[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

Martin Liška  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #21 from Martin Liška  ---
Fixed.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[jakub at gcc dot gnu.org]

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

--- Comment #20 from Jakub Jelinek  ---
That would be my preference (perhaps even for GCC 7, only do the warning and
attribute removal on the trunk).

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #19 from Martin Liška  ---
(In reply to Martin Liška from comment #18)
> (In reply to Jakub Jelinek from comment #17)
> > (In reply to Martin Liška from comment #16)
> > > --- a/gcc/c-family/c-common.c
> > > +++ b/gcc/c-family/c-common.c
> > > @@ -9447,7 +9447,6 @@ handle_target_attribute (tree *node, tree name, tree
> > > args, int flags,
> > > && TREE_STRING_LENGTH (value) == 1
> > > && TREE_STRING_POINTER (value)[0] == '\0')
> > >   {
> > > -   warning (OPT_Wattributes, "empty string in attribute %");
> > > *no_add_attrs = true;
> > >   }
> > >  }
> > > 
> > > That will however skip entire attribute:
> > > __attribute__((target("sse4.2", "", "")))
> > > 
> > > Or I can do patch that will just remove empty strings in a TREE_LIST.
> > > What way do you prefer?
> > 
> > Something that doesn't change behavior.  So the latter is better than the
> > former, but I wonder if just some return or continue when seeing empty
> > string later on isn't even easier/safer.
> 
> Well, for this purpose, maybe the original patch can be handy:
> https://patchwork.ozlabs.org/patch/794801/ ?

This one works for some situations, however it's quite painful to handle it
properly, for instance:

$ cat /tmp/ice.C
__attribute__((target("default")))
int foo() {return 99;}
__attribute__((target("sse4.2","")))
int foo() {return 1;}
__attribute__((target("","")))
int foo() {return 1;}

int main() {
return foo();
}

$ ./xg++ -B. /tmp/ice.C -S -c
/tmp/ice.C: In function ‘’:
/tmp/ice.C:6:5: error: No dispatcher found for the versioning attributes : 
 int foo() {return 1;}
 ^~~

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #18 from Martin Liška  ---
(In reply to Jakub Jelinek from comment #17)
> (In reply to Martin Liška from comment #16)
> > --- a/gcc/c-family/c-common.c
> > +++ b/gcc/c-family/c-common.c
> > @@ -9447,7 +9447,6 @@ handle_target_attribute (tree *node, tree name, tree
> > args, int flags,
> >   && TREE_STRING_LENGTH (value) == 1
> >   && TREE_STRING_POINTER (value)[0] == '\0')
> > {
> > - warning (OPT_Wattributes, "empty string in attribute %");
> >   *no_add_attrs = true;
> > }
> >  }
> > 
> > That will however skip entire attribute:
> > __attribute__((target("sse4.2", "", "")))
> > 
> > Or I can do patch that will just remove empty strings in a TREE_LIST.
> > What way do you prefer?
> 
> Something that doesn't change behavior.  So the latter is better than the
> former, but I wonder if just some return or continue when seeing empty
> string later on isn't even easier/safer.

Well, for this purpose, maybe the original patch can be handy:
https://patchwork.ozlabs.org/patch/794801/ ?

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[jakub at gcc dot gnu.org]

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

--- Comment #17 from Jakub Jelinek  ---
(In reply to Martin Liška from comment #16)
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -9447,7 +9447,6 @@ handle_target_attribute (tree *node, tree name, tree
> args, int flags,
> && TREE_STRING_LENGTH (value) == 1
> && TREE_STRING_POINTER (value)[0] == '\0')
>   {
> -   warning (OPT_Wattributes, "empty string in attribute %");
> *no_add_attrs = true;
>   }
>  }
> 
> That will however skip entire attribute:
> __attribute__((target("sse4.2", "", "")))
> 
> Or I can do patch that will just remove empty strings in a TREE_LIST.
> What way do you prefer?

Something that doesn't change behavior.  So the latter is better than the
former, but I wonder if just some return or continue when seeing empty string
later on isn't even easier/safer.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #16 from Martin Liška  ---
(In reply to Jakub Jelinek from comment #15)
> That looks risky to me, changes behavior.  Can't you instead of the warning
> + removal from attributes just do something that doesn't crash when it sees
> an empty string wherever it crashed?

Yes, we can either:

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 5089013ddcd..17c98bcba8b 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -9447,7 +9447,6 @@ handle_target_attribute (tree *node, tree name, tree
args, int flags,
  && TREE_STRING_LENGTH (value) == 1
  && TREE_STRING_POINTER (value)[0] == '\0')
{
- warning (OPT_Wattributes, "empty string in attribute %");
  *no_add_attrs = true;
}
 }

That will however skip entire attribute:
__attribute__((target("sse4.2", "", "")))

Or I can do patch that will just remove empty strings in a TREE_LIST.
What way do you prefer?

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[jakub at gcc dot gnu.org]

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

--- Comment #15 from Jakub Jelinek  ---
That looks risky to me, changes behavior.  Can't you instead of the warning +
removal from attributes just do something that doesn't crash when it sees an
empty string wherever it crashed?

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #14 from Martin Liška  ---
(In reply to Martin Liška from comment #13)
> (In reply to Jakub Jelinek from comment #12)
> > Or just remove or xfail the testcase.  The asan.c change is right even for
> > the branches...
> > 
> > BTW, just for completeness, I'm also seeing
> > +FAIL: gcc.target/i386/pr69255-2.c (test for excess errors)
> > +FAIL: gcc.target/i386/pr69255-3.c (test for excess errors)
> > regression on 6.x branch, in that case no idea what has caused it, just that
> > it appeared in between June 22nd and September 15th.
> 
> I'll mark them as XFAIL then.
> 
> The i386 test-case started to fail with mine r252795. I'll take a look.

We need there your revision r242740. May I backport it? Or just the hunk that
adjusts the test-case?

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #13 from Martin Liška  ---
(In reply to Jakub Jelinek from comment #12)
> Or just remove or xfail the testcase.  The asan.c change is right even for
> the branches...
> 
> BTW, just for completeness, I'm also seeing
> +FAIL: gcc.target/i386/pr69255-2.c (test for excess errors)
> +FAIL: gcc.target/i386/pr69255-3.c (test for excess errors)
> regression on 6.x branch, in that case no idea what has caused it, just that
> it appeared in between June 22nd and September 15th.

I'll mark them as XFAIL then.

The i386 test-case started to fail with mine r252795. I'll take a look.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek  ---
Or just remove or xfail the testcase.  The asan.c change is right even for the
branches...

BTW, just for completeness, I'm also seeing
+FAIL: gcc.target/i386/pr69255-2.c (test for excess errors)
+FAIL: gcc.target/i386/pr69255-3.c (test for excess errors)
regression on 6.x branch, in that case no idea what has caused it, just that it
appeared in between June 22nd and September 15th.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

Martin Liška  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #11 from Martin Liška  ---
You're right, but it's not ASAN-related: one needs to backport r236630
(PR70434).
I guess it's not subject for backport. Thus I'll probably revert the patch?

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek  ---
Also fails on 5.x branch.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek  ---
Are you sure?
At least on gcc-6-branch I'm seeing:
spawn -ignore SIGHUP /usr/src/gcc-6/obj28/gcc/xgcc -B/usr/src/gcc-6/obj28/gcc/
/usr/src/gcc-6/gcc/testsuite/gcc.dg/asan/pr81224.c
-B/usr/src/gcc-6/obj28/x86_64-pc-linux-gnu/./libsanitizer/
-B/usr/src/gcc-6/obj28/x86_64-pc-linux-gnu/./libsanitizer/asan/
-L/usr/src/gcc-6/obj28/x86_64-pc-linux-gnu/./libsanitizer/asan/.libs
-fsanitize=address -g -I/usr/src/gcc-6/gcc/testsuite/../../libsanitizer/include
-fno-diagnostics-show-caret -fdiagnostics-color=never -O0 -msse2 -S -o
pr81224.s
/usr/src/gcc-6/gcc/testsuite/gcc.dg/asan/pr81224.c: In function 'b':
/usr/src/gcc-6/gcc/testsuite/gcc.dg/asan/pr81224.c:10:11: internal compiler
error: in expand_expr_addr_expr_1, at expr.c:7672
0x868fe1 expand_expr_addr_expr_1
../../gcc/expr.c:7672
0x84ca06 expand_expr_addr_expr
../../gcc/expr.c:7785
0x84ca06 expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
../../gcc/expr.c:10869
0x85b820 expand_expr
../../gcc/expr.h:260
0x85b820 expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**,
rtx_def**, expand_modifier)
../../gcc/expr.c:7554
0x863113 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode,
expand_modifier)
../../gcc/expr.c:8395
0x73251d expand_gimple_stmt_1
../../gcc/cfgexpand.c:3653
0x73251d expand_gimple_stmt
../../gcc/cfgexpand.c:3714
0x733f8e expand_gimple_basic_block
../../gcc/cfgexpand.c:5720
0x739896 execute
../../gcc/cfgexpand.c:6335
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
compiler exited with status 1
FAIL: gcc.dg/asan/pr81224.c   -O0  (internal compiler error)
FAIL: gcc.dg/asan/pr81224.c   -O0  (test for excess errors)
and similarly for other -O levels.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

Martin Liška  changed:

   What|Removed |Added

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

--- Comment #8 from Martin Liška  ---
Fixed on all active branches.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #7 from Martin Liška  ---
Author: marxin
Date: Fri Sep 15 12:15:52 2017
New Revision: 252814

URL: https://gcc.gnu.org/viewcvs?rev=252814=gcc=rev
Log:
Backport r249728

2017-09-15  Martin Liska  

Backport from mainline
2017-06-28  Martin Liska  

PR sanitizer/81224
* asan.c (instrument_derefs): Bail out inner references
that are hard register variables.
2017-09-15  Martin Liska  

Backport from mainline
2017-06-28  Martin Liska  

PR sanitizer/81224
* gcc.dg/asan/pr81224.c: New test.

Added:
branches/gcc-6-branch/gcc/testsuite/gcc.dg/asan/pr81224.c
Modified:
branches/gcc-6-branch/gcc/ChangeLog
branches/gcc-6-branch/gcc/asan.c
branches/gcc-6-branch/gcc/testsuite/ChangeLog

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #6 from Martin Liška  ---
Author: marxin
Date: Fri Sep 15 12:14:40 2017
New Revision: 252813

URL: https://gcc.gnu.org/viewcvs?rev=252813=gcc=rev
Log:
Backport r249728

2017-09-15  Martin Liska  

Backport from mainline
2017-06-28  Martin Liska  

PR sanitizer/81224
* asan.c (instrument_derefs): Bail out inner references
that are hard register variables.
2017-09-15  Martin Liska  

Backport from mainline
2017-06-28  Martin Liska  

PR sanitizer/81224
* gcc.dg/asan/pr81224.c: New test.

Added:
branches/gcc-5-branch/gcc/testsuite/gcc.dg/asan/pr81224.c
Modified:
branches/gcc-5-branch/gcc/ChangeLog
branches/gcc-5-branch/gcc/asan.c
branches/gcc-5-branch/gcc/testsuite/ChangeLog

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #5 from Martin Liška  ---
Author: marxin
Date: Thu Jul 27 07:31:19 2017
New Revision: 250603

URL: https://gcc.gnu.org/viewcvs?rev=250603=gcc=rev
Log:
Backport r249728

2017-07-27  Martin Liska  

Backport from mainline
2017-06-28  Martin Liska  

PR sanitizer/81224
* asan.c (instrument_derefs): Bail out inner references
that are hard register variables.
2017-07-27  Martin Liska  

Backport from mainline
2017-06-28  Martin Liska  

PR sanitizer/81224
* gcc.dg/asan/pr81224.c: New test.

Added:
branches/gcc-7-branch/gcc/testsuite/gcc.dg/asan/pr81224.c
Modified:
branches/gcc-7-branch/gcc/ChangeLog
branches/gcc-7-branch/gcc/asan.c
branches/gcc-7-branch/gcc/testsuite/ChangeLog

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

Martin Liška  changed:

   What|Removed |Added

  Known to work||8.0
  Known to fail||5.4.0, 6.3.0, 7.1.0

--- Comment #4 from Martin Liška  ---
Fixed on trunk so far.

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[marxin at gcc dot gnu.org]

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

--- Comment #3 from Martin Liška  ---
Author: marxin
Date: Wed Jun 28 07:59:23 2017
New Revision: 249728

URL: https://gcc.gnu.org/viewcvs?rev=249728=gcc=rev
Log:
Bail out HARD_REGISTER vars in asan (PR sanitizer/81224).

2017-06-28  Martin Liska  

PR sanitizer/81224
* asan.c (instrument_derefs): Bail out inner references
that are hard register variables.
2017-06-28  Martin Liska  

PR sanitizer/81224
* gcc.dg/asan/pr81224.c: New test.

Added:
trunk/gcc/testsuite/gcc.dg/asan/pr81224.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/asan.c
trunk/gcc/testsuite/ChangeLog

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek  ---
Sure, and both tsan.c and ubsan.c already have some
if (VAR_P (base) && DECL_HARD_REGISTER (base))
  bail out;
tests, but asan.c doesn't (and should).

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[mpolacek at gcc dot gnu.org]

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

Marek Polacek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2017-06-27
 CC||mpolacek at gcc dot gnu.org
 Ever confirmed|0   |1

[Bug sanitizer/81224] ICE in -fsanitize=address w/ a register variable of a vector type

[rguenth at gcc dot gnu.org]

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

--- Comment #1 from Richard Biener  ---
You shouldn't sanitize DECL_HARD_REGISTER things