* [PATCH] c++: Check null pointer deref when calling memfn in constexpr [PR102420] @ 2023-12-17 21:51 Nathaniel Shead 2023-12-18 18:32 ` Jason Merrill 0 siblings, 1 reply; 3+ messages in thread From: Nathaniel Shead @ 2023-12-17 21:51 UTC (permalink / raw) To: gcc-patches; +Cc: Jason Merrill Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? An alternative approach for the lambda issue would be to modify 'maybe_add_lambda_conv_op' to not pass a null pointer, but I wasn't sure what the best approach for that would be. -- >8 -- Calling a non-static member function on a null pointer is undefined behaviour (see [expr.ref] p8) and should error in constant evaluation, even if the 'this' pointer is never actually accessed within that function. One catch is that currently, the function pointer conversion operator for lambda passes a null pointer as the 'this' pointer to the underlying 'operator()', so for now we ignore such calls. PR c++/102420 gcc/cp/ChangeLog: * constexpr.cc (cxx_bind_parameters_in_call): Check for calling non-static member functions with a null pointer. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/constexpr-memfn2.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/constexpr.cc | 17 +++++++++++++++++ gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C | 10 ++++++++++ 2 files changed, 27 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 051f73fb73f..9c18538b302 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -1884,6 +1884,23 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx, tree t, tree fun, TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm. */ arg = cxx_eval_constant_expression (ctx, x, vc_prvalue, non_constant_p, overflow_p); + /* Check we aren't dereferencing a null pointer when calling a non-static + member function, which is undefined behaviour. */ + if (i == 0 && DECL_NONSTATIC_MEMBER_FUNCTION_P (fun) + && integer_zerop (arg) + /* But ignore calls from within the lambda function pointer + conversion thunk, since this currently passes a null pointer. */ + && !(TREE_CODE (t) == CALL_EXPR + && CALL_FROM_THUNK_P (t) + && ctx->call + && ctx->call->fundef + && lambda_static_thunk_p (ctx->call->fundef->decl))) + { + if (!ctx->quiet) + error_at (cp_expr_loc_or_input_loc (x), + "dereferencing a null pointer"); + *non_constant_p = true; + } /* Don't VERIFY_CONSTANT here. */ if (*non_constant_p && ctx->quiet) break; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C new file mode 100644 index 00000000000..4749190a1f0 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C @@ -0,0 +1,10 @@ +// PR c++/102420 +// { dg-do compile { target c++11 } } + +struct X { + constexpr int f() { return 0; } +}; +constexpr int g(X* x) { + return x->f(); // { dg-error "dereferencing a null pointer" } +} +constexpr int t = g(nullptr); // { dg-message "in .constexpr. expansion" } -- 2.42.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++: Check null pointer deref when calling memfn in constexpr [PR102420] 2023-12-17 21:51 [PATCH] c++: Check null pointer deref when calling memfn in constexpr [PR102420] Nathaniel Shead @ 2023-12-18 18:32 ` Jason Merrill 2023-12-19 11:03 ` Nathaniel Shead 0 siblings, 1 reply; 3+ messages in thread From: Jason Merrill @ 2023-12-18 18:32 UTC (permalink / raw) To: Nathaniel Shead, gcc-patches On 12/17/23 16:51, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > An alternative approach for the lambda issue would be to modify > 'maybe_add_lambda_conv_op' to not pass a null pointer, but I wasn't sure > what the best approach for that would be. I don't see a way to do that. Better IMO would have been for the C++ committee to make the op() static in this case when we introduced static lambdas, but that didn't happen, so we're left with this kludge. > -- >8 -- > > Calling a non-static member function on a null pointer is undefined > behaviour (see [expr.ref] p8) and should error in constant evaluation, > even if the 'this' pointer is never actually accessed within that > function. > > One catch is that currently, the function pointer conversion operator > for lambda passes a null pointer as the 'this' pointer to the underlying > 'operator()', so for now we ignore such calls. > > PR c++/102420 > > gcc/cp/ChangeLog: > > * constexpr.cc (cxx_bind_parameters_in_call): Check for calling > non-static member functions with a null pointer. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/constexpr-memfn2.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/constexpr.cc | 17 +++++++++++++++++ > gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C | 10 ++++++++++ > 2 files changed, 27 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index 051f73fb73f..9c18538b302 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -1884,6 +1884,23 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx, tree t, tree fun, > TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm. */ > arg = cxx_eval_constant_expression (ctx, x, vc_prvalue, > non_constant_p, overflow_p); > + /* Check we aren't dereferencing a null pointer when calling a non-static > + member function, which is undefined behaviour. */ > + if (i == 0 && DECL_NONSTATIC_MEMBER_FUNCTION_P (fun) > + && integer_zerop (arg) > + /* But ignore calls from within the lambda function pointer > + conversion thunk, since this currently passes a null pointer. */ > + && !(TREE_CODE (t) == CALL_EXPR > + && CALL_FROM_THUNK_P (t) We might just stop here, since any CALL_FROM_THUNK_P is compiler internals, and we should have already complained (if appropriate) about calling the thunk. OK either way. > + && ctx->call > + && ctx->call->fundef > + && lambda_static_thunk_p (ctx->call->fundef->decl))) > + { > + if (!ctx->quiet) > + error_at (cp_expr_loc_or_input_loc (x), > + "dereferencing a null pointer"); > + *non_constant_p = true; > + } > /* Don't VERIFY_CONSTANT here. */ > if (*non_constant_p && ctx->quiet) > break; > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C > new file mode 100644 > index 00000000000..4749190a1f0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C > @@ -0,0 +1,10 @@ > +// PR c++/102420 > +// { dg-do compile { target c++11 } } > + > +struct X { > + constexpr int f() { return 0; } > +}; > +constexpr int g(X* x) { > + return x->f(); // { dg-error "dereferencing a null pointer" } > +} > +constexpr int t = g(nullptr); // { dg-message "in .constexpr. expansion" } ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++: Check null pointer deref when calling memfn in constexpr [PR102420] 2023-12-18 18:32 ` Jason Merrill @ 2023-12-19 11:03 ` Nathaniel Shead 0 siblings, 0 replies; 3+ messages in thread From: Nathaniel Shead @ 2023-12-19 11:03 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches On Mon, Dec 18, 2023 at 01:32:58PM -0500, Jason Merrill wrote: > On 12/17/23 16:51, Nathaniel Shead wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > An alternative approach for the lambda issue would be to modify > > 'maybe_add_lambda_conv_op' to not pass a null pointer, but I wasn't sure > > what the best approach for that would be. > > I don't see a way to do that. Better IMO would have been for the C++ > committee to make the op() static in this case when we introduced static > lambdas, but that didn't happen, so we're left with this kludge. I was thinking about doing something morally equivalent to (decltype(lambda){}).operator()(args...); (using 'build_value_init' or something?) but... > > -- >8 -- > > > > Calling a non-static member function on a null pointer is undefined > > behaviour (see [expr.ref] p8) and should error in constant evaluation, > > even if the 'this' pointer is never actually accessed within that > > function. > > > > One catch is that currently, the function pointer conversion operator > > for lambda passes a null pointer as the 'this' pointer to the underlying > > 'operator()', so for now we ignore such calls. > > > > PR c++/102420 > > > > gcc/cp/ChangeLog: > > > > * constexpr.cc (cxx_bind_parameters_in_call): Check for calling > > non-static member functions with a null pointer. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp0x/constexpr-memfn2.C: New test. > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > --- > > gcc/cp/constexpr.cc | 17 +++++++++++++++++ > > gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C | 10 ++++++++++ > > 2 files changed, 27 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C > > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > > index 051f73fb73f..9c18538b302 100644 > > --- a/gcc/cp/constexpr.cc > > +++ b/gcc/cp/constexpr.cc > > @@ -1884,6 +1884,23 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx, tree t, tree fun, > > TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm. */ > > arg = cxx_eval_constant_expression (ctx, x, vc_prvalue, > > non_constant_p, overflow_p); > > + /* Check we aren't dereferencing a null pointer when calling a non-static > > + member function, which is undefined behaviour. */ > > + if (i == 0 && DECL_NONSTATIC_MEMBER_FUNCTION_P (fun) > > + && integer_zerop (arg) > > + /* But ignore calls from within the lambda function pointer > > + conversion thunk, since this currently passes a null pointer. */ > > + && !(TREE_CODE (t) == CALL_EXPR > > + && CALL_FROM_THUNK_P (t) > > We might just stop here, since any CALL_FROM_THUNK_P is compiler internals, > and we should have already complained (if appropriate) about calling the > thunk. OK either way. > ... for future proofing it seems reasonable to ignore calls from compiler-generated code regardless, thanks. > > + && ctx->call > > + && ctx->call->fundef > > + && lambda_static_thunk_p (ctx->call->fundef->decl))) > > + { > > + if (!ctx->quiet) > > + error_at (cp_expr_loc_or_input_loc (x), > > + "dereferencing a null pointer"); > > + *non_constant_p = true; > > + } > > /* Don't VERIFY_CONSTANT here. */ > > if (*non_constant_p && ctx->quiet) > > break; > > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C > > new file mode 100644 > > index 00000000000..4749190a1f0 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C > > @@ -0,0 +1,10 @@ > > +// PR c++/102420 > > +// { dg-do compile { target c++11 } } > > + > > +struct X { > > + constexpr int f() { return 0; } > > +}; > > +constexpr int g(X* x) { > > + return x->f(); // { dg-error "dereferencing a null pointer" } > > +} > > +constexpr int t = g(nullptr); // { dg-message "in .constexpr. expansion" } > This is what I'll push once bootstrap & regtest is finished (currently only passed 'RUNTESTFLAGS="dg.exp=constexpr*"'): -- >8 -- Calling a non-static member function on a null pointer is undefined behaviour (see [expr.ref] p8) and should error in constant evaluation, even if the 'this' pointer is never actually accessed within that function. One catch is that currently, the function pointer conversion operator for lambdas passes a null pointer as the 'this' pointer to the underlying 'operator()', so for now we ignore such calls. PR c++/102420 gcc/cp/ChangeLog: * constexpr.cc (cxx_bind_parameters_in_call): Check for calling non-static member functions with a null pointer. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/constexpr-memfn2.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/constexpr.cc | 14 ++++++++++++++ gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C | 10 ++++++++++ 2 files changed, 24 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 051f73fb73f..eff43a42353 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -1884,6 +1884,20 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx, tree t, tree fun, TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm. */ arg = cxx_eval_constant_expression (ctx, x, vc_prvalue, non_constant_p, overflow_p); + /* Check we aren't dereferencing a null pointer when calling a non-static + member function, which is undefined behaviour. */ + if (i == 0 && DECL_NONSTATIC_MEMBER_FUNCTION_P (fun) + && integer_zerop (arg) + /* But ignore calls from within compiler-generated code, to handle + cases like lambda function pointer conversion operator thunks + which pass NULL as the 'this' pointer. */ + && !(TREE_CODE (t) == CALL_EXPR && CALL_FROM_THUNK_P (t))) + { + if (!ctx->quiet) + error_at (cp_expr_loc_or_input_loc (x), + "dereferencing a null pointer"); + *non_constant_p = true; + } /* Don't VERIFY_CONSTANT here. */ if (*non_constant_p && ctx->quiet) break; diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C new file mode 100644 index 00000000000..dde9416d86f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-memfn2.C @@ -0,0 +1,10 @@ +// PR c++/102420 +// { dg-do compile { target c++11 } } + +struct X { + constexpr int f() { return 0; } +}; +constexpr int g(X* x) { + return x->f(); // { dg-error "dereferencing a null pointer" } +} +constexpr int t = g(nullptr); // { dg-message "in .constexpr. expansion" } -- 2.42.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-19 11:04 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-12-17 21:51 [PATCH] c++: Check null pointer deref when calling memfn in constexpr [PR102420] Nathaniel Shead 2023-12-18 18:32 ` Jason Merrill 2023-12-19 11:03 ` Nathaniel Shead
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).