# [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash 7 messages from 2016-03-21 to 2016-03-21. Participants: Eric Sunshine, Jeff King, Christian Couder, Torsten Bögershausen. Thread: https://gitlist.dev/t/41765 ## Eric Sunshine, 2016-03-21 04:35 Subject: [PATCH 0/2] work around gcc 4.2.x compiler crash Message-ID: <1458534958-6956-1-git-send-email-sunshine@sunshineco.com> URL: https://gitlist.dev/e/1458534958-6956-1-git-send-email-sunshine%40sunshineco.com ``` This patch series works around a gcc 4.2.1 compiler crash reported on Mac OS X[1] and FreeBSD[2] which is triggered by perfectly valid change from 5b442c4 (tree-diff: catch integer overflow in combine_diff_path allocation, 2016-02-19). patch 1: sidestep the problem via a simple re-ordering of macro argument evaluation patch 2: revert an earlier Mac OS X 10.6-specific work-around which is no longer needed [1]: http://thread.gmane.org/gmane.comp.version-control.git/287486/focus=287706 [2]: http://thread.gmane.org/gmane.linux.kernel/2179363/focus=289357 Eric Sunshine (2): git-compat-util: st_add4: work around gcc 4.2.x compiler crash Revert "config.mak.uname: use clang for Mac OS X 10.6" config.mak.uname | 3 --- git-compat-util.h | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) -- 2.8.0.rc3.240.g104e649 ``` ## Eric Sunshine, 2016-03-21 04:35 Subject: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash Message-ID: <1458534958-6956-2-git-send-email-sunshine@sunshineco.com> URL: https://gitlist.dev/e/1458534958-6956-2-git-send-email-sunshine%40sunshineco.com In-Reply-To: <1458534958-6956-1-git-send-email-sunshine@sunshineco.com> ``` Although changes by 5b442c4 (tree-diff: catch integer overflow in combine_diff_path allocation, 2016-02-19) are perfectly valid, they unfortunately trigger an internal compiler error in gcc 4.2.x: combine-diff.c: In function 'diff_tree_combined': combine-diff.c:1391: internal compiler error: Segmentation fault: 11 Experimentation reveals that changing st_add4()'s argument evaluation order is sufficient to sidestep this problem. Although st_add3() does not trigger the compiler bug, for style consistency, change its argument evaluation order to match. Signed-off-by: Eric Sunshine --- git-compat-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index c07e0c1..4743954 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -715,8 +715,8 @@ static inline size_t st_add(size_t a, size_t b) (uintmax_t)a, (uintmax_t)b); return a + b; } -#define st_add3(a,b,c) st_add((a),st_add((b),(c))) -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d))) +#define st_add3(a,b,c) st_add(st_add((a),(b)),(c)) +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d)) static inline size_t st_mult(size_t a, size_t b) { -- 2.8.0.rc3.240.g104e649 ``` ## Eric Sunshine, 2016-03-21 04:35 Subject: [PATCH 2/2] Revert "config.mak.uname: use clang for Mac OS X 10.6" Message-ID: <1458534958-6956-3-git-send-email-sunshine@sunshineco.com> URL: https://gitlist.dev/e/1458534958-6956-3-git-send-email-sunshine%40sunshineco.com In-Reply-To: <1458534958-6956-1-git-send-email-sunshine@sunshineco.com> ``` This reverts commit 7b6daf8d2fee1a9866b1d4eddbfaa5dbc42c5dbb. Now that st_add4() has been patched to work around the gcc 4.2.x compiler crash, revert the sledge-hammer approach of forcing Mac OS X 10.6 to unconditionally use 'clang' rather than the default compiler (gcc). Signed-off-by: Eric Sunshine --- I verified that the work-around in patch 1/2 "fixes" the crash on FreeBSD, and I'm pretty confident that it fixes it as well on Mac OS X 10.6 since it manifested identically on both platforms, but it would still be nice to receive confirmation from Torsten. config.mak.uname | 3 --- 1 file changed, 3 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index 1139b44..fe8096f 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -103,9 +103,6 @@ ifeq ($(uname_S),Darwin) ifeq ($(shell expr "$(uname_R)" : '[15]\.'),2) NO_STRLCPY = YesPlease endif - ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -eq 10 && echo 1),1) - CC = clang - endif ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 11 && echo 1),1) HAVE_GETDELIM = YesPlease endif -- 2.8.0.rc3.240.g104e649 ``` ## Jeff King, 2016-03-21 04:43 Subject: Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash Message-ID: <20160321044335.GA20519@sigill.intra.peff.net> URL: https://gitlist.dev/e/20160321044335.GA20519%40sigill.intra.peff.net In-Reply-To: <1458534958-6956-2-git-send-email-sunshine@sunshineco.com> ``` On Mon, Mar 21, 2016 at 12:35:57AM -0400, Eric Sunshine wrote: > Although changes by 5b442c4 (tree-diff: catch integer overflow in > combine_diff_path allocation, 2016-02-19) are perfectly valid, they > unfortunately trigger an internal compiler error in gcc 4.2.x: > > combine-diff.c: In function 'diff_tree_combined': > combine-diff.c:1391: internal compiler error: Segmentation fault: 11 > > Experimentation reveals that changing st_add4()'s argument evaluation > order is sufficient to sidestep this problem. > > Although st_add3() does not trigger the compiler bug, for style > consistency, change its argument evaluation order to match. > > Signed-off-by: Eric Sunshine > --- > git-compat-util.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Thanks for digging into this, the result is a really pleasant solution. I think it's worth doing. -Peff ``` ## Christian Couder, 2016-03-21 04:56 Subject: Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash Message-ID: URL: https://gitlist.dev/e/CAP8UFD0Kp55bpwdNrXY3KhpDiHVDReWKpuNJz7_Q537DXeUXHA%40mail.gmail.com In-Reply-To: <1458534958-6956-2-git-send-email-sunshine@sunshineco.com> ``` On Mon, Mar 21, 2016 at 5:35 AM, Eric Sunshine wrote: > > diff --git a/git-compat-util.h b/git-compat-util.h > index c07e0c1..4743954 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -715,8 +715,8 @@ static inline size_t st_add(size_t a, size_t b) > (uintmax_t)a, (uintmax_t)b); > return a + b; > } > -#define st_add3(a,b,c) st_add((a),st_add((b),(c))) > -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d))) > +#define st_add3(a,b,c) st_add(st_add((a),(b)),(c)) > +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d)) Nit: maybe a comment around those lines would make sure that people do not inadvertently change them back later. Thanks, Christian. ``` ## Eric Sunshine, 2016-03-21 05:38 Subject: Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash Message-ID: URL: https://gitlist.dev/e/CAPig%2BcR2rBJ4Tn9h7d4pp_Uxrfgc-BgHMLOhxKhWtYbm2wnARA%40mail.gmail.com In-Reply-To: ``` On Mon, Mar 21, 2016 at 12:56 AM, Christian Couder wrote: > On Mon, Mar 21, 2016 at 5:35 AM, Eric Sunshine wrote: >> >> diff --git a/git-compat-util.h b/git-compat-util.h >> index c07e0c1..4743954 100644 >> --- a/git-compat-util.h >> +++ b/git-compat-util.h >> @@ -715,8 +715,8 @@ static inline size_t st_add(size_t a, size_t b) >> (uintmax_t)a, (uintmax_t)b); >> return a + b; >> } >> -#define st_add3(a,b,c) st_add((a),st_add((b),(c))) >> -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d))) >> +#define st_add3(a,b,c) st_add(st_add((a),(b)),(c)) >> +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d)) > > Nit: maybe a comment around those lines would make sure that people do > not inadvertently change them back later. Maybe, maybe not. I'm hesitant for the following reason. Unless we determine the exact compiler bug and patch which fixed it, we don't really have a good handle on what triggers this crash. Consequently, even with a comment saying "don't change the code in such and such a way", if someone ever does need to modify st_add4() in some fashion, it's entirely possible that the modification will trigger the crash again, even if the current evaluation order is kept or only modified slightly. We just don't know. So, such a comment doesn't strike me as having a lot of value, and whatever value it does have wanes as this old compiler (gcc 4.2.1) and these old platforms (Mac OS X 10.6 and FreeBSD 9.x) become less and less relevant over time. ``` ## Torsten Bögershausen, 2016-03-21 17:46 Subject: Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash Message-ID: <56F0336E.6000103@web.de> URL: https://gitlist.dev/e/56F0336E.6000103%40web.de In-Reply-To: <1458534958-6956-2-git-send-email-sunshine@sunshineco.com> ``` On 2016-03-21 05.35, Eric Sunshine wrote: > } > -#define st_add3(a,b,c) st_add((a),st_add((b),(c))) > -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d))) > +#define st_add3(a,b,c) st_add(st_add((a),(b)),(c)) > +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d)) > That fix compiles on my test machine, and passes the test suite. thanks for digging. ```