From c0e4db96cba0b46a06ecd1f81ed1ef747d9f78ba Mon Sep 17 00:00:00 2001 From: Dmitrryy Date: Sun, 7 Jan 2024 15:45:40 +0300 Subject: [PATCH] [PATCH v3] c++: side effect in nullptr_t conversion fix Hi, > This seems to assume that a CONVERT_EXPR can't have any other > side-effects nested in it. > > It seems to me a better approach is the one in keep_unused_object_arg > and cp_gimplify_expr, basically > > if (TREE_THIS_VOLATILE (e)) > e = cp_build_addr_expr (e); > > since the address of a volatile lvalue is safe to put on the lhs of a > COMPOUND_EXPR. Thank you for your review! I implemented what you suggested with a little modification: I cut the conversion nodes using STRIP_NOPS (expr), since the value is not actually converted anywhere and should be ignored. > I don't see why ocp_convert needs to change at all; if TREE_SIDE_EFFECTS > is set we'll eventually get to cp_convert_to_pointer and can do the > right thing there. Without changes, it does not happen to call the cp_convert_to_pointer function. Due to the equality of the 'to' and 'from' types, identity conversion occurs with preserving the side effect. > Why not just > > DECL_INITIAL (dest) = nullptr_node; > > ? Because it doesn't work. This does not completely replace initialization, reading from volatile nullptr remains. I also added a test suite 'nullptr47.C' to check the original tree. --- gcc/cp/cvt.cc | 20 ++++++++++++++--- gcc/cp/typeck2.cc | 8 +++++++ gcc/testsuite/g++.dg/cpp0x/nullptr47.C | 30 ++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/nullptr47.C diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc index cbed847b343..578c9b8ef20 100644 --- a/gcc/cp/cvt.cc +++ b/gcc/cp/cvt.cc @@ -218,8 +218,20 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold, ? build_int_cst_type (type, -1) : build_int_cst (type, 0)); - return (TREE_SIDE_EFFECTS (expr) - ? build2 (COMPOUND_EXPR, type, expr, val) : val); + tree e = STRIP_NOPS (expr); + if (TREE_THIS_VOLATILE (e)) + { + /* can't drop expr with side effect (e.g. function call). */ + e = cp_build_addr_expr(e, tf_warning_or_error); + return build2 (COMPOUND_EXPR, type, e, val); + } + + /* C++ [conv.lval]p3: + If T is cv std::nullptr_t, the result is a null pointer constant. */ + /* expr value must be ignored. */ + return ((TREE_SIDE_EFFECTS(e)) + ? build2(COMPOUND_EXPR, type, e, val) + : val); } else if (TYPE_PTRMEM_P (type) && INTEGRAL_CODE_P (form)) { @@ -743,9 +755,11 @@ ocp_convert (tree type, tree expr, int convtype, int flags, { if (complain & tf_warning) maybe_warn_zero_as_null_pointer_constant (e, loc); - if (!TREE_SIDE_EFFECTS (e)) return nullptr_node; + else + /* process nullptr to nullptr conversion with side effect. */ + return cp_convert_to_pointer(type, e, dofold, complain); } if (MAYBE_CLASS_TYPE_P (type) && (convtype & CONV_FORCE_TEMP)) diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc index ac0fefa24f2..cdabb32b289 100644 --- a/gcc/cp/typeck2.cc +++ b/gcc/cp/typeck2.cc @@ -770,6 +770,14 @@ split_nonconstant_init (tree dest, tree init) && array_of_runtime_bound_p (TREE_TYPE (dest))) code = build_vec_init (dest, NULL_TREE, init, /*value-init*/false, /*from array*/1, tf_warning_or_error); + else if (TREE_CODE (TREE_TYPE(init)) == NULLPTR_TYPE) + { + /* C++ [conv.lval]p3: + If T is cv std::nullptr_t, the result is a null pointer constant. */ + tree ie = cp_build_init_expr(dest, nullptr_node); + // remain expr with side effect + code = add_stmt_to_compound(init, ie); + } else code = cp_build_init_expr (dest, init); diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr47.C b/gcc/testsuite/g++.dg/cpp0x/nullptr47.C new file mode 100644 index 00000000000..f05e91465f7 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/nullptr47.C @@ -0,0 +1,30 @@ +// { dg-do compile { target c++11 } } +// { dg-options "-O2 -fdump-tree-original" } + +int* foo() +{ + volatile auto a_0 = nullptr; + int* b_0 = a_0; + return b_0; +} +/* nullptr to int pointer conversion: no load from a_0. Init b_0 by zero. */ +// { dg-final { scan-tree-dump-times "cleanup_point int \\* b_0 = 0" 1 "original"} } + +auto __attribute__ ((noinline)) bar() +{ + /* side effect. Elimination of 'call bar()' will be incorrect. */ + volatile int* b = (int *)0xff; + *b = 10; + + volatile auto n = nullptr; + return n; +} +/* nullptr to nullptr conversion (identity): no load from n. Just return zero. */ +// { dg-final { scan-tree-dump-times "return =\[^\\n\\r\]* 0;" 1 "original"} } + +void foo_2() +{ + volatile auto a_2 = bar(); +} +/* non-constant init: no store bar() result to a_2. Call bar() and init a_2 by zero. */ +// { dg-final { scan-tree-dump-times "bar \\(\\);, a_2 = 0;" 1 "original"} } -- 2.34.1 --- gcc/cp/cvt.cc | 20 ++++++++++++++--- gcc/cp/typeck2.cc | 8 +++++++ gcc/testsuite/g++.dg/cpp0x/nullptr47.C | 30 ++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/nullptr47.C diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc index cbed847b343..578c9b8ef20 100644 --- a/gcc/cp/cvt.cc +++ b/gcc/cp/cvt.cc @@ -218,8 +218,20 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold, ? build_int_cst_type (type, -1) : build_int_cst (type, 0)); - return (TREE_SIDE_EFFECTS (expr) - ? build2 (COMPOUND_EXPR, type, expr, val) : val); + tree e = STRIP_NOPS (expr); + if (TREE_THIS_VOLATILE (e)) + { + /* can't drop expr with side effect (e.g. function call). */ + e = cp_build_addr_expr(e, tf_warning_or_error); + return build2 (COMPOUND_EXPR, type, e, val); + } + + /* C++ [conv.lval]p3: + If T is cv std::nullptr_t, the result is a null pointer constant. */ + /* expr value must be ignored. */ + return ((TREE_SIDE_EFFECTS(e)) + ? build2(COMPOUND_EXPR, type, e, val) + : val); } else if (TYPE_PTRMEM_P (type) && INTEGRAL_CODE_P (form)) { @@ -743,9 +755,11 @@ ocp_convert (tree type, tree expr, int convtype, int flags, { if (complain & tf_warning) maybe_warn_zero_as_null_pointer_constant (e, loc); - if (!TREE_SIDE_EFFECTS (e)) return nullptr_node; + else + /* process nullptr to nullptr conversion with side effect. */ + return cp_convert_to_pointer(type, e, dofold, complain); } if (MAYBE_CLASS_TYPE_P (type) && (convtype & CONV_FORCE_TEMP)) diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc index ac0fefa24f2..cdabb32b289 100644 --- a/gcc/cp/typeck2.cc +++ b/gcc/cp/typeck2.cc @@ -770,6 +770,14 @@ split_nonconstant_init (tree dest, tree init) && array_of_runtime_bound_p (TREE_TYPE (dest))) code = build_vec_init (dest, NULL_TREE, init, /*value-init*/false, /*from array*/1, tf_warning_or_error); + else if (TREE_CODE (TREE_TYPE(init)) == NULLPTR_TYPE) + { + /* C++ [conv.lval]p3: + If T is cv std::nullptr_t, the result is a null pointer constant. */ + tree ie = cp_build_init_expr(dest, nullptr_node); + // remain expr with side effect + code = add_stmt_to_compound(init, ie); + } else code = cp_build_init_expr (dest, init); diff --git a/gcc/testsuite/g++.dg/cpp0x/nullptr47.C b/gcc/testsuite/g++.dg/cpp0x/nullptr47.C new file mode 100644 index 00000000000..f05e91465f7 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/nullptr47.C @@ -0,0 +1,30 @@ +// { dg-do compile { target c++11 } } +// { dg-options "-O2 -fdump-tree-original" } + +int* foo() +{ + volatile auto a_0 = nullptr; + int* b_0 = a_0; + return b_0; +} +/* nullptr to int pointer conversion: no load from a_0. Init b_0 by zero. */ +// { dg-final { scan-tree-dump-times "cleanup_point int \\* b_0 = 0" 1 "original"} } + +auto __attribute__ ((noinline)) bar() +{ + /* side effect. Elimination of 'call bar()' will be incorrect. */ + volatile int* b = (int *)0xff; + *b = 10; + + volatile auto n = nullptr; + return n; +} +/* nullptr to nullptr conversion (identity): no load from n. Just return zero. */ +// { dg-final { scan-tree-dump-times "return =\[^\\n\\r\]* 0;" 1 "original"} } + +void foo_2() +{ + volatile auto a_2 = bar(); +} +/* non-constant init: no store bar() result to a_2. Call bar() and init a_2 by zero. */ +// { dg-final { scan-tree-dump-times "bar \\(\\);, a_2 = 0;" 1 "original"} } -- 2.34.1