Created attachment 52281 [details] reproducer.cpp g++ 12 emits a bogus use-after-free warning on any virtual dtor on ARM if -ffat-lto-objects is used. See the attached minimal reproducer.
Created attachment 52282 [details] reproducer.s
Confirmed with an arm-linux-gnueabihf cross: $ ./cc1plus -quiet -Iinclude reproducer.cpp -flto=auto -ffat-lto-objects -Wuse-after-free reproducer.cpp: In member function ‘C::~C()’: reproducer.cpp:8:10: warning: pointer used after ‘operator delete(void*, unsigned int)’ [-Wuse-after-free] 8 | C::~C() {} | ^ reproducer.cpp:8:10: note: call to ‘operator delete(void*, unsigned int)’ here
-flto=auto -ffat-lto-objects is not needed.
The warning points out the return statement in the dtor below, so it's doing what it's been taught to do. I'm not sure why a dtor would have a return type other than void or why it would need to return the this pointer (it's invalid after the object has been deleted). If that's necessary then the code that emits the return statement could suppress the warning via suppress_warning(ret_stmt, OPT_Wuse_after_free). ;; Function C::~C (_ZN1CD0Ev, funcdef_no=3, decl_uid=4619, cgraph_uid=4, symbol_order=3) void * C::~C (struct C * const this) { void * D.4660; void * _5; <bb 2> : C::~C (this_2(D)); operator delete (this_2(D), 4); <<< delete (this) _5 = this_2(D); <bb 3> : <L0>: return _5; <<< return this } pr104213.C: In member function 'C::~C()': pr104213.C:8:10: warning: pointer used after 'operator delete(void*, unsigned int)' [-Wuse-after-free] 8 | C::~C() {} | ^ pr104213.C:8:10: note: call to 'operator delete(void*, unsigned int)' here tmp$
Return type from ctors/dtors is an ARM EABI special thing, the warning should ignore such returns from dtors if targetm.cxx.cdtor_returns_this ().
> I'm not sure why a dtor would have a return type other than void or why it would need to return the this pointer Because that is what is required by the ARM C++ EABI: From the PDF on: https://developer.arm.com/documentation/ihi0041/latest GC++ABI §3.1.512 Constructor return values This ABI requires C1 and C2 constructors to return this (instead of being void functions) so that a C3 constructor can tail call the C1 constructor and the C1 constructor can tail call C2. Similarly, we require D2 and D1 to return this so that D0 need not save and restore this and D1 can tail call D2 (if there are no virtual bases). D0 is still a void function. We do not require thunks to virtual destructors to return this. Such a thunk would have to adjust the destructor’s result, preventing it from tail calling the destructor, and nullifying any possible saving. Consequently, only non-virtual calls of D1 and D2 destructors can be relied on to return this.
Now that you cite that, seems this error is because we go beyond what the EABI requires and make even the D0 dtor return this that nothing can really use. So one way to fix that would be to also make sure that D0 dtors return void always. We'd need to check what other targets use true for this target hook though and whether they have similar wordings...
I've tried diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 22d3dd1e2ad..c2a0f0c24e2 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -17319,6 +17319,7 @@ finish_constructor_body (void) DECL_RESULT (current_function_decl), val); /* Return the address of the object. */ exprstmt = build_stmt (input_location, RETURN_EXPR, val); + suppress_warning (exprstmt, OPT_Wuse_after_free); add_stmt (exprstmt); } } @@ -17412,6 +17413,7 @@ finish_destructor_body (void) DECL_RESULT (current_function_decl), val); /* Return the address of the object. */ exprstmt = build_stmt (input_location, RETURN_EXPR, val); + suppress_warning (exprstmt, OPT_Wuse_after_free); add_stmt (exprstmt); } } diff --git a/gcc/cp/optimize.cc b/gcc/cp/optimize.cc index 4ad3f1dc9aa..51393710526 100644 --- a/gcc/cp/optimize.cc +++ b/gcc/cp/optimize.cc @@ -168,6 +168,7 @@ build_delete_destructor_body (tree delete_dtor, tree complete_dtor) tree val = DECL_ARGUMENTS (delete_dtor); val = build2 (MODIFY_EXPR, TREE_TYPE (val), DECL_RESULT (delete_dtor), val); + suppress_warning (val, OPT_Wuse_after_free); add_stmt (build_stmt (0, RETURN_EXPR, val)); } } the last hunk should have done the trick but the suppress_warning bits don't survive gimplification, it seems.
Try: val = build2 (MODIFY_EXPR, TREE_TYPE (val), DECL_RESULT (delete_dtor), val); tree stmt = build_stmt (0, RETURN_EXPR, val); suppress_warning (stmt, OPT_Wuse_after_free); add_stmt (stmt); That is do it for the RETURN_EXPR instead of the MODIFY_EXPR. It might be saved. Otherwise the gimplification of RETURN_EXPR to GIMPLE_RETURN needs to save it around.
I did but it didn't work either. So I think we need to properly propagate the suppression bits. Something like this: diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index bf2f60cce9a..0d266241b8c 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -6126,6 +6126,7 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, else { assign = gimple_build_assign (*to_p, *from_p); + copy_warning (assign, *expr_p); gimple_set_location (assign, EXPR_LOCATION (*expr_p)); if (COMPARISON_CLASS_P (*from_p)) copy_warning (assign, *from_p); which, along with the patch above, works.
Obviously the 6131 if (COMPARISON_CLASS_P (*from_p)) 6132 copy_warning (assign, *from_p); doesn't work because we are not dealing with a comparison here.
Removing the COMPARISON_CLASS_P check regresses uninit-pr74762.C. So how about 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..f39092ec416 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -3880,9 +3880,19 @@ 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; + else if (DECL_ARTIFICIAL (var)) + { + /* Don't warn for cases like when a cdtor returns 'this' on ARM. */ + if (warning_suppressed_p (var, OPT_Wuse_after_free)) + return; + ref = NULL_TREE; + } + } location_t use_loc = gimple_location (use_stmt); if (use_loc == UNKNOWN_LOCATION) I guess I can test & post it.
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>: https://gcc.gnu.org/g:7bd1e1296cc36b558a27bbe09352c5c2aca4c5d5 commit r12-6880-g7bd1e1296cc36b558a27bbe09352c5c2aca4c5d5 Author: Marek Polacek <polacek@redhat.com> Date: Tue Jan 25 15:12:51 2022 -0500 warn-access: Prevent -Wuse-after-free on ARM [PR104213] 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. 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. * g++.dg/warn/Wuse-after-free3.C: New test.
Fixed.
*** Bug 104190 has been marked as a duplicate of this bug. ***