Google ref: b/8856275 Test case: typedef decltype(sizeof(0)) size_t; struct Foo { ~Foo(); // comment out -> problem disappears int x[20]; }; template <typename T, int N> struct InlinedVector { inline InlinedVector() : size_(0) { } inline size_t size() const { return size_; } T* mutable_array(); void resize(size_t n) { size_t s = size(); DestroyElements(mutable_array() + n, s - n); } static void DestroyElements(T* ptr, size_t num) { for (size_t i = 0; i < num; i++) { ptr[i].~T(); } } size_t size_; }; struct CP { void Add(); InlinedVector<Foo,1> foo_; }; void CP::Add() { foo_.resize(foo_.size() + 1); } Using trunk r198689: g++ -std=c++11 -O2 -c tt.cc tt.cc: In member function ‘void CP::Add()’: tt.cc:21:7: warning: iteration 230584300921369396ul invokes undefined behavior [-Waggressive-loop-optimizations] ptr[i].~T(); ^ tt.cc:20:5: note: containing loop for (size_t i = 0; i < num; i++) { ^
Note that there seems to be an error in the source: void resize(size_t n) { size_t s = size(); DestroyElements(mutable_array() + n, s - n); for the call with n == size () + 1 the function calls DestroyElements with -1U elements which we figure out and optimize the caller to <bb 3>: _13 = i_12 * 80; _14 = _11 + _13; Foo::~Foo (_14); i_15 = i_12 + 1; <bb 4>: # i_12 = PHI <0(2), i_15(3)> if (i_12 != 18446744073709551615) goto <bb 3>; else goto <bb 5>; which then causes the addition to overflow and invoke undefined behavior and we warn. Thus, the warning, while not 100% helpful, points at a real problem.
> Thus, the warning, while not 100% helpful, points at a real problem. There is no real problem in the original source, which reads: size_t s = size(); if (n < s) DestroyElements(mutable_array() + n, s - n); else ... and produces the same warning. Here is updated test with above condition; confirmed to still show the warning with trunk @r198709. typedef decltype(sizeof(0)) size_t; struct Foo { ~Foo(); // comment out -> problem disappears int x[20]; }; template <typename T, int N> struct InlinedVector { inline InlinedVector() : size_(0) { } inline size_t size() const { return size_; } T* mutable_array(); void resize(size_t n) { size_t s = size(); if (n < s) DestroyElements(mutable_array() + n, s - n); } static void DestroyElements(T* ptr, size_t num) { for (size_t i = 0; i < num; i++) { ptr[i].~T(); } } size_t size_; }; struct CP { void Add(); InlinedVector<Foo,1> foo_; }; void CP::Add() { foo_.resize(foo_.size() + 1); }
I don't think the warning is bogus. At phiprop we have: long unsigned int _3; size_t _6; size_t _7; long unsigned int _8; <bb 2>: _6 = MEM[(const struct InlinedVector *)this_1(D)].size_; _3 = _6 + 1; ... _7 = MEM[(const struct InlinedVector *)this_1(D)].size_; if (_3 < _7) goto <bb 3>; else goto <bb 6>; <bb 3>: _8 = _7 - _3; and fre2 turns this into: _6 = MEM[(const struct InlinedVector *)this_1(D)].size_; _3 = _6 + 1; ... _7 = _6; if (_3 < _7) goto <bb 3>; else goto <bb 6>; <bb 3>: _8 = 18446744073709551615; i.e. it can't prove the following loop that uses _8 as upper bound is dead, but the loop has at that point constant bounds and triggers undefined behavior in it if executed. The loop can be only executed if foo_.size() is (size_t) -1, but if you call it with this value, you'll surely hit the undefined behavior the warning is complaining about.
(In reply to Jakub Jelinek from comment #3) > it can't prove the following loop that uses _8 as upper bound is dead, ... Do we need a separate "may invoke undefined behavior" warning? In our codebase of 100+ million lines of C++, we've found ~20 instances of real "invokes undefined behavior" (so we don't want to turn the -Waggressive-loop-optimizations off). But we also have ~5 instances where the warning is bogus, and there is no easy way to turn them off, short of sprinkling -Wno-aggresive-loop-optimizations over our BUILD files.
But this isn't any form of the may invoke, the loop certainly unconditionally invokes undefined behavior, just the whole loop is very unlikely to be ever executed (in this case if size is supposed to represent the length of an array with elements bigger than 1, then already the size would need to be invalid, but that is something the compiler can't understand, for it the size_t field is likely any other field, and there is no guarantee it won't be -1). It is in principle no different from say: void foo (size_t x) { if (x == (size_t) -1) { unsigned int a[128]; int i; for (i = 0; i < 128; ++i) /* { dg-message "note: containing loop" } */ a[i] = i * 0x02000001; /* { dg-warning "invokes undefined behavior" } */ bar (a); } } where you know you are never going to call foo with (size_t) -1, but the compiler doesn't know. How is the above different from say: void bar (void) { unsigned int a[128]; int i; for (i = 0; i < 128; ++i) /* { dg-message "note: containing loop" } */ a[i] = i * 0x02000001; /* { dg-warning "invokes undefined behavior" } */ bar (a); } ... /* in another CU */ void baz (size_t x) { if (x == (size_t) -1) bar (); } In your original testcase, you wouldn't get the warning if size was a signed integer instead of unsigned one, then the compiler would know it is undefined behavior if the size wraps and would just optimize the loop away altogether. Or perhaps some __builtin_unreachable assert that size isn't (size_t) -1?
> perhaps some __builtin_unreachable assert that size isn't (size_t) -1? That works. Thanks for the suggestion.
(In reply to Paul Pluzhnikov from comment #6) > > perhaps some __builtin_unreachable assert that size isn't (size_t) -1? > > That works. Thanks for the suggestion. Please, can you explain how you fixed this? We have almost the same problem.
(In reply to Mikhail Veltishchev from comment #7) > Please, can you explain how you fixed this? We have almost the same problem. Here is the fix we deployed (test case from comment#2): $ diff -u pr57199.cc pr57199a.cc --- pr57199.cc 2014-03-26 13:50:25.682351842 -0700 +++ pr57199a.cc 2014-03-26 13:54:12.880279926 -0700 @@ -18,6 +18,9 @@ DestroyElements(mutable_array() + n, s - n); } static void DestroyElements(T* ptr, size_t num) { + if (__builtin_expect (num == (size_t) -1, 0)) { + __builtin_unreachable(); + } for (size_t i = 0; i < num; i++) { ptr[i].~T(); }