Created attachment 46592 [details] gtest llvm small_vector It's google test for llvm small_vector. Compile: g++ -Wall -Wextra -O2 -Werror -fsanitize=address -c main.cpp And get: small_vector.h:505:7: ошибка: array subscript 1 is outside array bounds of «int [1]» [-Werror=array-bounds] 505 | ++EltPtr; | ^~ main.cpp:7:26: замечание: while referencing «<anonymous>» 7 | v.insert (v.begin (), 1); | If compile "g++ -Wall -Wextra -O2 -Werror -c main.cpp" there is no warning. I don't have this problem with gcc 8.3.0 . I compiled my project with gcc 9.1.0 without optimization and sanitizer don't detect problems in runtime.
I can reproduce the warning but I have a few comments: 1) A smaller test case would make the issue easier to debug. 2) The instrumentation inserted by the sanitizers is known to cause false positive warnings. This is unfortunate but until we come up with a solution we advise to either avoid combining sanitizers and warnings or be prepared for false positives. 3) The code in small_vector_impl::insert copied below doesn't look quite right to me: iterator insert(iterator I, const T &Elt) { ... // If we just moved the element we're inserting, be sure to update // the reference. const T *EltPtr = &Elt; if (I <= EltPtr && EltPtr < this->EndX) ++EltPtr; *I = ::std::move(*EltPtr); Calling it as is done in the test but on a non-empty vector: v.insert (v.begin (), 1); sets EltPtr to point to the temporary 1 while I ans this->EndX point to an object unrelated to the temporary. The result of an inequality expression between pointers to unrelated objects is unspecified in C++ and so the increment could in theory be evaluated regardless of whether Elt is a reference to an element of the vector or some other object (such as the temporary). A false positive warning can likely be reproduced with a valid test case but I don't think this one qualifies.
I remove gtest.h. #include "small_vector.h" class A { public: virtual void f () = 0; }; class B : public A { public: void f () override { small_vector<int, 20> v; v.insert (v.begin (), 1); } }; int main () { B b; (void)b; return 0; } I get the warning in this case. If comment inheritance then the warning disappears.
Created attachment 46593 [details] llvm small_vector
Just for the record, it started with r9-1948-gd893b683f40884cd00b5beb392566ecc7b67f721.
GCC 9.3.0 has been released, adjusting target milestone.
The testcase isn't very good, because it is optimized to nothing without the sanitization. But using __attribute__((used)) void f () override { small_vector<int, 20> v; v.insert (v.begin (), 1); __asm volatile ("" : : "r" (&v) : "memory"); } prevents that. With that, the dumps look comparable until fre3. Initially, I thought the issue is that ASAN_MARK ifn isn't said to affect just the pointed memory (and directly only). In reality it affects something different (the shadow memory corresponding to that memory), but perhaps it would work. Unfortunately: --- gcc/internal-fn.def.jj 2020-01-18 13:47:09.517357706 +0100 +++ gcc/internal-fn.def 2020-03-17 16:04:08.092861394 +0100 @@ -309,7 +309,7 @@ DEF_INTERNAL_FN (UBSAN_OBJECT_SIZE, ECF_ DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL) DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, "..R..") -DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, NULL) +DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, ".W.") DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) doesn't fix it. So, at the start of f without asan we have: v ={v} {CLOBBER}; MEM[(struct &)&v] ={v} {CLOBBER}; MEM[(struct small_vector_base *)&v] ={v} {CLOBBER}; MEM[(struct small_vector_base *)&v].BeginX = &MEM[(struct small_vector_template_common *)&v].FirstEl; MEM[(struct small_vector_base *)&v].EndX = &MEM[(struct small_vector_template_common *)&v].FirstEl; MEM[(struct small_vector_base *)&v].CapacityX = &MEM <union U> [(void *)&v + 104B]; D.48526 = 1; _17 = MEM[(struct small_vector_template_common *)&v].D.48095.EndX; if (&MEM[(struct small_vector_template_common *)&v].FirstEl == _17) where small_vector_template_common is derived from small_vector_base and thus MEM[(struct small_vector_template_common *)&v].D.48095 and MEM[(struct small_vector_base *)&v] is the same thing. Now, with asan we have: .ASAN_MARK (UNPOISON, &v, 104); v ={v} {CLOBBER}; MEM[(struct &)&v] ={v} {CLOBBER}; MEM[(struct small_vector_base *)&v] ={v} {CLOBBER}; MEM[(struct small_vector_base *)&v].BeginX = &MEM[(struct small_vector_template_common *)&v].FirstEl; MEM[(struct small_vector_base *)&v].EndX = &MEM[(struct small_vector_template_common *)&v].FirstEl; MEM[(struct small_vector_base *)&v].CapacityX = &MEM <union U> [(void *)&v + 104B]; .ASAN_MARK (UNPOISON, &D.48886, 4); D.48886 = 1; _10 = MEM[(struct small_vector_template_common *)&v].D.48455.BeginX; _20 = MEM[(struct small_vector_template_common *)&v].D.48455.EndX; if (_10 == _20) The only difference other than .ASAN_MARK calls is that in the asan case _10 hasn't been propagated into the comparison. Anyway, fre3 seems to be able to value number the _17 load to &MEM[(struct small_vector_template_common *)&v].FirstEl and thus optimize the comparison into true, while with the .ASAN_MARK calls it doesn't.
Ah, should have been: --- gcc/internal-fn.def.jj 2020-01-18 13:47:09.517357706 +0100 +++ gcc/internal-fn.def 2020-03-17 16:04:08.092861394 +0100 @@ -309,7 +309,7 @@ DEF_INTERNAL_FN (UBSAN_OBJECT_SIZE, ECF_ DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL) DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, "..R..") -DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, NULL) +DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, "..W.") DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) the ifn has 3 arguments and the pointer is the middle one. Anyway, even with this sccvn isn't able to see through it.
Created attachment 48050 [details] pr91146.ii.xz Preprocessed source. -O2 -fsanitize=address -Wall
Seems even with that patch 3677 if (stmt_may_clobber_ref_p_1 (def_stmt, ref, tbaa_p)) in tree-ssa-sccvn.c thinks the .ASAN_MARK (UNPOISON, &D.48886, 4); call may clobber MEM[(struct small_vector_template_common *)&v].D.48455.BeginX With the following completely untested patch we optimize like withouf -fsanitize=address and don't warn. --- gcc/internal-fn.def.jj 2020-01-18 13:47:09.517357706 +0100 +++ gcc/internal-fn.def 2020-03-17 16:32:41.210800195 +0100 @@ -309,7 +309,7 @@ DEF_INTERNAL_FN (UBSAN_OBJECT_SIZE, ECF_ DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL) DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, "..R..") -DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, NULL) +DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, "..W.") DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL) DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) --- gcc/tree-ssa-alias.c.jj 2020-03-03 11:04:46.367821907 +0100 +++ gcc/tree-ssa-alias.c 2020-03-17 17:40:22.295385869 +0100 @@ -3032,6 +3032,14 @@ call_may_clobber_ref_p_1 (gcall *call, a default: /* Fallthru to general call handling. */; } + else if (gimple_call_internal_p (call) + && gimple_call_internal_fn (call) == IFN_ASAN_MARK) + { + ao_ref dref; + tree size = gimple_call_arg (call, 2); + ao_ref_init_from_ptr_and_size (&dref, gimple_call_arg (call, 1), size); + return refs_may_alias_p_1 (&dref, ref, false); + } /* Check if base is a global static variable that is not written by the function. */
The #c9 patch passed bootstrap, but regresses: FAIL: g++.dg/asan/use-after-scope-types-1.C -O1 execution test FAIL: g++.dg/asan/use-after-scope-types-1.C -O2 execution test FAIL: g++.dg/asan/use-after-scope-types-1.C -O3 -g execution test FAIL: g++.dg/asan/use-after-scope-types-1.C -Os execution test FAIL: g++.dg/asan/use-after-scope-types-1.C -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test FAIL: g++.dg/asan/use-after-scope-types-1.C -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test FAIL: g++.dg/asan/use-after-scope-types-2.C -O1 execution test FAIL: g++.dg/asan/use-after-scope-types-2.C -O2 execution test FAIL: g++.dg/asan/use-after-scope-types-2.C -O3 -g execution test FAIL: g++.dg/asan/use-after-scope-types-2.C -Os execution test FAIL: g++.dg/asan/use-after-scope-types-2.C -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test FAIL: g++.dg/asan/use-after-scope-types-2.C -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test FAIL: g++.dg/asan/use-after-scope-types-3.C -O1 execution test FAIL: g++.dg/asan/use-after-scope-types-3.C -O2 execution test FAIL: g++.dg/asan/use-after-scope-types-3.C -O3 -g execution test FAIL: g++.dg/asan/use-after-scope-types-3.C -Os execution test FAIL: g++.dg/asan/use-after-scope-types-3.C -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test FAIL: g++.dg/asan/use-after-scope-types-3.C -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test FAIL: g++.dg/asan/use-after-scope-types-5.C -O1 execution test FAIL: g++.dg/asan/use-after-scope-types-5.C -O2 execution test FAIL: g++.dg/asan/use-after-scope-types-5.C -O3 -g execution test FAIL: g++.dg/asan/use-after-scope-types-5.C -Os execution test FAIL: g++.dg/asan/use-after-scope-types-5.C -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test FAIL: g++.dg/asan/use-after-scope-types-5.C -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test The thing is that those tests do those use after scope accesses but don't really have anything afterwards that would allow optimizing those accesses away. With the previous behavior of ASAN_MARK ifns such barriers were in those ifns. So, the big question is, what do we want. For -O0 -fsanitize=address both the unpatched and patched versions work similarly, and for -O1 -fsanitize=address and higher, do we want the sanitizer to detect just those after scope uses that would remain after optimization even in non-instrumented code? Then we want the patch. Or do we want to detect more after scope uses, even at the expense of some false positive warnings? If I try clang++ -O{0,1} -fsanitize=address on the use-after-scope-types-1.C testcase, then it detects the violation (patched g++ with already -O1 -fsanitize=address doesn't), but clang++ -O{2,3} -fsanitize=address doesn't detect it (while unpatched g++ -O{2,3} -fsanitize=address does).
Regarding the original testcase, small tweak results in the same warning even without sanitization: #include "pr91146.h" class A { public: virtual void f () = 0; }; template <typename T> void foo (T &); class B : public A { public: void f () override { small_vector<int, 20> v; auto i = v.begin (); foo (i); v.insert (i, 1); foo (v); } }; int main () { B b; b.f (); return 0; } Here we just make sure the method isn't optimized away completely and that the optimizer doesn't know v.insert is called with v.begin () iterator. And the problem is that the llvm small vector implementation does: iterator insert(iterator I, T &&Elt) { ... T *EltPtr = &Elt; if (I <= EltPtr && EltPtr < this->EndX) ++EltPtr; *I = ::std::move(*EltPtr); return I; } *++EltPtr on an int (not array) is obviously UB, but the condition should guard that this doesn't happen. Except that there is UB already in the condition that guards this, because non-equality pointer comparisons are valid only if both pointers point into the same object or one past the end of it, or both are NULL. That isn't the case here, as I points into the payload of the v variable and so does this->endX, while EltPtr points to a temporary holding 1. So, I'd say this is LLVM bug.
Though, admittedly fixing that: T *EltPtr = &Elt; - if (I <= EltPtr && EltPtr < this->EndX) + if ((std::uintptr_t) I <= (std::uintptr_t) EltPtr && (std::uintptr_t) EltPtr < (std::uintptr_t) this->EndX) ++EltPtr; *I = ::std::move(*EltPtr); doesn't make the warning go away. We have: int D.48534; ... <bb 30> [local count: 885408257]: # EltPtr_49 = PHI <&D.48534(27), &D.48534(28), &MEM <int> [(void *)&D.48534 + 4B](29)> _50 = MEM[(type &)EltPtr_49]; *I_42 = _50; before phiprop and phiprop changes that into: <bb 38> [local count: 442704129]: _33 = MEM[(type &)&D.48534]; goto <bb 30>; [100.00%] ... <bb 39> [local count: 221352064]: _34 = MEM[(type &)&D.48534]; goto <bb 30>; [100.00%] <bb 29> [local count: 221352064]: _48 = MEM[(type &)&D.48534 + 4]; <bb 30> [local count: 885408257]: # EltPtr_49 = PHI <&MEM <int> [(void *)&D.48534 + 4B](29), &D.48534(38), &D.48534(39)> # _50 = PHI <_48(29), _33(38), _34(39)> *I_42 = _50; where the _48 load is clearly UB, but with -fsanitize=address the compiler can't prove that it is unreachable, or with the adjusted testcase either. So we are back to the dozens of other PRs of this kind, late warnings warning on UB in dead code and the question what the users actually want, whether a (false positive) warning, or no warning, or whether the compiler should replace the UB code with __builtin_unreachable and let the code be further simplified, or replace it with __builtin_trap.
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
GCC 9 branch is being closed
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
GCC 10 branch is being closed.