aboutsummaryrefslogtreecommitdiff
path: root/gdb/frame.c
AgeCommit message (Collapse)AuthorFilesLines
2023-03-13Fix crash in inside_main_funcTom Tromey1-0/+8
gdb 13.1 crashes while running the rust compiler's debugger tests. The crash has a number of causes. First, the rust compiler still uses the C++-like _Z mangling, but with its own twist -- some hex digits added to the end of a symbol. So, while gdb finds the correct name of "main": (top-gdb) p name $13 = 0x292e0c0 "rustc_gdb_1031745::main" It isn't found in the minsyms, because C++ demangling yields: [99] t 0x90c0 _ZN17rustc_gdb_10317454main17h5b5be7fe16a97225E section .text rustc_gdb_1031745::main::h5b5be7fe16a97225 zko06yobckx336v This could perhaps be fixed. I also filed a new PR to suggest preferring the linkage name of the main program. Next, the rust compiler emits both a DW_TAG_subprogram and a DW_TAG_namespace for "main". This happens because the file is named "main.rs" -- i.e., the bug is specific to the source file name. The crash also seems to require the nested function inside of 'main', at least for me. The namespace always is generated, but perhaps this changes the ordering in the DWARF. When inside_main_func looks up the main symbol, it finds the namespace symbol rather than the function. (I filed a bug about fixing gdb's symbol tables -- long overdue.) Meanwhile, as I think it's important to fix this crash sooner rather than later, this patch changes inside_main_func to check that the symbol that is found is LOC_BLOCK. This perhaps should have been done in the first place, anyway. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30158
2023-02-19Convert contained_in to methodTom Tromey1-1/+1
This converts contained_in to be a method of block.
2023-02-15gdb: fix dealloc function not being called for frame 0Simon Marchi1-8/+21
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 compute its 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 therefore that frame is 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 its frame id is computed, it is expected that it will be in the frame stash, so it will be "deleted" through that. [1] https://inbox.sourceware.org/gdb-patches/20230130200249.131155-1-simon.marchi@efficios.com/T/#mcf1340ce2906a72ec7ed535ec0c97dba11c3d977 Reported-By: Tom de Vries <tdevries@suse.de> Tested-By: Tom de Vries <tdevries@suse.de> Change-Id: I2351882dd511f3bbc01e4152e9db13b69b3ba384
2023-02-13Remove deprecated_lval_hackTom Tromey1-4/+4
This removes deprecated_lval_hack and the VALUE_LVAL macro, replacing all uses with a call to value::lval. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn many optimized-out value functions into methodsTom Tromey1-11/+11
This turns many functions that are related to optimized-out or availability-checking to be methods of value. The static function value_entirely_covered_by_range_vector is also converted to be a private method. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn remaining value_contents functions into methodsTom Tromey1-6/+6
This turns the remaining value_contents functions -- value_contents, value_contents_all, value_contents_for_printing, and value_contents_for_printing_const -- into methods of value. It also converts the static functions require_not_optimized_out and require_available to be private methods. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn some value_contents functions into methodsTom Tromey1-2/+2
This turns value_contents_raw, value_contents_writeable, and value_contents_all_raw into methods on value. The remaining functions will be changed later in the series; they were a bit trickier and so I didn't include them in this patch. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn value_address and set_value_address functions into methodsTom Tromey1-2/+2
This changes the value_address and set_value_address functions to be methods of value. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn value_lazy and set_value_lazy functions into methodsTom Tromey1-1/+1
This changes the value_lazy and set_value_lazy functions to be methods of value. Much of this patch was written by script. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-13Turn value_type into methodTom Tromey1-2/+2
This changes value_type to be a method of value. Much of this patch was written by script. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2023-02-10[gdb/cli] Add maint info frame-unwindersTom de Vries1-2/+2
Add a new command "maint info frame-unwinders": ... (gdb) help maint info frame-unwinders List the frame unwinders currently in effect, starting with the highest \ priority. ... Output for i386: ... $ gdb -q -batch -ex "set arch i386" -ex "maint info frame-unwinders" The target architecture is set to "i386". dummy DUMMY_FRAME dwarf2 tailcall TAILCALL_FRAME inline INLINE_FRAME i386 epilogue NORMAL_FRAME dwarf2 NORMAL_FRAME dwarf2 signal SIGTRAMP_FRAME i386 stack tramp NORMAL_FRAME i386 sigtramp SIGTRAMP_FRAME i386 prologue NORMAL_FRAME ... Output for x86_64: ... $ gdb -q -batch -ex "set arch i386:x86-64" -ex "maint info frame-unwinders" The target architecture is set to "i386:x86-64". dummy DUMMY_FRAME dwarf2 tailcall TAILCALL_FRAME inline INLINE_FRAME python NORMAL_FRAME amd64 epilogue NORMAL_FRAME i386 epilogue NORMAL_FRAME dwarf2 NORMAL_FRAME dwarf2 signal SIGTRAMP_FRAME amd64 sigtramp SIGTRAMP_FRAME amd64 prologue NORMAL_FRAME i386 stack tramp NORMAL_FRAME i386 sigtramp SIGTRAMP_FRAME i386 prologue NORMAL_FRAME ... Tested on x86_64-linux. Reviewed-By: Tom Tromey <tom@tromey.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
2023-02-08gdb: give sentinel for user frames distinct IDs, register sentinel frames to ↵Simon Marchi1-14/+49
the frame cache The test gdb.base/frame-view.exp fails like this on AArch64: frame^M #0 baz (z1=hahaha, /home/simark/src/binutils-gdb/gdb/value.c:4056: internal-error: value_fetch_lazy_register: Assertion `next_frame != NULL' failed.^M A problem internal to GDB has been detected,^M further debugging may prove unreliable.^M FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error) The sequence of events leading to this is the following: - When we create the user frame (the "select-frame view" command), we create a sentinel frame just for our user-created frame, in create_new_frame. This sentinel frame has the same id as the regular sentinel frame. - When printing the frame, after doing the "select-frame view" command, the argument's pretty printer is invoked, which does an inferior function call (this is the point of the test). This clears the frame cache, including the "real" sentinel frame, which sets the sentinel_frame global to nullptr. - Later in the frame-printing process (when printing the second argument), the auto-reinflation mechanism re-creates the user frame by calling create_new_frame again, creating its own special sentinel frame again. However, note that the "real" sentinel frame, the sentinel_frame global, is still nullptr. If the selected frame had been a regular frame, we would have called get_current_frame at some point during the reinflation, which would have re-created the "real" sentinel frame. But it's not the case when reinflating a user frame. - Deep down the stack, something wants to fill in the unwind stop reason for frame 0, which requires trying to unwind frame 1. This leads us to trying to unwind the PC of frame 1: #0 gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955 #1 0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390 #2 0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089 #3 0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101 #4 0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281 #5 0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376 #6 0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051 #7 0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356 #8 0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110 #9 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239 #10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811 #11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239 #12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0, type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078 #13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513 #14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557 #15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052 - AArch64 defines a special "prev register" function, aarch64_dwarf2_prev_register, to handle unwinding the PC. This function does frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM); - frame_unwind_register_unsigned ultimately creates a lazy register value, saving the frame id of this_frame->next. this_frame is the user-created frame, to this_frame->next is the special sentinel frame we created for it. So the saved ID is the sentinel frame ID. - When time comes to un-lazify the value, value_fetch_lazy_register calls frame_find_by_id, to find the frame with the ID we saved. - frame_find_by_id sees it's the sentinel frame ID, so returns the sentinel_frame global, which is, if you remember, nullptr. - We hit the `gdb_assert (next_frame != NULL)` assertion in value_fetch_lazy_register. The issues I see here are: - The ID of the sentinel frame created for the user-created frame is not distinguishable from the ID of the regular sentinel frame. So there's no way frame_find_by_id could find the right frame, in value_fetch_lazy_register. - Even if they had distinguishable IDs, sentinel frames created for user frames are not registered anywhere, so there's no easy way frame_find_by_id could find it. This patch addresses these two issues: - Give sentinel frames created for user frames their own distinct IDs - Register sentinel frames in the frame cache, so they can be found with frame_find_by_id. I initially had this split in two patches, but I then found that it was easier to explain as a single patch. Rergarding the first part of the change: with this patch, the sentinel frames created for user frames (in create_new_frame) still have stack_status == FID_STACK_SENTINEL, but their code_addr and stack_addr fields are now filled with the addresses used to create the user frame. This ensures this sentinel frame ID is different from the "target" sentinel frame ID, as well as any other "user" sentinel frame ID. If the user tries to create the same frame, with the same addresses, multiple times, create_sentinel_frame just reuses the existing frame. So we won't end up with multiple user sentinels with the same ID. Regular "target" sentinel frames remain with code_addr and stack_addr unset. The concrete changes for that part are: - Remove the sentinel_frame_id constant, since there isn't one "sentinel frame ID" now. Add the frame_id_build_sentinel function for building sentinel frame IDs and a is_sentinel_frame_id function to check if a frame id represents a sentinel frame. - Replace the sentinel_frame_id check in frame_find_by_id with a comparison to `frame_id_build_sentinel (0, 0)`. The sentinel_frame global is meant to contain a reference to the "target" sentinel, so the one with addresses (0, 0). - Add stack and code address parameters to create_sentinel_frame, to be able to create the various types of sentinel frames. - Adjust get_current_frame to create the regular "target" sentinel. - Adjust create_new_frame to create a sentinel with the ID specific to the created user frame. - Adjust sentinel_frame_prev_register to get the sentinel frame ID from the frame_info object, since there isn't a single "sentinel frame ID" now. - Change get_next_frame_sentinel_okay to check for a sentinel-frame-id-like frame ID, rather than for sentinel_frame specifically, since this function could be called with another sentinel frame (and we would want the assert to catch it). The rest of the change is about registering the sentinel frame in the frame cache: - Change frame_stash_add's assertion to allow sentinel frame levels (-1). - Make create_sentinel_frame add the frame to the frame cache. - Change the "sentinel_frame != NULL" check in reinit_frame_cache for a check that the frame stash is not empty. The idea is that if we only have some user-created frames in the cache when reinit_frame_cache is called, we probably want to emit the frames invalid annotation. The goal of that check is to avoid unnecessary repeated annotations, I suppose, so the "frame cache not empty" check should achieve that. After this change, I think we could theoritically get rid of the sentienl_frame global. That sentinel frame could always be found by looking up `frame_id_build_sentinel (0, 0)` in the frame cache. However, I left the global there to avoid slowing the typical case down for nothing. I however, noted in its comment that it is an optimization. With this fix applied, the gdb.base/frame-view.exp now passes for me on AArch64. value_of_register_lazy now saves the special sentinel frame ID in the value, and value_fetch_lazy_register is able to find that sentinel frame after the frame cache reinit and after the user-created frame was reinflated. Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com> Tested-By: Luis Machado <luis.machado@arm.com> Change-Id: I8b77b3448822c8aab3e1c3dda76ec434eb62704f
2023-02-08gdb: call frame unwinders' dealloc_cache methods through destroying the ↵Simon Marchi1-15/+24
frame cache Currently, some frame resources are deallocated by iterating on the frame chain (starting from the sentinel), calling dealloc_cache. The problem is that user-created frames are not part of that chain, so we never call dealloc_cache for them. I propose to make it so the dealloc_cache callbacks are called when the frames are removed from the frame_stash hash table, by registering a deletion function to the hash table. This happens when frame_stash_invalidate is called by reinit_frame_cache. This way, all frames registered in the cache will get their unwinder's dealloc_cache callbacks called. Note that at the moment, the sentinel frames are not registered in the cache, so we won't call dealloc_cache for them. However, it's just a theoritical problem, because the sentinel frame unwinder does not provide this callback. Also, a subsequent patch will change things so that sentinel frames are registered to the cache. I moved the obstack_free / obstack_init pair below the frame_stash_invalidate call in reinit_frame_cache, because I assumed that some dealloc_cache would need to access some data on that obstack, so it would be better to free it after clearing the hash table. Change-Id: If4f9b38266b458c4e2f7eb43e933090177c22190
2023-01-20gdb: make frame_info_ptr auto-reinflatableSimon Marchi1-3/+4
This is the second step of making frame_info_ptr automatic, reinflate on demand whenever trying to obtain the wrapper frame_info pointer, either through the get method or operator->. Make the reinflate method private, it is used as a convenience method in those two. Add an "is_null" method, because it is often needed to know whether the frame_info_ptr wraps an frame_info or is empty. Make m_ptr mutable, so that it's possible to reinflate const frame_info_ptr objects. Whether m_ptr is nullptr or not does not change the logical state of the object, because we re-create it on demand. I believe this is the right use case for mutable. Change-Id: Icb0552d0035e227f81eb3c121d8a9bb2f9d25794 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: make frame_info_ptr grab frame level and id on constructionSimon Marchi1-8/+11
This is the first step of making frame_info_ptr automatic. Remove the frame_info_ptr::prepare_reinflate method, move that code to the constructor. Change-Id: I85cdae3ab1c043c70e2702e7fb38e9a4a8a675d8 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: make user-created frames reinflatableSimon Marchi1-6/+16
This patch teaches frame_info_ptr to reinflate user-created frames (frames created through create_new_frame, with the "select-frame view" command). Before this patch, frame_info_ptr doesn't support reinflating user-created frames, because it currently reinflates by getting the current target frame (for frame 0) or frame_find_by_id (for other frames). To reinflate a user-created frame, we need to call create_new_frame, to make it lookup an existing user-created frame, or otherwise create one. So, in prepare_reinflate, get the frame id even if the frame has level 0, if it is user-created. In reinflate, if the saved frame id is user create it, call create_new_frame. In order to test this, I initially enhanced the gdb.base/frame-view.exp test added by the previous patch by setting a pretty-printer for the type of the function parameters, in which we do an inferior call. This causes print_frame_args to not reinflate its frame (which is a user-created one) properly. On one machine (my Arch Linux one), it properly catches the bug, as the frame is not correctly restored after printing the first parameter, so it messes up the second parameter: frame #0 baz (z1=hahaha, z2=<error reading variable: frame address is not available.>) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:40 40 return z1.m + z2.n; (gdb) FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame frame #0 baz (z1=hahaha, z2=<error reading variable: frame address is not available.>) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:40 40 return z1.m + z2.n; (gdb) FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame again However, on another machine (my Ubuntu 22.04 one), it just passes fine, without the appropriate fix. I then thought about writing a selftest for that, it's more reliable. I left the gdb.base/frame-view.exp pretty printer test there, it's already written, and we never know, it might catch some unrelated issue some day. Change-Id: I5849baf77991fc67a15bfce4b5e865a97265b386 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: make it possible to restore selected user-created framesSimon Marchi1-7/+23
I would like to improve frame_info_ptr to automatically grab the information needed to reinflate a frame, and automatically reinflate it as needed. One thing that is in the way is the fact that some frames can be created out of thin air by the create_new_frame function. These frames are not the fruit of unwinding from the target's current frame. These frames are created by the "select-frame view" command. These frames are not correctly handled by the frame save/restore functions, save_selected_frame, restore_selected_frame and lookup_selected_frame. This can be observed here, using the test included in this patch: $ ./gdb --data-directory=data-directory -nx -q testsuite/outputs/gdb.base/frame-view/frame-view Reading symbols from testsuite/outputs/gdb.base/frame-view/frame-view... (gdb) break thread_func Breakpoint 1 at 0x11a2: file /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c, line 42. (gdb) run Starting program: /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/frame-view/frame-view [Thread debugging using libthread_db enabled] Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1". [New Thread 0x7ffff7cc46c0 (LWP 4171134)] [Switching to Thread 0x7ffff7cc46c0 (LWP 4171134)] Thread 2 "frame-view" hit Breakpoint 1, thread_func (p=0x0) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42 42 foo (11); (gdb) info frame Stack level 0, frame at 0x7ffff7cc3ee0: rip = 0x5555555551a2 in thread_func (/home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42); saved rip = 0x7ffff7d4e8fd called by frame at 0x7ffff7cc3f80 source language c. Arglist at 0x7ffff7cc3ed0, args: p=0x0 Locals at 0x7ffff7cc3ed0, Previous frame's sp is 0x7ffff7cc3ee0 Saved registers: rbp at 0x7ffff7cc3ed0, rip at 0x7ffff7cc3ed8 (gdb) thread 1 [Switching to thread 1 (Thread 0x7ffff7cc5740 (LWP 4171122))] #0 0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6 Here, we create a custom frame for thread 1 (using the stack from thread 2, for convenience): (gdb) select-frame view 0x7ffff7cc3f80 0x5555555551a2 The first calls to "frame" looks good: (gdb) frame #0 thread_func (p=0x7ffff7d4e630) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:42 42 foo (11); But not the second one: (gdb) frame #0 0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6 This second "frame" command shows the current target frame instead of the user-created frame. It's not totally clear how the "select-frame view" feature is expected to behave, especially since it's not tested. I heard accounts that it used to be possible to select a frame like this and do "up" and "down" to navigate the backtrace starting from that frame. The fact that create_new_frame calls frame_unwind_find_by_frame to install the right unwinder suggest that it used to be possible. But that doesn't work today: (gdb) select-frame view 0x7ffff7cc3f80 0x5555555551a2 (gdb) up Initial frame selected; you cannot go up. (gdb) down Bottom (innermost) frame selected; you cannot go down. and "backtrace" always shows the actual thread's backtrace, it ignores the user-created frame: (gdb) bt #0 0x00007ffff7d4b4b6 in ?? () from /usr/lib/libc.so.6 #1 0x00007ffff7d50403 in ?? () from /usr/lib/libc.so.6 #2 0x000055555555521a in main () at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/frame-view.c:56 I don't want to address all the `select-frame view` issues , but I think we can agree that the "frame" command changing the selected frame, as shown above, is a bug. I would expect that command to show the currently selected frame and not change it. This happens because of the scoped_restore_selected_frame object in print_frame_args. The frame information is saved in the constructor (the backtrace below), and restored in the destructor. #0 save_selected_frame (frame_id=0x7ffdc0020ad0, frame_level=0x7ffdc0020af0) at /home/simark/src/binutils-gdb/gdb/frame.c:1682 #1 0x00005631390242f0 in scoped_restore_selected_frame::scoped_restore_selected_frame (this=0x7ffdc0020ad0) at /home/simark/src/binutils-gdb/gdb/frame.c:324 #2 0x000056313993581e in print_frame_args (fp_opts=..., func=0x62100023bde0, frame=..., num=-1, stream=0x60b000000300) at /home/simark/src/binutils-gdb/gdb/stack.c:755 #3 0x000056313993ad49 in print_frame (fp_opts=..., frame=..., print_level=1, print_what=SRC_AND_LOC, print_args=1, sal=...) at /home/simark/src/binutils-gdb/gdb/stack.c:1401 #4 0x000056313993835d in print_frame_info (fp_opts=..., frame=..., print_level=1, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1126 #5 0x0000563139932e0b in print_stack_frame (frame=..., print_level=1, print_what=SRC_AND_LOC, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:368 #6 0x0000563139932bbe in print_stack_frame_to_uiout (uiout=0x611000016840, frame=..., print_level=1, print_what=SRC_AND_LOC, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:346 #7 0x0000563139b0641e in print_selected_thread_frame (uiout=0x611000016840, selection=...) at /home/simark/src/binutils-gdb/gdb/thread.c:1993 #8 0x0000563139940b7f in frame_command_core (fi=..., ignored=true) at /home/simark/src/binutils-gdb/gdb/stack.c:1871 #9 0x000056313994db9e in frame_command_helper<frame_command_core>::base_command (arg=0x0, from_tty=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1976 Since the user-created frame has level 0 (identified by the saved level -1), lookup_selected_frame just reselects the target's current frame, and the user-created frame is lost. My goal here is to fix this particular problem. Currently, select_frame does not set selected_frame_id and selected_frame_level for frames with level 0. It leaves them at null_frame_id / -1, indicating to restore_selected_frame to use the target's current frame. User-created frames also have level 0, so add a special case them such that select_frame saves their selected id and level. save_selected_frame does not need any change. Change the assertion in restore_selected_frame that checks `frame_level != 0` to account for the fact that we can restore user-created frames, which have level 0. Finally, change lookup_selected_frame to make it able to re-create user-created frame_info objects from selected_frame_level and selected_frame_id. Add a minimal test case for the case described above, that is the "select-frame view" command followed by the "frame" command twice. In order to have a known stack frame to switch to, the test spawns a second thread, and tells the first thread to use the other thread's top frame. Change-Id: Ifc77848dc465fbd21324b9d44670833e09fe98c7 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: add create_new_frame(frame_id) overloadSimon Marchi1-8/+18
The subsequent patches will need to call create_new_frame with an existing frame_id representing a user created frame. They could call the existing create_new_frame, passing both addresses, but it seems nicer to have a version of the function that takes a frame_id directly. Change-Id: If31025314fec0c3e644703e4391a5ef8079e1a32 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: add user-created frames to stashSimon Marchi1-2/+17
A subsequent patch makes it possible for frame_info_ptr to reinflate user-created frames. If two frame_info_ptr objects wrapping the same user-created frame_info need to do reinflation, we want them to end up pointing to the same frame_info instance, and not create two separate frame_infos. Otherwise, GDB gets confused down the line, as the state kept in one frame_info object starts differing from the other frame_info. Achieve this by making create_new_frame place the user-created frames in the frame stash. This way, when the second frame_info_ptr does reinflation, it will find the existing frame_info object, created by the other frame_info_ptr, in the frame stash. To make the frame stash differentiate between regular and user-created frame infos which would otherwise be equal, change frame_addr_hash and frame_id::operator== to account for frame_id::user_created_p. I made create_new_frame look up existing frames in the stash, and only create one if it doesn't find one. The goal is to avoid the "select-frame view"/"info frame view"/"frame view" commands from overriding existing entries into the stash, should the user specify the same frame more than once. This will also help in the subsequent patch that makes frame_info_ptr capable of reinflating user-created frames. It will be able to just call create_new_frame and it will do the right thing. Change-Id: I14ba5799012056c007b4992ecb5c7adafd0c2404 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: add frame_id::user_created_pSimon Marchi1-0/+1
Later in this series, we'll need to differentiate frame ids for regular frames (obtained from the target state and unwinding from it) vs frame ids for user-created frames (created with create_new_frame). Add the frame_id::user_created_p field to indicate a frame is user-created, and set it in create_new_frame. The field is otherwise not used yet, so not changes in behavior are expected. Change-Id: I60de3ce581ed01bf0fddb30dff9bd932840120c3 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-20gdb: move frame_info_ptr to frame.{c,h}Simon Marchi1-1/+42
A patch later in this series will make frame_info_ptr access some fields internal to frame_info, which we don't want to expose outside of frame.c. Move the frame_info_ptr class to frame.h, and the definitions to frame.c. Remove frame-info.c and frame-info.h. Change-Id: Ic5949759e6262ea0da6123858702d48fe5673fea Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2023-01-01Update copyright year range in header of all files managed by GDBJoel Brobecker1-1/+1
This commit is the result of running the gdb/copyright.py script, which automated the update of the copyright year range for all source files managed by the GDB project to be updated to include year 2023.
2022-12-07gdb: add invalidate_selected_frame functionSimon Marchi1-2/+14
Instead of using `select_frame (nullptr)` to invalidate the selected frame, introduce a function to do that. There is no change in behavior, but it makes the intent a bit clearer. It also allows adding an assert in select_frame that fi is not nullptr, so it avoids passing nullptr by mistake. Change-Id: I61643f46bc8eca428334513ebdaadab63997bdd0 Reviewed-By: Bruno Larsen <blarsen@redhat.com>
2022-12-01gdb: make frame_register staticSimon Marchi1-1/+6
It is only used inside frame.c. Change-Id: I44eb46a5992412f8f8b4954b2284b0ef3b549504
2022-11-10gdb: move frame_info_ptr method implementations to frame-info.cSimon Marchi1-3/+1
I don't see any particular reason why the implementations of the frame_info_ptr object are in the header file. It only seems to add some complexity. Since we can't include frame.h in frame-info.h, we have to add declarations of functions defined in frame.c, in frame-info.h. By moving the implementations to a new frame-info.c, we can avoid that. Change-Id: I435c828f81b8a3392c43ef018af31effddf6be9c Reviewed-By: Bruno Larsen <blarsen@redhat.com> Reviewed-By: Tom Tromey <tom@tromey.com>
2022-11-07gdb: make lookup_selected_frame staticSimon Marchi1-2/+6
Change-Id: Ide2749a34333110c7f0112b25852c78cace0d2b4
2022-10-28Convert compunit_language to a methodTom Tromey1-4/+4
This changes compunit_language to be a method on compunit_symtab. Approved-By: Simon Marchi <simon.marchi@efficios.com>
2022-10-25gdb: remove spurious spaces after frame_info_ptrSimon Marchi1-16/+16
Fix some whitespace issues introduced with the frame_info_ptr patch. Change-Id: I158d30d8108c97564276c647fc98283ff7b12163
2022-10-19internal_error: remove need to pass __FILE__/__LINE__Pedro Alves1-6/+3
Currently, every internal_error call must be passed __FILE__/__LINE__ explicitly, like: internal_error (__FILE__, __LINE__, "foo %d", var); The need to pass in explicit __FILE__/__LINE__ is there probably because the function predates widespread and portable variadic macros availability. We can use variadic macros nowadays, and in fact, we already use them in several places, including the related gdb_assert_not_reached. So this patch renames the internal_error function to something else, and then reimplements internal_error as a variadic macro that expands __FILE__/__LINE__ itself. The result is that we now should call internal_error like so: internal_error ("foo %d", var); Likewise for internal_warning. The patch adjusts all calls sites. 99% of the adjustments were done with a perl/sed script. The non-mechanical changes are in gdbsupport/errors.h, gdbsupport/gdb_assert.h, and gdb/gdbarch.py. Approved-By: Simon Marchi <simon.marchi@efficios.com> Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
2022-10-10Change GDB to use frame_info_ptrTom Tromey1-146/+146
This changes GDB to use frame_info_ptr instead of frame_info * The substitution was done with multiple sequential `sed` commands: sed 's/^struct frame_info;/class frame_info_ptr;/' sed 's/struct frame_info \*/frame_info_ptr /g' - which left some issues in a few files, that were manually fixed. sed 's/\<frame_info \*/frame_info_ptr /g' sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace problems. The changed files were then manually checked and some 'sed' changes undone, some constructors and some gets were added, according to what made sense, and what Tromey originally did Co-Authored-By: Bruno Larsen <blarsen@redhat.com> Approved-by: Tom Tomey <tom@tromey.com>
2022-10-10Introduce frame_info_ptr smart pointer classTom Tromey1-0/+6
This adds frame_info_ptr, a smart pointer class. Every instance of the class is kept on an intrusive list. When reinit_frame_cache is called, the list is traversed and all the pointers are invalidated. This should help catch the typical GDB bug of keeping a frame_info pointer alive where a frame ID was needed instead. Co-Authored-By: Bruno Larsen <blarsen@redhat.com> Approved-by: Tom Tomey <tom@tromey.com>
2022-10-10Remove frame_id_eqTom Tromey1-14/+13
This replaces frame_id_eq with operator== and operator!=. I wrote this for a version of this series that I later abandoned; but since it simplifies the code, I left this patch in. Approved-by: Tom Tomey <tom@tromey.com>
2022-09-21gdb: remove TYPE_LENGTHSimon Marchi1-2/+2
Remove the macro, replace all uses with calls to type::length. Change-Id: Ib9bdc954576860b21190886534c99103d6a47afb
2022-08-24gdb: new 'maint print frame-id' commandAndrew Burgess1-0/+28
When debugging a certain class of GDB bug, I often end up wanting to know what GDB thinks the frame-id is in a particular frame. It's not too hard to pull this from some debug output, but I thought it might be nice if there was a maintenance command that could tell us. This commit adds 'maint print frame-id' which prints the frame-id of the currently selected frame. You can also pass a frame level number to find the frame-id for a specific frame. There's a new test too.
2022-04-27gdb: remove BLOCK_{START,END} macrosSimon Marchi1-1/+1
Replace with equivalent methods. Change-Id: I10a6c8a2a86462d9d4a6a6409a3f07a6bea66310
2022-04-20Replace symbol_symtab with symbol::symtabTom Tromey1-1/+1
This turns symbol_symtab into a method on symbol. It also replaces symbol_set_symtab with a method.
2022-04-11gdb: remove symbol value macrosSimon Marchi1-2/+2
Remove all macros related to getting and setting some symbol value: #define SYMBOL_VALUE(symbol) (symbol)->value.ivalue #define SYMBOL_VALUE_ADDRESS(symbol) \ #define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value) \ #define SYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes #define SYMBOL_VALUE_COMMON_BLOCK(symbol) (symbol)->value.common_block #define SYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block #define SYMBOL_VALUE_CHAIN(symbol) (symbol)->value.chain #define MSYMBOL_VALUE(symbol) (symbol)->value.ivalue #define MSYMBOL_VALUE_RAW_ADDRESS(symbol) ((symbol)->value.address + 0) #define MSYMBOL_VALUE_ADDRESS(objfile, symbol) \ #define BMSYMBOL_VALUE_ADDRESS(symbol) \ #define SET_MSYMBOL_VALUE_ADDRESS(symbol, new_value) \ #define MSYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes #define MSYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block Replace them with equivalent methods on the appropriate objects. Change-Id: Iafdab3b8eefc6dc2fd895aa955bf64fafc59ed50
2022-03-29Unify gdb printf functionsTom Tromey1-25/+25
Now that filtered and unfiltered output can be treated identically, we can unify the printf family of functions. This is done under the name "gdb_printf". Most of this patch was written by script.
2022-02-06gdb: remove SYMBOL_LINE macroSimon Marchi1-2/+2
Add a getter and a setter for a symbol's line. Remove the corresponding macro and adjust all callers. Change-Id: I229f2b8fcf938c07975f641361313a8761fad9a5
2022-01-18Move gdb obstack code to gdbsupportTom Tromey1-1/+1
This moves the gdb-specific obstack code -- both extensions like obconcat and obstack_strdup, and things like auto_obstack -- to gdbsupport.
2022-01-01Automatic Copyright Year update after running gdb/copyright.pyJoel Brobecker1-1/+1
This commit brings all the changes made by running gdb/copyright.py as per GDB's Start of New Year Procedure. For the avoidance of doubt, all changes in this commits were performed by the script.
2021-12-03gdb: trivial changes to use array_viewSimon Marchi1-1/+1
Change a few relatively obvious spots using value contents to propagate the use array_view a bit more. Change-Id: I5338a60986f06d5969fec803d04f8423c9288a15
2021-12-03gdb: make extract_integer take an array_viewSimon Marchi1-3/+1
I think it would make sense for extract_integer, extract_signed_integer and extract_unsigned_integer to take an array_view. This way, when we extract an integer, we can validate that we don't overflow the buffer passed by the caller (e.g. ask to extract a 4-byte integer but pass a 2-byte buffer). - Change extract_integer to take an array_view - Add overloads of extract_signed_integer and extract_unsigned_integer that take array_views. Keep the existing versions so we don't need to change all callers, but make them call the array_view versions. This shortens some places like: result = extract_unsigned_integer (value_contents (result_val).data (), TYPE_LENGTH (value_type (result_val)), byte_order); into result = extract_unsigned_integer (value_contents (result_val), byte_order); value_contents returns an array view that is of length `TYPE_LENGTH (value_type (result_val))` already, so the length is implicitly communicated through the array view. Change-Id: Ic1c1f98c88d5c17a8486393af316f982604d6c95
2021-10-28gdb: add add_setshow_prefix_cmdSimon Marchi1-6/+5
There's a common pattern to call add_basic_prefix_cmd and add_show_prefix_cmd to add matching set and show commands. Add the add_setshow_prefix_cmd function to factor that out and use it at a few places. Change-Id: I6e9e90a30e9efb7b255bf839cac27b85d7069cfd
2021-10-25gdb: change functions returning value contents to use gdb::array_viewSimon Marchi1-10/+13
The bug fixed by this [1] patch was caused by an out-of-bounds access to a value's content. The code gets the value's content (just a pointer) and then indexes it with a non-sensical index. This made me think of changing functions that return value contents to return array_views instead of a plain pointer. This has the advantage that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view are checked, making bugs more apparent / easier to find. This patch changes the return types of these functions, and updates callers to call .data() on the result, meaning it's not changing anything in practice. Additional work will be needed (which can be done little by little) to make callers propagate the use of array_view and reap the benefits. [1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3
2021-09-27gdb: prevent an assertion when computing the frame_id for an inline frameAndrew Burgess1-8/+52
I ran into this assertion while GDB was trying to unwind the stack: gdb/inline-frame.c:173: internal-error: void inline_frame_this_id(frame_info*, void**, frame_id*): Assertion `frame_id_p (*this_id)' failed. That is, when building the frame_id for an inline frame, GDB asks for the frame_id of the previous frame. Unfortunately, no valid frame_id was returned for the previous frame, and so the assertion triggers. What is happening is this, I had a stack that looked something like this (the arrows '->' point from caller to callee): normal_frame -> inline_frame However, for whatever reason (e.g. broken debug information, or corrupted stack contents in the inferior), when GDB tries to unwind "normal_frame", it ends up getting back effectively the same frame, thus the call stack looks like this to GDB: .-> normal_frame -> inline_frame | | '-----' Given such a situation we would expect GDB to terminate the stack with an error like this: Backtrace stopped: previous frame identical to this frame (corrupt stack?) However, the inline_frame causes a problem, and here's why: When unwinding we start from the sentinel frame and call get_prev_frame. We eventually end up in get_prev_frame_if_no_cycle, in here we create a raw frame, and as this is frame #0 we immediately return. However, eventually we will try to unwind the stack further. When we do this we inevitably needing to know the frame_id for frame #0, and so, eventually, we end up in compute_frame_id. In compute_frame_id we first find the right unwinder for this frame, in our case (i.e. for inline_frame) the $pc is within the function normal_frame, but also within a block associated with the inlined function inline_frame, as such the inline frame unwinder claims this frame. Back in compute_frame_id we next compute the frame_id, for our inline_frame this means a call to inline_frame_this_id. The ID of an inline frame is based on the id of the previous frame, so from inline_frame_this_id we call get_prev_frame_always, this eventually calls get_prev_frame_if_no_cycle again, which creates another raw frame and calls compute_frame_id (for frames other than frame 0 we immediately compute the frame_id). In compute_frame_id we again identify the correct unwinder for this frame. Our $pc is unchanged, however, the fact that the next frame is of type INLINE_FRAME prevents the inline frame unwinder from claiming this frame again, and so, the standard DWARF frame unwinder claims normal_frame. We return to compute_frame_id and call the standard DWARF function to build the frame_id for normal_frame. With the frame_id of normal_frame figured out we return to compute_frame_id, and then to get_prev_frame_if_no_cycle, where we add the ID for normal_frame into the frame_id cache, and return the frame back to inline_frame_this_id. From inline_frame_this_id we build a frame_id for inline_frame and return to compute_frame_id, and then to get_prev_frame_if_no_cycle, which adds the frame_id for inline_frame into the frame_id cache. So far, so good. However, as we are trying to unwind the complete stack, we eventually ask for the previous frame of normal_frame, remember, at this point GDB doesn't know the stack is corrupted (with a cycle), GDB still needs to figure that out. So, we eventually end up in get_prev_frame_if_no_cycle where we create a raw frame and call compute_frame_id, remember, this is for the frame before normal_frame. The first task for compute_frame_id is to find the unwinder for this frame, so all of the frame sniffers are tried in order, this includes the inline frame sniffer. The inline frame sniffer asks for the $pc, this request is sent up the stack to normal_frame, which, due to its cyclic behaviour, tells GDB that the $pc in the previous frame was the same as the $pc in normal_frame. GDB spots that this $pc corresponds to both the function normal_frame and also the inline function inline_frame. As the next frame is not an INLINE_FRAME then GDB figures that we have not yet built a frame to cover inline_frame, and so the inline sniffer claims this new frame. Our stack is now looking like this: inline_frame -> normal_frame -> inline_frame But, we have not yet computed the frame id for the outer most (on the left) inline_frame. After the frame sniffer has claimed the inline frame GDB returns to compute_frame_id and calls inline_frame_this_id. In here GDB calls get_prev_frame_always, which eventually ends up in get_prev_frame_if_no_cycle again, where we create a raw frame and call compute_frame_id. Just like before, compute_frame_id tries to find an unwinder for this new frame, it sees that the $pc is within both normal_frame and inline_frame, but the next frame is, again, an INLINE_FRAME, so, just like before the standard DWARF unwinder claims this frame. Back in compute_frame_id we again call the standard DWARF function to build the frame_id for this new copy of normal_frame. At this point the stack looks like this: normal_frame -> inline_frame -> normal_frame -> inline_frame After compute_frame_id we return to get_prev_frame_if_no_cycle, where we try to add the frame_id for the new normal_frame into the frame_id cache, however, unlike before, we fail to add this frame_id as it is a duplicate of the previous normal_frame frame_id. Having found a duplicate get_prev_frame_if_no_cycle unlinks the new frame from the stack, and returns nullptr, the stack now looks like this: inline_frame -> normal_frame -> inline_frame The nullptr result from get_prev_frame_if_no_cycle is fed back to inline_frame_this_id, which forwards this to get_frame_id, which immediately returns null_frame_id. As null_frame_id is not considered a valid frame_id, this is what triggers the assertion. In summary then: - inline_frame_this_id currently assumes that as the inline frame exists, we will always get a valid frame back from get_prev_frame_always, - get_prev_frame_if_no_cycle currently assumes that it is safe to return nullptr when it sees a cycle. Notice that in frame.c:compute_frame_id, this code: fi->this_id.value = outer_frame_id; fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value); gdb_assert (frame_id_p (fi->this_id.value)); The assertion makes it clear that the this_id function must always return a valid frame_id (e.g. null_frame_id is not a valid return value), and similarly in inline_frame.c:inline_frame_this_id this code: *this_id = get_frame_id (get_prev_frame_always (this_frame)); /* snip comment */ gdb_assert (frame_id_p (*this_id)); Makes it clear that every inline frame expects to be able to get a previous frame, which will have a valid frame_id. As I have discussed above, these assumptions don't currently hold in all cases. One possibility would be to move the call to get_prev_frame_always forward from inline_frame_this_id to inline_frame_sniffer, however, this falls foul of (in frame.c:frame_cleanup_after_sniffer) this assertion: /* No sniffer should extend the frame chain; sniff based on what is already certain. */ gdb_assert (!frame->prev_p); This assert prohibits any sniffer from trying to get the previous frame, as getting the previous frame is likely to depend on the next frame, I can understand why this assertion is a good thing, and I'm in no rush to alter this rule. The solution proposed here takes onboard feedback from both Pedro, and Simon (see the links below). The get_prev_frame_if_no_cycle function is renamed to get_prev_frame_maybe_check_cycle, and will now not do cycle detection for inline frames, even when we spot a duplicate frame it is still returned. This is fine, as, if the normal frame has a duplicate frame-id then the inline frame will also have a duplicate frame-id. And so, when we reject the inline frame, the duplicate normal frame, which is previous to the inline frame, will also be rejected. In inline-frame.c the call to get_prev_frame_always is no longer nested inside the call to get_frame_id. There are reasons why get_prev_frame_always can return nullptr, for example, if there is a memory error while trying to get the previous frame, if this should happen then we now give a more informative error message. Historical Links: Patch v2: https://sourceware.org/pipermail/gdb-patches/2021-June/180208.html Feedback: https://sourceware.org/pipermail/gdb-patches/2021-July/180651.html https://sourceware.org/pipermail/gdb-patches/2021-July/180663.html Patch v3: https://sourceware.org/pipermail/gdb-patches/2021-July/181029.html Feedback: https://sourceware.org/pipermail/gdb-patches/2021-July/181035.html Additional input: https://sourceware.org/pipermail/gdb-patches/2021-September/182040.html
2021-09-07gdb: make thread_info::executing privateAndrew Burgess1-1/+1
Rename thread_info::executing to thread_info::m_executing, and make it private. Add a new get/set member functions, and convert GDB to make use of these. The only real change of interest in this patch is in thread.c where I have deleted the helper function set_executing_thread, and now just use the new set function thread_info::set_executing. However, the old helper function set_executing_thread included some code to reset the thread's stop_pc, so I moved this code into the new function thread_info::set_executing. However, I don't believe there is anywhere that this results in a change of behaviour, previously the executing flag was always set true through a call to set_executing_thread anyway.
2021-08-04gdb: Use unwinder name in frame_info::to_stringLancelot SIX1-2/+2
While working on a stack unwinding issue using 'set debug frame on', I noticed the frame_info::to_string method could be slightly improved. Unwinders have been given a name in a154d838a70e96d888620c072e2d6ea8bdf044ca. Before this patch, frame_info debug output prints the host address of the used unwinder, which is not easy to interpret. This patch proposes to use the unwinder name instead since we now have it. Before the patch: {level=1,type=NORMAL_FRAME,unwind=0x2ac1763ec0,pc=0x3ff7fc3460,id={stack=0x3ff7ea79b0,code=0x0000003ff7fc33ac,!special},func=0x3ff7fc33ac} With the patch: {level=1,type=NORMAL_FRAME,unwinder="riscv prologue",pc=0x3ff7fc3460,id={stack=0x3ff7ea79b0,code=0x0000003ff7fc33ac,!special},func=0x3ff7fc33ac} Tested on riscv64-linux-gnu.
2021-07-27gdb: remove VALUE_FRAME_ID and fix another frame debug issueAndrew Burgess1-18/+7
This commit was originally part of this patch series: (v1): https://sourceware.org/pipermail/gdb-patches/2021-May/179357.html (v2): https://sourceware.org/pipermail/gdb-patches/2021-June/180208.html (v3): https://sourceware.org/pipermail/gdb-patches/2021-July/181028.html However, that series is being held up in review, so I wanted to break out some of the non-related fixes in order to get these merged. This commit addresses two semi-related issues, both of which are problems exposed by using 'set debug frame on'. The first issue is in frame.c in get_prev_frame_always_1, and was introduced by this commit: commit a05a883fbaba69d0f80806e46a9457727fcbe74c Date: Tue Jun 29 12:03:50 2021 -0400 gdb: introduce frame_debug_printf This commit replaced fprint_frame with frame_info::to_string. However, the former could handle taking a nullptr while the later, a member function, obviously requires a non-nullptr in order to make the function call. In one place we are not-guaranteed to have a non-nullptr, and so, there is the possibility of triggering undefined behaviour. The second issue addressed in this commit has existed for a while in GDB, and would cause this assertion: gdb/frame.c:622: internal-error: frame_id get_frame_id(frame_info*): Assertion `fi->this_id.p != frame_id_status::COMPUTING' failed. We attempt to get the frame_id for a frame while we are computing the frame_id for that same frame. What happens is that when GDB stops we create a frame_info object for the sentinel frame (frame #-1) and then we attempt to unwind this frame to create a frame_info object for frame #0. In the test case used here to expose the issue we have created a Python frame unwinder. In the Python unwinder we attemt to read the program counter register. Reading this register will initially create a lazy register value. The frame-id stored in the lazy register value will be for the sentinel frame (lazy register values hold the frame-id for the frame from which the register will be unwound). However, the Python unwinder does actually want to examine the value of the program counter, and so the lazy register value is resolved into a non-lazy value. This sends GDB into value_fetch_lazy_register in value.c. Now, inside this function, if 'set debug frame on' is in effect, then we want to print something like: frame=%d, regnum=%d(%s), .... Where 'frame=%d' will be the relative frame level of the frame for which the register is being fetched, so, in this case we would expect to see 'frame=0', i.e. we are reading a register as it would be in frame #0. But, remember, the lazy register value actually holds the frame-id for frame #-1 (the sentinel frame). So, to get the frame_info for frame #0 we used to call: frame = frame_find_by_id (VALUE_FRAME_ID (val)); Where VALUE_FRAME_ID is: #define VALUE_FRAME_ID(val) (get_prev_frame_id_by_id (VALUE_NEXT_FRAME_ID (val))) That is, we start with the frame-id for the next frame as obtained by VALUE_NEXT_FRAME_ID, then call get_prev_frame_id_by_id to get the frame-id of the previous frame. The get_prev_frame_id_by_id function finds the frame_info for the given frame-id (in this case frame #-1), calls get_prev_frame to get the previous frame, and then calls get_frame_id. The problem here is that calling get_frame_id requires that we know the frame unwinder, so then have to try each frame unwinder in turn, which would include the Python unwinder.... which is where we started, and thus we have a loop! To prevent this loop GDB has an assertion in place, which is what actually triggers. Solving the assertion failure is pretty easy, if we consider the code in value_fetch_lazy_register and get_prev_frame_id_by_id then what we do is: 1. Start with a frame_id taken from a value, 2. Lookup the corresponding frame, 3. Find the previous frame, 4. Get the frame_id for that frame, and 5. Lookup the corresponding frame 6. Print the frame's level Notice that steps 3 and 5 give us the exact same result, step 4 is just wasted effort. We could shorten this process such that we drop steps 4 and 5, thus: 1. Start with a frame_id taken from a value, 2. Lookup the corresponding frame, 3. Find the previous frame, 6. Print the frame's level This will give the exact same frame as a result, and this is what I have done in this patch by removing the use of VALUE_FRAME_ID from value_fetch_lazy_register. Out of curiosity I looked to see how widely VALUE_FRAME_ID was used, and saw it was only used in one other place in valops.c:value_assign, where, once again, we take the result of VALUE_FRAME_ID and pass it to frame_find_by_id, thus introducing a redundant frame_id lookup. I don't think the value_assign case risks triggering the assertion though, as we are unlikely to call value_assign while computing the frame_id for a frame, however, we could make value_assign slightly more efficient, with no real additional complexity, by removing the use of VALUE_FRAME_ID. So, in this commit, I completely remove VALUE_FRAME_ID, and replace it with a use of VALUE_NEXT_FRAME_ID, followed by a direct call to get_prev_frame_always, this should make no difference in either case, and resolves the assertion issue from value.c. As I said, this patch was originally part of another series, the original test relied on the fixes in that original series. However, I was able to create an alternative test for this issue by enabling frame debug within an existing test script. This commit probably fixes bug PR gdb/27938, though the bug doesn't have a reproducer attached so it is not possible to know for sure. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27938
2021-06-29gdb: introduce FRAME_SCOPED_DEBUG_ENTER_EXITSimon Marchi1-9/+12
Introduce FRAME_SCOPED_DEBUG_ENTER_EXIT and use it to print enter/exit messages in important frame-related functions. I think this helps understand which lower-level operations are done as part of which higher-level operation. And it helps visually skip over a higher-level operation you are not interested in. Here's an example, combined with some py-unwind messages: [frame] frame_unwind_find_by_frame: enter [frame] frame_unwind_find_by_frame: this_frame=0 [frame] frame_unwind_try_unwinder: trying unwinder "dummy" [frame] frame_unwind_try_unwinder: no [frame] frame_unwind_try_unwinder: trying unwinder "dwarf2 tailcall" [frame] frame_unwind_try_unwinder: no [frame] frame_unwind_try_unwinder: trying unwinder "inline" [frame] frame_unwind_try_unwinder: no [frame] frame_unwind_try_unwinder: trying unwinder "jit" [frame] frame_unwind_try_unwinder: no [frame] frame_unwind_try_unwinder: trying unwinder "python" [py-unwind] pyuw_sniffer: enter [frame] frame_unwind_register_value: enter [frame] frame_unwind_register_value: frame=-1, regnum=7(rsp) [frame] frame_unwind_register_value: -> register=7 bytes=[40ddffffff7f0000] [frame] frame_unwind_register_value: exit [py-unwind] pyuw_sniffer: frame=0, sp=0x7fffffffdd40, pc=0x5555555551ec [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_eq: l={stack=<sentinel>,!code,special=0x0000000000000000}, r={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_unwind_register_value: enter [frame] frame_unwind_register_value: frame=-1, regnum=6(rbp) [frame] frame_unwind_register_value: -> register=6 bytes=[50ddffffff7f0000] [frame] frame_unwind_register_value: exit [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_eq: l={stack=<sentinel>,!code,special=0x0000000000000000}, r={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] get_prev_frame: enter [frame] get_prev_frame_always_1: enter [frame] get_prev_frame_always_1: this_frame=-1 [frame] get_prev_frame_always_1: -> {level=0,type=NORMAL_FRAME,unwind=0x5588ee3d17c0,pc=0x5555555551ec,id=<not computed>,func=<unknown>} // cached [frame] get_prev_frame_always_1: exit [frame] get_prev_frame: exit [frame] value_fetch_lazy_register: (frame=0, regnum=6(rbp), ...) -> register=6 bytes=[50ddffffff7f0000] [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_eq: l={stack=<sentinel>,!code,special=0x0000000000000000}, r={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_unwind_register_value: enter [frame] frame_unwind_register_value: frame=-1, regnum=7(rsp) [frame] frame_unwind_register_value: -> register=7 bytes=[40ddffffff7f0000] [frame] frame_unwind_register_value: exit [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_eq: l={stack=<sentinel>,!code,special=0x0000000000000000}, r={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] get_prev_frame: enter [frame] get_prev_frame_always_1: enter [frame] get_prev_frame_always_1: this_frame=-1 [frame] get_prev_frame_always_1: -> {level=0,type=NORMAL_FRAME,unwind=0x5588ee3d1824,pc=0x5555555551ec,id=<not computed>,func=<unknown>} // cached [frame] get_prev_frame_always_1: exit [frame] get_prev_frame: exit [frame] value_fetch_lazy_register: (frame=0, regnum=7(rsp), ...) -> register=7 bytes=[40ddffffff7f0000] [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_eq: l={stack=<sentinel>,!code,special=0x0000000000000000}, r={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_unwind_register_value: enter [frame] frame_unwind_register_value: frame=-1, regnum=16(rip) [frame] frame_unwind_register_value: -> register=16 bytes=[ec51555555550000] [frame] frame_unwind_register_value: exit [frame] frame_id_p: l={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] frame_id_eq: l={stack=<sentinel>,!code,special=0x0000000000000000}, r={stack=<sentinel>,!code,special=0x0000000000000000} -> 1 [frame] get_prev_frame: enter [frame] get_prev_frame_always_1: enter [frame] get_prev_frame_always_1: this_frame=-1 [frame] get_prev_frame_always_1: -> {level=0,type=NORMAL_FRAME,unwind=0x5588ee3d1888,pc=0x5555555551ec,id=<not computed>,func=<unknown>} // cached [frame] get_prev_frame_always_1: exit [frame] get_prev_frame: exit [frame] value_fetch_lazy_register: (frame=0, regnum=16(rip), ...) -> register=16 bytes=[ec51555555550000] [py-unwind] pyuw_sniffer: frame claimed by unwinder test unwinder [py-unwind] pyuw_sniffer: exit [frame] frame_unwind_try_unwinder: yes [frame] frame_unwind_find_by_frame: exit gdb/ChangeLog: * frame.h (FRAME_SCOPED_DEBUG_ENTER_EXIT): New. * frame.c (compute_frame_id, get_prev_frame_always_1, get_prev_frame): Use FRAME_SCOPED_DEBUG_ENTER_EXIT. * frame-unwind.c (frame_unwind_find_by_frame): Likewise. (frame_unwind_register_value): Likewise. Change-Id: I45b69b4ed962e70572bc55b8adfb211483c1eeed
generated by cgit v1.1 at 2025-02-07 11:24:00 +0000