From: Tom de Vries <tdevries@suse.de>
To: Simon Marchi <simon.marchi@efficios.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: fix dealloc function not being called for frame 0
Date: Wed, 15 Feb 2023 19:01:52 +0100 [thread overview]
Message-ID: <5be9ed74-04cf-b909-9e8c-d0f38cf28501@suse.de> (raw)
In-Reply-To: <20230209195037.100368-1-simon.marchi@efficios.com>
On 2/9/23 20:50, Simon Marchi wrote:
> Tom de Vries reported [1] a regression in gdb.btrace/record_goto.exp
> caused by 6d3717d4c4 ("gdb: call frame unwinders' dealloc_cache methods
> through destroying the frame cache"). This issue is caught by ASan. On
> a non-ASan build, it may or may not cause a crash or some other issue, I
> haven't tried.
>
> I managed to narrow it down to:
>
> $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.btrace/record_goto/record_goto -ex "start" -ex "record btrace" -ex "next"
>
> ... and then doing repeatedly "record goto 19" and "record goto 27".
> Eventually, I get:
>
> (gdb) record goto 27
> =================================================================
> ==1527735==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210003392a8 at pc 0x55e4c26eef86 bp 0x7ffd229f24e0 sp 0x7ffd229f24d8
> READ of size 8 at 0x6210003392a8 thread T0
> #0 0x55e4c26eef85 in bfcache_eq /home/simark/src/binutils-gdb/gdb/record-btrace.c:1639
> #1 0x55e4c37cdeff in htab_find_slot_with_hash /home/simark/src/binutils-gdb/libiberty/hashtab.c:659
> #2 0x55e4c37ce24a in htab_find_slot /home/simark/src/binutils-gdb/libiberty/hashtab.c:703
> #3 0x55e4c26ef0c6 in bfcache_new /home/simark/src/binutils-gdb/gdb/record-btrace.c:1653
> #4 0x55e4c26f1242 in record_btrace_frame_sniffer /home/simark/src/binutils-gdb/gdb/record-btrace.c:1820
> #5 0x55e4c1b926a1 in frame_unwind_try_unwinder /home/simark/src/binutils-gdb/gdb/frame-unwind.c:136
> #6 0x55e4c1b930d7 in frame_unwind_find_by_frame(frame_info_ptr, void**) /home/simark/src/binutils-gdb/gdb/frame-unwind.c:196
> #7 0x55e4c1bb867f in get_frame_type(frame_info_ptr) /home/simark/src/binutils-gdb/gdb/frame.c:2925
> #8 0x55e4c2ae6798 in print_frame_info(frame_print_options const&, frame_info_ptr, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:1049
> #9 0x55e4c2ade3e1 in print_stack_frame(frame_info_ptr, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:367
> #10 0x55e4c26fda03 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2779
> #11 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
> #12 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
> #13 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
> #14 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383
>
> 0x6210003392a8 is located 424 bytes inside of 4064-byte region [0x621000339100,0x62100033a0e0)
> freed by thread T0 here:
> #0 0x7f6ca34a5b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
> #1 0x55e4c38a4c17 in rpl_free /home/simark/src/binutils-gdb/gnulib/import/free.c:44
> #2 0x55e4c1bbd378 in xfree<void> /home/simark/src/binutils-gdb/gdb/../gdbsupport/gdb-xfree.h:37
> #3 0x55e4c37d1b63 in call_freefun /home/simark/src/binutils-gdb/libiberty/obstack.c:103
> #4 0x55e4c37d25a2 in _obstack_free /home/simark/src/binutils-gdb/libiberty/obstack.c:280
> #5 0x55e4c1bad701 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2112
> #6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564
> #7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573
> #8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772
> #9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
> #10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
> #11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
> #12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383
>
> previously allocated by thread T0 here:
> #0 0x7f6ca34a5e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
> #1 0x55e4c0b55c60 in xmalloc /home/simark/src/binutils-gdb/gdb/alloc.c:57
> #2 0x55e4c37d1a6d in call_chunkfun /home/simark/src/binutils-gdb/libiberty/obstack.c:94
> #3 0x55e4c37d1c20 in _obstack_begin_worker /home/simark/src/binutils-gdb/libiberty/obstack.c:141
> #4 0x55e4c37d1ed7 in _obstack_begin /home/simark/src/binutils-gdb/libiberty/obstack.c:164
> #5 0x55e4c1bad728 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2113
> #6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564
> #7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573
> #8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772
> #9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
> #10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
> #11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
> #12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383
>
> The problem is a stale entry in the bfcache hash table (in
> record-btrace.c), left across a reinit_frame_cache. This entry points
> to something that used to be allocated on the frame obstack, that has
> since been wiped by reinit_frame_cache.
>
> Before the aforementioned, unwinder deallocation functions were called
> by iterating on the frame chain, starting with the sentinel frame, like
> so:
>
> /* Tear down all frame caches. */
> for (frame_info *fi = sentinel_frame; fi != NULL; fi = fi->prev)
> {
> if (fi->prologue_cache && fi->unwind->dealloc_cache)
> fi->unwind->dealloc_cache (fi, fi->prologue_cache);
> if (fi->base_cache && fi->base->unwind->dealloc_cache)
> fi->base->unwind->dealloc_cache (fi, fi->base_cache);
> }
>
> After that patch, we relied on the fact that all frames are (supposedly)
> in the frame_stash. A deletion function was added to the frame_stash
> hash table, so that dealloc functions would be called when emptying the
> frame stash. There is one case, however, where a frame_info is not in
> the frame stash. That is when we create the frame_info for the current
> frame (level 0, unwound from the sentinel frame), but don't computed its
computed -> compute
> frame id. The computation of the frame id for that frame (and only that
> frame, AFAIK) is done lazily. And putting a frame_info in the frame stash
> requires knowing its id. So a frame 0 whose frame id is not computed
> yet is necessarily not in the frame stash.
>
> When replaying with btrace, record_btrace_frame_sniffer insert entries
> corresponding to frames in the "bfcache" hash table. It then relies on
> record_btrace_frame_dealloc_cache being called for each frame to remove
> all those entries when the frames get invalidated. If a frame reinit
> happens while frame 0's id is not computed (and thefore that frame is
thefore -> therefore
> not in frame_stash), record_btrace_frame_dealloc_cache does not get
> called for it, and it leaves a stale entry in bfcache. That then leads
> to a use-after-free when that entry is accessed later, which ASan
> catches.
>
> The proposed solution is to explicitly call frame_info_del on frame 0,
> if it exists, and if its frame id is not computed. If it's frame id is
it's -> its
Thanks,
- Tom
> computed, it is expected that it will be in the frame stash, so it will
> be "deleted" through that.
>
> Reported-By: Tom de Vries <tdevries@suse.de>
next prev parent reply other threads:[~2023-02-15 18:01 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-09 19:50 Simon Marchi
2023-02-10 3:10 ` Simon Marchi
2023-02-15 9:39 ` Tom de Vries
2023-02-15 16:42 ` Simon Marchi
2023-02-15 17:56 ` Tom de Vries
2023-02-15 18:01 ` Tom de Vries [this message]
2023-02-15 18:45 ` Simon Marchi
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=5be9ed74-04cf-b909-9e8c-d0f38cf28501@suse.de \
--to=tdevries@suse.de \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@efficios.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).