# [RFH] gcc constant expression warning... 8 messages from 2007-10-28 to 2007-10-29. Participants: Junio C Hamano, Florian Weimer, Antti-Juhani Kaijanaho, Daniel Barkalow, Linus Torvalds, Nicolas Pitre, Stephen Rothwell. Thread: https://gitlist.dev/t/10495 ## Junio C Hamano, 2007-10-28 08:46 Subject: [RFH] gcc constant expression warning... Message-ID: <7vy7dnvd6w.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy7dnvd6w.fsf%40gitster.siamese.dyndns.org ``` With the recent gcc, we get: sha1_file.c: In check_packed_git_: sha1_file.c:527: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false sha1_file.c:527: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false when compiling with -O2 -Werror -Wall -Wold-style-definition \ -ansi -pedantic -std=c99 -Wdeclaration-after-statement The offending lines are: if (idx_size != min_size) { /* make sure we can deal with large pack offsets */ off_t x = 0x7fffffffUL, y = 0xffffffffUL; if (x > (x + 1) || y > (y + 1)) { munmap(idx_map, idx_size); ``` ## Florian Weimer, 2007-10-28 10:21 Subject: Re: [RFH] gcc constant expression warning... Message-ID: <87ir4rletv.fsf@mid.deneb.enyo.de> URL: https://gitlist.dev/e/87ir4rletv.fsf%40mid.deneb.enyo.de In-Reply-To: <7vy7dnvd6w.fsf@gitster.siamese.dyndns.org> ``` * Junio C. Hamano: > The offending lines are: > > if (idx_size != min_size) { > /* make sure we can deal with large pack offsets */ > off_t x = 0x7fffffffUL, y = 0xffffffffUL; > if (x > (x + 1) || y > (y + 1)) { > munmap(idx_map, idx_size); x and y must be unsigned for this test to work (signed overflow is undefined). ``` ## Antti-Juhani Kaijanaho, 2007-10-28 10:37 Subject: Re: [RFH] gcc constant expression warning... Message-ID: URL: https://gitlist.dev/e/slrnfi8pj7.mb4.antti-juhani%40kukkaseppele.kaijanaho.fi In-Reply-To: <7vy7dnvd6w.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano kirjoitti 28.10.2007: > With the recent gcc, we get: > > sha1_file.c: In check_packed_git_: > sha1_file.c:527: warning: assuming signed overflow does not > occur when assuming that (X + c) < X is always false > sha1_file.c:527: warning: assuming signed overflow does not > occur when assuming that (X + c) < X is always false > > when compiling with > > -O2 -Werror -Wall -Wold-style-definition \ > -ansi -pedantic -std=c99 -Wdeclaration-after-statement -ansi and -std=c99 in the same command line is a bit weird, BTW :) > The offending lines are: > > if (idx_size != min_size) { > /* make sure we can deal with large pack offsets */ > off_t x = 0x7fffffffUL, y = 0xffffffffUL; > if (x > (x + 1) || y > (y + 1)) { > munmap(idx_map, idx_size); The second if line invokes undefined behavior if off_t cannot represent 0x7fffffffUL + 1. GCC apparently takes that as a license to ignore overflow and rewrite that if as "if (0) { ...". A fast fix is to compile with -fwrapv or with -fno-strict-overflow: -fstrict-overflow Allow the compiler to assume strict signed overflow rules, depending on the language being compiled. For C (and C++) this means that overflow when doing arithmetic with signed numbers is undefined, which means that the compiler may assume that it will not happen. This permits various optimizations. For example, the compiler will assume that an expression like i + 10 > i will always be true for signed i. This assumption is only valid if signed overflow is undefined, as the expression is false if i + 10 overflows when using twos complement arithmetic. When this option is in effect any attempt to determine whether an operation on signed numbers will overflow must be written carefully to not actually involve overflow. See also the -fwrapv option. Using -fwrapv means that signed overflow is fully defined: it wraps. When -fwrapv is used, there is no difference between -fstrict-overflow and -fno-strict-overflow. With -fwrapv certain types of overflow are permitted. For example, if the compiler gets an overflow when doing arithmetic on constants, the overflowed value can still be used with -fwrapv, but not otherwise. The -fstrict-overflow option is enabled at levels -O2, -O3, -Os. -fwrapv This option instructs the compiler to assume that signed arithmetic overflow of addition, subtraction and multiplication wraps around using twos-complement representation. This flag enables some optimizations and disables others. This option is enabled by default for the Java front-end, as required by the Java language specification. (From the GCC 4.2.2 manual.) A correct fix would be to check for the size of off_t in some other (and defined) manner, but I don't know off_t well enough to suggest one. Considering that the size of off_t won't change at runtime, the test ought to be compile (or configure) time. Reading POSIX, there seem to be some rather cumbersome sysconf stuff one could test for, and of course CHAR_BIT * sizeof(off_t) also tells one something. GNU autoconf might also have a solution at hand. -- Antti-Juhani Kaijanaho, Jyväskylä, Finland ``` ## Daniel Barkalow, 2007-10-28 16:28 Subject: Re: [RFH] gcc constant expression warning... Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0710281204350.7345%40iabervon.org In-Reply-To: <87ir4rletv.fsf@mid.deneb.enyo.de> ``` On Sun, 28 Oct 2007, Florian Weimer wrote: > * Junio C. Hamano: > > > The offending lines are: > > > > if (idx_size != min_size) { > > /* make sure we can deal with large pack offsets */ > > off_t x = 0x7fffffffUL, y = 0xffffffffUL; > > if (x > (x + 1) || y > (y + 1)) { > > munmap(idx_map, idx_size); > > x and y must be unsigned for this test to work (signed overflow is > undefined). I believe the test is trying to determine if signed addition on numbers of a certain size is safe in this environment. Doing the test with unsigned variables would cause the test to give a predictable but irrelevant result. I think gcc is being annoying in assuming that signed overflow doesn't occur (even when it must), rather than assuming that the result of signed overflow is some arbitrary and likely not useful value. If we have an overflow possible with off_t in the way we'd use it, then one of those tests should be automatically true due to the limited size of the type (except that I think the test should be >= instead of >). I think we should be able to assume that the result of a signed overflow, whatever undefined value it is, is a possible value of its type and therefore not more than the maximum value of its type, but gcc may be screwing this up. It's probably best just to test the size of off_t. -Daniel *This .sig left intentionally blank* ``` ## Linus Torvalds, 2007-10-28 17:09 Subject: Re: [RFH] gcc constant expression warning... Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.999.0710281000260.30120%40woody.linux-foundation.org In-Reply-To: ``` On Sun, 28 Oct 2007, Antti-Juhani Kaijanaho wrote: > > A correct fix would be to check for the size of off_t in some other (and > defined) manner, but I don't know off_t well enough to suggest one. In this case, it's trying to make sense that "off_t" can hold more than 32 bits. So I think that test can just be rewritten as if (sizeof(off_t) <= 4) { munmap(idx_map, idx_size); return error("pack too large for current definition of off_t in %s", path); } instead. Linus ``` ## Nicolas Pitre, 2007-10-29 00:55 Subject: Re: [RFH] gcc constant expression warning... Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.9999.0710282053590.22100%40xanadu.home In-Reply-To: ``` On Sun, 28 Oct 2007, Linus Torvalds wrote: > > > On Sun, 28 Oct 2007, Antti-Juhani Kaijanaho wrote: > > > > A correct fix would be to check for the size of off_t in some other (and > > defined) manner, but I don't know off_t well enough to suggest one. > > In this case, it's trying to make sense that "off_t" can hold more than 32 > bits. So I think that test can just be rewritten as > > if (sizeof(off_t) <= 4) { > munmap(idx_map, idx_size); > return error("pack too large for current definition of off_t in %s", path); > } > > instead. The test must also make sure off_t isn't signed, since in that case it can only hold 31 bits. Nicolas ``` ## Stephen Rothwell, 2007-10-29 02:41 Subject: Re: [RFH] gcc constant expression warning... Message-ID: <20071029134135.ed72fe78.sfr@canb.auug.org.au> URL: https://gitlist.dev/e/20071029134135.ed72fe78.sfr%40canb.auug.org.au In-Reply-To: ``` On Sun, 28 Oct 2007 20:55:32 -0400 (EDT) Nicolas Pitre wrote: > > The test must also make sure off_t isn't signed, since in that case it > can only hold 31 bits. Posix says: "blkcnt_t and off_t shall be signed integer types." -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ ``` ## Linus Torvalds, 2007-10-29 04:37 Subject: Re: [RFH] gcc constant expression warning... Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.999.0710282136410.30120%40woody.linux-foundation.org In-Reply-To: ``` On Sun, 28 Oct 2007, Nicolas Pitre wrote: > > The test must also make sure off_t isn't signed, since in that case it > can only hold 31 bits. Sinc eneither 31 _nor_ 32 bits is really enough, it's perfectly fine to just check that the size of "off_t" is *bigger* than 4 bytes, which my pseudo-patch did. Linus ```