Starting from when -fsanitize=address was introduced, we ICE on: $ cat ice.i int a; int b () { register __attribute__ ((__vector_size__ (sizeof (int)))) int c asm("xmm0"); return c[a]; } $ gcc ice.i -c -fsanitize=address during RTL pass: expand ice.i: In function ‘b’: ice.i:6:11: internal compiler error: in expand_expr_addr_expr_1, at expr.c:7792 return c[a]; ~^~~ 0x859acf expand_expr_addr_expr_1 ../../gcc/expr.c:7792 0x859418 expand_expr_addr_expr_1 ../../gcc/expr.c:7830 0x84cb5a expand_expr_addr_expr ../../gcc/expr.c:7905 0x84cb5a expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) ../../gcc/expr.c:11058 0x85a2f3 store_expr_with_bounds(tree_node*, rtx_def*, int, bool, bool, tree_node*) ../../gcc/expr.c:5554 0x85baf7 expand_assignment(tree_node*, tree_node*, bool) ../../gcc/expr.c:5323 0x73af78 expand_gimple_stmt_1 ../../gcc/cfgexpand.c:3643 0x73af78 expand_gimple_stmt ../../gcc/cfgexpand.c:3741 0x73cfa7 expand_gimple_basic_block ../../gcc/cfgexpand.c:5745 0x742c86 execute ../../gcc/cfgexpand.c:6354 I'll take a look.
You shouldn't sanitize DECL_HARD_REGISTER things
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).
Author: marxin Date: Wed Jun 28 07:59:23 2017 New Revision: 249728 URL: https://gcc.gnu.org/viewcvs?rev=249728&root=gcc&view=rev Log: Bail out HARD_REGISTER vars in asan (PR sanitizer/81224). 2017-06-28 Martin Liska <mliska@suse.cz> PR sanitizer/81224 * asan.c (instrument_derefs): Bail out inner references that are hard register variables. 2017-06-28 Martin Liska <mliska@suse.cz> 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
Fixed on trunk so far.
Author: marxin Date: Thu Jul 27 07:31:19 2017 New Revision: 250603 URL: https://gcc.gnu.org/viewcvs?rev=250603&root=gcc&view=rev Log: Backport r249728 2017-07-27 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR sanitizer/81224 * asan.c (instrument_derefs): Bail out inner references that are hard register variables. 2017-07-27 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> 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
Author: marxin Date: Fri Sep 15 12:14:40 2017 New Revision: 252813 URL: https://gcc.gnu.org/viewcvs?rev=252813&root=gcc&view=rev Log: Backport r249728 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR sanitizer/81224 * asan.c (instrument_derefs): Bail out inner references that are hard register variables. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> 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
Author: marxin Date: Fri Sep 15 12:15:52 2017 New Revision: 252814 URL: https://gcc.gnu.org/viewcvs?rev=252814&root=gcc&view=rev Log: Backport r249728 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> PR sanitizer/81224 * asan.c (instrument_derefs): Bail out inner references that are hard register variables. 2017-09-15 Martin Liska <mliska@suse.cz> Backport from mainline 2017-06-28 Martin Liska <mliska@suse.cz> 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
Fixed on all active branches.
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 <http://gcc.gnu.org/bugs.html> 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.
Also fails on 5.x branch.
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?
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.
(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.
(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?
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?
(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 %<target%>"); *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?
(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 %<target%>"); > *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.
(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 %<target%>"); > > *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/ ?
(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 %<target%>"); > > > *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 ‘<built-in>’: /tmp/ice.C:6:5: error: No dispatcher found for the versioning attributes : int foo() {return 1;} ^~~
That would be my preference (perhaps even for GCC 7, only do the warning and attribute removal on the trunk).
Fixed.