| Message ID | c5c3c9d0-d424-d8f9-0cb6-c15adb391f31@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | avoid a couple of missing -Wuninitialized (PR 98583, 93100) | expand |
On 5/11/2021 1:49 PM, Martin Sebor via Gcc-patches wrote: > The attached change teaches the uninitialized pass about > __builtin_stack_restore and __builtin___asan_mark to avoid two > classes of -Wuninitialized false negatives. > > Richard, you already approved the __builtin_stack_restore change > in the bug but I figured I'd submit a patch with both changes for > approval since they affect the same piece of code. > > Martin > > gcc-93100.diff > > Avoid -Wuninitialized false negatives with sanitization and VLAs. > > Resolves: > PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized > PR middle-end/98583 - missing -Wuninitialized reading from a second VLA in its own block > > gcc/ChangeLog: > > PR tree-optimization/93100 > PR middle-end/98583 > * tree-ssa-uninit.c (check_defs): > > gcc/testsuite/ChangeLog: > > PR tree-optimization/93100 > PR middle-end/98583 > * g++.dg/warn/uninit-pr93100.C: New test. > * gcc.dg/uninit-pr93100.c: New test. > * gcc.dg/uninit-pr98583.c: New test. OK. I wonder if it would make sense to describe this property when we construct the builtin and check that property rather than each builtin we find over time. Your call on whether or not to explore that. Jeff
On 5/13/21 1:03 PM, Jeff Law wrote: > > On 5/11/2021 1:49 PM, Martin Sebor via Gcc-patches wrote: >> The attached change teaches the uninitialized pass about >> __builtin_stack_restore and __builtin___asan_mark to avoid two >> classes of -Wuninitialized false negatives. >> >> Richard, you already approved the __builtin_stack_restore change >> in the bug but I figured I'd submit a patch with both changes for >> approval since they affect the same piece of code. >> >> Martin >> >> gcc-93100.diff >> >> Avoid -Wuninitialized false negatives with sanitization and VLAs. >> >> Resolves: >> PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized >> PR middle-end/98583 - missing -Wuninitialized reading from a second VLA in its own block >> >> gcc/ChangeLog: >> >> PR tree-optimization/93100 >> PR middle-end/98583 >> * tree-ssa-uninit.c (check_defs): >> >> gcc/testsuite/ChangeLog: >> >> PR tree-optimization/93100 >> PR middle-end/98583 >> * g++.dg/warn/uninit-pr93100.C: New test. >> * gcc.dg/uninit-pr93100.c: New test. >> * gcc.dg/uninit-pr98583.c: New test. > > OK. I wonder if it would make sense to describe this property when we > construct the builtin and check that property rather than each builtin > we find over time. Your call on whether or not to explore that. I like the idea. Adding atrribute access to the built-ins would be one way. Attribute fn spec might be able to express the same thing although there I'm not sure if it would apply to the sanitizer functions. Either way it seems worth looking into. Thanks Martin > > > Jeff >
Avoid -Wuninitialized false negatives with sanitization and VLAs. Resolves: PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized PR middle-end/98583 - missing -Wuninitialized reading from a second VLA in its own block gcc/ChangeLog: PR tree-optimization/93100 PR middle-end/98583 * tree-ssa-uninit.c (check_defs): gcc/testsuite/ChangeLog: PR tree-optimization/93100 PR middle-end/98583 * g++.dg/warn/uninit-pr93100.C: New test. * gcc.dg/uninit-pr93100.c: New test. * gcc.dg/uninit-pr98583.c: New test. diff --git a/gcc/testsuite/g++.dg/warn/uninit-pr93100.C b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C new file mode 100644 index 00000000000..c9cd3ef0174 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C @@ -0,0 +1,59 @@ +/* PR tree-optimization/98508 - Sanitizer disable -Wall and -Wextra + { dg-do compile } + { dg-options "-O0 -Wall -fsanitize=address" } */ + +struct S +{ + int a; +}; + +void warn_init_self_O0 () +{ + S s = S (s); // { dg-warning "\\\[-Wuninitialized" } + (void)&s; +} + + +void warn_init_self_use_O0 () +{ + S s = S (s); // { dg-warning "\\\[-Wuninitialized" } + + void sink (void*); + sink (&s); +} + + +#pragma GCC optimize ("1") + +void warn_init_self_O1 () +{ + S s = S (s); // { dg-warning "\\\[-Wuninitialized" } + (void)&s; +} + + +void warn_init_self_use_O1 () +{ + S s = S (s); // { dg-warning "\\\[-Wuninitialized" } + + void sink (void*); + sink (&s); +} + + +#pragma GCC optimize ("2") + +void warn_init_self_O2 () +{ + S s = S (s); // { dg-warning "\\\[-Wuninitialized" } + (void)&s; +} + + +void warn_init_self_use_O2 () +{ + S s = S (s); // { dg-warning "\\\[-Wuninitialized" } + + void sink (void*); + sink (&s); +} diff --git a/gcc/testsuite/gcc.dg/uninit-pr93100.c b/gcc/testsuite/gcc.dg/uninit-pr93100.c new file mode 100644 index 00000000000..61b7e434038 --- /dev/null +++ b/gcc/testsuite/gcc.dg/uninit-pr93100.c @@ -0,0 +1,74 @@ +/* PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized + { dg-do compile } + { dg-options "-Wall -fsanitize=address" } */ + +struct A +{ + _Bool b; + int i; +}; + +void warn_A_b_O0 (void) +{ + struct A a; + + if (a.b) // { dg-warning "\\\[-Wuninitialized" } + { + (void)&a; + } +} + +void warn_A_i_O0 (void) +{ + struct A a; + + if (a.i) // { dg-warning "\\\[-Wuninitialized" } + { + (void)&a; + } +} + +#pragma GCC optimize ("1") + +void warn_A_b_O1 (void) +{ + struct A a; + + if (a.b) // { dg-warning "\\\[-Wuninitialized" } + { + (void)&a; + } +} + +void warn_A_i_O1 (void) +{ + struct A a; + + if (a.i) // { dg-warning "\\\[-Wuninitialized" } + { + (void)&a; + } +} + + +#pragma GCC optimize ("2") + +void warn_A_b_O2 (void) +{ + struct A a; + + if (a.b) // { dg-warning "\\\[-Wuninitialized" } + { + (void)&a; + } +} + +void warn_A_i_O2 (void) +{ + struct A a; + + if (a.i) // { dg-warning "\\\[-Wuninitialized" } + { + (void)&a; + } +} diff --git a/gcc/testsuite/gcc.dg/uninit-pr98583.c b/gcc/testsuite/gcc.dg/uninit-pr98583.c new file mode 100644 index 00000000000..638b0295809 --- /dev/null +++ b/gcc/testsuite/gcc.dg/uninit-pr98583.c @@ -0,0 +1,31 @@ +/* PR middle-end/98583 - missing -Wuninitialized reading from a second VLA + in its own block + { dg-do compile } + { dg-options "-O2 -Wall" } */ + +void f (int*); +void g (int); + +void h1 (int n) +{ + int a[n]; + f (a); + + int b[n]; + g (b[1]); // { dg-warning "\\\[-Wuninitialized" } +} + +void h2 (int n, int i, int j) +{ + if (i) + { + int a[n]; + f (a); + } + + if (j) + { + int b[n]; + g (b[1]); // { dg-warning "\\\[-Wmaybe-uninitialized" } + } +} diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c index 0800f596ab1..f55ce1939ac 100644 --- a/gcc/tree-ssa-uninit.c +++ b/gcc/tree-ssa-uninit.c @@ -209,6 +209,16 @@ check_defs (ao_ref *ref, tree vdef, void *data_) { check_defs_data *data = (check_defs_data *)data_; gimple *def_stmt = SSA_NAME_DEF_STMT (vdef); + + /* The ASAN_MARK intrinsic doesn't modify the variable. */ + if (is_gimple_call (def_stmt) + && gimple_call_internal_p (def_stmt, IFN_ASAN_MARK)) + return false; + + /* End of VLA scope is not a kill. */ + if (gimple_call_builtin_p (def_stmt, BUILT_IN_STACK_RESTORE)) + return false; + /* If this is a clobber then if it is not a kill walk past it. */ if (gimple_clobber_p (def_stmt)) {