diff options
author | Marek Polacek <polacek@redhat.com> | 2022-10-14 10:05:57 -0400 |
---|---|---|
committer | Marek Polacek <polacek@redhat.com> | 2022-10-26 15:13:04 -0400 |
commit | d2249cd9adf5ae638577139177a50f7e62d8abd9 (patch) | |
tree | 646929046e4c2d8962270cc50acc17c6bad9f869 /gcc/cp | |
parent | f896c13489d22b30d01257bc8316ab97b3359d1c (diff) | |
download | gcc-d2249cd9adf5ae638577139177a50f7e62d8abd9.zip gcc-d2249cd9adf5ae638577139177a50f7e62d8abd9.tar.gz gcc-d2249cd9adf5ae638577139177a50f7e62d8abd9.tar.bz2 |
c++: Implement -Wdangling-reference [PR106393]
This patch implements a new experimental warning (enabled by -Wall) to
detect references bound to temporaries whose lifetime has ended. The
primary motivation is the Note in
<https://en.cppreference.com/w/cpp/algorithm/max>:
Capturing the result of std::max by reference produces a dangling reference
if one of the parameters is a temporary and that parameter is returned:
int n = 1;
const int& r = std::max(n-1, n+1); // r is dangling
That's because both temporaries for n-1 and n+1 are destroyed at the end
of the full expression. With this warning enabled, you'll get:
g.C:3:12: warning: possibly dangling reference to a temporary [-Wdangling-reference]
3 | const int& r = std::max(n-1, n+1);
| ^
g.C:3:24: note: the temporary was destroyed at the end of the full expression 'std::max<int>((n - 1), (n + 1))'
3 | const int& r = std::max(n-1, n+1);
| ~~~~~~~~^~~~~~~~~~
The warning works by checking if a reference is initialized with a function
that returns a reference, and at least one parameter of the function is
a reference that is bound to a temporary. It assumes that such a function
actually returns one of its arguments! (I added code to check_return_expr
to suppress the warning when we've seen the definition of the function
and we can say that it can return a variable with static storage
duration.)
It warns when the function in question is a member function, but only if
the function is invoked on a temporary object, otherwise the warning
would emit loads of warnings for valid code like obj.emplace<T>({0}, 0).
It does detect the dangling reference in:
struct S {
const S& self () { return *this; }
};
const S& s = S().self();
It warns in member initializer lists as well:
const int& f(const int& i) { return i; }
struct S {
const int &r;
S() : r(f(10)) { }
};
I've run the testsuite/bootstrap with the warning enabled by default.
There were just a few FAILs, all of which look like genuine bugs.
A bootstrap with the warning enabled by default passed as well.
When testing a previous version of the patch, there were many FAILs in
libstdc++'s 22_locale/; all of them because the warning triggered on
const test_type& obj = std::use_facet<test_type>(std::locale());
but this code looks valid -- std::use_facet doesn't return a reference
to its parameter. Therefore I added a #pragma and code to suppress the
warning.
PR c++/106393
gcc/c-family/ChangeLog:
* c.opt (Wdangling-reference): New.
gcc/cp/ChangeLog:
* call.cc (expr_represents_temporary_p): New, factored out of...
(conv_binds_ref_to_temporary): ...here. Don't return false just
because a ck_base is missing. Use expr_represents_temporary_p.
(do_warn_dangling_reference): New.
(maybe_warn_dangling_reference): New.
(extend_ref_init_temps): Call maybe_warn_dangling_reference.
* cp-tree.h: Adjust comment.
* typeck.cc (check_return_expr): Suppress -Wdangling-reference
warnings.
gcc/ChangeLog:
* doc/invoke.texi: Document -Wdangling-reference.
libstdc++-v3/ChangeLog:
* include/bits/locale_classes.tcc: Add #pragma to disable
-Wdangling-reference with std::use_facet.
gcc/testsuite/ChangeLog:
* g++.dg/cpp23/elision4.C: Use -Wdangling-reference, add dg-warning.
* g++.dg/cpp23/elision7.C: Likewise.
* g++.dg/warn/Wdangling-pointer-2.C: Use -Wno-dangling-reference.
* g++.dg/warn/Wdangling-reference1.C: New test.
* g++.dg/warn/Wdangling-reference2.C: New test.
* g++.dg/warn/Wdangling-reference3.C: New test.
Diffstat (limited to 'gcc/cp')
-rw-r--r-- | gcc/cp/call.cc | 148 | ||||
-rw-r--r-- | gcc/cp/cp-tree.h | 4 | ||||
-rw-r--r-- | gcc/cp/typeck.cc | 11 |
3 files changed, 150 insertions, 13 deletions
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 6a34e9c..951b9fd 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -9313,6 +9313,16 @@ conv_binds_ref_to_prvalue (conversion *c) return conv_is_prvalue (next_conversion (c)); } +/* True iff EXPR represents a (subobject of a) temporary. */ + +static bool +expr_represents_temporary_p (tree expr) +{ + while (handled_component_p (expr)) + expr = TREE_OPERAND (expr, 0); + return TREE_CODE (expr) == TARGET_EXPR; +} + /* True iff C is a conversion that binds a reference to a temporary. This is a superset of conv_binds_ref_to_prvalue: here we're also interested in xvalues. */ @@ -9330,18 +9340,14 @@ conv_binds_ref_to_temporary (conversion *c) struct Derived : Base {}; const Base& b(Derived{}); where we bind 'b' to the Base subobject of a temporary object of type - Derived. The subobject is an xvalue; the whole object is a prvalue. */ - if (c->kind != ck_base) - return false; - c = next_conversion (c); - if (c->kind == ck_identity && c->u.expr) - { - tree expr = c->u.expr; - while (handled_component_p (expr)) - expr = TREE_OPERAND (expr, 0); - if (TREE_CODE (expr) == TARGET_EXPR) - return true; - } + Derived. The subobject is an xvalue; the whole object is a prvalue. + + The ck_base doesn't have to be present for cases like X{}.m. */ + if (c->kind == ck_base) + c = next_conversion (c); + if (c->kind == ck_identity && c->u.expr + && expr_represents_temporary_p (c->u.expr)) + return true; return false; } @@ -13428,6 +13434,121 @@ initialize_reference (tree type, tree expr, return expr; } +/* Helper for maybe_warn_dangling_reference to find a problematic CALL_EXPR + that initializes the LHS (and at least one of its arguments represents + a temporary, as outlined in maybe_warn_dangling_reference), or NULL_TREE + if none found. For instance: + + const S& s = S().self(); // S::self (&TARGET_EXPR <...>) + const int& r = (42, f(1)); // f(1) + const int& t = b ? f(1) : f(2); // f(1) + const int& u = b ? f(1) : f(g); // f(1) + const int& v = b ? f(g) : f(2); // f(2) + const int& w = b ? f(g) : f(g); // NULL_TREE + const int& y = (f(1), 42); // NULL_TREE + const int& z = f(f(1)); // f(f(1)) + + EXPR is the initializer. */ + +static tree +do_warn_dangling_reference (tree expr) +{ + STRIP_NOPS (expr); + switch (TREE_CODE (expr)) + { + case CALL_EXPR: + { + tree fndecl = cp_get_callee_fndecl_nofold (expr); + if (!fndecl + || warning_suppressed_p (fndecl, OPT_Wdangling_reference) + || !warning_enabled_at (DECL_SOURCE_LOCATION (fndecl), + OPT_Wdangling_reference) + /* If the function doesn't return a reference, don't warn. This + can be e.g. + const int& z = std::min({1, 2, 3, 4, 5, 6, 7}); + which doesn't dangle: std::min here returns an int. */ + || !TYPE_REF_OBJ_P (TREE_TYPE (TREE_TYPE (fndecl)))) + return NULL_TREE; + + /* Here we're looking to see if any of the arguments is a temporary + initializing a reference parameter. */ + for (int i = 0; i < call_expr_nargs (expr); ++i) + { + tree arg = CALL_EXPR_ARG (expr, i); + /* Check that this argument initializes a reference, except for + the argument initializing the object of a member function. */ + if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl) + && !TYPE_REF_P (TREE_TYPE (arg))) + continue; + /* It could also be another call taking a temporary and returning + it and initializing this reference parameter. */ + if (do_warn_dangling_reference (arg)) + return expr; + STRIP_NOPS (arg); + if (TREE_CODE (arg) == ADDR_EXPR) + arg = TREE_OPERAND (arg, 0); + if (expr_represents_temporary_p (arg)) + return expr; + /* Don't warn about member function like: + std::any a(...); + S& s = a.emplace<S>({0}, 0); + which constructs a new object and returns a reference to it, but + we still want to detect: + struct S { const S& self () { return *this; } }; + const S& s = S().self(); + where 's' dangles. If we've gotten here, the object this function + is invoked on is not a temporary. */ + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)) + break; + } + return NULL_TREE; + } + case COMPOUND_EXPR: + return do_warn_dangling_reference (TREE_OPERAND (expr, 1)); + case COND_EXPR: + if (tree t = do_warn_dangling_reference (TREE_OPERAND (expr, 1))) + return t; + return do_warn_dangling_reference (TREE_OPERAND (expr, 2)); + case PAREN_EXPR: + return do_warn_dangling_reference (TREE_OPERAND (expr, 0)); + default: + return NULL_TREE; + } +} + +/* Implement -Wdangling-reference, to detect cases like + + int n = 1; + const int& r = std::max(n - 1, n + 1); // r is dangling + + This creates temporaries from the arguments, returns a reference to + one of the temporaries, but both temporaries are destroyed at the end + of the full expression. + + This works by checking if a reference is initialized with a function + that returns a reference, and at least one parameter of the function + is a reference that is bound to a temporary. It assumes that such a + function actually returns one of its arguments. + + DECL is the reference being initialized, INIT is the initializer. */ + +static void +maybe_warn_dangling_reference (const_tree decl, tree init) +{ + if (!warn_dangling_reference) + return; + if (!TYPE_REF_P (TREE_TYPE (decl))) + return; + if (tree call = do_warn_dangling_reference (init)) + { + auto_diagnostic_group d; + if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wdangling_reference, + "possibly dangling reference to a temporary")) + inform (EXPR_LOCATION (call), "the temporary was destroyed at " + "the end of the full expression %qE", call); + } +} + /* If *P is an xvalue expression, prevent temporary lifetime extension if it gets used to initialize a reference. */ @@ -13525,6 +13646,9 @@ extend_ref_init_temps (tree decl, tree init, vec<tree, va_gc> **cleanups, tree type = TREE_TYPE (init); if (processing_template_decl) return init; + + maybe_warn_dangling_reference (decl, init); + if (TYPE_REF_P (type)) init = extend_ref_init_temps_1 (decl, init, cleanups, cond_guard); else diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 867096b..40f5bf8 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -459,7 +459,6 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX]; TI_PENDING_TEMPLATE_FLAG. TEMPLATE_PARMS_FOR_INLINE. DELETE_EXPR_USE_VEC (in DELETE_EXPR). - (TREE_CALLS_NEW) (in _EXPR or _REF) (commented-out). ICS_ELLIPSIS_FLAG (in _CONV) DECL_INITIALIZED_P (in VAR_DECL) TYPENAME_IS_CLASS_P (in TYPENAME_TYPE) @@ -4567,6 +4566,9 @@ get_vec_init_expr (tree t) When appearing in a CONSTRUCTOR, the expression is an unconverted compound literal. + When appearing in a CALL_EXPR, it means that it is a call to + a constructor. + When appearing in a FIELD_DECL, it means that this field has been duly initialized in its constructor. */ #define TREE_HAS_CONSTRUCTOR(NODE) (TREE_LANG_FLAG_4 (NODE)) diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index ab6979b..4605f73 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -11246,6 +11246,17 @@ check_return_expr (tree retval, bool *no_warning) if (processing_template_decl) return saved_retval; + /* A naive attempt to reduce the number of -Wdangling-reference false + positives: if we know that this function can return a variable with + static storage duration rather than one of its parameters, suppress + the warning. */ + if (warn_dangling_reference + && TYPE_REF_P (functype) + && bare_retval + && VAR_P (bare_retval) + && TREE_STATIC (bare_retval)) + suppress_warning (current_function_decl, OPT_Wdangling_reference); + /* Actually copy the value returned into the appropriate location. */ if (retval && retval != result) retval = cp_build_init_expr (result, retval); |