{"thread":{"id":"41089","subject":"[PATCH 0/2] compiling with -fsanitize=undefined","startedAt":"2015-12-29T06:34:49Z","lastAt":"2016-01-04T23:32:30Z","messageCount":12,"participants":["Jeff King","Eric Sunshine","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"275076","messageId":"20151229063449.GA28755@sigill.intra.peff.net","threadId":"41089","inReplyTo":null,"subject":"[PATCH 0/2] compiling with -fsanitize=undefined","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-29T06:34:49Z","receivedAt":"2015-12-29T06:34:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I was playing around with the new-ish \"-fsanitize=undefined\" compiler\nflag, and it detected a few problems:\n\n 1. We sometimes bit-shift signed constants too far (fixed by the first\n patch).\n\n 2. We have some unaligned memory accesses that presumably work OK on\n x86, but would blow up on ARM or other platforms (I didn't test).\n\nThe latter looks like it's in the untracked cache code (Duy and\nChristian cc'd). Running t7063 gets me this:\n\ndir.c:2631:45: runtime error: member access within misaligned address 0x7f19806ff185 for type 'const struct ondisk_untracked_cache', which requires 4 byte alignment\n0x7f19806ff185: note: pointer points here\n 31 33 29 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00\n ^\n\nWe also do unaligned loads in the get_be* functions, but only on x86 and\nsimilar platforms. I'm counting these as a false positive, since it is\npresumably OK there. The second patch makes it easier to squelch these.\n\nI also got false positives from feeding NULL to qsort(). This is\ntechnically wrong, but OK in practice when we tell it we have zero\nelements. Compiling with INTERNAL_QSORT silences these (and if somebody\nis on a platform where their qsort() segfaults, it's what I'd tell them\nto use).\n\nSo if you want to play along at home, my build is something like:\n\n make \\\n CC=clang \\\n INTERNAL_QSORT=YesPlease \\\n CFLAGS='-O2 -g -fsanitize=undefined -fno-sanitize-recover=undefined -DNO_UNALIGNED_LOADS' \\\n test\n\nand it passes except for t7063.\n\nThe patches are:\n\n [1/2]: avoid shifting signed integers 31 bits\n [2/2]: bswap: add NO_UNALIGNED_LOADS define\n\n-Peff\n"},{"id":"275077","messageId":"20151229063545.GA30340@sigill.intra.peff.net","threadId":"41089","inReplyTo":"20151229063449.GA28755@sigill.intra.peff.net","subject":"[PATCH 1/2] avoid shifting signed integers 31 bits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-29T06:35:46Z","receivedAt":"2015-12-29T06:35:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We sometimes use 32-bit unsigned integers as bit-fields.\nIt's fine to access the MSB, because it's unsigned. However,\ndoing so as \"1 << 31\" is wrong, because the constant \"1\" is\na signed int, and we shift into the sign bit, causing\nundefined behavior.\n\nWe can fix this by using \"1U\" as the constant.\n\nSigned-off-by: Jeff King \n---\n builtin/receive-pack.c | 2 +-\n diff.h | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ca38131..2b3b746 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1618,7 +1618,7 @@ static void prepare_shallow_update(struct command *commands,\n \t\t\t\tcontinue;\n \t\t\tsi->need_reachability_test[i]++;\n \t\t\tfor (k = 0; k < 32; k++)\n-\t\t\t\tif (si->used_shallow[i][j] & (1 << k))\n+\t\t\t\tif (si->used_shallow[i][j] & (1U << k))\n \t\t\t\t\tsi->shallow_ref[j * 32 + k]++;\n \t\t}\n \ndiff --git a/diff.h b/diff.h\nindex f7208ad..893f446 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -91,7 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)\n #define DIFF_OPT_FUNCCONTEXT (1 << 29)\n #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n-#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)\n+#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31)\n \n #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_TOUCHED(opts, flag) ((opts)->touched_flags & DIFF_OPT_##flag)\n-- \n2.7.0.rc2.368.g1cbb535\n"},{"id":"275078","messageId":"20151229063600.GB30340@sigill.intra.peff.net","threadId":"41089","inReplyTo":"20151229063449.GA28755@sigill.intra.peff.net","subject":"[PATCH 2/2] bswap: add NO_UNALIGNED_LOADS define","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-29T06:36:00Z","receivedAt":"2015-12-29T06:36:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The byte-swapping code automatically decides, based on the\nplatform, whether it is sensible to cast an do a potentially\nunaligned ntohl(), or to pick individual bytes out of an\narray.\n\nIt can be handy to override this decision, though, when\nturning on compiler flags that will complain about unaligned\nloads (such as -fsanitize=undefined). This patch adds a\nmacro check to make this possible.\n\nThere's no nice Makefile knob here; this is for prodding at\nGit's internals, and anybody using it can set\n\"-DNO_UNALIGNED_LOADS\" in the same place they are setting up\n\"-fsanitize\".\n\nSigned-off-by: Jeff King \n---\n compat/bswap.h | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex 7fed637..d47c003 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -149,11 +149,12 @@ static inline uint64_t git_bswap64(uint64_t x)\n * and is faster on architectures with memory alignment issues.\n */\n \n-#if defined(__i386__) || defined(__x86_64__) || \\\n+#if !defined(NO_UNALIGNED_LOADS) && ( \\\n+ defined(__i386__) || defined(__x86_64__) || \\\n defined(_M_IX86) || defined(_M_X64) || \\\n defined(__ppc__) || defined(__ppc64__) || \\\n defined(__powerpc__) || defined(__powerpc64__) || \\\n- defined(__s390__) || defined(__s390x__)\n+ defined(__s390__) || defined(__s390x__))\n \n #define get_be16(p)\tntohs(*(unsigned short *)(p))\n #define get_be32(p)\tntohl(*(unsigned int *)(p))\n-- \n2.7.0.rc2.368.g1cbb535\n"},{"id":"275079","messageId":"CAPig+cQ_u2Y6Z6SO3JQ0XtaoRCPf7r_gnEh-kTj0mn36ahEoHg@mail.gmail.com","threadId":"41089","inReplyTo":"20151229063600.GB30340@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] bswap: add NO_UNALIGNED_LOADS define","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-12-29T06:42:45Z","receivedAt":"2015-12-29T06:42:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Dec 29, 2015 at 1:36 AM, Jeff King wrote:\n> The byte-swapping code automatically decides, based on the\n> platform, whether it is sensible to cast an do a potentially\n\ns/an/and/ or something?\n\n> unaligned ntohl(), or to pick individual bytes out of an\n> array.\n>\n> It can be handy to override this decision, though, when\n> turning on compiler flags that will complain about unaligned\n> loads (such as -fsanitize=undefined). This patch adds a\n> macro check to make this possible.\n>\n> There's no nice Makefile knob here; this is for prodding at\n> Git's internals, and anybody using it can set\n> \"-DNO_UNALIGNED_LOADS\" in the same place they are setting up\n> \"-fsanitize\".\n>\n> Signed-off-by: Jeff King \n"},{"id":"275080","messageId":"20151229064435.GA31944@sigill.intra.peff.net","threadId":"41089","inReplyTo":"20151229063449.GA28755@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] compiling with -fsanitize=undefined","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-29T06:44:35Z","receivedAt":"2015-12-29T06:44:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 29, 2015 at 01:34:49AM -0500, Jeff King wrote:\n\n> 2. We have some unaligned memory accesses that presumably work OK on\n> x86, but would blow up on ARM or other platforms (I didn't test).\n> \n> The latter looks like it's in the untracked cache code (Duy and\n> Christian cc'd). Running t7063 gets me this:\n> \n> dir.c:2631:45: runtime error: member access within misaligned address 0x7f19806ff185 for type 'const struct ondisk_untracked_cache', which requires 4 byte alignment\n> 0x7f19806ff185: note: pointer points here\n> 31 33 29 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00\n> ^\n\nI took a brief look at fixing this, and it may actually be a false\npositive, as well (in a manner of speaking; if the compiler thinks it is\nundefined behavior, it may still be worth fixing so we don't fall prey\nto some optimizations-gone-wild).\n\nThe line in question is:\n\n load_sha1_stat(&uc->ss_info_exclude, &ouc->info_exclude_stat,\n\t\t ouc->info_exclude_sha1);\n\nwhere \"ouc\" is the on-disk data cast to a struct. So we definitely\ngenerate an unaligned pointer, but then we only access its contents via\nget_be32(), which handles alignment. So it might actually be OK. I\ndunno.\n\n-Peff\n"},{"id":"275081","messageId":"20151229064509.GA31992@sigill.intra.peff.net","threadId":"41089","inReplyTo":"CAPig+cQ_u2Y6Z6SO3JQ0XtaoRCPf7r_gnEh-kTj0mn36ahEoHg@mail.gmail.com","subject":"Re: [PATCH 2/2] bswap: add NO_UNALIGNED_LOADS define","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-29T06:45:09Z","receivedAt":"2015-12-29T06:45:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 29, 2015 at 01:42:45AM -0500, Eric Sunshine wrote:\n\n> On Tue, Dec 29, 2015 at 1:36 AM, Jeff King wrote:\n> > The byte-swapping code automatically decides, based on the\n> > platform, whether it is sensible to cast an do a potentially\n> \n> s/an/and/ or something?\n\nYes, it should be \"and\". Thanks.\n\n-Peff\n"},{"id":"275163","messageId":"xmqq4mf0g5la.fsf@gitster.mtv.corp.google.com","threadId":"41089","inReplyTo":"20151229063545.GA30340@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] avoid shifting signed integers 31 bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-30T00:09:21Z","receivedAt":"2015-12-30T00:09:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/diff.h b/diff.h\n> index f7208ad..893f446 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -91,7 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n> #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)\n> #define DIFF_OPT_FUNCCONTEXT (1 << 29)\n> #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n> -#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)\n> +#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31)\n> \n> #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)\n> #define DIFF_OPT_TOUCHED(opts, flag) ((opts)->touched_flags & DIFF_OPT_##flag)\n\nThanks.\n\nSeeing (1 << 30) and (1U <<31) together made me feel that we are way\n_too_ explicit being careful about 32-bit archs (iow, it would be\nmore consistent to turn all of these \"1 <<\" into \"1U <<\"), but at\nthe same time, (1 << 30) won't be broken unless we are on 31-bit\narch in the sense that if we are on 30-bit or smaller arch the\nexpression is already broken with or without \"U\", and if we are on\n32-bit or more, then with or without \"U\" we are OK---which made me\nfeel somewhat funny.\n\nIn any case, these two are good changes. Thanks.\n"},{"id":"275164","messageId":"20151230042519.GA5037@sigill.intra.peff.net","threadId":"41089","inReplyTo":"xmqq4mf0g5la.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] avoid shifting signed integers 31 bits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-30T04:25:19Z","receivedAt":"2015-12-30T04:25:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 29, 2015 at 04:09:21PM -0800, Junio C Hamano wrote:\n\n> > diff --git a/diff.h b/diff.h\n> > index f7208ad..893f446 100644\n> > --- a/diff.h\n> > +++ b/diff.h\n> > @@ -91,7 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n> > #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)\n> > #define DIFF_OPT_FUNCCONTEXT (1 << 29)\n> > #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n> > -#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)\n> > +#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31)\n> > \n> > #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)\n> > #define DIFF_OPT_TOUCHED(opts, flag) ((opts)->touched_flags & DIFF_OPT_##flag)\n> \n> Thanks.\n> \n> Seeing (1 << 30) and (1U <<31) together made me feel that we are way\n> _too_ explicit being careful about 32-bit archs (iow, it would be\n> more consistent to turn all of these \"1 <<\" into \"1U <<\"), but at\n> the same time, (1 << 30) won't be broken unless we are on 31-bit\n> arch in the sense that if we are on 30-bit or smaller arch the\n> expression is already broken with or without \"U\", and if we are on\n> 32-bit or more, then with or without \"U\" we are OK---which made me\n> feel somewhat funny.\n\nYeah, I was tempted to convert them all, but there's no benefit except\nconsistency. Interestingly, if we spelled these as:\n\n 0x01\n 0x02\n ...\n 0x80000000\n\nthat _is_ OK by the C standard (the type of the constant is \"the first\nthing big enough to hold it\" from a list of int, unsigned, etc[1]). I find\nthat style more error-prone, though, so I don't think it's a good idea\nto move to it.\n\nWe pretty much assume an int of at least 32-bits anyway; the flags field\nitself is a simple \"unsigned\". We could make that a uint32_t, but in\npractice I hope that sub-32-bit platforms are all dead, at least for\ngeneral purpose application code like git.\n\n-Peff\n\n[1] The list of possible types is actually _different_ for decimal and\n hex constants. Which seems slightly insane, but hey, it's C.\n Notably, the decimal equivalent of 0x80000000 is guaranteed to be\n signed (but would be a \"long int\" on a 32-bit platform).\n"},{"id":"275226","messageId":"CACsJy8CAAqZFQCfadUov7DFhSgh=dtGfE-onbXZQXO-0Y2652g@mail.gmail.com","threadId":"41089","inReplyTo":"20151229063545.GA30340@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] avoid shifting signed integers 31 bits","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-12-31T05:10:33Z","receivedAt":"2015-12-31T05:10:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 29, 2015 at 1:35 PM, Jeff King wrote:\n> We sometimes use 32-bit unsigned integers as bit-fields.\n> It's fine to access the MSB, because it's unsigned. However,\n> doing so as \"1 << 31\" is wrong, because the constant \"1\" is\n> a signed int, and we shift into the sign bit, causing\n> undefined behavior.\n>\n> We can fix this by using \"1U\" as the constant.\n\nWe have this in cache.h, should it be fixed as well?\n\n/* CE_EXTENDED2 is for future extension */\n#define CE_EXTENDED2 (1 << 31)\n-- \nDuy\n"},{"id":"275227","messageId":"20151231052029.GA10238@sigill.intra.peff.net","threadId":"41089","inReplyTo":"CACsJy8CAAqZFQCfadUov7DFhSgh=dtGfE-onbXZQXO-0Y2652g@mail.gmail.com","subject":"Re: [PATCH 1/2] avoid shifting signed integers 31 bits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-31T05:20:29Z","receivedAt":"2015-12-31T05:20:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 31, 2015 at 12:10:33PM +0700, Duy Nguyen wrote:\n\n> On Tue, Dec 29, 2015 at 1:35 PM, Jeff King wrote:\n> > We sometimes use 32-bit unsigned integers as bit-fields.\n> > It's fine to access the MSB, because it's unsigned. However,\n> > doing so as \"1 << 31\" is wrong, because the constant \"1\" is\n> > a signed int, and we shift into the sign bit, causing\n> > undefined behavior.\n> >\n> > We can fix this by using \"1U\" as the constant.\n> \n> We have this in cache.h, should it be fixed as well?\n> \n> /* CE_EXTENDED2 is for future extension */\n> #define CE_EXTENDED2 (1 << 31)\n\nSort of. We don't actually use it, and since it's a macro, that means it\nnever even hits the compiler proper itself. So it's not a bug, but it's\na bug waiting to happen. :)\n\n-Peff\n"},{"id":"275310","messageId":"xmqqh9itp705.fsf@gitster.mtv.corp.google.com","threadId":"41089","inReplyTo":"20151231052029.GA10238@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] avoid shifting signed integers 31 bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-04T17:52:10Z","receivedAt":"2016-01-04T17:52:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Dec 31, 2015 at 12:10:33PM +0700, Duy Nguyen wrote:\n>\n>> On Tue, Dec 29, 2015 at 1:35 PM, Jeff King wrote:\n>> > We sometimes use 32-bit unsigned integers as bit-fields.\n>> > It's fine to access the MSB, because it's unsigned. However,\n>> > doing so as \"1 << 31\" is wrong, because the constant \"1\" is\n>> > a signed int, and we shift into the sign bit, causing\n>> > undefined behavior.\n>> >\n>> > We can fix this by using \"1U\" as the constant.\n>> \n>> We have this in cache.h, should it be fixed as well?\n>> \n>> /* CE_EXTENDED2 is for future extension */\n>> #define CE_EXTENDED2 (1 << 31)\n>\n> Sort of. We don't actually use it, and since it's a macro, that means it\n> never even hits the compiler proper itself. So it's not a bug, but it's\n> a bug waiting to happen. :)\n>\n> -Peff\n\nLet's squash an obvious change for that in to 1/2, then, before I\nmerge the series to 'next'.\n\nThanks.\n\n-- >8 --\nFrom: Jeff King \nDate: Tue, 29 Dec 2015 01:35:46 -0500\nSubject: [PATCH] avoid shifting signed integers 31 bits\n\nWe sometimes use 32-bit unsigned integers as bit-fields.\nIt's fine to access the MSB, because it's unsigned. However,\ndoing so as \"1 << 31\" is wrong, because the constant \"1\" is\na signed int, and we shift into the sign bit, causing\nundefined behavior.\n\nWe can fix this by using \"1U\" as the constant.\n\nSigned-off-by: Jeff King \nSigned-off-by: Junio C Hamano \n---\n builtin/receive-pack.c | 2 +-\n cache.h | 2 +-\n diff.h | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e6b93d0..e35ed40 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1597,7 +1597,7 @@ static void prepare_shallow_update(struct command *commands,\n \t\t\t\tcontinue;\n \t\t\tsi->need_reachability_test[i]++;\n \t\t\tfor (k = 0; k < 32; k++)\n-\t\t\t\tif (si->used_shallow[i][j] & (1 << k))\n+\t\t\t\tif (si->used_shallow[i][j] & (1U << k))\n \t\t\t\t\tsi->shallow_ref[j * 32 + k]++;\n \t\t}\n \ndiff --git a/cache.h b/cache.h\nindex 6f53962..9088843 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -214,7 +214,7 @@ struct cache_entry {\n #define CE_INTENT_TO_ADD (1 << 29)\n #define CE_SKIP_WORKTREE (1 << 30)\n /* CE_EXTENDED2 is for future extension */\n-#define CE_EXTENDED2 (1 << 31)\n+#define CE_EXTENDED2 (1U << 31)\n \n #define CE_EXTENDED_FLAGS (CE_INTENT_TO_ADD | CE_SKIP_WORKTREE)\n \ndiff --git a/diff.h b/diff.h\nindex f7208ad..893f446 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -91,7 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)\n #define DIFF_OPT_FUNCCONTEXT (1 << 29)\n #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n-#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)\n+#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31)\n \n #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_TOUCHED(opts, flag) ((opts)->touched_flags & DIFF_OPT_##flag)\n-- \n2.7.0-rc3-132-g73ad441\n"},{"id":"275332","messageId":"20160104233230.GA28274@sigill.intra.peff.net","threadId":"41089","inReplyTo":"xmqqh9itp705.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] avoid shifting signed integers 31 bits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-01-04T23:32:30Z","receivedAt":"2016-01-04T23:32:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 04, 2016 at 09:52:10AM -0800, Junio C Hamano wrote:\n\n> >> We have this in cache.h, should it be fixed as well?\n> >> \n> >> /* CE_EXTENDED2 is for future extension */\n> >> #define CE_EXTENDED2 (1 << 31)\n> >\n> > Sort of. We don't actually use it, and since it's a macro, that means it\n> > never even hits the compiler proper itself. So it's not a bug, but it's\n> > a bug waiting to happen. :)\n> >\n> \n> Let's squash an obvious change for that in to 1/2, then, before I\n> merge the series to 'next'.\n\nThanks, I agree it is worth fixing while we're visiting the topic.\n\n-Peff\n"}]}