| Message ID | 6493bd88-c2d4-eb68-a06b-46c1d404248b@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | avoid -Wnonnull with lambda (PR 100684) | expand |
On 5/19/2021 3:01 PM, Martin Sebor via Gcc-patches wrote: > The front end -Wnonnull handler has code to suppress warning for > lambdas with null this pointers but the middle end handler has > no corresponding logic. This leads to spurious -Wnonnull in > lambda calls that aren't inlined. That might happen at low > optimization levels such as -O1 or -Og and with sanitization. > > The attached patch enhances the middle end -Wnonnull to deal > with this case and avoid the false positives. > > Tested on x86_64-linux. > > Martin > > gcc-100684.diff > > PR middle-end/100684 - spurious -Wnonnull with -O1 on a C++ lambda > > gcc/ChangeLog: > > PR middle-end/100684 > * tree-ssa-ccp.c (pass_post_ipa_warn::execute): > > gcc/testsuite/ChangeLog: > > PR middle-end/100684 > * g++.dg/warn/Wnonnull13.C: New test. > * g++.dg/warn/Wnonnull14.C: New test. > * g++.dg/warn/Wnonnull15.C: New test. OK with a ChangeLog entry for the tree-ssa-ccp change. Jeff
On Thu, May 20, 2021 at 7:00 AM Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > On 5/19/2021 3:01 PM, Martin Sebor via Gcc-patches wrote: > > The front end -Wnonnull handler has code to suppress warning for > > lambdas with null this pointers but the middle end handler has > > no corresponding logic. This leads to spurious -Wnonnull in > > lambda calls that aren't inlined. That might happen at low > > optimization levels such as -O1 or -Og and with sanitization. > > > > The attached patch enhances the middle end -Wnonnull to deal > > with this case and avoid the false positives. > > > > Tested on x86_64-linux. > > > > Martin > > > > gcc-100684.diff > > > > PR middle-end/100684 - spurious -Wnonnull with -O1 on a C++ lambda > > > > gcc/ChangeLog: > > > > PR middle-end/100684 > > * tree-ssa-ccp.c (pass_post_ipa_warn::execute): > > > > gcc/testsuite/ChangeLog: > > > > PR middle-end/100684 > > * g++.dg/warn/Wnonnull13.C: New test. > > * g++.dg/warn/Wnonnull14.C: New test. > > * g++.dg/warn/Wnonnull15.C: New test. > > OK with a ChangeLog entry for the tree-ssa-ccp change. Note it won't work with LTO because when the lambda type was introduced it was added to the hashing code but the streaming only streams the sub-state DECL_IS_OPERATOR_NEW_P rather than the full FUNCTION_DECL_DECL_TYPE ... Martin - was this intended? Can you fix it up please? (g:cb50701ec2c7) Thanks, Richard. > > Jeff
On 5/20/21 10:03 AM, Richard Biener wrote:
> Martin - was this intended? Can you fix it up please? (g:cb50701ec2c7)
It was not intentional. Fixed in the attached patch (where I did
a refactoring as well).
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
On 5/24/2021 4:12 AM, Martin Liška wrote: > On 5/20/21 10:03 AM, Richard Biener wrote: >> Martin - was this intended? Can you fix it up please? (g:cb50701ec2c7) > > It was not intentional. Fixed in the attached patch (where I did > a refactoring as well). > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > 0001-LTO-stream-properly-FUNCTION_DECL_DECL_TYPE.patch > > From fa91b695b595d2fa1a6245eafe26a83b7aeb0787 Mon Sep 17 00:00:00 2001 > From: Martin Liska <mliska@suse.cz> > Date: Mon, 24 May 2021 11:18:21 +0200 > Subject: [PATCH] LTO: stream properly FUNCTION_DECL_DECL_TYPE. > > gcc/lto/ChangeLog: > > * lto-common.c (compare_tree_sccs_1): Compare > FUNCTION_DECL_DECL_TYPE. > > gcc/ChangeLog: > > * tree-streamer-in.c (unpack_ts_function_decl_value_fields): > Unpack FUNCTION_DECL_DECL_TYPE. > * tree-streamer-out.c (pack_ts_function_decl_value_fields): > Stream FUNCTION_DECL_DECL_TYPE instead of > DECL_IS_OPERATOR_NEW_P. > * tree.h (set_function_decl_type): Use FUNCTION_DECL_DECL_TYPE > macro. > (DECL_IS_OPERATOR_NEW_P): Likewise. > (DECL_IS_OPERATOR_DELETE_P): Likewise. > (DECL_LAMBDA_FUNCTION_P): Likewise. OK jeff
PR middle-end/100684 - spurious -Wnonnull with -O1 on a C++ lambda gcc/ChangeLog: PR middle-end/100684 * tree-ssa-ccp.c (pass_post_ipa_warn::execute): gcc/testsuite/ChangeLog: PR middle-end/100684 * g++.dg/warn/Wnonnull13.C: New test. * g++.dg/warn/Wnonnull14.C: New test. * g++.dg/warn/Wnonnull15.C: New test. diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull13.C b/gcc/testsuite/g++.dg/warn/Wnonnull13.C new file mode 100644 index 00000000000..e3279764ac0 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wnonnull13.C @@ -0,0 +1,28 @@ +/* PR middle-end/100684 - spurious -Wnonnull with -O1 on a C++ lambda + { dg-do compile { target c++11 } } + { dg-options "-O0 -Wall -fsanitize=undefined" } */ + +#define NONNULL __attribute__ ((nonnull)) + +typedef int F (const char *); + +NONNULL int f (const char *); + +int nowarn_O0 () +{ + return static_cast<F*>([](const char *s){ return f (s); })("O0"); + // { dg-bogus "\\\[-Wnonnull" "" { target *-*-* } .-1 } +} + +int warn_O0 () +{ + return static_cast<F*>([] NONNULL (const char *){ return 0; })(0); + // { dg-warning "\\\[-Wnonnull" "" { target *-*-* } .-1 } +} + +int warn_O0_inline () +{ + return static_cast<F*>([](const char *s){ return f (s); })(0); + // { dg-warning "\\\[-Wnonnull" "lambda not inlined" { xfail *-*-* } .-1 } +} + diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull14.C b/gcc/testsuite/g++.dg/warn/Wnonnull14.C new file mode 100644 index 00000000000..16d7ec3f573 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wnonnull14.C @@ -0,0 +1,28 @@ +/* PR middle-end/100684 - spurious -Wnonnull with -O1 on a C++ lambda + { dg-do compile { target c++11 } } + { dg-options "-Og -Wall -fsanitize=undefined" } */ + +#define NONNULL __attribute__ ((nonnull)) + +typedef int F (const char *); + +__attribute__ ((nonnull)) int f (const char *); + +int nowarn_Og () +{ + return static_cast<F*>([](const char *s){ return f (s); })("Og"); + // { dg-bogus "'this' pointer is null" "" { target *-*-* } .-1 } +} + +int warn_Og () +{ + return static_cast<F*>([] NONNULL (const char *){ return 0; })(0); + // { dg-warning "\\\[-Wnonnull" "" { target *-*-* } .-1 } +} + +int warn_Og_inline () +{ + const char *p = 0; + return static_cast<F*>([](const char *s){ return f (s); })(p); + // { dg-warning "\\\[-Wnonnull" "lambda not inlined" { xfail *-*-* } .-1 } +} diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull15.C b/gcc/testsuite/g++.dg/warn/Wnonnull15.C new file mode 100644 index 00000000000..36a2ab48789 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wnonnull15.C @@ -0,0 +1,28 @@ +/* PR middle-end/100684 - spurious -Wnonnull with -O1 on a C++ lambda + { dg-do compile { target c++11 } } + { dg-options "-O1 -Wall -fsanitize=undefined" } */ + +#define NONNULL __attribute__ ((nonnull)) + +typedef int F (const char *); + +NONNULL int f (const char *); + +int nowarn_O1 () +{ + return static_cast<F*>([](const char *s){ return f (s); })("O1"); + // { dg-bogus "\\\[-Wnonnull" "" { target *-*-* } .-1 } +} + +int warn_O1 () +{ + return static_cast<F*>([] NONNULL (const char *){ return 0; })(0); + // { dg-warning "\\\[-Wnonnull" "" { target *-*-* } .-1 } +} + +int warn_O1_inline () +{ + const char *p = 0; + return static_cast<F*>([](const char *s){ return f (s); })(p); + // { dg-warning "\\\[-Wnonnull" "lambda not inlined" { xfail *-*-* } .-1 } +} diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c index bf31f035153..3834212b867 100644 --- a/gcc/tree-ssa-ccp.c +++ b/gcc/tree-ssa-ccp.c @@ -3536,6 +3536,7 @@ pass_post_ipa_warn::execute (function *fun) continue; tree fndecl = gimple_call_fndecl (stmt); + const bool closure = fndecl && DECL_LAMBDA_FUNCTION_P (fndecl); for (unsigned i = 0; i < gimple_call_num_args (stmt); i++) { @@ -3544,6 +3545,9 @@ pass_post_ipa_warn::execute (function *fun) continue; if (!integer_zerop (arg)) continue; + if (i == 0 && closure) + /* Avoid warning for the first argument to lambda functions. */ + continue; if (!bitmap_empty_p (nonnullargs) && !bitmap_bit_p (nonnullargs, i)) continue;