[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146 Jeffrey A. Law changed: What|Removed |Added CC||law at redhat dot com Priority|P3 |P2
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146 --- Comment #12 from Jakub Jelinek --- Though, admittedly fixing that: T *EltPtr = -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; ... [local count: 885408257]: # EltPtr_49 = PHI <(27), (28), [(void *) + 4B](29)> _50 = MEM[(type &)EltPtr_49]; *I_42 = _50; before phiprop and phiprop changes that into: [local count: 442704129]: _33 = MEM[(type &)]; goto ; [100.00%] ... [local count: 221352064]: _34 = MEM[(type &)]; goto ; [100.00%] [local count: 221352064]: _48 = MEM[(type &) + 4]; [local count: 885408257]: # EltPtr_49 = PHI < [(void *) + 4B](29), (38), (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.
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146
--- Comment #11 from Jakub Jelinek ---
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
void foo (T &);
class B : public A
{
public:
void f () override
{
small_vector 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 &) {
...
T *EltPtr =
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.
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146
Jakub Jelinek changed:
What|Removed |Added
CC||marxin at gcc dot gnu.org
--- Comment #10 from Jakub Jelinek ---
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).
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146
--- Comment #9 from Jakub Jelinek ---
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, , 4);
call may clobber
MEM[(struct small_vector_template_common *)].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.c2020-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 (, gimple_call_arg (call, 1), size);
+ return refs_may_alias_p_1 (, ref, false);
+}
/* Check if base is a global static variable that is not written
by the function. */
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146 --- Comment #8 from Jakub Jelinek --- Created attachment 48050 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48050=edit pr91146.ii.xz Preprocessed source. -O2 -fsanitize=address -Wall
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146 --- Comment #7 from Jakub Jelinek --- 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.
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #6 from Jakub Jelinek ---
The testcase isn't very good, because it is optimized to nothing without the
sanitization.
But using
__attribute__((used)) void f () override
{
small_vector v;
v.insert (v.begin (), 1);
__asm volatile ("" : : "r" () : "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} {CLOBBER};
MEM[(struct small_vector_base *)] ={v} {CLOBBER};
MEM[(struct small_vector_base *)].BeginX = [(struct
small_vector_template_common *)].FirstEl;
MEM[(struct small_vector_base *)].EndX = [(struct
small_vector_template_common *)].FirstEl;
MEM[(struct small_vector_base *)].CapacityX = [(void *) +
104B];
D.48526 = 1;
_17 = MEM[(struct small_vector_template_common *)].D.48095.EndX;
if ([(struct small_vector_template_common *)].FirstEl == _17)
where small_vector_template_common is derived from small_vector_base and thus
MEM[(struct small_vector_template_common *)].D.48095 and MEM[(struct
small_vector_base *)] is the same thing.
Now, with asan we have:
.ASAN_MARK (UNPOISON, , 104);
v ={v} {CLOBBER};
MEM[(struct &)] ={v} {CLOBBER};
MEM[(struct small_vector_base *)] ={v} {CLOBBER};
MEM[(struct small_vector_base *)].BeginX = [(struct
small_vector_template_common *)].FirstEl;
MEM[(struct small_vector_base *)].EndX = [(struct
small_vector_template_common *)].FirstEl;
MEM[(struct small_vector_base *)].CapacityX = [(void *) +
104B];
.ASAN_MARK (UNPOISON, , 4);
D.48886 = 1;
_10 = MEM[(struct small_vector_template_common *)].D.48455.BeginX;
_20 = MEM[(struct small_vector_template_common *)].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 [(struct
small_vector_template_common *)].FirstEl and thus optimize the comparison
into true, while with the .ASAN_MARK calls it doesn't.
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146 Jakub Jelinek changed: What|Removed |Added Target Milestone|9.3 |9.4 --- Comment #5 from Jakub Jelinek --- GCC 9.3.0 has been released, adjusting target milestone.
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146 Richard Biener changed: What|Removed |Added Target Milestone|--- |9.3
[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146 Martin Liška changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2020-01-23 CC||marxin at gcc dot gnu.org Known to work||8.3.0 Summary|-Werror=array-bounds if |[9/10 Regression] |compile with|-Werror=array-bounds if |-fsanitize=address |compile with ||-fsanitize=address Ever confirmed|0 |1 Known to fail||10.0, 9.2.0 --- Comment #4 from Martin Liška --- Just for the record, it started with r9-1948-gd893b683f40884cd00b5beb392566ecc7b67f721.
