{"thread":{"id":"41765","subject":"[PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash","startedAt":"2016-03-21T04:35:56Z","lastAt":"2016-03-21T17:46:22Z","messageCount":7,"participants":["Eric Sunshine","Jeff King","Christian Couder","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"281321","messageId":"1458534958-6956-1-git-send-email-sunshine@sunshineco.com","threadId":"41765","inReplyTo":null,"subject":"[PATCH 0/2] work around gcc 4.2.x compiler crash","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-21T04:35:56Z","receivedAt":"2016-03-21T04:35:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This patch series works around a gcc 4.2.1 compiler crash reported on\nMac OS X[1] and FreeBSD[2] which is triggered by perfectly valid change\nfrom 5b442c4 (tree-diff: catch integer overflow in combine_diff_path\nallocation, 2016-02-19).\n\npatch 1: sidestep the problem via a simple re-ordering of macro argument\n evaluation\n\npatch 2: revert an earlier Mac OS X 10.6-specific work-around which is\n no longer needed\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/287486/focus=287706\n[2]: http://thread.gmane.org/gmane.linux.kernel/2179363/focus=289357\n\nEric Sunshine (2):\n git-compat-util: st_add4: work around gcc 4.2.x compiler crash\n Revert \"config.mak.uname: use clang for Mac OS X 10.6\"\n\n config.mak.uname | 3 ---\n git-compat-util.h | 4 ++--\n 2 files changed, 2 insertions(+), 5 deletions(-)\n\n-- \n2.8.0.rc3.240.g104e649\n"},{"id":"281320","messageId":"1458534958-6956-2-git-send-email-sunshine@sunshineco.com","threadId":"41765","inReplyTo":"1458534958-6956-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-21T04:35:57Z","receivedAt":"2016-03-21T04:35:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Although changes by 5b442c4 (tree-diff: catch integer overflow in\ncombine_diff_path allocation, 2016-02-19) are perfectly valid, they\nunfortunately trigger an internal compiler error in gcc 4.2.x:\n\n combine-diff.c: In function 'diff_tree_combined':\n combine-diff.c:1391: internal compiler error: Segmentation fault: 11\n\nExperimentation reveals that changing st_add4()'s argument evaluation\norder is sufficient to sidestep this problem.\n\nAlthough st_add3() does not trigger the compiler bug, for style\nconsistency, change its argument evaluation order to match.\n\nSigned-off-by: Eric Sunshine \n---\n git-compat-util.h | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex c07e0c1..4743954 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -715,8 +715,8 @@ static inline size_t st_add(size_t a, size_t b)\n \t\t (uintmax_t)a, (uintmax_t)b);\n \treturn a + b;\n }\n-#define st_add3(a,b,c) st_add((a),st_add((b),(c)))\n-#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d)))\n+#define st_add3(a,b,c) st_add(st_add((a),(b)),(c))\n+#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))\n \n static inline size_t st_mult(size_t a, size_t b)\n {\n-- \n2.8.0.rc3.240.g104e649\n"},{"id":"281322","messageId":"1458534958-6956-3-git-send-email-sunshine@sunshineco.com","threadId":"41765","inReplyTo":"1458534958-6956-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 2/2] Revert \"config.mak.uname: use clang for Mac OS X 10.6\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-21T04:35:58Z","receivedAt":"2016-03-21T04:35:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This reverts commit 7b6daf8d2fee1a9866b1d4eddbfaa5dbc42c5dbb.\n\nNow that st_add4() has been patched to work around the gcc 4.2.x\ncompiler crash, revert the sledge-hammer approach of forcing Mac OS X\n10.6 to unconditionally use 'clang' rather than the default compiler\n(gcc).\n\nSigned-off-by: Eric Sunshine \n---\n\nI verified that the work-around in patch 1/2 \"fixes\" the crash on\nFreeBSD, and I'm pretty confident that it fixes it as well on Mac OS X\n10.6 since it manifested identically on both platforms, but it would\nstill be nice to receive confirmation from Torsten.\n\nconfig.mak.uname | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 1139b44..fe8096f 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -103,9 +103,6 @@ ifeq ($(uname_S),Darwin)\n \tifeq ($(shell expr \"$(uname_R)\" : '[15]\\.'),2)\n \t\tNO_STRLCPY = YesPlease\n \tendif\n-\tifeq ($(shell test \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.'`\" -eq 10 && echo 1),1)\n-\t\tCC = clang\n-\tendif\n \tifeq ($(shell test \"`expr \"$(uname_R)\" : '\\([0-9][0-9]*\\)\\.'`\" -ge 11 && echo 1),1)\n \t\tHAVE_GETDELIM = YesPlease\n \tendif\n-- \n2.8.0.rc3.240.g104e649\n"},{"id":"281324","messageId":"20160321044335.GA20519@sigill.intra.peff.net","threadId":"41765","inReplyTo":"1458534958-6956-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-21T04:43:35Z","receivedAt":"2016-03-21T04:43:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 21, 2016 at 12:35:57AM -0400, Eric Sunshine wrote:\n\n> Although changes by 5b442c4 (tree-diff: catch integer overflow in\n> combine_diff_path allocation, 2016-02-19) are perfectly valid, they\n> unfortunately trigger an internal compiler error in gcc 4.2.x:\n> \n> combine-diff.c: In function 'diff_tree_combined':\n> combine-diff.c:1391: internal compiler error: Segmentation fault: 11\n> \n> Experimentation reveals that changing st_add4()'s argument evaluation\n> order is sufficient to sidestep this problem.\n> \n> Although st_add3() does not trigger the compiler bug, for style\n> consistency, change its argument evaluation order to match.\n> \n> Signed-off-by: Eric Sunshine \n> ---\n> git-compat-util.h | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n\nThanks for digging into this, the result is a really pleasant solution.\nI think it's worth doing.\n\n-Peff\n"},{"id":"281325","messageId":"CAP8UFD0Kp55bpwdNrXY3KhpDiHVDReWKpuNJz7_Q537DXeUXHA@mail.gmail.com","threadId":"41765","inReplyTo":"1458534958-6956-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2016-03-21T04:56:56Z","receivedAt":"2016-03-21T04:56:56Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Mar 21, 2016 at 5:35 AM, Eric Sunshine wrote:\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index c07e0c1..4743954 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -715,8 +715,8 @@ static inline size_t st_add(size_t a, size_t b)\n> (uintmax_t)a, (uintmax_t)b);\n> return a + b;\n> }\n> -#define st_add3(a,b,c) st_add((a),st_add((b),(c)))\n> -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d)))\n> +#define st_add3(a,b,c) st_add(st_add((a),(b)),(c))\n> +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))\n\nNit: maybe a comment around those lines would make sure that people do\nnot inadvertently change them back later.\n\nThanks,\nChristian.\n"},{"id":"281326","messageId":"CAPig+cR2rBJ4Tn9h7d4pp_Uxrfgc-BgHMLOhxKhWtYbm2wnARA@mail.gmail.com","threadId":"41765","inReplyTo":"CAP8UFD0Kp55bpwdNrXY3KhpDiHVDReWKpuNJz7_Q537DXeUXHA@mail.gmail.com","subject":"Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-21T05:38:37Z","receivedAt":"2016-03-21T05:38:37Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 21, 2016 at 12:56 AM, Christian Couder\n wrote:\n> On Mon, Mar 21, 2016 at 5:35 AM, Eric Sunshine wrote:\n>>\n>> diff --git a/git-compat-util.h b/git-compat-util.h\n>> index c07e0c1..4743954 100644\n>> --- a/git-compat-util.h\n>> +++ b/git-compat-util.h\n>> @@ -715,8 +715,8 @@ static inline size_t st_add(size_t a, size_t b)\n>> (uintmax_t)a, (uintmax_t)b);\n>> return a + b;\n>> }\n>> -#define st_add3(a,b,c) st_add((a),st_add((b),(c)))\n>> -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d)))\n>> +#define st_add3(a,b,c) st_add(st_add((a),(b)),(c))\n>> +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))\n>\n> Nit: maybe a comment around those lines would make sure that people do\n> not inadvertently change them back later.\n\nMaybe, maybe not. I'm hesitant for the following reason.\n\nUnless we determine the exact compiler bug and patch which fixed it,\nwe don't really have a good handle on what triggers this crash.\nConsequently, even with a comment saying \"don't change the code in\nsuch and such a way\", if someone ever does need to modify st_add4() in\nsome fashion, it's entirely possible that the modification will\ntrigger the crash again, even if the current evaluation order is kept\nor only modified slightly. We just don't know.\n\nSo, such a comment doesn't strike me as having a lot of value, and\nwhatever value it does have wanes as this old compiler (gcc 4.2.1) and\nthese old platforms (Mac OS X 10.6 and FreeBSD 9.x) become less and\nless relevant over time.\n"},{"id":"281351","messageId":"56F0336E.6000103@web.de","threadId":"41765","inReplyTo":"1458534958-6956-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-03-21T17:46:22Z","receivedAt":"2016-03-21T17:46:22Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2016-03-21 05.35, Eric Sunshine wrote:\n> }\n> -#define st_add3(a,b,c) st_add((a),st_add((b),(c)))\n> -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d)))\n> +#define st_add3(a,b,c) st_add(st_add((a),(b)),(c))\n> +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))\n> \n\nThat fix compiles on my test machine,\nand passes the test suite.\nthanks for digging.\n"}]}