[PATCH] warn-access: Prevent -Wuse-after-free on ARM [PR104213]

Martin Sebor msebor@gmail.com
Wed Jan 26 16:39:46 GMT 2022


On 1/26/22 08:24, Jason Merrill via Gcc-patches wrote:
> On 1/25/22 18:33, Marek Polacek wrote:
>> Here, -Wuse-after-free warns about using 'this' which, on ARM, cdtors
>> return, as mandated by the EABI.  To be entirely correct, it only
>> requires it for C1 and C2 ctors and D2 and D1 dtors, but I don't feel
>> like changing that now and possibly running into issues later on.
>>
>> This patch uses suppress_warning on 'this' for certain cdtor_returns_this
>> cases in the C++ FE, and then warn_invalid_pointer makes use of this
>> information and doesn't warn.
>>
>> In my first attempt I tried suppress_warning the MODIFY_EXPR or 
>> RETURN_EXPR
>> we build in build_delete_destructor_body, but the complication is that
>> the suppress_warning bits don't always survive gimplification; see e.g.
>> gimplify_modify_expr where we do
>>
>>   6130       if (COMPARISON_CLASS_P (*from_p))
>>   6131         copy_warning (assign, *from_p);
>>
>> but here we're not dealing with a comparison.  Removing that check
>> regresses uninit-pr74762.C.  Adding copy_warning (assign, *expr_p)
>> regresses c-c++-common/uninit-17.c.
>>
>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> OK if Martin doesn't have any suggestions.

Nothing further from me.

Thanks
Martin

> 
>>     PR target/104213
>>
>> gcc/cp/ChangeLog:
>>
>>     * decl.cc (finish_constructor_body): Suppress -Wuse-after-free.
>>     (finish_destructor_body): Likewise.
>>     * optimize.cc (build_delete_destructor_body): Likewise.
>>
>> gcc/ChangeLog:
>>
>>     * gimple-ssa-warn-access.cc (pass_waccess::warn_invalid_pointer): 
>> Don't
>>     warn when the SSA_NAME_VAR of REF has supressed -Wuse-after-free.
>>
>> gcc/testsuite/ChangeLog:
>>
>>     * g++.dg/warn/Wuse-after-free2.C: New test.
>> ---
>>   gcc/cp/decl.cc                               |  2 ++
>>   gcc/cp/optimize.cc                           |  1 +
>>   gcc/gimple-ssa-warn-access.cc                | 14 +++++++++++---
>>   gcc/testsuite/g++.dg/warn/Wuse-after-free2.C | 10 ++++++++++
>>   4 files changed, 24 insertions(+), 3 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/warn/Wuse-after-free2.C
>>
>> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
>> index 22d3dd1e2ad..6534a7fd320 100644
>> --- a/gcc/cp/decl.cc
>> +++ b/gcc/cp/decl.cc
>> @@ -17315,6 +17315,7 @@ finish_constructor_body (void)
>>         add_stmt (build_stmt (input_location, LABEL_EXPR, cdtor_label));
>>         val = DECL_ARGUMENTS (current_function_decl);
>> +      suppress_warning (val, OPT_Wuse_after_free);
>>         val = build2 (MODIFY_EXPR, TREE_TYPE (val),
>>               DECL_RESULT (current_function_decl), val);
>>         /* Return the address of the object.  */
>> @@ -17408,6 +17409,7 @@ finish_destructor_body (void)
>>         tree val;
>>         val = DECL_ARGUMENTS (current_function_decl);
>> +      suppress_warning (val, OPT_Wuse_after_free);
>>         val = build2 (MODIFY_EXPR, TREE_TYPE (val),
>>               DECL_RESULT (current_function_decl), val);
>>         /* Return the address of the object.  */
>> diff --git a/gcc/cp/optimize.cc b/gcc/cp/optimize.cc
>> index 4ad3f1dc9aa..13ab8b7361e 100644
>> --- a/gcc/cp/optimize.cc
>> +++ b/gcc/cp/optimize.cc
>> @@ -166,6 +166,7 @@ build_delete_destructor_body (tree delete_dtor, 
>> tree complete_dtor)
>>     if (targetm.cxx.cdtor_returns_this ())
>>       {
>>         tree val = DECL_ARGUMENTS (delete_dtor);
>> +      suppress_warning (val, OPT_Wuse_after_free);
>>         val = build2 (MODIFY_EXPR, TREE_TYPE (val),
>>                       DECL_RESULT (delete_dtor), val);
>>         add_stmt (build_stmt (0, RETURN_EXPR, val));
>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>> b/gcc/gimple-ssa-warn-access.cc
>> index 8bc33eeb6fa..484bcd34c25 100644
>> --- a/gcc/gimple-ssa-warn-access.cc
>> +++ b/gcc/gimple-ssa-warn-access.cc
>> @@ -3880,9 +3880,17 @@ pass_waccess::warn_invalid_pointer (tree ref, 
>> gimple *use_stmt,
>>                       bool maybe, bool equality /* = false */)
>>   {
>>     /* Avoid printing the unhelpful "<unknown>" in the diagnostics.  */
>> -  if (ref && TREE_CODE (ref) == SSA_NAME
>> -      && (!SSA_NAME_VAR (ref) || DECL_ARTIFICIAL (SSA_NAME_VAR (ref))))
>> -    ref = NULL_TREE;
>> +  if (ref && TREE_CODE (ref) == SSA_NAME)
>> +    {
>> +      tree var = SSA_NAME_VAR (ref);
>> +      if (!var)
>> +    ref = NULL_TREE;
>> +      /* Don't warn for cases like when a cdtor returns 'this' on 
>> ARM.  */
>> +      else if (warning_suppressed_p (var, OPT_Wuse_after_free))
>> +    return;
>> +      else if (DECL_ARTIFICIAL (var))
>> +    ref = NULL_TREE;
>> +    }
>>     location_t use_loc = gimple_location (use_stmt);
>>     if (use_loc == UNKNOWN_LOCATION)
>> diff --git a/gcc/testsuite/g++.dg/warn/Wuse-after-free2.C 
>> b/gcc/testsuite/g++.dg/warn/Wuse-after-free2.C
>> new file mode 100644
>> index 00000000000..6d5f2bf01b5
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/warn/Wuse-after-free2.C
>> @@ -0,0 +1,10 @@
>> +// PR target/104213
>> +// { dg-do compile }
>> +// { dg-options "-Wuse-after-free" }
>> +
>> +class C
>> +{
>> +    virtual ~C();
>> +};
>> +
>> +C::~C() {} // { dg-bogus "used after" }
>>
>> base-commit: aeac414923aa1e87986c7fc6f9b921d89a9b86cf
> 



More information about the Gcc-patches mailing list