From: Kevin Buettner <kevinb@redhat.com>
To: Mark Wielaard <mark@klomp.org>
Cc: gdb-patches@sourceware.org, Tom Tromey <tom@tromey.com>
Subject: Re: [PATCH] Pass const frame_info_ptr reference for skip_[language_]trampoline
Date: Tue, 2 May 2023 17:33:09 -0700 [thread overview]
Message-ID: <20230502173309.764aae3c@f37-zws-nv> (raw)
In-Reply-To: <20230502183444.1445634-1-mark@klomp.org>
Hi Mark,
On Tue, 2 May 2023 20:34:44 +0200
Mark Wielaard <mark@klomp.org> wrote:
> 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 inlined enough code in this specific case).
>
> Since passing the frame_info_ptr as const reference is cheaper, use
> that as workaround for this warning.
I've tested your patch on Fedora 38 w/ gcc (GCC) 13.1.1 20230426 (Red
Hat 13.1.1-1) and find that GDB builds again. Currently, it doesn't
build without your patch.
Your explanation and fix looks reasonable to me too.
Tested-by: Kevin Buettner <kevinb@redhat.com>
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
next prev parent reply other threads:[~2023-05-03 0:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-02 18:34 Mark Wielaard
2023-05-03 0:33 ` Kevin Buettner [this message]
2023-05-03 14:50 ` Tom Tromey
2023-05-03 15:11 ` Mark Wielaard
2023-05-03 18:07 ` Tom de Vries
2023-05-03 19:18 ` Mark Wielaard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230502173309.764aae3c@f37-zws-nv \
--to=kevinb@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=mark@klomp.org \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).