public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
* [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
@ 2023-05-02 16:57 vries at gcc dot gnu.org
  2023-05-02 16:59 ` [Bug build/30413] " vries at gcc dot gnu.org
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-02 16:57 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

            Bug ID: 30413
           Summary: [gdb/build] error: storing the address of local
                    variable ‘<anonymous>’ in
                    ‘frame_info_ptr::frame_list.intrusive_list<frame_info_
                    ptr>::m_back’ [-Werror=dangling-pointer=]
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: build
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

The buildbot for opensuse tumbleweed fails (
https://builder.sourceware.org/buildbot/#/builders/97/builds/3582/steps/4/logs/stdio
):
...
In file included from ../../binutils-gdb/gdb/frame.h:75,
                 from ../../binutils-gdb/gdb/symtab.h:40,
                 from ../../binutils-gdb/gdb/language.c:33:
In member function ‘void intrusive_list<T, AsNode>::push_empty(T&) [with T =
frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]’,
    inlined from ‘void intrusive_list<T, AsNode>::push_back(reference) [with T
= frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]’ at
../../binutils-gdb/gdb/../gdbsupport/intrusive_list.h:332:24,
    inlined from ‘frame_info_ptr::frame_info_ptr(const frame_info_ptr&)’ at
../../binutils-gdb/gdb/frame.h:241:26,
    inlined from ‘CORE_ADDR skip_language_trampoline(frame_info_ptr,
CORE_ADDR)’ at ../../binutils-gdb/gdb/language.c:530:49:
../../binutils-gdb/gdb/../gdbsupport/intrusive_list.h:415:12: error: storing
the address of local variable ‘<anonymous>’ in
‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’
[-Werror=dangling-pointer=]
  415 |     m_back = &elem;
      |     ~~~~~~~^~~~~~~
../../binutils-gdb/gdb/language.c: In function ‘CORE_ADDR
skip_language_trampoline(frame_info_ptr, CORE_ADDR)’:
../../binutils-gdb/gdb/language.c:530:49: note: ‘<anonymous>’ declared here
  530 |       CORE_ADDR real_pc = lang->skip_trampoline (frame, pc);
      |                           ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
../../binutils-gdb/gdb/frame.h:359:41: note: ‘frame_info_ptr::frame_list’
declared here
  359 |   static intrusive_list<frame_info_ptr> frame_list;
      |                                         ^~~~~~~~~~
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:1922: language.o] Error 1
make[1]: *** Waiting for unfinished jobs....
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
@ 2023-05-02 16:59 ` vries at gcc dot gnu.org
  2023-05-02 20:48 ` tromey at sourceware dot org
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-02 16:59 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #0)
> The buildbot for opensuse tumbleweed fails 

Same for rawhide.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
  2023-05-02 16:59 ` [Bug build/30413] " vries at gcc dot gnu.org
@ 2023-05-02 20:48 ` tromey at sourceware dot org
  2023-05-02 20:52 ` mark at klomp dot org
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: tromey at sourceware dot org @ 2023-05-02 20:48 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at sourceware dot org

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
https://sourceware.org/pipermail/gdb-patches/2023-May/199291.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
  2023-05-02 16:59 ` [Bug build/30413] " vries at gcc dot gnu.org
  2023-05-02 20:48 ` tromey at sourceware dot org
@ 2023-05-02 20:52 ` mark at klomp dot org
  2023-05-02 20:54 ` mark at klomp dot org
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: mark at klomp dot org @ 2023-05-02 20:52 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2023-05-02 20:52 ` mark at klomp dot org
@ 2023-05-02 20:54 ` mark at klomp dot org
  2023-05-03 10:07 ` vries at gcc dot gnu.org
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: mark at klomp dot org @ 2023-05-02 20:54 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #3 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Tom de Vries from comment #1)
> (In reply to Tom de Vries from comment #0)
> > The buildbot for opensuse tumbleweed fails 
> 
> Same for rawhide.

and fedora-latest (that is f38 with an update to gcc 13.1.1)

The patch proposed in comment #2 should fix it.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2023-05-02 20:54 ` mark at klomp dot org
@ 2023-05-03 10:07 ` vries at gcc dot gnu.org
  2023-05-03 10:49 ` vries at gcc dot gnu.org
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-03 10:07 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
FWIW, I've managed to reproduce this outside of the gdb build, while using
gdbsupport/intrusive_list.h:
...
$ cat test.C
#include <iterator>

#include <assert.h>
#define gdb_assert(A) assert (A)

#include "src/gdbsupport/intrusive_list.h"

class void_ptr;

class void_ptr : public intrusive_list_node<void_ptr>
{
public:
  void_ptr () : m_ptr (nullptr) {
    void_ptr_list.push_back (*this);
  }

  void_ptr (const void_ptr &other) : m_ptr (other.m_ptr)
  {
    void_ptr_list.push_back (*this);
  }

  ~void_ptr ()
  {
    if (is_linked ())
      void_ptr_list.erase (void_ptr_list.iterator_to (*this));
  }

private:
  mutable void *m_ptr = nullptr;
  static intrusive_list<void_ptr> void_ptr_list;
};

intrusive_list<void_ptr> void_ptr::void_ptr_list;

void 
bar (void_ptr arg)
{

}

void
foo (void_ptr arg)
{
  bar (arg);
}

int
main (void)
{
  void_ptr ptr_to_a;
  foo (ptr_to_a);

  return 0;
}
...

With -Wdangling-pointer=0:
...
$ g++ test.C -Wdangling-pointer=0 -pedantic -O1 -g -Werror
$
...

With -Wdangling-pointer=1:
...
$ g++ test.C -Wdangling-pointer=1 -pedantic -O1 -g -Werror
In file included from test.C:6:
In member function ‘void intrusive_list<T, AsNode>::push_empty(T&) [with T =
void_ptr; AsNode = intrusive_base_node<void_ptr>]’,
    inlined from ‘void intrusive_list<T, AsNode>::push_back(reference) [with T
= void_ptr; AsNode = intrusive_base_node<void_ptr>]’ at
src/gdbsupport/intrusive_list.h:332:24,
    inlined from ‘void_ptr::void_ptr(const void_ptr&)’ at test.C:19:29,
    inlined from ‘void foo(void_ptr)’ at test.C:44:7:
src/gdbsupport/intrusive_list.h:415:12: error: storing the address of local
variable ‘<anonymous>’ in
‘void_ptr::void_ptr_list.intrusive_list<void_ptr>::m_back’
[-Werror=dangling-pointer=]
  415 |     m_back = &elem;
      |     ~~~~~~~^~~~~~~
test.C: In function ‘void foo(void_ptr)’:
test.C:44:7: note: ‘<anonymous>’ declared here
   44 |   bar (arg);
      |   ~~~~^~~~~
test.C:33:26: note: ‘void_ptr::void_ptr_list’ declared here
   33 | intrusive_list<void_ptr> void_ptr::void_ptr_list;
      |                          ^~~~~~~~
cc1plus: all warnings being treated as errors
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2023-05-03 10:07 ` vries at gcc dot gnu.org
@ 2023-05-03 10:49 ` vries at gcc dot gnu.org
  2023-05-03 10:49 ` vries at gcc dot gnu.org
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-03 10:49 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
I think the thing the warning complains about is that adding to the list is
unconditional, and removing from the list is conditional (on is_linked), and
the compiler can't figure out that for the temporary object is_linked () ==
true.

[ You could reason that the warning is too aggressive, but that may be
intentional, I'm not sure. ]

The test on is_linked was introduced because of trying to accommodate the
scenario mentioned in the comment:
...
  ~frame_info_ptr ()
  {
    /* If this node has static storage, it may be deleted after                 
       frame_list.  Attempting to erase ourselves would then trigger            
       internal errors, so make sure we are still linked first.  */
    if (is_linked ())
      frame_list.erase (frame_list.iterator_to (*this));
  }
...

So perhaps we can fix this by removing the is_linked test.  The code was added
because the static frame_list and the static frame_info_ptr where in different
compilation units, but that's no longer the case.

So I'm thinking of:
...
diff --git a/gdb/frame.c b/gdb/frame.c
index 36fb02f3c8e..c47244b8cb2 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1733,6 +1733,11 @@ get_current_frame (void)
 static frame_id selected_frame_id = null_frame_id;
 static int selected_frame_level = -1;

+/* See frame-info-ptr.h.  This definition should come before any definition of
+   a static frame_info_ptr.  */
+
+intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
+
 /* The cached frame_info object pointing to the selected frame.
    Looked up on demand by get_selected_frame.  */
 static frame_info_ptr selected_frame;
@@ -3275,10 +3280,6 @@ maintenance_print_frame_id (const char *args, int
from_tty)

 /* See frame-info-ptr.h.  */

-intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
-
-/* See frame-info-ptr.h.  */
-
 frame_info_ptr::frame_info_ptr (struct frame_info *ptr)
   : m_ptr (ptr)
 {
diff --git a/gdb/frame.h b/gdb/frame.h
index 6ed8db0af56..fff5c248070 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -257,8 +257,8 @@ class frame_info_ptr : public
intrusive_list_node<frame_info_ptr>
     /* If this node has static storage, it may be deleted after
        frame_list.  Attempting to erase ourselves would then trigger
        internal errors, so make sure we are still linked first.  */
-    if (is_linked ())
-      frame_list.erase (frame_list.iterator_to (*this));
+    gdb_assert (is_linked ());
+    frame_list.erase (frame_list.iterator_to (*this));
   }

   frame_info_ptr &operator= (const frame_info_ptr &other)


...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2023-05-03 10:49 ` vries at gcc dot gnu.org
@ 2023-05-03 10:49 ` vries at gcc dot gnu.org
  2023-05-03 13:56 ` simon.marchi at polymtl dot ca
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-03 10:49 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simark at simark dot ca

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2023-05-03 10:49 ` vries at gcc dot gnu.org
@ 2023-05-03 13:56 ` simon.marchi at polymtl dot ca
  2023-05-03 15:11 ` cvs-commit at gcc dot gnu.org
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: simon.marchi at polymtl dot ca @ 2023-05-03 13:56 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

Simon Marchi <simon.marchi at polymtl dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at polymtl dot ca

--- Comment #6 from Simon Marchi <simon.marchi at polymtl dot ca> ---
Thanks Tom, this analysis makes sense to me.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2023-05-03 13:56 ` simon.marchi at polymtl dot ca
@ 2023-05-03 15:11 ` cvs-commit at gcc dot gnu.org
  2023-05-03 15:43 ` mark at klomp dot org
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-03 15:11 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Mark Wielaard <mark@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9b0ccb1ebaef7474d4f7242f778531ef042a55fc

commit 9b0ccb1ebaef7474d4f7242f778531ef042a55fc
Author: Mark Wielaard <mark@klomp.org>
Date:   Tue May 2 20:23:32 2023 +0200

    Pass const frame_info_ptr reference for skip_[language_]trampoline

    g++ 13.1.1 produces a -Werror=dangling-pointer=

    In file included from ../../binutils-gdb/gdb/frame.h:75,
                     from ../../binutils-gdb/gdb/symtab.h:40,
                     from ../../binutils-gdb/gdb/language.c:33:
    In member function âvoid intrusive_list<T, AsNode>::push_empty(T&) [with
T = frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]â,
        inlined from âvoid intrusive_list<T, AsNode>::push_back(reference)
[with T = frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]â at
gdbsupport/intrusive_list.h:332:24,
        inlined from âframe_info_ptr::frame_info_ptr(const
frame_info_ptr&)â at gdb/frame.h:241:26,
        inlined from âCORE_ADDR skip_language_trampoline(frame_info_ptr,
CORE_ADDR)â at gdb/language.c:530:49:
    gdbsupport/intrusive_list.h:415:12: error: storing the address of local
variable â<anonymous>â in
âframe_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_backâ
[-Werror=dangling-pointer=]
      415 |     m_back = &elem;
          |     ~~~~~~~^~~~~~~
    gdb/language.c: In function âCORE_ADDR
skip_language_trampoline(frame_info_ptr, CORE_ADDR)â:
    gdb/language.c:530:49: note: â<anonymous>â declared here
      530 |       CORE_ADDR real_pc = lang->skip_trampoline (frame, pc);
          |                           ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
    gdb/frame.h:359:41: note: âframe_info_ptr::frame_listâ declared here
      359 |   static intrusive_list<frame_info_ptr> frame_list;
          |                                         ^~~~~~~~~~

    Each new frame_info_ptr is being pushed on a static frame list and g++
    cannot see why that is safe in case the frame_info_ptr is created and
    destroyed immediately when passed as value.

    It isn't clear why only in this one place g++ sees the issue (probably
    because it can inline enough code in this specific case).

    Since passing the frame_info_ptr as const reference is cheaper, use
    that as workaround for this warning.

    PR build/30413
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30413

    Tested-by: Kevin Buettner <kevinb@redhat.com>
    Reviewed-by: Kevin Buettner <kevinb@redhat.com>
    Reviewed-by: Tom Tromey <tom@tromey.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2023-05-03 15:11 ` cvs-commit at gcc dot gnu.org
@ 2023-05-03 15:43 ` mark at klomp dot org
  2023-05-03 16:40 ` vries at gcc dot gnu.org
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: mark at klomp dot org @ 2023-05-03 15:43 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #8 from Mark Wielaard <mark at klomp dot org> ---
Note that the workaround commit in comment #7 fixed this issue for
fedora-rawhide, fedora-latest and suse-tumbleweed on x86_64. But there is
another instance of this issue on gentoo-sparc:
https://builder.sourceware.org/buildbot/#/builders/230/builds/1290

In file included from ../../binutils-gdb/gdb/frame.h:75,
                 from ../../binutils-gdb/gdb/frame.c:21:
In member function ‘void intrusive_list<T, AsNode>::push_empty(T&) [with T =
frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]’,
    inlined from ‘void intrusive_list<T, AsNode>::push_back(reference) [with T
= frame_info_ptr; AsNode = intrusive_base_node<frame_info_ptr>]’ at
../../binutils-gdb/gdb/../gdbsupport/intrusive_list.h:332:24,
    inlined from ‘frame_info_ptr::frame_info_ptr(std::nullptr_t)’ at
../../binutils-gdb/gdb/frame.h:233:26,
    inlined from ‘frame_info_ptr get_next_frame(frame_info_ptr)’ at
../../binutils-gdb/gdb/frame.c:2065:12,
    inlined from ‘void frame_register_unwind_location(frame_info_ptr, int,
int*, lval_type*, CORE_ADDR*, int*)’ at ../../binutils-gdb/gdb/frame.c:2159:35:
../../binutils-gdb/gdb/../gdbsupport/intrusive_list.h:415:12: error: storing
the address of local variable ‘<anonymous>’ in
‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’
[-Werror=dangling-pointer=]
  415 |     m_back = &elem;
      |     ~~~~~~~^~~~~~~
../../binutils-gdb/gdb/frame.c: In function ‘void
frame_register_unwind_location(frame_info_ptr, int, int*, lval_type*,
CORE_ADDR*, int*)’:
../../binutils-gdb/gdb/frame.c:2159:35: note: ‘<anonymous>’ declared here
 2159 |       this_frame = get_next_frame (this_frame);
      |                    ~~~~~~~~~~~~~~~^~~~~~~~~~~~
../../binutils-gdb/gdb/frame.c:3278:32: note: ‘frame_info_ptr::frame_list’
declared here
 3278 | intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
      |                                ^~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:1922: frame.o] Error 1
make[1]: *** Waiting for unfinished jobs....

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2023-05-03 15:43 ` mark at klomp dot org
@ 2023-05-03 16:40 ` vries at gcc dot gnu.org
  2023-05-03 16:53 ` simon.marchi at polymtl dot ca
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-03 16:40 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #9 from Tom de Vries <vries at gcc dot gnu.org> ---
I've also looked into the following.

The definition order in frame.c is:
...
static frame_info_ptr selected_frame;
  ...
intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
...

All the constructors of frame_info_ptr use frame_list, but selected_frame comes
first, so the constructor for selected_frame is called first, in other words
frame_list is used before its constructor is called.

But this doesn't cause problems because the constructor is trivial (i.e.
performs no action).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2023-05-03 16:40 ` vries at gcc dot gnu.org
@ 2023-05-03 16:53 ` simon.marchi at polymtl dot ca
  2023-05-03 18:00 ` vries at gcc dot gnu.org
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: simon.marchi at polymtl dot ca @ 2023-05-03 16:53 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #10 from Simon Marchi <simon.marchi at polymtl dot ca> ---
I don't think that intrusive_list is trivially constructible, since we assign
the fields like this:

  T *m_front = nullptr;
  T *m_back = nullptr;

I verified using std::is_trivially_constructible.  I suppose that things still
work because the object is put in .bss, so the fields are cleared from the
start.  Still, I'd put the declarations in the right order, just to be extra
safe.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2023-05-03 16:53 ` simon.marchi at polymtl dot ca
@ 2023-05-03 18:00 ` vries at gcc dot gnu.org
  2023-05-03 19:43 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-03 18:00 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #11 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #5)
> So I'm thinking of:
> ...
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 36fb02f3c8e..c47244b8cb2 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1733,6 +1733,11 @@ get_current_frame (void)
>  static frame_id selected_frame_id = null_frame_id;
>  static int selected_frame_level = -1;
>  
> +/* See frame-info-ptr.h.  This definition should come before any definition
> of
> +   a static frame_info_ptr.  */
> +
> +intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
> +
>  /* The cached frame_info object pointing to the selected frame.
>     Looked up on demand by get_selected_frame.  */
>  static frame_info_ptr selected_frame;
> @@ -3275,10 +3280,6 @@ maintenance_print_frame_id (const char *args, int
> from_tty)
>  
>  /* See frame-info-ptr.h.  */
>  
> -intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
> -
> -/* See frame-info-ptr.h.  */
> -
>  frame_info_ptr::frame_info_ptr (struct frame_info *ptr)
>    : m_ptr (ptr)
>  {
> diff --git a/gdb/frame.h b/gdb/frame.h
> index 6ed8db0af56..fff5c248070 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -257,8 +257,8 @@ class frame_info_ptr : public
> intrusive_list_node<frame_info_ptr>
>      /* If this node has static storage, it may be deleted after
>         frame_list.  Attempting to erase ourselves would then trigger
>         internal errors, so make sure we are still linked first.  */
> -    if (is_linked ())
> -      frame_list.erase (frame_list.iterator_to (*this));
> +    gdb_assert (is_linked ());
> +    frame_list.erase (frame_list.iterator_to (*this));
>    }
>  
>    frame_info_ptr &operator= (const frame_info_ptr &other)
> 
> 
> ...

Submitted here:
https://sourceware.org/pipermail/gdb-patches/2023-May/199342.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2023-05-03 18:00 ` vries at gcc dot gnu.org
@ 2023-05-03 19:43 ` cvs-commit at gcc dot gnu.org
  2023-05-03 19:56 ` vries at gcc dot gnu.org
  2023-05-03 20:15 ` vries at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-03 19:43 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #12 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=751c7c72c0105c7d55e58bba3c069c36a74c8937

commit 751c7c72c0105c7d55e58bba3c069c36a74c8937
Author: Tom de Vries <tdevries@suse.de>
Date:   Wed May 3 21:43:03 2023 +0200

    [gdb/build] Fix frame_list position in frame.c

    In commit 995a34b1772 ("Guard against frame.c destructors running before
    frame-info.c's") the following problem was addressed.

    The frame_info_ptr destructor:
    ...
      ~frame_info_ptr ()
      {
        frame_list.erase (frame_list.iterator_to (*this));
      }
    ...
    uses frame_list, which is a static member of class frame_info_ptr,
    instantiated in frame-info.c:
    ...
    intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
    ...

    Then there's a static frame_info_pointer variable named selected_frame in
    frame.c:
    ...
    static frame_info_ptr selected_frame;
    ...

    Because the destructor of selected_frame uses frame_list, its destructor
needs
    to be called before the destructor of frame_list.

    But because they're in different compilation units, the initialization
order and
    consequently destruction order is not guarantueed.

    The commit fixed this by handling the case that the destructor of
frame_list
    is called first, adding a check on is_linked ():
    ...
       ~frame_info_ptr ()
       {
    -    frame_list.erase (frame_list.iterator_to (*this));
    +    /* If this node has static storage, it may be deleted after
    +       frame_list.  Attempting to erase ourselves would then trigger
    +       internal errors, so make sure we are still linked first.  */
    +    if (is_linked ())
    +      frame_list.erase (frame_list.iterator_to (*this));
       }
    ...

    However, since then frame_list has been moved into frame.c, and
    initialization/destruction order is guarantueed inside a compilation unit.

    Revert aforementioned commit, and fix the destruction order problem by
moving
    frame_list before selected_frame.

    Reverting the commit is another way of fixing the already fixed
    Wdangling-pointer warning reported in PR build/30413, in a different way
than
    commit 9b0ccb1ebae ("Pass const frame_info_ptr reference for
    skip_[language_]trampoline").

    Approved-By: Simon Marchi <simon.marchi@efficios.com>
    Tested on x86_64-linux.
    PR build/30413
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30413

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2023-05-03 19:43 ` cvs-commit at gcc dot gnu.org
@ 2023-05-03 19:56 ` vries at gcc dot gnu.org
  2023-05-03 20:15 ` vries at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-03 19:56 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

--- Comment #13 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #10)
> I don't think that intrusive_list is trivially constructible, since we
> assign the fields like this:
> 
>   T *m_front = nullptr;
>   T *m_back = nullptr;
> 
> I verified using std::is_trivially_constructible.  I suppose that things
> still work because the object is put in .bss, so the fields are cleared from
> the start.

Yes, I've now found the additional info I needed here (
https://en.cppreference.com/w/cpp/language/initialization#Non-local_variables
).

So what happens with frame_list is static initialization, which happens before
dynamic initialization, and the ordering within a compilation unit applies to
the dynamic initialization.

> Still, I'd put the declarations in the right order, just to be
> extra safe.

Sure, done as part of the by now committed cleanup patch.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug build/30413] [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=]
  2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2023-05-03 19:56 ` vries at gcc dot gnu.org
@ 2023-05-03 20:15 ` vries at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: vries at gcc dot gnu.org @ 2023-05-03 20:15 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=30413

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |14.1

--- Comment #14 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-05-03 20:15 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-02 16:57 [Bug build/30413] New: [gdb/build] error: storing the address of local variable ‘<anonymous>’ in ‘frame_info_ptr::frame_list.intrusive_list<frame_info_ptr>::m_back’ [-Werror=dangling-pointer=] vries at gcc dot gnu.org
2023-05-02 16:59 ` [Bug build/30413] " vries at gcc dot gnu.org
2023-05-02 20:48 ` tromey at sourceware dot org
2023-05-02 20:52 ` mark at klomp dot org
2023-05-02 20:54 ` mark at klomp dot org
2023-05-03 10:07 ` vries at gcc dot gnu.org
2023-05-03 10:49 ` vries at gcc dot gnu.org
2023-05-03 10:49 ` vries at gcc dot gnu.org
2023-05-03 13:56 ` simon.marchi at polymtl dot ca
2023-05-03 15:11 ` cvs-commit at gcc dot gnu.org
2023-05-03 15:43 ` mark at klomp dot org
2023-05-03 16:40 ` vries at gcc dot gnu.org
2023-05-03 16:53 ` simon.marchi at polymtl dot ca
2023-05-03 18:00 ` vries at gcc dot gnu.org
2023-05-03 19:43 ` cvs-commit at gcc dot gnu.org
2023-05-03 19:56 ` vries at gcc dot gnu.org
2023-05-03 20:15 ` vries at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).