[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[bonzini at gnu dot org]

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

Paolo Bonzini  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #15 from Paolo Bonzini  ---
Reverted.

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[bonzini at gnu dot org]

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

--- Comment #14 from Paolo Bonzini  ---
I'll just revert the original PR84307 patch.  Changing the fnspec has way too
many ramifications.  PR84307 can either be fixed with an early UNPOISON
elimination pass, or delayed to GCC 9 where we can play with pass reordering.

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[bonzini at gnu dot org]

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

--- Comment #13 from Paolo Bonzini  ---
Author: bonzini
Date: Tue Feb 13 13:03:22 2018
New Revision: 257625

URL: https://gcc.gnu.org/viewcvs?rev=257625=gcc=rev
Log:
gcc:
2018-02-13  Paolo Bonzini 

PR sanitizer/84340
* internal-fn.def (ASAN_CHECK, ASAN_MARK): Revert changes to fnspec.

gcc/testsuite:
2018-02-13  Paolo Bonzini  

PR sanitizer/84307
* gcc.dg/asan/pr84307.c: Remove test.


Removed:
trunk/gcc/testsuite/gcc.dg/asan/pr84307.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/internal-fn.def
trunk/gcc/testsuite/ChangeLog

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[bonzini at gnu dot org]

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

--- Comment #12 from Paolo Bonzini  ---
No, I don't think computing a shadow memory address counts as memory
indirection.

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[marxin at gcc dot gnu.org]

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

--- Comment #11 from Martin Liška  ---
(In reply to Paolo Bonzini from comment #10)
> > Note that we only instrument ASAN_CHECK for memory references. x=0 is not 
> > that 
> > case.
> 
> That depends... in use-after-scope-types-1.C there is inlining involved. 
> With my pass ordering change ASAN_CHECK is added before inlining, when there
> is still a *ptr.t dereference.  After inlining you get:
> 

Note that we're in stage4 and the pass reordering is not possible now. Thus we
should fix current regressions. Is the consensus that we can't use 'W' and 'R'
fn spec as there's memory indirection (which happens for shadow memory)?

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[bonzini at gnu dot org]

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

--- Comment #10 from Paolo Bonzini  ---
> Note that we only instrument ASAN_CHECK for memory references. x=0 is not 
> that 
> case.

That depends... in use-after-scope-types-1.C there is inlining involved.  With
my pass ordering change ASAN_CHECK is added before inlining, when there is
still a *ptr.t dereference.  After inlining you get:

  ASAN_MARK (UNPOISON, , 8);
  ASAN_MARK (UNPOISON, , 1);
  ASAN_CHECK (7, , 8, 8);
  _9 = 
  ASAN_CHECK (7, _9, 8, 8);
  ptr.t = 
  ASAN_MARK (POISON, , 1);
  ASAN_CHECK (6, , 8, 8);
  ASAN_CHECK (7, , 1, 1);
  _10 = 
  ASAN_CHECK (7, _10, 1, 8);
  x = 0;
  ASAN_MARK (POISON, , 8);

But even if this weren't the case, the ASAN_MARK would be lowered to
ASAN_POISON() and everything seems to work:

int main()
{
bool *ptr;
{
bool x;
ptr = 
}
return *ptr;
}

becomes

  x_8 = ASAN_POISON ();
  _6 = (int) x_8;
  return _6;

(and it is very similar to gcc.dg/asan/use-after-scope-10.c).

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[marxin at gcc dot gnu.org]

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

--- Comment #9 from Martin Liška  ---
(In reply to Paolo Bonzini from comment #7)
> The problem is not the transformation from *ptr to x, the problem is that
> x=0 is later considered dead because ASAN_CHECK references are introduced
> too late.

Note that we only instrument ASAN_CHECK for memory references. x=0 is not that
case.

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[marxin at gcc dot gnu.org]

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

--- Comment #8 from Martin Liška  ---
(In reply to Richard Biener from comment #6)
> (In reply to Richard Biener from comment #5)
> > Doens't have anything todo with EAF_DIRECT (which is correct) but POISON has
> > to be marked with reading from the argument as otherwise stores are not kept
> > live.
> 
> Ah, no, in this case it is the EAF_DIRECT.  But I'm not sure how ASAN works
> here

ASAN_CHECK(In reply to Richard Biener from comment #6)
> (In reply to Richard Biener from comment #5)
> > Doens't have anything todo with EAF_DIRECT (which is correct) but POISON has
> > to be marked with reading from the argument as otherwise stores are not kept
> > live.
> 
> Ah, no, in this case it is the EAF_DIRECT.  But I'm not sure how ASAN works
> here

E.g. ASAN_MARK (UNPOISON, , 8); is later expanded to:

  _1 = 
  _13 = (unsigned long) _1;
  _14 = _13 >> 3;
  _15 = _14 + 2147450880;
  _16 = (signed char *) _15;
  *_16 = 0;

Thus EAF_DIRECT shoud NOT be set. Or do I miss something?

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[bonzini at gnu dot org]

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

--- Comment #7 from Paolo Bonzini  ---
The problem is not the transformation from *ptr to x, the problem is that x=0
is later considered dead because ASAN_CHECK references are introduced too late.

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener  ---
(In reply to Richard Biener from comment #5)
> Doens't have anything todo with EAF_DIRECT (which is correct) but POISON has
> to be marked with reading from the argument as otherwise stores are not kept
> live.

Ah, no, in this case it is the EAF_DIRECT.  But I'm not sure how ASAN works
here

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener  ---
Doens't have anything todo with EAF_DIRECT (which is correct) but POISON has to
be marked with reading from the argument as otherwise stores are not kept live.

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška  ---
There's example of how a check is optimized out:

before the r257585:

__attribute__((noinline))
void test() [with T = bool] ()
{
  bool x;
  struct Ptr ptr;
  bool * _8;

   [100.00%]:
  ASAN_MARK (UNPOISON, , 8);
  ASAN_MARK (UNPOISON, , 1);
  ptr.t = 
  ASAN_MARK (POISON, , 1);
  _8 = ptr.t;
  *_8 = 0;
  ASAN_MARK (POISON, , 8);
  return;

}

after:

__attribute__((noinline))
test ()
{
  bool x;
  struct Ptr ptr;

   :
  ASAN_MARK (UNPOISON, , 8);
  ASAN_MARK (UNPOISON, , 1);
  ptr.t = 
  ASAN_MARK (POISON, , 1);
  x = 0;
  ASAN_MARK (POISON, , 8);
  return;

}

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[marxin at gcc dot gnu.org]

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

--- Comment #3 from Martin Liška  ---
Created attachment 43401
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43401=edit
Untested patch

Problem here is that we can't have 'W' ('R' respectively) as the arguments are
read addresses and the internal functions write/read to shadow memory. Thus
can't have EAF_DIRECT flag set:

/* Call argument flags.  */
/* Nonzero if the argument is not dereferenced recursively, thus only
   directly reachable memory is read or written.  */
#define EAF_DIRECT  (1 << 0)

int
gimple_call_arg_flags (const gcall *stmt, unsigned arg)
{
  const_tree attr = gimple_call_fnspec (stmt);

  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
return 0;

  switch (TREE_STRING_POINTER (attr)[1 + arg])
{
case 'x':
case 'X':
  return EAF_UNUSED;

case 'R':
  return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;

case 'r':
  return EAF_NOCLOBBER | EAF_NOESCAPE;

case 'W':
  return EAF_DIRECT | EAF_NOESCAPE;

case 'w':
  return EAF_NOESCAPE;

case '.':
default:
  return 0;
}
}

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[marxin at gcc dot gnu.org]

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

Martin Liška  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2018-02-13
   Assignee|unassigned at gcc dot gnu.org  |marxin at gcc dot 
gnu.org
 Ever confirmed|0   |1

--- Comment #2 from Martin Liška  ---
Let me take a look.

[Bug sanitizer/84340] [8 regression] g++.dg/asan/use-after-scope-types-1.C (and others) fails after r257585

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

   Target Milestone|--- |8.0

--- Comment #1 from Jakub Jelinek  ---
It fails on x86_64-linux and i686-linux as well.