[Bug c++/104213] New: bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[Bug c++/104213] New: bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[dtardon at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

            Bug ID: 104213
           Summary: bogus use-after-free in virtual dtor with
                    -ffat-lto-objects on ARM
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dtardon at redhat dot com
  Target Milestone: ---

Created attachment 52281
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52281&action=edit
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.

[Bug c++/104213] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[dtardon at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #1 from David Tardon <dtardon at redhat dot com> ---
Created attachment 52282
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52282&action=edit
reproducer.s

[Bug c++/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|bogus use-after-free in     |[12 Regression] bogus
                   |virtual dtor with           |use-after-free in virtual
                   |-ffat-lto-objects on ARM    |dtor with -ffat-lto-objects
                   |                            |on ARM
           Keywords|                            |diagnostic
   Target Milestone|---                         |12.0

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-01-24
     Ever confirmed|0                           |1
                 CC|                            |mpolacek at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
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

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
-flto=auto -ffat-lto-objects is not needed.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |104075
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #4 from Martin Sebor <msebor at gcc dot gnu.org> ---
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$


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104075
[Bug 104075] bogus/missing -Wuse-after-free

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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 ().

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> 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.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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...

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #8 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
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.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #10 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
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.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #11 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
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.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #12 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
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.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #14 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed.

[Bug target/104213] [12 Regression] bogus use-after-free in virtual dtor with -ffat-lto-objects on ARM

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104213

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |unlvsur at live dot com

--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 104190 has been marked as a duplicate of this bug. ***