[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address

[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.3                         |9.4

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48050
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48050&action=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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.  */

[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug middle-end/91146] [9/10 Regression] -Werror=array-bounds if compile with -fsanitize=address

[law at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com
           Priority|P3                          |P2

[Bug middle-end/91146] [9/10/11/12 Regression] -Werror=array-bounds if compile with -fsanitize=address

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.4                         |9.5

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.

[Bug middle-end/91146] [10/11/12/13 Regression] -Werror=array-bounds if compile with -fsanitize=address

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug middle-end/91146] [10/11/12/13 Regression] -Werror=array-bounds if compile with -fsanitize=address

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug middle-end/91146] [11/12/13/14 Regression] -Werror=array-bounds if compile with -fsanitize=address

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91146

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.