aboutsummaryrefslogtreecommitdiff
path: root/gdb/ChangeLog
AgeCommit message (Collapse)AuthorFilesLines
2021-02-09Fix typo in stap_parse_argument_conditionallyTom Tromey1-0/+4
This fixes a typo in an error message in stap_parse_argument_conditionally. gdb/ChangeLog 2021-02-09 Tom Tromey <tom@tromey.com> * stap-probe.c (stap_parse_argument_conditionally): Fix typo.
2021-02-09[gdb/symtab] Fix element type modification in read_array_typeTom de Vries1-0/+7
When running test-case gdb.fortran/function-calls.exp with target board unix/gdb:debug_flags=-gdwarf-5, I run into: ... (gdb) PASS: gdb.fortran/function-calls.exp: \ p derived_types_and_module_calls::pass_cart(c) p derived_types_and_module_calls::pass_cart_nd(c_nd)^M ^M Program received signal SIGSEGV, Segmentation fault.^M 0x0000000000400f73 in derived_types_and_module_calls::pass_cart_nd \ (c=<error reading variable: Cannot access memory at address 0xc>) at \ function-calls.f90:130^M 130 pass_cart_nd = ubound(c%d,1,4)^M The program being debugged was signaled while in a function called from GDB.^M GDB has restored the context to what it was before the call.^M To change this behavior use "set unwindonsignal off".^M Evaluation of the expression containing the function^M (derived_types_and_module_calls::pass_cart_nd) will be abandoned.^M (gdb) FAIL: gdb.fortran/function-calls.exp: p ... The problem originates in read_array_type, when reading a DW_TAG_array_type with a dwarf-5 DW_TAG_generic_subrange child. This is not supported, and the fallout of this is that rather than constructing a new array type, the code proceeds to modify the element type. Fix this conservatively by issuing a complaint and bailing out in read_array_type when not being able to construct an array type, such that we have: ... (gdb) maint expand-symtabs function-calls.f90^M During symbol reading: unable to find array range \ - DIE at 0xe1e [in module function-calls]^M During symbol reading: unable to find array range \ - DIE at 0xe1e [in module function-calls]^M (gdb) KFAIL: gdb.fortran/function-calls.exp: no complaints in srcfile \ (PRMS: symtab/27388) ... Tested on x86_64-linux. gdb/ChangeLog: 2021-02-09 Tom de Vries <tdevries@suse.de> PR symtab/27341 * dwarf2/read.c (read_array_type): Return NULL when not being able to construct an array type. Add assert to ensure that element_type is not being modified. gdb/testsuite/ChangeLog: 2021-02-09 Tom de Vries <tdevries@suse.de> PR symtab/27341 * lib/gdb.exp (with_complaints): New proc, factored out of ... (gdb_load_no_complaints): ... here. * gdb.fortran/function-calls.exp: Add test-case.
2021-02-09gdb: revert "gdb: unify parts of the Linux and FreeBSD core dumping code"Andrew Burgess1-0/+26
This reverts commit 82a1fd3a4935fe665cf08bc6820942c4a091184c. It was pointed out: https://sourceware.org/pipermail/gdb-patches/2021-February/175750.html that commit 82a1fd3a4935 caused GDB to have an unconditional dependency on ELF specific parts of BFD. What this means is that if GDB and BFD are built for a non-elf target then there will be undefined symbol references within GDB. The right solution isn't immediately obvious. So rather than rush a fix in I'm reverting this commit for now, and will bring it back once I have a good solution. gdb/ChangeLog: * gcore.c (struct gcore_collect_regset_section_cb_data): Delete. (gcore_collect_regset_section_cb): Delete. (gcore_collect_thread_registers): Delete. (gcore_build_thread_register_notes): Delete. (gcore_find_signalled_thread): Delete. * gcore.h: Remove 'gdbsupport/gdb_signals.h' include and delete 'gdbarch' and 'thread_info' declarations. (gcore_build_thread_register_notes): Delete declaration. (gcore_find_signalled_thread): Likewise. * fbsd-tdep.c: Remove 'gcore.h' include. (struct fbsd_collect_regset_section_cb_data): New struct. (fbsd_collect_regset_section_cb): New function. (fbsd_collect_thread_registers): New function. (struct fbsd_corefile_thread_data): New struct. (fbsd_corefile_thread): New function. (fbsd_make_corefile_notes): Call FreeBSD specific code. * linux-tdep.c: Remove 'gcore.h' include. (struct linux_collect_regset_section_cb_data): New struct. (linux_collect_regset_section_cb): New function. (linux_collect_thread_registers): New function. (linux_corefile_thread): Call Linux specific code. (find_signalled_thread): New function. (linux_make_corefile_notes): Call find_signalled_thread.
2021-02-09Avoid crash from coerce_unspec_val_to_typeTom Tromey1-0/+7
With a certain Ada program, ada-lang.c:coerce_unspec_val_to_type can cause a crash. This function may copy a value, and in the particular case in the crash, the new value's type is smaller than the original type. This causes coerce_unspec_val_to_type to create a lazy value -- but the original value is also not_lval, so later, when the value is un-lazied, gdb asserts. As with the previous patch, we believe there is a compiler bug here, but it is difficult to reproduce, so we're not completely certain. In the particular case we saw, the original value has record type, and the record holds some variable-length arrays. This leads to the type's length being 0. At the same time, the value is optimized out. This patch changes coerce_unspec_val_to_type to handle an optimized-out value correctly. It also slightly restructures this code to avoid a crash should a not_lval value wind up here. This is a purely defensive change. This change also made it clear that value_contents_copy_raw can now be made static, so that is also done. gdb/ChangeLog 2021-02-09 Tom Tromey <tromey@adacore.com> * ada-lang.c (coerce_unspec_val_to_type): Avoid making lazy not_lval value. * value.c (value_contents_copy_raw): Now static. * value.h (value_contents_copy_raw): Don't declare.
2021-02-09Avoid crash in resolve_dynamic_structTom Tromey1-0/+5
resolve_dynamic_struct says: gdb_assert (type->num_fields () > 0); However, a certain Ada program has a structure with no fields but with a dynamic size, causing this assertion to fire. It is difficult to be certain, but we think this is a compiler bug. However, in the meantime this assertion does not seem to be checking any kind of internal consistency; so this patch removes it. gdb/ChangeLog 2021-02-09 Tom Tromey <tromey@adacore.com> * gdbtypes.c (resolve_dynamic_struct): Handle structure with no fields.
2021-02-08gdb: Do not interrupt atomic sequences for ARCShahab Vahedi1-0/+6
When stepping over thread-lock related codes (in uClibc), the inferior process gets stuck and never manages to enter the critical section: ------8<------- 1 size_t fwrite(const void * __restrict ptr, size_t size, 2 size_t nmemb, register FILE * __restrict stream) 3 { 4 size_t retval; 5 __STDIO_AUTO_THREADLOCK_VAR; 6 7 > __STDIO_AUTO_THREADLOCK(stream); 8 9 retval = fwrite_unlocked(ptr, size, nmemb, stream); 10 11 __STDIO_AUTO_THREADUNLOCK(stream); 12 13 return retval; 14 } ------>8------- Here, we are at line 7. Using the "next" command leads no where. However, setting a breakpoint on line 9 and issuing "continue" works. Looking at the assembly instructions reveals that we're dealing with the critical section entry code [1] that should never be interrupted, in this case by the debugger's implicit breakpoints: ------8<------- ... 1 add_s r0,r13,0x38 2 mov_s r3,1 3 llock r2,[r0] <-. 4 brne.nt r2,0,14 --. | 5 scond r3,[r0] | | 6 bne -10 --|--' 7 brne_s r2,0,84 <-' ... ------>8------- Lines 3 until 5 (inclusive) are supposed to be executed atomically. Therefore, GDB should never (implicitly) insert a breakpoint on lines 4 and 5, else the program will try to acquire the lock again by jumping back to line 3 and gets stuck in an infinite loop. The solution is to make GDB aware of these patterns so it inserts breakpoints after the sequence -- line 6 in this example. [1] https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/arc/bits/atomic.h#n46 ------8<------- ({ \ __typeof(oldval) prev; \ \ __asm__ __volatile__( \ "1: llock %0, [%1] \n" \ " brne %0, %2, 2f \n" \ " scond %3, [%1] \n" \ " bnz 1b \n" \ "2: \n" \ : "=&r"(prev) \ : "r"(mem), "ir"(oldval), \ "r"(newval) /* can't be "ir". scond can't take limm for "b" */\ : "cc", "memory"); \ \ prev; \ }) ------>8------- "llock" (Load Locked) loads the 32-bit word pointed by the source operand. If the load is completed without any interruption or exception, the physical address is remembered, in Lock Physical Address (LPA), and the Lock Flag (LF) is set to 1. LF is a non-architecturally visible flag and is cleared whenever an interrupt or exception takes place. LF is also cleared (atomically) whenever another process writes to the LPA. "scond" (Store Conditional) will write to the destination address if and only if the LF is set to 1. When finished, with or without a write, it atomically copies the LF value to ZF (Zero Flag). These two instructions together provide the mechanism for entering a critical section. The code snippet above comes from uClibc: ----------------------- v3 (after Tom's remarks[2]): handle_atomic_sequence() - no need to initialize the std::vector with "{}" - fix typo in comments: "conditial" -> "conditional" - add braces to the body of "if" condition because of the comment line arc_linux_software_single_step() - make the performance slightly more efficient by moving a few variables after the likely "return" point. v2 (after Simon's remarks[3]): - handle_atomic_sequence() gets a copy of an instruction instead of a reference. - handle_atomic_sequence() asserts if the given instruction is an llock. [2] https://sourceware.org/pipermail/gdb-patches/2021-February/175805.html [3] https://sourceware.org/pipermail/gdb-patches/2021-January/175487.html gdb/ChangeLog: PR tdep/27369 * arc-linux-tdep.c (handle_atomic_sequence): New. (arc_linux_software_single_step): Call handle_atomic_sequence().
2021-02-08gdb: return true in TuiWindow.is_valid only if TUI is enabledAndrew Burgess1-0/+14
If the user implements a TUI window in Python, and this window responds to GDB events and then redraws its window contents then there is currently an edge case which can lead to problems. The Python API documentation suggests that calling methods like erase or write on a TUI window (from Python code) will raise an exception if the window is not valid. And the description for is_valid says: This method returns True when this window is valid. When the user changes the TUI layout, windows no longer visible in the new layout will be destroyed. At this point, the gdb.TuiWindow will no longer be valid, and methods (and attributes) other than is_valid will throw an exception. From this I, as a user, would expect that if I did 'tui disable' to switch back to CLI mode, then the window would no longer be valid. However, this is not the case. When the TUI is disabled the windows in the TUI are not deleted, they are simply hidden. As such, currently, the is_valid method continues to return true. This means that if the users Python code does something like: def event_handler (e): global tui_window_object if tui_window_object->is_valid (): tui_window_object->erase () tui_window_object->write ("Hello World") gdb.events.stop.connect (event_handler) Then when a stop event arrives GDB will try to draw the TUI window, even when the TUI is disabled. This exposes two bugs. First, is_valid should be returning false in this case, second, if the user forgot to add the is_valid call, then I believe the erase and write calls should be throwing an exception (when the TUI is disabled). The solution to both of these issues is I think bound together, as it depends on having a working 'is_valid' check. There's a rogue assert added into tui-layout.c as part of this commit. While working on this commit I managed to break GDB such that TUI_CMD_WIN was nullptr, this was causing GDB to abort. I'm leaving the assert in as it might help people catch issues in the future. This patch is inspired by the work done here: https://sourceware.org/pipermail/gdb-patches/2020-December/174338.html gdb/ChangeLog: * python/py-tui.c (gdbpy_tui_window) <is_valid>: New member function. (REQUIRE_WINDOW): Call is_valid member function. (REQUIRE_WINDOW_FOR_SETTER): New define. (gdbpy_tui_is_valid): Call is_valid member function. (gdbpy_tui_set_title): Call REQUIRE_WINDOW_FOR_SETTER instead. * tui/tui-data.h (struct tui_win_info) <is_visible>: Check tui_active too. * tui/tui-layout.c (tui_apply_current_layout): Add an assert. * tui/tui.c (tui_enable): Move setting of tui_active earlier in the function. gdb/doc/ChangeLog: * python.texinfo (TUI Windows In Python): Extend description of TuiWindow.is_valid. gdb/testsuite/ChangeLog: * gdb.python/tui-window-disabled.c: New file. * gdb.python/tui-window-disabled.exp: New file. * gdb.python/tui-window-disabled.py: New file.
2021-02-08gdb/python: don't allow the user to delete window title attributesAndrew Burgess1-0/+5
There's a bug in the python tui API. If the user tries to delete the window title attribute then this will trigger undefined behaviour in GDB due to a missing nullptr check. gdb/ChangeLog: * python/py-tui.c (gdbpy_tui_set_title): Check that the new value for the title is not nullptr. gdb/testsuite/ChangeLog: * gdb.python/tui-window.exp: Add new tests. * gdb.python/tui-window.py (TestWindow) <__init__>: Store TestWindow object into global the_window. <remote_title>: New method. (delete_window_title): New function.
2021-02-08gdb/tui: don't add windows to global list from tui_layout:window::applyAndrew Burgess1-0/+11
This commit was inspired by this mailing list patch: https://sourceware.org/pipermail/gdb-patches/2021-January/174713.html Currently, calling tui_layout_window::apply will add the window from the layout object to the global tui_windows list. Unfortunately, when the user runs the 'winheight' command, this calls tui_adjust_window_height, which calls the tui_layout_base::adjust_size function, which can then call tui_layout_base::apply. The consequence of this is that when the user does 'winheight' duplicate copies of a window can be added to the global tui_windows list. The original patch fixed this by changing the apply function to only update the global list some of the time. This patch takes a different approach. The apply function no longer updates the global tui_windows list. Instead a new virtual function is added to tui_layout_base which is used to gather all the currently applied windows into a vector. Finally tui_apply_current_layout is updated to make use of this new function to update the tui_windows list. The benefits I see in this approach are, (a) the apply function now no longer touches global state, this solves the immediate problem, and (b) now that tui_windows is updated directly in the function tui_apply_current_layout, we can drop the saved_tui_windows global. gdb/ChangeLog: * tui-layout.c (saved_tui_windows): Delete. (tui_apply_current_layout): Don't make use of saved_tui_windows, call new get_windows member function instead. (tui_get_window_by_name): Check in tui_windows. (tui_layout_window::apply): Don't add to tui_windows. * tui-layout.h (tui_layout_base::get_windows): New member function. (tui_layout_window::get_windows): Likewise. (tui_layout_split::get_windows): Likewise. gdb/testsuite/ChangeLog: * gdb.tui/winheight.exp: Add more tests.
2021-02-08gdb/tui: restore delete of window objectsAndrew Burgess1-0/+5
In commit: commit f237f998d1168139d599c550d54169cd8f94052d Date: Mon Jan 25 18:43:19 2021 +0000 gdb/tui: remove special handling of locator/status window I accidentally remove a call to delete the tui window objects. Now every time GDB changes tui layout it is leaking windows. gdb/ChangeLog: * tui/tui-layout.c (tui_apply_current_layout): Restore the delete of the window objects.
2021-02-08gdb/python: reformat an error stringAndrew Burgess1-0/+4
While working on another patch I noticed an oddly formatted error message in the Python code. When 'set python print-stack message' is in effect then consider this Python script: class TestCommand (gdb.Command): def __init__ (self): gdb.Command.__init__ (self, "test-cmd", gdb.COMMAND_DATA) def invoke(self, args, from_tty): raise RuntimeError ("bad") TestCommand () And this GDB session: (gdb) source path/to/python/script.py (gdb) test-cmd Python Exception <class 'RuntimeError'> bad: Error occurred in Python: bad The line 'Python Exception <class 'RuntimeError'> bad:' doesn't look terrible in this situation, the colon at the end of the first line makes sense given the second line. However, there are places in GDB where there is no second line printed, for example consider this python script: def stop_listener (e): raise RuntimeError ("bad") gdb.events.stop.connect (stop_listener) Then this GDB session: (gdb) file helloworld.exe (gdb) start Temporary breakpoint 1 at 0x40112a: file hello.c, line 6. Starting program: helloworld.exe Temporary breakpoint 1, main () at hello.c:6 6 printf ("Hello World\n"); Python Exception <class 'RuntimeError'> bad: (gdb) si 0x000000000040112f 6 printf ("Hello World\n"); Python Exception <class 'RuntimeError'> bad: In this case there is no auxiliary information displayed after the warning, and the line ending in the colon looks weird to me. A quick survey of the code seems to indicate that it is not uncommon for there to be no auxiliary information line printed, its not just the one case I found above. I propose that the line that currently looks like this: Python Exception <class 'RuntimeError'> bad: Be reformatted like this: Python Exception <class 'RuntimeError'>: bad I think this looks fine then in either situation. The first now looks like this: (gdb) test-cmd Python Exception <class 'RuntimeError'>: bad Error occurred in Python: bad And the second like this: (gdb) si 0x000000000040112f 6 printf ("Hello World\n"); Python Exception <class 'RuntimeError'>: bad There's just two tests that needed updating. Errors are checked for in many more tests, but most of the time the pattern doesn't care about the colon. gdb/ChangeLog: * python/python.c (gdbpy_print_stack): Reformat an error message. gdb/testsuite/ChangeLog: * gdb.python/py-framefilter.exp: Update expected results. * gdb.python/python.exp: Update expected results.
2021-02-08gdb/tui: fix issue with handling the return characterAndrew Burgess1-0/+10
My initial goal was to fix our gdb/testsuite/lib/tuiterm.exp such that it would correctly support (some limited) scrolling of the command window. What I observe is that when sending commands to the tui command window in a test script with: Term::command "p 1" The command window would be left looking like this: (gdb) (gdb) p 1$1 = 1 (gdb) When I would have expected it to look like this: (gdb) p 1 $1 = 1 (gdb) Obviously a bug in our tuiterm.exp library, right??? Wrong! Turns out there's a bug in GDB. If in GDB I enable the tui and then type (slowly) the 'p 1\r' (the \r is pressing the return key at the end of the string), then you do indeed get the "expected" terminal output. However, if instead I copy the 'p 1\r' string and paste it into the tui in one go then I now see the same corrupted output as we do when using tuiterm.exp. It turns out the problem is that GDB fails when handling lots of input arriving quickly with a \r (or \n) on the end. The reason for this bug is as follows: When the tui is active the terminal is in no-echo mode, so characters sent to the terminal are not echoed out again. This means that when the user types \r, this is not echoed to the terminal. The characters read in are passed to readline and \r indicates that the command line is complete and ready to be processed. However, the \r is not included in readlines command buffer, and is NOT printed by readline when is displays its buffer to the screen. So, in GDB we have to manually spot the \r when it is read in and update the display. Printing a newline character to the output and moving the cursor to the next line. This is done in tui_getc_1. Now readline tries to reduce the number of write calls. So if we very quickly (as in paste in one go) the text 'p 1' to readline (this time with no \r on the end), then readline will fetch the fist character and add it to its internal buffer. But before printing the character out readline checks to see if there's more input incoming. As we pasted multiple characters, then yes, readline sees the ' ' and adds this to its buffer, and finally the '1', this too is added to the buffer. Now if at this point we take a break, readline sees there is no more input available, and so prints its buffer out. Now when we press \r the code in tui_getc_1 kicks in, adds a \n to the output and moves the cursor to the next line. But, if instead we paste 'p 1\r' in one go then readline adds 'p 1' to its buffer as before, but now it sees that there is still more input available. Now it fetches the '\r', but this triggers the newline behaviour, we print '\n' and move to the next line - however readline has not printed its buffer yet! So finally we end up on the next line. There's no more input available so readline prints its buffer, then GDB gets passed the buffer, handles it, and prints the result. The solution I think is to put of our special newline insertion code until we know that readline has finished printing its buffer. Handily we know when this is - the next thing readline does is pass us the command line buffer for processing. So all we need to do is hook in to the command line processing, and before we pass the command line to GDB's internals we do all of the magic print a newline and move the cursor to the next line stuff. Luckily, GDB's interpreter mechanism already provides the hooks we need to do this. So all I do here is move the newline printing code from tui_getc_1 into a new function, setup a new input_handler hook for the tui, and call my new newline printing function. After this I can enable the tui and paste in 'p 1\r' and see the correct output. Also the tuiterm.exp library will now see non-corrupted output. gdb/ChangeLog: * tui/tui-interp.c (tui_command_line_handler): New function. (tui_interp::resume): Register tui_command_line_handler as the input_handler. * tui/tui-io.c (tui_inject_newline_into_command_window): New function. (tui_getc_1): Delete handling of '\n' and '\r'. * tui-io.h (tui_inject_newline_into_command_window): Declare. gdb/testsuite/ChangeLog: * gdb.tui/scroll.exp: Tighten expected results. Remove comment about bug in GDB, update expected results, and add more tests.
2021-02-07Don't draw register sub windows outside the visible areaHannes Domani1-0/+7
If the regs window is not big enough to show all registers, the registers whose values changed are always drawn, even if they are not in the currently visible area. So this marks the invisible register sub windows with y=0, and skips their rerender call in check_register_values. gdb/ChangeLog: 2021-02-07 Hannes Domani <ssbssa@yahoo.de> * tui/tui-regs.c (tui_data_window::display_registers_from): Mark invisible register sub windows. (tui_data_window::check_register_values): Ignore invisible register sub windows.
2021-02-07Don't fill regs window with a negative number of spacesHannes Domani1-0/+5
Function n_spaces can't handle negative values, and returns an invalid pointer in this case. gdb/ChangeLog: 2021-02-07 Hannes Domani <ssbssa@yahoo.de> * tui/tui-regs.c (tui_data_item_window::rerender): Don't call n_spaces with a negative value.
2021-02-07Refresh regs window in display_registers_fromHannes Domani1-0/+5
Otherwise the register window is redrawn empty when scrolling or changing its size with winheight. gdb/ChangeLog: 2021-02-07 Hannes Domani <ssbssa@yahoo.de> * tui/tui-regs.c (tui_data_window::display_registers_from): Add refresh_window call.
2021-02-07Also compare frame_id_is_next in frapy_richcompareHannes Domani1-0/+4
The last frame in a corrupt stack stores the frame_id of the next frame, so these two frames currently compare as equal. So if you have a backtrace where the oldest frame is corrupt, this happens: (gdb) py >f = gdb.selected_frame() >while f.older(): > f = f.older() >print(f == f.newer()) >end True With this change, that same example returns False. gdb/ChangeLog: 2021-02-07 Hannes Domani <ssbssa@yahoo.de> * python/py-frame.c (frapy_richcompare): Compare frame_id_is_next.
2021-02-05gdb: symmisc.c: remove std_{in,out,err}Simon Marchi1-0/+5
These are likely not very useful, remove them. gdb/ChangeLog: * symmisc.c (std_in, std_out, std_err): Remove. (_initialize_symmisc): Don't set std_in, std_out and std_err. Change-Id: I140bfffd7fb655d39c32333bb53924b91b1eb13c
2021-02-05[gdb/breakpoints] Handle glibc with debuginfo in ↵Tom de Vries1-0/+6
create_exception_master_breakpoint The test-case nextoverthrow.exp is failing on targets with unstripped libc. This is a regression since commit 1940319c0ef "[gdb] Fix internal-error in process_event_stop_test". The problem is that this code in create_exception_master_breakpoint: ... for (objfile *sepdebug = obj->separate_debug_objfile; sepdebug != nullptr; sepdebug = sepdebug->separate_debug_objfile) if (create_exception_master_breakpoint_hook (sepdebug)) ... iterates over all the separate debug object files, but fails to handle the case that obj itself has the debug info we're looking for. Fix this by using the separate_debug_objfiles () range instead, which does iterate both over obj and the obj->separate_debug_objfile chain. Tested on x86_64-linux. gdb/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR breakpoints/27330 * breakpoint.c (create_exception_master_breakpoint): Handle case that glibc object file has debug info.
2021-02-05[gdb/symtab] Handle DW_TAG_type_unit in process_psymtab_comp_unitTom de Vries1-0/+5
When running test-case gdb.cp/cpexprs-debug-types.exp with target board unix/gdb:debug_flags=-gdwarf-5, I run into: ... (gdb) file cpexprs-debug-types^M Reading symbols from cpexprs-debug-types...^M ERROR: Couldn't load cpexprs-debug-types into GDB (eof). ERROR: Couldn't send delete breakpoints to GDB. ERROR: GDB process no longer exists GDB process exited with wait status 23054 exp9 0 0 CHILDKILLED SIGABRT SIGABRT ... We're running into this abort in process_psymtab_comp_unit: ... switch (reader.comp_unit_die->tag) { case DW_TAG_compile_unit: this_cu->unit_type = DW_UT_compile; break; case DW_TAG_partial_unit: this_cu->unit_type = DW_UT_partial; break; default: abort (); } ... because reader.comp_unit_die->tag == DW_TAG_type_unit. Fix this by adding a DW_TAG_type_unit case. Tested on x86_64-linux. gdb/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR symtab/27333 * dwarf2/read.c (process_psymtab_comp_unit): Handle DW_TAG_type_unit.
2021-02-05[gdb/breakpoints] Fix segfault for catch syscall -1Tom de Vries1-0/+6
Using a hello world a.out, I run into a segfault: ... $ gcc hello.c $ gdb -batch a.out -ex "catch syscall -1" -ex r Catchpoint 1 (syscall -1) Aborted (core dumped) ... Fix this by erroring out if a negative syscall number is used in the catch syscall command. Tested on x86_64-linux. gdb/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR breakpoints/27313 * break-catch-syscall.c (catch_syscall_split_args): Reject negative syscall numbers. gdb/testsuite/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR breakpoints/27313 * gdb.base/catch-syscall.exp: Check that "catch syscall -1" is rejected.
2021-02-05Return unique_ptr from language_defn::get_compile_contextTom Tromey1-0/+14
This changes language_defn::get_compile_context to return a unique_ptr. This makes the ownership transfer clear. gdb/ChangeLog 2021-02-05 Tom Tromey <tom@tromey.com> * compile/compile-c-support.c (get_compile_context) (c_get_compile_context, cplus_get_compile_context): Change return type. * language.c (language_defn::get_compile_instance): New method. * language.h (language_defn::get_compile_instance): Change return type. No longer inline. * c-lang.c (c_language::get_compile_instance): Change return type. (cplus_language::get_compile_instance): Change return type. * c-lang.h (c_get_compile_context, cplus_get_compile_context): Change return type. * compile/compile.c (compile_to_object): Update.
2021-02-05Extract symbol-writing function from parsersTom Tromey1-0/+10
I noticed that several parsers shared the same code to write a symbol reference to an expression. This patch factors this code out into a new function. Regression tested on x86-64 Fedora 32. gdb/ChangeLog 2021-02-05 Tom Tromey <tom@tromey.com> * parser-defs.h (write_exp_symbol_reference): Declare. * parse.c (write_exp_symbol_reference): New function. * p-exp.y (variable): Use write_exp_symbol_reference. * m2-exp.y (variable): Use write_exp_symbol_reference. * f-exp.y (variable): Use write_exp_symbol_reference. * d-exp.y (PrimaryExpression): Use write_exp_symbol_reference. * c-exp.y (variable): Use write_exp_symbol_reference.
2021-02-05[gdb/exp] Fix assert when adding ptr to imaginary unitTom de Vries1-0/+6
I'm running into this assertion failure: ... $ gdb -batch -ex "p (void *)0 - 5i" gdbtypes.c:3430: internal-error: \ type* init_complex_type(const char*, type*): Assertion \ `target_type->code () == TYPE_CODE_INT \ || target_type->code () == TYPE_CODE_FLT' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. ... This is a regression since commit c34e8714662 "Implement complex arithmetic". Before that commit we had: ... (gdb) p (void *)0 - 5i Argument to arithmetic operation not a number or boolean. ... Fix this in complex_binop by throwing an error, such that we have: ... (gdb) print (void *)0 - 5i Argument to complex arithmetic operation not supported. ... Tested on x86_64-linux. gdb/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR exp/27265 * valarith.c (complex_binop): Throw an error if complex type can't be created. gdb/testsuite/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR exp/27265 * gdb.base/complex-parts.exp: Add tests.
2021-02-05[gdb/symtab] Fix duplicate CUs in create_cus_from_debug_names_listTom de Vries1-0/+6
When running test-case gdb.dwarf2/clang-debug-names.exp, I run into the following warning: ... (gdb) file clang-debug-names^M Reading symbols from clang-debug-names...^M warning: Section .debug_aranges in clang-debug-names has duplicate \ debug_info_offset 0xc7, ignoring .debug_aranges.^M ... This is caused by a missing return in commit 3ee6bb113af "[gdb/symtab] Fix incomplete CU list assert in .debug_names". Fix this by adding the missing return, such that we have instead this warning: ... (gdb) file clang-debug-names^M Reading symbols from clang-debug-names...^M warning: Section .debug_aranges in clang-debug-names \ entry at offset 0 debug_info_offset 0 does not exists, \ ignoring .debug_aranges.^M ... which is a known problem filed as PR25969 - "Ignoring .debug_aranges with clang .debug_names". Tested on x86_64-linux. gdb/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR symtab/27307 * dwarf2/read.c (create_cus_from_debug_names_list): Add missing return. gdb/testsuite/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> PR symtab/27307 * gdb.dwarf2/clang-debug-names.exp: Check file command warnings.
2021-02-05[gdb/symtab] Fix indentation in create_cus_from_debug_names_listTom de Vries1-0/+4
Fix indentation in !map.augmentation_is_gdb part of create_cus_from_debug_names_list. gdb/ChangeLog: 2021-02-05 Tom de Vries <tdevries@suse.de> * dwarf2/read.c (create_cus_from_debug_names_list): Fix indentation.
2021-02-04gdb: riscv: enable sim integrationMike Frysinger1-0/+4
Now the simulator can be loaded via gdb using "target sim".
2021-02-04gdb: make target_is_non_stop_p return boolSimon Marchi1-0/+5
gdb/ChangeLog: * target.c (target_is_non_stop_p): Return bool. * target.h (target_is_non_stop_p): Return bool. Change-Id: Icdb37ffe917798e59b822976794d4b1b7aafd709
2021-02-04gdb: make record-full clear async handler in waitSimon Marchi1-0/+7
For the same reason explained in the previous patch (which was for the record-btrace target), move clearing of the async event handler of the record-full target to the wait method. I'm not sure if/where that target needs to re-set its async event handler in the wait method. Since it only supports a single thread, there probably can't be multiple events to report at the same time. gdb/ChangeLog: * record-full.c (record_full_async_inferior_event_handler): Don't clear async event handler. (record_full_base_target::wait): Clear async event handler at beginning. Change-Id: I146fbdb53d99e3a32766ac7cd337ac5ed7fd9adf
2021-02-04gdb: make record-btrace clear event handler in waitSimon Marchi1-0/+7
For the same reason explained in the previous patch (which was for the remote target), move clearing of the async event handler of the record-btrace target to the wait method. The record-btrace target already re-sets its async event handler in its wait method, so that part doesn't need to be changed: /* In async mode, we need to announce further events. */ if (target_is_async_p ()) record_btrace_maybe_mark_async_event (moving, no_history); gdb/ChangeLog: * record-btrace.c (record_btrace_handle_async_inferior_event): Don't clear async event handler. (record_btrace_target::wait): Clear async event handler at beginning. Change-Id: Ib32087a81bf94f1b884a938c8167ac8bbe09e362
2021-02-04gdb: make remote target clear its handler in remote_target::waitSimon Marchi1-0/+7
The remote target's remote_async_inferior_event_token is a flag that tells when it wants the infrun loop to call its wait method. The flag is cleared in the async_event_handler's callback (remote_async_inferior_event_handler), just before calling inferior_event_handler. However, since inferior_event_handler may actually call another target's wait method, there needs to be code that checks if we need to re-raise the flag. It would be simpler instead for remote_target::wait to clear the flag when it returns an event and there are no more to report after that. If another target's wait method gets called by inferior_event_handler, the remote target's flag will stay naturally stay marked. Note that this is already partially implemented in remote_target::wait, since the remote target may have multiple events to report (and it can only report one at the time): if (target_is_async_p ()) { remote_state *rs = get_remote_state (); /* If there are are events left in the queue tell the event loop to return here. */ if (!rs->stop_reply_queue.empty ()) mark_async_event_handler (rs->remote_async_inferior_event_token); } The code in remote_async_inferior_event_handler also checks for pending events as well, in addition to the stop reply queue, so I've made remote_target::wait check for that as well. I'm not completely sure this is ok, since I don't understand very well how the pending events mechanism works. But I figured it was safer to do this, worst case it just leads to unnecessary calls to remote_target::wait. gdb/ChangeLog: * remote.c (remote_target::wait): Clear async event handler at beginning, mark if needed at the end. (remote_async_inferior_event_handler): Don't set or clear async event handler. Change-Id: I20117f5b5acc8a9972c90f16280249b766c1bf37
2021-02-04gdb: make async event handlers clear themselvesSimon Marchi1-0/+15
The `ready` flag of async event handlers is cleared by the async event handler system right before invoking the associated callback, in check_async_event_handlers. This is not ideal with how the infrun subsystem consumes events: all targets' async event handler callbacks essentially just invoke `inferior_event_handler`, which eventually calls `fetch_inferior_event` and `do_target_wait`. `do_target_wait` picks an inferior at random, and thus a target at random (it could be the target whose `ready` flag was cleared, or not), and pulls one event from it. So it's possible that: - the async event handler for a target A is called - we end up consuming an event for target B - all threads of target B are stopped, target_async(0) is called on it, so its async event handler is cleared (e.g. record_btrace_target::async) As a result, target A still has events to report while its async event handler is left unmarked, so these events are not consumed. To counter this, at the end of their async event handler callbacks, targets check if they still have something to report and re-mark their async event handler (e.g. remote_async_inferior_event_handler). The linux_nat target does not suffer from this because it doesn't use an async event handler at the moment. It only uses a pipe registered with the event loop. It is written to in the SIGCHLD handler (and in other spots that want to get target wait method called) and read from in the target's wait method. So if linux_nat happened to be target A in the example above, the pipe would just stay readable, and the event loop would wake up again, until linux_nat's wait method is finally called and consumes the contents of the pipe. I think it would be nicer if targets using async_event_handler worked in a similar way, where the flag would stay set until the target's wait method is actually called. As a first step towards that, this patch moves the responsibility of clearing the ready flags of async event handlers to the invoked callback. All async event handler callbacks are modified to clear their ready flag before doing anything else. So in practice, nothing changes with this patch. It's only the responsibility of clearing the flag that is shifted toward the callee. gdb/ChangeLog: * async-event.h (async_event_handler_func): Add documentation. * async-event.c (check_async_event_handlers): Don't clear async_event_handler ready flag. * infrun.c (infrun_async_inferior_event_handler): Clear ready flag. * record-btrace.c (record_btrace_handle_async_inferior_event): Likewise. * record-full.c (record_full_async_inferior_event_handler): Likewise. * remote-notif.c (remote_async_get_pending_events_handler): Likewise. * remote.c (remote_async_inferior_event_handler): Likewise. Change-Id: I179ef8e99580eae642d332846fd13664dbddc0c1
2021-02-03gdb: infrun: move stop_soon variable to inner scoped in handle_inferior_eventSimon Marchi1-0/+5
Moving it to an inner scope makes it clearer where it's used (only while handling the TARGET_WAITKIND_LOADED event). gdb/ChangeLog: * infrun.c (handle_inferior_event): Move stop_soon variable to inner scope. Change-Id: Ic57685a21714cfbb38f1487ee96cea1d12b44652
2021-02-03detach in all-stop with threads runningPedro Alves1-0/+10
A following patch will add a testcase that has a number of threads constantly stepping over a breakpoint, and then has GDB detach the process, while threads are running. If we have more than one inferior running, and we detach from just one of the inferiors, we expect that the remaining inferior continues running. However, in all-stop, if GDB needs to pause the target for the detach, nothing is re-resuming the other inferiors after the detach. "info threads" shows the threads as running, but they really aren't. This fixes it. gdb/ChangeLog: * infcmd.c (detach_command): Hold strong reference to target, and if all-stop on entry, restart threads on exit. * infrun.c (switch_back_to_stepped_thread): Factor out bits to ... (restart_stepped_thread): ... this new function. Also handle trap_expected. (restart_after_all_stop_detach): New function. * infrun.h (restart_after_all_stop_detach): Declare.
2021-02-03detach with in-line step over in progressPedro Alves1-0/+5
A following patch will add a testcase that has a number of threads constantly stepping over a breakpoint, and then has GDB detach the process. That testcase exercises both "set displaced-stepping on/off". Testing with "set displaced-stepping off" reveals that GDB does not handle the case of the user typing "detach" just while some thread is in the middle of an in-line step over. If that thread belongs to the inferior that is being detached, then the step-over never finishes, and threads of other inferiors are never re-resumed. This fixes it. gdb/ChangeLog: * infrun.c (struct step_over_info): Initialize fields. (prepare_for_detach): Handle ongoing in-line step over.
2021-02-03detach and breakpoint removalPedro Alves1-0/+8
A following patch will add a testcase that has a number of threads constantly stepping over a breakpoint, and then has GDB detach the process. That testcase sometimes fails with the inferior crashing with SIGTRAP after the detach because of the bug fixed by this patch, when tested with the native target. The problem is that target_detach removes breakpoints from the target immediately, and that does not work with the native GNU/Linux target (and probably no other native target) currently. The test wouldn't fail with this issue when testing against gdbserver, because gdbserver does allow accessing memory while the current thread is running, by transparently pausing all threads temporarily, without GDB noticing. Implementing that in gdbserver was a lot of work, so I'm not looking forward right now to do the same in the native target. Instead, I came up with a simpler solution -- push the breakpoints removal down to the targets. The Linux target conveniently already pauses all threads before detaching them, since PTRACE_DETACH only works with stopped threads, so we move removing breakpoints to after that. Only the remote and GNU/Linux targets support support async execution, so no other target should really need this. gdb/ChangeLog: * linux-nat.c (linux_nat_target::detach): Remove breakpoints here... * remote.c (remote_target::remote_detach_1): ... and here ... * target.c (target_detach): ... instead of here. * target.h (target_ops::detach): Add comment.
2021-02-03prepare_for_detach and ongoing displaced steppingPedro Alves1-0/+10
I noticed that "detach" while a program was running sometimes resulted in the process crashing. I tracked it down to this change to prepare_for_detach in commit 187b041e ("gdb: move displaced stepping logic to gdbarch, allow starting concurrent displaced steps"): /* Is any thread of this process displaced stepping? If not, there's nothing else to do. */ - if (displaced->step_thread == nullptr) + if (displaced_step_in_progress (inf)) return; The problem above is that the condition was inadvertently flipped. It should have been: if (!displaced_step_in_progress (inf)) So I fixed it, and wrote a testcase to exercise it. The testcase has a number of threads constantly stepping over a breakpoint, and then GDB detaches the process, while threads are running and stepping over the breakpoint. And then I was surprised that my testcase would hang -- GDB would get stuck in an infinite loop in prepare_for_detach, here: while (displaced_step_in_progress (inf)) { ... What is going on is that since we now have two displaced stepping buffers, as one displaced step finishes, GDB starts another, and there's another one already in progress, and on and on, so the displaced_step_in_progress condition never turns false. This happens because we go via the whole handle_inferior_event, which tries to start new step overs when one finishes. And also because while we remove breakpoints from the target before prepare_for_detach is called, handle_inferior_event ends up calling insert_breakpoints via e.g. keep_going. Thinking through all this, I came to the conclusion that going through the whole handle_inferior_event isn't ideal. A _lot_ is done by that function, e.g., some thread may get a signal which is passed to the inferior, and gdb decides to try to get over the signal handler, which reinstalls breakpoints. Or some process may exit. We can end up reporting these events via normal_stop while detaching, maybe end up running some breakpoint commands, or maybe even something runs an inferior function call. Etc. All this after the user has already declared they don't want to debug the process anymore, by asking to detach. I came to the conclusion that it's better to do the minimal amount of work possible, in a more controlled fashion, without going through handle_inferior_event. So in the new approach implemented by this patch, if there are threads of the inferior that we're detaching in the middle of a displaced step, stop them, and cancel the displaced step. This is basically what stop_all_threads already does, via wait_one and (the now factored out) handle_one, so I'm reusing those. gdb/ChangeLog: * infrun.c (struct wait_one_event): Move higher up. (prepare_for_detach): Abort in-progress displaced steps instead of letting them complete. (handle_one): If the inferior is detaching, don't add the thread back to the global step-over chain. (restart_threads): Don't restart threads if detaching. (handle_signal_stop): Remove inferior::detaching reference.
2021-02-03prepare_for_detach: don't release scoped_restore at the endPedro Alves1-0/+5
After detaching from a process, the inf->detaching flag is inadvertently left set to true. If you afterwards reuse the same inferior to start a new process, GDB will mishave... The problem is that prepare_for_detach discards the scoped_restore at the end, while the intention is for the flag to be set only for the duration of prepare_for_detach. This was already a bug in the original commit that added prepare_for_detach, commit 24291992dac3 ("PR gdb/11321"), by yours truly. Back then, we still used cleanups, and the function called discard_cleanups instead of do_cleanups, by mistake. gdb/ChangeLog: * infrun.c (prepare_for_detach): Don't release scoped_restore before returning.
2021-02-03Factor out after-stop event handling code from stop_all_threadsPedro Alves1-0/+5
This moves the code handling an event out of wait_one to a separate function, to be used in another context in a following patch. gdb/ChangeLog: * infrun.c (handle_one): New function, factored out from ... (stop_all_threads): ... here.
2021-02-03Fix a couple vStopped pending ack bugsPedro Alves1-0/+7
A following patch will add a testcase that has two processes with threads stepping over a breakpoint continuously, and then detaches from one of the processes while threads are running. The other process continues stepping over its breakpoint. And then the testcase sends a SIGUSR1, expecting that GDB reports it. That would sometimes hang against gdbserver, due to the bugs fixed here. Both bugs are related, in that they're about remote protocol asynchronous Stop notifications. There's a bug in GDB, and another in GDBserver. The GDB bug: - when we detach from a process, the remote target discards any pending RSP notification related to that process, including the in-flight, yet-unacked notification. Discarding the in-flight notification is the problem. Until the in-flight notification is acked with a vStopped packet, the server won't send another %Stop notification. As a result, the debug session gets messed up. In the new testcase's case, GDB would hang inside stop_all_threads, waiting for a stop for one of the process'es threads, which never arrived -- its stop reply was permanently stuck in the stop reply queue, waiting for a vStopped packet that never arrived. In summary: 1. GDBserver sends stop notification about thread X, the remote target receives it and stores it 2. At the same time, GDB detaches thread X's inferior 3. The remote target discards the received stop notification 4. GDBserver waits forever for the ack The GDBserver bug: GDBserver has the opposite bug. It also discards notifications for the process being detached. If that discards the head of the notification queue, when gdb sends an ack, it ends up acking the _next_ notification. Meaning, gdb loses one notification. In the testcase, this results in a similar hang in stop_all_threads. So we have two very similar bugs in GDB and GDBserver, both resulting in a similar symptom. That's why I'm fixing them both at the same time. gdb/ChangeLog: * remote.c (remote_notif_stop_ack): Don't error out on TARGET_WAITKIND_IGNORE; instead, just ignore the notification. (remote_target::discard_pending_stop_replies): Don't delete in-flight notification; instead, clear its contents. gdbserver/ChangeLog: * server.cc (discard_queued_stop_replies): Don't ever discard the notification at the head of the list.
2021-02-03Fix "target extended-remote" + "maint set target-non-stop" + "attach"Pedro Alves1-0/+5
With "target extended-remote" + "maint set target-non-stop", attaching hangs like so: (gdb) attach 1244450 Attaching to process 1244450 [New Thread 1244450.1244450] [New Thread 1244450.1244453] [New Thread 1244450.1244454] [New Thread 1244450.1244455] [New Thread 1244450.1244456] [New Thread 1244450.1244457] [New Thread 1244450.1244458] [New Thread 1244450.1244459] [New Thread 1244450.1244461] [New Thread 1244450.1244462] [New Thread 1244450.1244463] * hang * Attaching to the hung GDB shows that GDB is busy in an infinite loop in stop_all_threads: (top-gdb) bt #0 stop_all_threads () at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:4755 #1 0x000055555597b424 in stop_waiting (ecs=0x7fffffffd930) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:7738 #2 0x0000555555976fba in handle_signal_stop (ecs=0x7fffffffd930) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5868 #3 0x0000555555975f6a in handle_inferior_event (ecs=0x7fffffffd930) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:5527 #4 0x0000555555971da4 in fetch_inferior_event () at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:3910 #5 0x00005555559540b2 in inferior_event_handler (event_type=INF_REG_EVENT) at /home/pedro/gdb/binutils-gdb/src/gdb/inf-loop.c:42 #6 0x000055555597e825 in infrun_async_inferior_event_handler (data=0x0) at /home/pedro/gdb/binutils-gdb/src/gdb/infrun.c:9162 #7 0x0000555555687d1d in check_async_event_handlers () at /home/pedro/gdb/binutils-gdb/src/gdb/async-event.c:328 #8 0x0000555555e48284 in gdb_do_one_event () at /home/pedro/gdb/binutils-gdb/src/gdbsupport/event-loop.cc:216 #9 0x00005555559e7512 in start_event_loop () at /home/pedro/gdb/binutils-gdb/src/gdb/main.c:347 #10 0x00005555559e765d in captured_command_loop () at /home/pedro/gdb/binutils-gdb/src/gdb/main.c:407 #11 0x00005555559e8f80 in captured_main (data=0x7fffffffdb70) at /home/pedro/gdb/binutils-gdb/src/gdb/main.c:1239 #12 0x00005555559e8ff2 in gdb_main (args=0x7fffffffdb70) at /home/pedro/gdb/binutils-gdb/src/gdb/main.c:1254 #13 0x0000555555627c86 in main (argc=12, argv=0x7fffffffdc88) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:32 The problem is that the remote sends stops for all the threads: Packet received: l/home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.threads/attach-non-stop/attach-non-stop Sending packet: $vStopped#55...Packet received: T0006:f06e25edec7f0000;07:f06e25edec7f0000;10:f14190ccf4550000;thread:p12fd22.12fd2f;core:15; Sending packet: $vStopped#55...Packet received: T0006:f0dea5f0ec7f0000;07:f0dea5f0ec7f0000;10:e84190ccf4550000;thread:p12fd22.12fd27;core:4; Sending packet: $vStopped#55...Packet received: T0006:f0ee25f1ec7f0000;07:f0ee25f1ec7f0000;10:f14190ccf4550000;thread:p12fd22.12fd26;core:5; Sending packet: $vStopped#55...Packet received: T0006:f0bea5efec7f0000;07:f0bea5efec7f0000;10:f14190ccf4550000;thread:p12fd22.12fd29;core:1; Sending packet: $vStopped#55...Packet received: T0006:f0ce25f0ec7f0000;07:f0ce25f0ec7f0000;10:e84190ccf4550000;thread:p12fd22.12fd28;core:a; Sending packet: $vStopped#55...Packet received: T0006:f07ea5edec7f0000;07:f07ea5edec7f0000;10:e84190ccf4550000;thread:p12fd22.12fd2e;core:f; Sending packet: $vStopped#55...Packet received: T0006:f0ae25efec7f0000;07:f0ae25efec7f0000;10:df4190ccf4550000;thread:p12fd22.12fd2a;core:6; Sending packet: $vStopped#55...Packet received: T0006:0000000000000000;07:c0e8a381fe7f0000;10:bf43b4f1ec7f0000;thread:p12fd22.12fd22;core:2; Sending packet: $vStopped#55...Packet received: T0006:f0fea5f1ec7f0000;07:f0fea5f1ec7f0000;10:df4190ccf4550000;thread:p12fd22.12fd25;core:8; Sending packet: $vStopped#55...Packet received: T0006:f09ea5eeec7f0000;07:f09ea5eeec7f0000;10:e84190ccf4550000;thread:p12fd22.12fd2b;core:b; Sending packet: $vStopped#55...Packet received: OK But then wait_one never consumes them, always hitting this path: 4473 if (nfds == 0) 4474 { 4475 /* No waitable targets left. All must be stopped. */ 4476 return {NULL, minus_one_ptid, {TARGET_WAITKIND_NO_RESUMED}}; 4477 } Resulting in GDB constanly calling target_stop to stop threads, but the remote target never reporting back the stops to infrun. That TARGET_WAITKIND_NO_RESUMED path shown above is always taken because here, in wait_one too, just above: 4428 for (inferior *inf : all_inferiors ()) 4429 { 4430 process_stratum_target *target = inf->process_target (); 4431 if (target == NULL 4432 || !target->is_async_p () ^^^^^^^^^^^^^^^^^^^^^ 4433 || !target->threads_executing) 4434 continue; ... the remote target is not async. And in turn that happened because extended_remote_target::attach misses enabling async in the target-non-stop path. A testcase exercising this will be added in a following patch. gdb/ChangeLog: * remote.c (extended_remote_target::attach): Set target async in the target-non-stop path too.
2021-02-03Fix attaching in non-stop mode (PR gdb/27055)Pedro Alves1-0/+6
Attaching in non-stop mode currently misbehaves, like so: (gdb) attach 1244450 Attaching to process 1244450 [New LWP 1244453] [New LWP 1244454] [New LWP 1244455] [New LWP 1244456] [New LWP 1244457] [New LWP 1244458] [New LWP 1244459] [New LWP 1244461] [New LWP 1244462] [New LWP 1244463] No unwaited-for children left. At this point, GDB's stopped/running thread state is out of sync with the inferior: (gdb) info threads Id Target Id Frame * 1 LWP 1244450 "attach-non-stop" 0xf1b443bf in ?? () 2 LWP 1244453 "attach-non-stop" (running) 3 LWP 1244454 "attach-non-stop" (running) 4 LWP 1244455 "attach-non-stop" (running) 5 LWP 1244456 "attach-non-stop" (running) 6 LWP 1244457 "attach-non-stop" (running) 7 LWP 1244458 "attach-non-stop" (running) 8 LWP 1244459 "attach-non-stop" (running) 9 LWP 1244461 "attach-non-stop" (running) 10 LWP 1244462 "attach-non-stop" (running) 11 LWP 1244463 "attach-non-stop" (running) (gdb) (gdb) interrupt -a (gdb) *nothing* The problem is that attaching installs an inferior continuation, called when the target reports the initial attach stop, here, in inf-loop.c:inferior_event_handler: /* Do all continuations associated with the whole inferior (not a particular thread). */ if (inferior_ptid != null_ptid) do_all_inferior_continuations (0); However, currently in non-stop mode, inferior_ptid is still null_ptid when we get here. If you try to do "set debug infrun 1" to debug the problem, however, then the attach completes correctly, with GDB reporting a stop for each thread. The bug is that we're missing a switch_to_thread/context_switch call when handling the initial stop, here: if (stop_soon == STOP_QUIETLY_NO_SIGSTOP && (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_STOP || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_0)) { stop_print_frame = true; stop_waiting (ecs); ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; return; } Note how the STOP_QUIETLY / STOP_QUIETLY_REMOTE case above that does call context_switch. And the reason "set debug infrun 1" "fixes" it, is that the debug path has a switch_to_thread call. This patch fixes it by moving the main context_switch call earlier. It also removes the: if (ecs->ptid != inferior_ptid) check at the same time because: #1 - that is half of what context_switch already does #2 - deprecated_context_hook is only used in Insight, and all it does is set an int. It won't care if we call it when the current thread hasn't actually changed. A testcase exercising this will be added in a following patch. gdb/ChangeLog: PR gdb/27055 * infrun.c (handle_signal_stop): Move main context_switch call earlier, before STOP_QUIETLY_NO_SIGSTOP.
2021-02-02Inferior without argument prints detail of current inferior.Lancelot SIX1-0/+8
This patch makes the inferior command display information about the current inferior when called with no argument. This behavior is similar to the one of the thread command. Before patch: (gdb) info inferior Num Description Connection Executable * 1 process 19221 1 (native) /home/lsix/tmp/a.out 2 process 19239 1 (native) /home/lsix/tmp/a.out (gdb) inferior 2 [Switching to inferior 2 [process 19239] (/home/lsix/tmp/a.out)] [Switching to thread 2.1 (process 19239)] #0 0x0000000000401146 in main () (gdb) inferior Argument required (expression to compute). After patch: (gdb) info inferior Num Description Connection Executable * 1 process 18699 1 (native) /home/lsix/tmp/a.out 2 process 18705 1 (native) /home/lsix/tmp/a.out (gdb) inferior 2 [Switching to inferior 2 [process 18705] (/home/lsix/tmp/a.out)] [Switching to thread 2.1 (process 18705)] #0 0x0000000000401146 in main () (gdb) inferior [Current inferior is 2 [process 18705] (/home/lsix/tmp/a.out)] gdb/doc/ChangeLog: * gdb.texinfo (Inferiors Connections and Programs): Document the inferior command when used without argument. gdb/ChangeLog: * NEWS: Add entry for the behavior change of the inferior command. * inferior.c (inferior_command): When no argument is given to the inferior command, display info about the currently selected inferior. gdb/testsuite/ChangeLog: * gdb.base/inferior-noarg.c: New test. * gdb.base/inferior-noarg.exp: New test.
2021-02-02gdb/dwarf: make read_{loc,rng}list_index return sect_offsetSimon Marchi1-0/+6
I think it's wrong that read_loclist_index and read_rnglist_index return a CORE_ADDR. A CORE_ADDR is an address in the program. These functions return offset in sections (.debug_loclists and .debug_rnglists). I think sect_offset is more appropriate. I'm wondering if struct attribute should have a "set_sect_offset" method, that takes a sect_offset parameter, or if it's better to be left as a simple "unsigned". gdb/ChangeLog: * dwarf2/read.c (read_loclist_index, read_rnglist_index): Return a sect_offset. (read_attribute_reprocess): Adjust. Change-Id: I0e22e0864130fb490072b41ae099762918b8ad4d
2021-02-02gdb/dwarf: split dwarf2_cu::ranges_base in twoSimon Marchi1-0/+15
Consider the test case added in this patch. It defines a compilation unit with a DW_AT_rnglists_base attribute (used for attributes of form DW_FORM_rnglistx), but also uses DW_AT_ranges of form DW_FORM_sec_offset: 0x00000027: DW_TAG_compile_unit DW_AT_ranges [DW_FORM_sec_offset] (0x0000004c [0x0000000000005000, 0x0000000000006000)) DW_AT_rnglists_base [DW_FORM_sec_offset] (0x00000044) The DW_AT_rnglists_base does not play a role in reading the DW_AT_ranges of form DW_FORM_sec_offset, but it should also not do any harm. This case is currently not handled correctly by GDB. This is not something that a compiler is likely to emit, but in my opinion there's no reason why GDB should fail reading it. The problem is that in partial_die_info::read and a few other places where the same logic is replicated, the cu->ranges_base value, containing the DW_AT_rnglists_base value, is wrongfully added to the DW_AT_ranges value. It is quite messy how to decide whether cu->ranges_base should be added to the attribute's value or not. But to summarize, the only time we want to add it is when the attribute comes from a pre-DWARF 5 split unit file (a .dwo) [1]. In this case, the DW_AT_ranges attribute from the split unit file will have form DW_FORM_sec_offset, pointing somewhere in the linked file's .debug_ranges section. *But* it's not a "true" DW_FORM_sec_offset, in that it's an offset relative to the beginning of that CU's contribution in the section, not relative to the beginning of the section. So in that case, and only that case, do we want to add the ranges base value, which we found from the DW_AT_GNU_ranges_base attribute on the skeleton unit. Almost all instances of the DW_AT_ranges attribute will be found in the split unit (on DW_TAG_subprogram, for example), and therefore need to have the ranges base added. However, the DW_TAG_compile_unit DIE in the skeleton may also have a DW_AT_ranges attribute. For that one, the ranges base must not be added. Once the DIEs have been loaded in GDB, however, the distinction between what's coming from the skeleton and what's coming from the split unit is not clear. It is all merged in one big happy tree. So how do we know if a given attribute comes from the split unit or not? We use the fact that in pre-DWARF 5 split DWARF, DW_AT_ranges is found on the skeleton's DW_TAG_compile_unit (in the linked file) and never in the split unit's DW_TAG_compile_unit. This is why you have this in partial_die_info::read: int need_ranges_base = (tag != DW_TAG_compile_unit && attr.form != DW_FORM_rnglistx); However, with the corner case described above (where we have a DW_AT_rnglists_base attribute and a DW_AT_ranges attribute of form DW_FORM_sec_offset) the condition gets it wrong when it encounters an attribute like DW_TAG_subprogram with a DW_AT_ranges attribute of DW_FORM_sec_offset form: it thinks that it is necessary to add the base, when it reality it is not. The problem boils down to failing to differentiate these cases: - a DW_AT_ranges attribute of form DW_FORM_sec_offset in a pre-DWARF 5 split unit (in which case we need to add the base) - a DW_AT_ranges attribute of form DW_FORM_sec_offset in a DWARF 5 non-split unit (in which case we must not add the base) What makes it unnecessarily complex is that the cu->ranges_base field is overloaded, used to hold the pre-DWARF 5, non-standard DW_AT_GNU_ranges_base and the DWARF 5 DW_AT_rnglists_base. In reality, these two are called "bases" but are not the same thing. The result is that we need twisted conditions to try to determine whether or not we should add the base to the attribute's value. To fix it, split the field in two distinct fields. I renamed everything related to the "old" ranges base to "gnu_ranges_base", to make it clear that it's about the non-standard, pre-DWARF 5 thing. And everything related to the DWARF 5 thing gets renamed "rnglists". I think it becomes much easier to reason this way. The issue described above gets fixed by the fact that the DW_AT_rnglists_base value does not end up in cu->gnu_ranges_base, so cu->gnu_ranges_base stays 0. The condition to determine whether gnu_ranges_base should be added can therefore be simplified back to: tag != DW_TAG_compile_unit ... as it was before rnglistx support was added. Extend the gdb.dwarf2/rnglists-sec-offset.exp to cover this case. I also extended the test case for loclists similarly, just to see if there would be some similar problem. There wasn't, but I think it's not a bad idea to test that case for loclists as well, so I left it in the patch. [1] https://gcc.gnu.org/wiki/DebugFission gdb/ChangeLog: * dwarf2/die.h (struct die_info) <ranges_base>: Split in... <gnu_ranges_base>: ... this... <rnglists_base>: ... and this. * dwarf2/read.c (struct dwarf2_cu) <ranges_base>: Split in... <gnu_ranges_base>: ... this... <rnglists_base>: ... and this. (read_cutu_die_from_dwo): Adjust (dwarf2_get_pc_bounds): Adjust (dwarf2_record_block_ranges): Adjust. (read_full_die_1): Adjust (partial_die_info::read): Adjust. (read_rnglist_index): Adjust. gdb/testsuite/ChangeLog: * gdb.dwarf2/rnglists-sec-offset.exp: Add test for DW_AT_ranges of DW_FORM_sec_offset form plus DW_AT_rnglists_base attribute. * gdb.dwarf2/loclists-sec-offset.exp: Add test for DW_AT_location of DW_FORM_sec_offset plus DW_AT_loclists_base attribute Change-Id: Icd109038634b75d0e6e9d7d1dcb62fb9eb951d83
2021-02-02gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_indexSimon Marchi1-0/+9
When loading the binary from PR 26813 in GDB, we get: DW_FORM_rnglistx index pointing outside of .debug_rnglists offset array [in module /home/simark/build/binutils-gdb/gdb/MagicPurse] ... and the symbols fail to load. In read_rnglist_index and read_loclist_index, we read the header (documented in sections 7.28 and 7.29 of DWARF 5) of the CU's contribution to the .debug_rnglists / .debug_loclists sections to validate that the index we want to read makes sense. However, we always read the header at the beginning of the section, rather than the header for the contribution from which we want to read the index. To illustrate, here's what the binary from PR 26813 contains. There are two compile units: 0x0000000c: DW_TAG_compile_unit 1 DW_AT_ranges [DW_FORM_rnglistx]: 0x0 DW_AT_rnglists_base [DW_FORM_sec_offset]: 0xC 0x00003ec9: DW_TAG_compile_unit 2 DW_AT_ranges [DW_FORM_rnglistx]: 0xB DW_AT_rnglists_base [DW_FORM_sec_offset]: 0x85 The layout of the .debug_rnglists is the following: [0x00, 0x0B]: header for CU 1's contribution [0x0C, 0x0F]: list of offsets for CU 1 (1 element) [0x10, 0x78]: range lists data for CU 1 [0x79, 0x84]: header for CU 2's contribution [0x85, 0xB4]: list of offsets for CU 2 (12 elements) [0xB5, 0xBD7]: range lists data for CU 2 The DW_AT_rnglists_base attrbute points to the beginning of the list of offsets for that CU, relative to the start of the .debug_rnglists section. That's right after the header for that contribution. When we try to read the DW_AT_ranges attribute for CU 2, read_rnglist_index reads the header for CU 1 instead of the one for CU 2. Since there's only one element in CU 1's offset list, it believes (wrongfully) that the index 0xB is out of range. Fix it by reading the header just before where DW_AT_rnglists_base points to. With this patch, I am able to load GDB built with clang-11 and -gdwarf-5 in itself, with and without -readnow. gdb/ChangeLog: PR gdb/26813 * dwarf2/read.c (read_loclists_rnglists_header): Add header_offset parameter and use it. (read_loclist_index): Read header of the current contribution, not the one at the beginning of the section. (read_rnglist_index): Likewise. Change-Id: Ie53ff8251af8c1556f0a83a31aa8572044b79e3d
2021-02-02gdb/dwarf: few fixes for handling DW_FORM_{rng,loc}listxSimon Marchi1-0/+13
We hit an assertion when loading the binary from PR 26813. When fixing it, execution goes a up bit further but then hits another assert, and another, and another. With these fours fixes, I am able to load the binary and get to the prompt. An error is shown (index pointing outside of the section), because the DW_FORM_rnglistx attribute is not read correctly, but that one is taken care of by the next patch. The four fixes are: - attribute::form_requires_reprocessing needs to handle forms DW_FORM_rnglistx and DW_FORM_loclistx, because set_unsigned_reprocess is called for them in read_attribute_value. - read_attribute_reprocess must call set_unsigned for them, not set_address. The parameter of set_address is a CORE_ADDR, meaning it's for program addresses. Post-reprocess, DW_FORM_rnglistx and DW_FORM_loclistx are offsets into their respective sections (.debug_rnglists and .debug_loclists). set_unsigned is the current attribute value setter that fits the best. But perhaps we should have a setter that takes a sect_offset? - read_attribute_process must call as_unsigned_reprocess instead of as_unsigned to get the pre-reprocess value, otherwise we hit the assert inside as_unsigned that makes sure the attribute doesn't need reprocessing. - attribute::set_unsigned needs to clear the requires_reprocessing flag, otherwise it stays set when reprocessing DW_FORM_rnglistx and DW_FORM_loclistx attributes. There's another assert that we hit once the next patch is applied, but since it's in the same vein as the changes in this patch, I included it in this patch: - attribute::form_is_unsigned must handle form DW_FORM_loclistx, otherwise we hit the assert when trying to call set_unsigned for an attribute of this form. DW_FORM_rnglistx is already handled. gdb/ChangeLog: PR gdb/26813 * dwarf2/attribute.h (struct attribute) <set_unsigned>: Clear requires_reprocessing flag. * dwarf2/attribute.c (attribute::form_is_unsigned): Handle DW_FORM_loclistx. (attribute::form_requires_reprocessing): Handle DW_FORM_rnglistx and DW_FORM_loclistx. * dwarf2/read.c (read_attribute_reprocess): Use set_unsigned instead of set_address for DW_FORM_loclistx and DW_FORM_rnglistx. Change-Id: I06c156fa3913ca98e4e39085f4ef171645b4bc1e
2021-02-02gdb/dwarf: remove unnecessary check in read_{rng,loc}list_indexSimon Marchi1-0/+6
In read_rnglist_index and read_loclist_index, we check that both the start and end of the offset that we read from the offset table are within the section. I think it's unecessary to do both: if the end of the offset is within the section, then surely the start of the offset is within it. Remove the check for the start of the offset in both functions. gdb/ChangeLog: * dwarf2/read.c (read_loclist_index): Remove bound check for start of offset. (read_rnglist_index): Likewise. Change-Id: I7b57ddf4f8a8a28971738f0e3f3af62108f9e19a
2021-02-02gdb/dwarf: add missing bound check to read_loclist_indexSimon Marchi1-0/+5
read_rnglist_index has a bound check to make sure that we don't go past the end of the section while reading the offset, but read_loclist_index doesn't. Add it to read_loclist_index. gdb/ChangeLog: * dwarf2/read.c (read_loclist_index): Add bound check for the end of the offset. Change-Id: Ic4b55c88860fdc3e007740949c78ec84cdb4da60
2021-02-02gdb/dwarf: fix bound check in read_rnglist_indexSimon Marchi1-0/+4
I think this check in read_rnglist_index is wrong: /* Validate that reading won't go beyond the end of the section. */ if (start_offset + cu->header.offset_size > rnglist_base + section->size) error (_("Reading DW_FORM_rnglistx index beyond end of" ".debug_rnglists section [in module %s]"), objfile_name (objfile)); The addition `rnglist_base + section->size` doesn't make sense. rnglist_base is an offset into `section`, so it doesn't make sense to add it to `section`'s size. `start_offset` also is an offset into `section`, so we should just compare it to just `section->size`. gdb/ChangeLog: * dwarf2/read.c (read_rnglist_index): Fix bound check. Change-Id: If0ff7c73f4f80f79aac447518f4e8f131f2db8f2
2021-02-02gdb/dwarf: change read_loclist_index complaints into errorsSimon Marchi1-0/+5
Unlike read_rnglists_index, read_loclist_index uses complaints when it detects an inconsistency (a DW_FORM_loclistx value without a .debug_loclists section or an offset outside of the section). I really think they should be errors, since there's no point in continuing if this situation happens, we will likely segfault or read garbage. gdb/ChangeLog: * dwarf2/read.c (read_loclist_index): Change complaints into errors. Change-Id: Ic3a1cf6e682d47cb6e739dd76fd7ca5be2637e10