Skip to content

Conversation

daeseokyoun
Copy link

This didn't seem to be fixed since 2012 Mar.
Attach gdbserver on qemu from client and set a breakpoint in
start_kernel function but it will be failed with below error:
(gdb) c
Remote 'g' packet reply is too long: 00004082fffffff
...

Refer https://wiki.osdev.org/QEMU_and_GDB_in_long_mode OS-Dev wiki
to fix issue with workaround patch.

My Scenario for kernel debugging

  • Build binutild-gdb after apply the patch for fixing issue
$ ./configure
$ make -j8 && sudo make install
  • the gdb binary will be installed in /usr/local/bin/gdb

  • Run qemu for x86_64 architecture

$ qemu-system-x86_64 -s -S -kernel arch/x86/boot/bzImage -boot c -m 2049M \
   -hda ../buildroot/output/images/rootfs.ext2 \
   -append "root=/dev/sda rw console=ttyS0,115200 acpi=off nokaslr" \
   -serial stdio -display none
  • Run gdb with ./vmlinux
$ cd /path/to/linux/kernel/top
$ gdb ./vmlinux
(gdb) target remote :1234
(gdb) b start_kernel
Breakpoint 1 at 0xffffffff82973ac0: file init/main.c, line 538.
(gdb) c
Breakpoint 1, start_kernel () at init/main.c:538
538	{
(gdb) list
533	{
534		rest_init();
535	}
536	
537	asmlinkage __visible void __init start_kernel(void)
538	{
539		char *command_line;
540		char *after_dashes;
541	
542		set_task_stack_end_magic(&init_task);

This didn't seem to be fixed since 2012 Mar.
Attach gdbserver on qemu from client and set a breakpoint in
start_kernel function but it will be failed with below error:
(gdb) c
 Remote 'g' packet reply is too long: 00004082fffffff
 ...

Refer https://wiki.osdev.org/QEMU_and_GDB_in_long_mode OS-Dev wiki
to fix issue with workaround patch.
@daeseokyoun daeseokyoun closed this Jan 3, 2019
wilsonk pushed a commit to wilsonk/binutils-gdb that referenced this pull request Jan 4, 2019
Unlike Solaris/SPARC, the __sighndlr function isn't recognized as part
of a signal handler, causing a couple of testcases to fail.

The following patch fixes that.  A followup patch will move this to
common code to avoid such unnecessary discrepancies between
Solaris/SPARC and x86 in the future.

While this fixes a couple of backtraces to now correctly print

bminor#1  <signal handler called>

they often fail later with

bminor#2  0x0ff3ffffff00857f in ?? ()
Backtrace stopped: Cannot access memory at address 0xff3000002e0886f

which needs further investigation.

Tested on amd64-pc-solaris2.11 (running the tests with both -m64 and
-m32).

	* amd64-sol2-tdep.c (amd64_sol2_sigtramp_p): Also recognize
	__sighndlr.
	* i386-sol2-tdep.c (i386_sol2_sigtramp_p): Likewise.
wilsonk pushed a commit to wilsonk/binutils-gdb that referenced this pull request Jan 4, 2019
…,write}

Running "maintenance selftest" on an amd64 build with AddressSanitizer
enabled, I get this:

==18126==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7ffdf72397c1 at pc 0x7fb5f437b011 bp 0x7ffdf7239740 sp 0x7ffdf7238ee8
WRITE of size 8 at 0x7ffdf72397c1 thread T0
    #0 0x7fb5f437b010 in __interceptor_memcpy /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:737
    bminor#1 0x55a1f899c1b3 in readable_regcache::raw_read(int, unsigned char*) /home/simark/src/binutils-gdb/gdb/regcache.c:530
    bminor#2 0x55a1f7db241b in amd64_pseudo_register_read_value /home/simark/src/binutils-gdb/gdb/amd64-tdep.c:384
    bminor#3 0x55a1f8413a2e in gdbarch_pseudo_register_read_value(gdbarch*, readable_regcache*, int) /home/simark/src/binutils-gdb/gdb/gdbarch.c:1992
    bminor#4 0x55a1f899c9d1 in readable_regcache::cooked_read(int, unsigned char*) /home/simark/src/binutils-gdb/gdb/regcache.c:636
    bminor#5 0x55a1f89a2251 in cooked_read_test /home/simark/src/binutils-gdb/gdb/regcache.c:1649

In amd64_pseudo_register_read_value, when we try to read the al
register, for example, we need to read rax and extract al from it.  We
allocate a buffer of the size of al (1 byte):

  gdb_byte *raw_buf = (gdb_byte *) alloca (register_size (gdbarch, regnum));

but read in it the whole rax value (8 bytes):

  status = regcache->raw_read (gpnum, raw_buf);

Fix it by allocating a buffer correctly sized for the full register from
which the smaller register is extracted.  The
amd64_pseudo_register_write function had the same problem.

gdb/ChangeLog:

	* amd64-tdep.c (amd64_pseudo_register_read_value): Use
	correctly-sized buffer with raw_read.
	(amd64_pseudo_register_write): Use correctly-sized buffer for
	raw_read/raw_write.
wilsonk pushed a commit to wilsonk/binutils-gdb that referenced this pull request Jan 4, 2019
bminor#1- Check that the warning is emitted.

bminor#2- Avoid overriding INTERNAL_GDBFLAGS, as per documentated in
    gdb/testsuite/README:

 ~~~
 The testsuite does not override a value provided by the user.
 ~~~

We don't actually need to tweak INTERNAL_GDBFLAGS, we just need to
append out -data-directory to GDBFLAGS, because each passed
-data-directory option leads to a call to the warning:

 $ ./gdb -data-directory=foo -data-directory=bar
 Warning: foo: No such file or directory.
 Warning: bar: No such file or directory.
 [...]

gdb/ChangeLog
2018-11-19  Pedro Alves  <palves@redhat.com>

	* gdb.base/warning.exp: Don't override INTERNAL_FLAGS.  Use
	gdb_spawn_with_cmdline_opts instead of gdb_start.  Check that we
	see the expected warning.
wilsonk pushed a commit to wilsonk/binutils-gdb that referenced this pull request Jan 4, 2019
First of all, I would like to express my gratitude to Keith Seitz, Jan
Kratochvil and Tom Tromey, who were really kind and helped a lot with
this bug.  The patch itself was authored by Jan.

This all began with:

  https://bugzilla.redhat.com/show_bug.cgi?id=1639242
  py-bt is broken, results in exception

In summary, the error reported by the bug above is:

  $ gdb -args python3
  GNU gdb (GDB) Fedora 8.1.1-3.fc28
  (...)
  Reading symbols from python3...Reading symbols from /usr/lib/debug/usr/bin/python3.6-3.6.6-1.fc28.x86_64.debug...done.
  done.
  Dwarf Error: could not find partial DIE containing offset 0x316 [in module /usr/lib/debug/usr/bin/python3.6-3.6.6-1.fc28.x86_64.debug]

After a long investigation, and after thinking that the problem might
actually be on DWZ's side, we were able to determine that there's
something wrong going on when
dwarf2read.c:dwarf2_find_containing_comp_unit performs a binary search
over all of the CUs belonging to an objfile in order to find the CU
which contains a DIE at an specific offset.  The current algorithm is:

  static struct dwarf2_per_cu_data *
  dwarf2_find_containing_comp_unit (sect_offset sect_off,
				    unsigned int offset_in_dwz,
				    struct dwarf2_per_objfile *dwarf2_per_objfile)
  {
    struct dwarf2_per_cu_data *this_cu;
    int low, high;
    const sect_offset *cu_off;

    low = 0;
    high = dwarf2_per_objfile->all_comp_units.size () - 1;
    while (high > low)
      {
	struct dwarf2_per_cu_data *mid_cu;
	int mid = low + (high - low) / 2;

	mid_cu = dwarf2_per_objfile->all_comp_units[mid];
	cu_off = &mid_cu->sect_off;
	if (mid_cu->is_dwz > offset_in_dwz
	    || (mid_cu->is_dwz == offset_in_dwz && *cu_off >= sect_off))
	  high = mid;
	else
	  low = mid + 1;
      }

For the sake of this example, let's consider that "sect_off =
0x7d".

There are a few important things going on here.  First,
"dwarf2_per_objfile->all_comp_units ()" will be sorted first by
whether the CU is a DWZ CU, and then by cu->sect_off.  In this
specific bug, "offset_in_dwz" is false, which means that, for the most
part of the loop, we're going to do "high = mid" (i.e, we'll work with
the lower part of the vector).

In our particular case, when we reach the part where "mid_cu->is_dwz
== offset_in_dwz" (i.e, both are false), we end up with "high = 2" and
"mid = 1".  I.e., there are only 2 elements in the vector who are not
DWZ.  The vector looks like this:

  #0: cu->sect_off = 0;   length = 114;  is_dwz = false  <-- low
  bminor#1: cu->sect_off = 114; length = 7796; is_dwz = false  <-- mid
  bminor#2: cu->sect_off = 0;   length = 28;   is_dwz = true   <-- high
  ...

The CU we want is bminor#1, which is exactly where "mid" is.  Also, bminor#1 is
not DWZ, which is also exactly what we want.  So we perform the second
comparison:

  (mid_cu->is_dwz == offset_in_dwz && *cu_off >= sect_off)
                                      ^^^^^^^^^^^^^^^^^^^

Because "*cu_off = 114" and "sect_off = 0x7d", this evaluates to
false, so we end up with "low = mid + 1 = 2", which actually gives us
the wrong CU (i.e., a CU that is DWZ).  Next in the code, GDB does:

    gdb_assert (low == high);
    this_cu = dwarf2_per_objfile->all_comp_units[low];
    cu_off = &this_cu->sect_off;
    if (this_cu->is_dwz != offset_in_dwz || *cu_off > sect_off)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      {
	if (low == 0 || this_cu->is_dwz != offset_in_dwz)
	  error (_("Dwarf Error: could not find partial DIE containing "
		 "offset %s [in module %s]"),
		 sect_offset_str (sect_off),
		 bfd_get_filename (dwarf2_per_objfile->objfile->obfd));
	...

Triggering the error we saw in the original bug report.

It's important to notice that we see the error message because the
selected CU is a DWZ one, but we're looking for a non-DWZ CU here.
However, even when the selected CU is *not* a DWZ (and we don't see
any error message), we still end up with the wrong CU.  For example,
suppose that the vector had:

  #0: cu->sect_off = 0;    length = 114;  is_dwz = false
  bminor#1: cu->sect_off = 114;  length = 7796; is_dwz = false
  bminor#2: cu->sect_off = 7910; length = 28;   is_dwz = false
  ...

I.e., bminor#2's "is_dwz" is false instead of true.  In this case, we still
want bminor#1, because that's where the DIE is located.  After the loop ends
up in bminor#2, we have "is_dwz" as false, which is what we wanted, so we
compare offsets.  In this case, "7910 >= 0x7d", so we set "mid = high
= 2".  Next iteration, we have "mid = 0 + (2 - 0) / 2 = 1", and thus
we examining bminor#1.  "is_dwz" is still false, but "114 >= 0x7d" also
evaluates to false, so "low = mid + 1 = 2", which makes the loop stop.
Therefore, we end up choosing bminor#2 as our CU, even though bminor#1 is the
right one.

The problem here is happening because we're comparing "sect_off"
directly against "*cu_off", while we should actually be comparing
against "*cu_off + mid_cu->length" (i.e., the end offset):

  ...
  || (mid_cu->is_dwz == offset_in_dwz
      && *cu_off + mid_cu->length >= sect_off))
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ...

And this is what the patch does.  The idea is that if GDB is searching
for an offset that falls above the *end* of the CU being
analyzed (i.e., "mid"), then the next iteration should try a
higher-offset CU next.  The previous algorithm was using
the *beginning* of the CU.

Unfortunately, I could not devise a testcase for this problem, so I am
proposing a fix with this huge explanation attached to it in the hope
that it is sufficient.  After talking a bit to Keith (our testcase
guru), it seems that one would have to create an objfile with both DWZ
and non-DWZ sections, which may prove very hard to do, I think.

I ran this patch on our BuildBot, and no regressions were detected.

gdb/ChangeLog:
2018-11-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Keith Seitz  <keiths@redhat.com>
	    Tom Tromey  <tom@tromey.com>
	    Sergio Durigan Junior  <sergiodj@redhat.com>

	https://bugzilla.redhat.com/show_bug.cgi?id=1613614
	* dwarf2read.c (dwarf2_find_containing_comp_unit): Add
	'mid_cu->length' to '*cu_off' when checking if 'sect_off' is
	inside the CU.
berenm pushed a commit that referenced this pull request Feb 16, 2019
ravenscar-thread.c intercepts resume and wait target requests and
replaces the requested ptid with the ptid of the underlying CPU.
However, this is incorrect when a request is made with a wildcard
ptid.

This patch adds a special case to ravenscar-thread.c for
minus_one_ptid.  I don't believe a special case for process wildcards
is necessary, so I have not added that.

Joel's description explains the bug well:

At the user level, we noticed the issue because we had a test were
we insert a breakpoint one some code which is only run from, say,
CPU #2, whereas we unfortunately resumed the execution after having
stopped somewhere in CPU #1. As a result, we sent an order to resume
CPU #1, which starves CPU #2 forever, because the code in CPU #1
waits for some of the Ada tasks allocated to CPU #2 (and we never
reach our breakpoint either).

gdb/ChangeLog
2019-02-15  Tom Tromey  <tromey@adacore.com>

	* ravenscar-thread.c (ravenscar_thread_target::resume)
	(ravenscar_thread_target::wait): Special case wildcard requests.
berenm pushed a commit that referenced this pull request Feb 21, 2019
Errors that happen in nested sourced files (when a sourced file sources
another file) lead to a wrong error message, or use-after-free.

For example, if I put this in "a.gdb":

    command_that_doesnt_exist

and this in "b.gdb":

   source a.gdb

and try to "source b.gdb" in GDB, the result may look like this:

    (gdb) source b.gdb
    b.gdb:1: Error in sourced command file:
    _that_doesnt_exist:1: Error in sourced command file:
    Undefined command: "command_that_doesnt_exist".  Try "help".

Notice the wrong file name where "a.gdb" should be.  The exact result
may differ, depending on the feelings of the memory allocator.

What happens is:

- The "source a.gdb" command is saved by command_line_append_input_line
  in command_line_input's static buffer.
- Since we are sourcing a file, the script_from_file function stores the
  script name (a.gdb) in the source_file_name global.  However, it doesn't
  do a copy, it just saves a pointer to command_line_input's static buffer.
- The "command_that_doesnt_exist" command is saved by
  command_line_append_input_line in command_line_input's static buffer.
  Depending on what xrealloc does, source_file_name may now point to
  freed memory, or at the minimum the data it was pointing to was
  overwritten.
- When the error is handled in script_from_file, we dererence
  source_file_name to print the name of the file in which the error
  occured.

To fix it, I made source_file_name an std::string, so that keeps a copy of
the file name instead of pointing to a buffer with a too small
lifetime.

With this patch, the expected filename is printed, and no use-after-free
occurs:

    (gdb) source b.gdb
    b.gdb:1: Error in sourced command file:
    a.gdb:1: Error in sourced command file:
    Undefined command: "command_that_doesnt_exist".  Try "help".

I passed explicit template parameters to make_scoped_restore
(<std::string, const std::string &>), so that the second parameter is
passed by reference and avoid a copy.

It was not as obvious as I first thought to change gdb.base/source.exp
to test this, because source commands inside sourced files are
interpreted relative to GDB's current working directory, not the
directory of the currently sourced file.  As a workaround, I moved the
snippet that tests errors after the snippet that adds the source
directory to the search path.  This way, the "source source-error-1.gdb"
line in source-error.exp manages to find the file.

For reference, here is what ASAN reports when use-after-free occurs:

(gdb) source b.gdb
=================================================================
==18498==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000019847 at pc 0x7f1d3645de8e bp 0x7ffdcb892e50 sp 0x7ffdcb8925c8
READ of size 6 at 0x60c000019847 thread T0
    #0 0x7f1d3645de8d in printf_common /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:546
    #1 0x7f1d36477175 in __interceptor_vasprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1525
    #2 0x5632eaffa277 in xstrvprintf(char const*, __va_list_tag*) /home/simark/src/binutils-gdb/gdb/common/common-utils.c:122
    #3 0x5632eaff96d1 in throw_it /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:351
    #4 0x5632eaff98df in throw_verror(errors, char const*, __va_list_tag*) /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:379
    #5 0x5632eaff9a2a in throw_error(errors, char const*, ...) /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:394
    #6 0x5632eafca21a in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1553
    #7 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #8 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #9 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #10 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #11 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #12 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #13 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #14 0x5632ebf3cf09 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:425
    #15 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #16 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #17 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #18 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #19 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #20 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #21 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #22 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #23 0x5632eb3b2f87 in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770
    #24 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213
    #25 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220
    #26 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175
    #27 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192
    #28 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511
    #29 0x5632eb3aa6a9 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #30 0x5632eb3aaf41 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #31 0x5632eb3a88ea in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:347
    #32 0x5632eb3a89bf in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #33 0x5632eb76fbfc in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:330
    #34 0x5632eb772ea8 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1176
    #35 0x5632eb773071 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1192
    #36 0x5632eabfe7f9 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #37 0x7f1d3554f222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #38 0x5632eabfe5dd in _start (/home/simark/build/binutils-gdb/gdb/gdb+0x195d5dd)

0x60c000019847 is located 7 bytes inside of 128-byte region [0x60c000019840,0x60c0000198c0)
freed by thread T0 here:
    #0 0x7f1d36502491 in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:105
    #1 0x5632eaff9f47 in xrealloc /home/simark/src/binutils-gdb/gdb/common/common-utils.c:62
    #2 0x5632eaff6b44 in buffer_grow(buffer*, char const*, unsigned long) /home/simark/src/binutils-gdb/gdb/common/buffer.c:40
    #3 0x5632eb3b271d in command_line_append_input_line /home/simark/src/binutils-gdb/gdb/event-top.c:614
    #4 0x5632eb3b28c6 in handle_line_of_input(buffer*, char const*, int, char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:654
    #5 0x5632ebf402a6 in command_line_input(char const*, char const*) /home/simark/src/binutils-gdb/gdb/top.c:1252
    #6 0x5632ebf3cee9 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:422
    #7 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #8 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #9 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #10 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #11 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #12 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #13 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #14 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #15 0x5632ebf3cf09 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:425
    #16 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #17 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #18 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #19 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #20 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #21 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #22 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #23 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #24 0x5632eb3b2f87 in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770
    #25 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213
    #26 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220
    #27 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175
    #28 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192
    #29 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511

previously allocated by thread T0 here:
    #0 0x7f1d36502491 in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:105
    #1 0x5632eaff9f47 in xrealloc /home/simark/src/binutils-gdb/gdb/common/common-utils.c:62
    #2 0x5632eaff6b44 in buffer_grow(buffer*, char const*, unsigned long) /home/simark/src/binutils-gdb/gdb/common/buffer.c:40
    #3 0x5632eb3b271d in command_line_append_input_line /home/simark/src/binutils-gdb/gdb/event-top.c:614
    #4 0x5632eb3b28c6 in handle_line_of_input(buffer*, char const*, int, char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:654
    #5 0x5632ebf402a6 in command_line_input(char const*, char const*) /home/simark/src/binutils-gdb/gdb/top.c:1252
    #6 0x5632ebf3cee9 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:422
    #7 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #8 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #9 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #10 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #11 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #12 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #13 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #14 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #15 0x5632eb3b2f87 in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770
    #16 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213
    #17 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220
    #18 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175
    #19 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192
    #20 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511
    #21 0x5632eb3aa6a9 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #22 0x5632eb3aaf41 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #23 0x5632eb3a88ea in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:347
    #24 0x5632eb3a89bf in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #25 0x5632eb76fbfc in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:330
    #26 0x5632eb772ea8 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1176
    #27 0x5632eb773071 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1192
    #28 0x5632eabfe7f9 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #29 0x7f1d3554f222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

SUMMARY: AddressSanitizer: heap-use-after-free /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:546 in printf_common

gdb/ChangeLog:

	* top.h (source_file_name): Change to std::string.
	* top.c (source_file_name): Likewise.
	(command_line_input): Adjust.
	* cli/cli-script.c (script_from_file): Adjust.

gdb/testsuite/ChangeLog:

	* gdb.base/source.exp: Move "error in sourced script" code to
	the end.
	* gdb.base/source-error.gdb: Move contents to
	source-error-1.gdb.  Add new code to source source-error-1.gdb.
	* gdb.base/source-error-1.gdb: New file, from previous
	source-error.gdb.
berenm pushed a commit that referenced this pull request Feb 28, 2019
This patch removes the non-IS_PY3K code in infpy_write_memory()
and infpy_search_memory().  In both cases, the remaining code
from these ifdefs is related to use of the PEP 3118 buffer protocol.
(Deleted code is either due to simplification or related to use of the
old buffer protocol.)  PEP 3118 is sometimes referred to as the "new"
buffer protocol, though it's not that new anymore.

The link below describes new features in Python 2.6.  In particular,
it says that the buffer protocol described by PEP 3118 is in Python
2.6.  It also says (at the top of the page) that Python 2.6 was
released on Oct 1, 2008.

https://docs.python.org/3/whatsnew/2.6.html#pep-3118-revised-buffer-protocol

The last security release for the Python 2.6 series was 2.6.9.  It was
released on Oct 29, 2013.  According to this document...

https://www.python.org/download/releases/2.6.9/

...support for the 2.6 series has ended:

    With the 2.6.9 release, and five years after its first release,
    the Python 2.6 series is now officially retired.  All official
    maintenance for Python 2.6, including security patches, has ended.
    For ongoing maintenance releases, please see the Python 2.7
    series.

As noted earlier, Python 2.6, Python 2.7, and Python 3.X all have
support for the PEP 3118 buffer protocol.  Python releases prior
to 2.6 use an older buffer protocol.  Since Python 2.6 has been
retired for a good while now, it seems reasonable to me to remove
code using the older buffer protocol from GDB.

I have also simplified some of the code via use of the Py_buffer
unique_ptr specialization which I introduced in the two argument
gdb.Value constructor patch series.  Therefore, there is a dependency
on patch #1 from that series.

I have tested against both Python 2.7.15 and 3.7.2.  I see no
regressions among the non-racy tests.  I've also verified that
PyBuffer_Release is being called when the affected functions exit
while running the tests in gdb.python/py-inferior.exp by hand.  I've
also tried running valgrind on GDB while running this test, but I'm
puzzled by the results that I'm seeing - I'm seeing no additional
leaks when I comment out the Py_buffer_up lines that I introduced.
That said, I'm not seeing any leaks that obviously originate from
either infpy_write_memory() or infpy_search_memory().

gdb/ChangeLog:

	* python/py-inferior.c (infpy_write_memory): Remove non-IS_PY3K
	code from these functions.  Remove corresponding ifdefs.  Use
	Py_buffer_up instead of explicit calls to PyBuffer_Release.
	Remove gotos and target of gotos.
	(infpy_search_memory): Likewise.
berenm pushed a commit that referenced this pull request Mar 19, 2019
This commit fixes two issues in scrolling right in the TUI:

#1 - Scrolling right with the arrow keys, the first keypress doesn't
do anything.  The problem is that copy_source_line() checks if
(column < first_col), and because of the ++column directly before, it
basically starts with 1 instead of 0.

#2 - Scrolling right handles TABS and escaped characters as single
characters, which just looks weird.  The problem is that there's a
spot that misses handling TABS.

gdb/ChangeLog:
2019-03-18  Hannes Domani  <ssbssa@yahoo.de>

	* tui/tui-source.c (copy_source_line): Fix handling of 'column'.
	Handle tabs.
berenm pushed a commit that referenced this pull request Mar 19, 2019
This commit fixes two issues in scrolling right in the TUI:

#1 - Scrolling right with the arrow keys, the first keypress doesn't
do anything.  The problem is that copy_source_line() checks if
(column < first_col), and because of the ++column directly before, it
basically starts with 1 instead of 0.

#2 - Scrolling right handles TABS and escaped characters as single
characters, which just looks weird.  The problem is that there's a
spot that misses handling TABS.

gdb/ChangeLog:
2019-03-18  Hannes Domani  <ssbssa@yahoo.de>

	* tui/tui-source.c (copy_source_line): Fix handling of 'column'.
	Handle tabs.
berenm pushed a commit that referenced this pull request Mar 26, 2019
Commit ab42892 ("Fix vertical scrolling of TUI source window")
introduced a use-after-free in source_cache::get_source_lines.

At the beginning of the method, we get the fullname of the symtab:

    const char *fullname = symtab_to_fullname (s);

fullname points to the string owned by the symtab (s.fullname).  When we
later do

    scoped_fd desc = open_source_file (s);

s.fullname gets reallocated (even though the string contents may not
change).  The fullname local variable now points to freed memory.

To avoid it, refresh the value of fullname after calling
open_source_file.

Here is the ASan report:

$ ./gdb -nx --data-directory=data-directory ./a.out
(gdb) start
Temporary breakpoint 1 at 0x1130: file test.cpp, line 12.
Starting program: /home/simark/build/binutils-gdb/gdb/a.out

Temporary breakpoint 1, main () at test.cpp:12
=================================================================
==26068==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210003d4100 at pc 0x7fed89a34681 bp 0x7ffd8d185d80 sp 0x7ffd8d185528
READ of size 2 at 0x6210003d4100 thread T0
    #0 0x7fed89a34680 in __interceptor_strlen /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:301
    #1 0x55b6edf6c2f7 in std::char_traits<char>::length(char const*) /usr/include/c++/8.2.1/bits/char_traits.h:320
    #2 0x55b6edf6c9b2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) /usr/include/c++/8.2.1/bits/basic_string.h:516
    #3 0x55b6ef09121b in source_cache::get_source_lines(symtab*, int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) /home/simark/src/binutils-gdb/gdb/source-cache.c:214
    #4 0x55b6ef0a15cb in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1340
    #5 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #6 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #7 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #8 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #9 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #10 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #11 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #12 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #13 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #14 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #15 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #16 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #17 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #18 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #19 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #20 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #21 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #22 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #23 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #24 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #25 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #26 0x7fed88ad8222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #27 0x55b6edf4f86d in _start (/home/simark/build/binutils-gdb/gdb/gdb+0x197186d)

0x6210003d4100 is located 0 bytes inside of 4096-byte region [0x6210003d4100,0x6210003d5100)
freed by thread T0 here:
    #0 0x7fed89a8ac19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:66
    #1 0x55b6edfe12df in xfree<char> /home/simark/src/binutils-gdb/gdb/common/common-utils.h:60
    #2 0x55b6edfea675 in gdb::xfree_deleter<char>::operator()(char*) const /home/simark/src/binutils-gdb/gdb/common/gdb_unique_ptr.h:34
    #3 0x55b6edfe532c in std::unique_ptr<char, gdb::xfree_deleter<char> >::reset(char*) /usr/include/c++/8.2.1/bits/unique_ptr.h:382
    #4 0x55b6edfe7329 in std::unique_ptr<char, gdb::xfree_deleter<char> >::operator=(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /usr/include/c++/8.2.1/bits/unique_ptr.h:289
    #5 0x55b6ef09ec2b in find_and_open_source(char const*, char const*, std::unique_ptr<char, gdb::xfree_deleter<char> >*) /home/simark/src/binutils-gdb/gdb/source.c:990
    #6 0x55b6ef09f56a in open_source_file(symtab*) /home/simark/src/binutils-gdb/gdb/source.c:1069
    #7 0x55b6ef090f78 in source_cache::get_source_lines(symtab*, int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) /home/simark/src/binutils-gdb/gdb/source-cache.c:205
    #8 0x55b6ef0a15cb in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1340
    #9 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #10 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #11 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #12 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #13 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #14 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #15 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #16 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #17 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #18 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #19 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #20 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #21 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #22 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #23 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #24 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #25 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #26 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #27 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #28 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #29 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32

previously allocated by thread T0 here:
    #0 0x7fed89a8b019 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
    #1 0x7fed88af983f in realpath@@GLIBC_2.3 (/usr/lib/libc.so.6+0x4583f)
    #2 0x7fed899dbbbc in __interceptor_canonicalize_file_name /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:3297
    #3 0x55b6ee376a03 in gdb_realpath(char const*) /home/simark/src/binutils-gdb/gdb/common/pathstuff.c:72
    #4 0x55b6ef09ec12 in find_and_open_source(char const*, char const*, std::unique_ptr<char, gdb::xfree_deleter<char> >*) /home/simark/src/binutils-gdb/gdb/source.c:990
    #5 0x55b6ef09f56a in open_source_file(symtab*) /home/simark/src/binutils-gdb/gdb/source.c:1069
    #6 0x55b6ef0a0f12 in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1270
    #7 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #8 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #9 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #10 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #11 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #12 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #13 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #14 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #15 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #16 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #17 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #18 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #19 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #20 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #21 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #22 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #23 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #24 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #25 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #26 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #27 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #28 0x7fed88ad8222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

gdb/ChangeLog:

	* source-cache.c (source_cache::get_source_lines): Re-read
	fullname after calling open_source_file.
berenm pushed a commit that referenced this pull request Mar 26, 2019
Commit ab42892 ("Fix vertical scrolling of TUI source window")
introduced a use-after-free in source_cache::get_source_lines.

At the beginning of the method, we get the fullname of the symtab:

    const char *fullname = symtab_to_fullname (s);

fullname points to the string owned by the symtab (s.fullname).  When we
later do

    scoped_fd desc = open_source_file (s);

s.fullname gets reallocated (even though the string contents may not
change).  The fullname local variable now points to freed memory.

To avoid it, refresh the value of fullname after calling
open_source_file.

Here is the ASan report:

$ ./gdb -nx --data-directory=data-directory ./a.out
(gdb) start
Temporary breakpoint 1 at 0x1130: file test.cpp, line 12.
Starting program: /home/simark/build/binutils-gdb/gdb/a.out

Temporary breakpoint 1, main () at test.cpp:12
=================================================================
==26068==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210003d4100 at pc 0x7fed89a34681 bp 0x7ffd8d185d80 sp 0x7ffd8d185528
READ of size 2 at 0x6210003d4100 thread T0
    #0 0x7fed89a34680 in __interceptor_strlen /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:301
    #1 0x55b6edf6c2f7 in std::char_traits<char>::length(char const*) /usr/include/c++/8.2.1/bits/char_traits.h:320
    #2 0x55b6edf6c9b2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) /usr/include/c++/8.2.1/bits/basic_string.h:516
    #3 0x55b6ef09121b in source_cache::get_source_lines(symtab*, int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) /home/simark/src/binutils-gdb/gdb/source-cache.c:214
    #4 0x55b6ef0a15cb in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1340
    #5 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #6 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #7 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #8 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #9 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #10 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #11 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #12 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #13 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #14 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #15 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #16 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #17 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #18 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #19 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #20 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #21 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #22 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #23 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #24 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #25 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #26 0x7fed88ad8222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #27 0x55b6edf4f86d in _start (/home/simark/build/binutils-gdb/gdb/gdb+0x197186d)

0x6210003d4100 is located 0 bytes inside of 4096-byte region [0x6210003d4100,0x6210003d5100)
freed by thread T0 here:
    #0 0x7fed89a8ac19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:66
    #1 0x55b6edfe12df in xfree<char> /home/simark/src/binutils-gdb/gdb/common/common-utils.h:60
    #2 0x55b6edfea675 in gdb::xfree_deleter<char>::operator()(char*) const /home/simark/src/binutils-gdb/gdb/common/gdb_unique_ptr.h:34
    #3 0x55b6edfe532c in std::unique_ptr<char, gdb::xfree_deleter<char> >::reset(char*) /usr/include/c++/8.2.1/bits/unique_ptr.h:382
    #4 0x55b6edfe7329 in std::unique_ptr<char, gdb::xfree_deleter<char> >::operator=(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /usr/include/c++/8.2.1/bits/unique_ptr.h:289
    #5 0x55b6ef09ec2b in find_and_open_source(char const*, char const*, std::unique_ptr<char, gdb::xfree_deleter<char> >*) /home/simark/src/binutils-gdb/gdb/source.c:990
    #6 0x55b6ef09f56a in open_source_file(symtab*) /home/simark/src/binutils-gdb/gdb/source.c:1069
    #7 0x55b6ef090f78 in source_cache::get_source_lines(symtab*, int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) /home/simark/src/binutils-gdb/gdb/source-cache.c:205
    #8 0x55b6ef0a15cb in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1340
    #9 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #10 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #11 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #12 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #13 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #14 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #15 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #16 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #17 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #18 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #19 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #20 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #21 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #22 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #23 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #24 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #25 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #26 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #27 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #28 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #29 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32

previously allocated by thread T0 here:
    #0 0x7fed89a8b019 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
    #1 0x7fed88af983f in realpath@@GLIBC_2.3 (/usr/lib/libc.so.6+0x4583f)
    #2 0x7fed899dbbbc in __interceptor_canonicalize_file_name /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:3297
    #3 0x55b6ee376a03 in gdb_realpath(char const*) /home/simark/src/binutils-gdb/gdb/common/pathstuff.c:72
    #4 0x55b6ef09ec12 in find_and_open_source(char const*, char const*, std::unique_ptr<char, gdb::xfree_deleter<char> >*) /home/simark/src/binutils-gdb/gdb/source.c:990
    #5 0x55b6ef09f56a in open_source_file(symtab*) /home/simark/src/binutils-gdb/gdb/source.c:1069
    #6 0x55b6ef0a0f12 in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1270
    #7 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #8 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #9 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #10 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #11 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #12 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #13 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #14 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #15 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #16 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #17 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #18 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #19 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #20 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #21 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #22 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #23 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #24 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #25 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #26 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #27 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #28 0x7fed88ad8222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

gdb/ChangeLog:

	* source-cache.c (source_cache::get_source_lines): Re-read
	fullname after calling open_source_file.
berenm pushed a commit that referenced this pull request Apr 2, 2019
…trace

The test  gdb.threads/watchthreads-reorder.exp verifies that the
'set debug infrun 1' debug output does not crash GDB.

Under high load, the test can still cause a GDB internal error (see details
below).

This patch fixes this crash, and improves/factorises some wait kind traces.

Tested on debian/amd64 + run one test with 'set debug infrun 1'.

Changes compared to the first version:
  * Handles the suggestions of Kevin to trace the relevant elements
    of the wait status (this is done by calling target_waitstatus_to_string).
  * Some other changes to factorise wait status tracing.

Note that using target_waitstatus_to_string instead of the 'locally printed'
status kind strings means that debug trace that was using strings such as:
   "EXITED" or "TARGET_WAITKIND_EXITED"
will now use what is printed by target_waitstatus_to_string e.g.
   "exited".

gdb/ChangeLog
2019-04-01  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* infrun.c (stop_all_threads): If debug_infrun, always
	trace the wait status after wait_one, using
	target_waitstatus_to_string and target_pid_to_str.
	(handle_inferior_event): Replace various trace of
	wait status kind by a single trace.
	* gdb/gnu-nat.c (gnu_nat_target::wait): Replace local
	wait status kind image by target_waitstatus_to_string.
	* target/waitstatus.c (target_waitstatus_to_string): Fix
	obsolete comment.

  (top-gdb) bt
  #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x00007f3d54a0642a in __GI_abort () at abort.c:89
  #2  0x0000555c24c60e66 in dump_core () at ../../fixleaks/gdb/utils.c:201
  #3  0x0000555c24c63d49 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=problem@entry=0x555c25338d40 <internal_error_problem>, file=<optimized out>, line=287,
      fmt=<optimized out>, ap=<optimized out>) at ../../fixleaks/gdb/utils.c:411
  #4  0x0000555c24c63eab in internal_verror (file=<optimized out>, line=<optimized out>, fmt=<optimized out>,
      ap=<optimized out>) at ../../fixleaks/gdb/utils.c:436
  #5  0x0000555c249e8c22 in internal_error (file=file@entry=0x555c24e0f2ad "../../fixleaks/gdb/inferior.c",
      line=line@entry=287, fmt=<optimized out>) at ../../fixleaks/gdb/common/errors.c:55
  #6  0x0000555c247d3f5c in find_inferior_pid (pid=<optimized out>) at ../../fixleaks/gdb/inferior.c:287
  #7  0x0000555c24ad2248 in find_inferior_pid (pid=<optimized out>) at ../../fixleaks/gdb/inferior.c:302
  #8  find_inferior_ptid (ptid=...) at ../../fixleaks/gdb/inferior.c:301
  #9  0x0000555c24c35f25 in find_thread_ptid (ptid=...) at ../../fixleaks/gdb/thread.c:522
  #10 0x0000555c24b0ab4d in thread_db_target::pid_to_str[abi:cxx11](ptid_t) (
      this=0x555c2532e3e0 <the_thread_db_target>, ptid=...) at ../../fixleaks/gdb/linux-thread-db.c:1637
  #11 0x0000555c24c2f420 in target_pid_to_str[abi:cxx11](ptid_t) (ptid=...) at ../../fixleaks/gdb/target.c:2083
  #12 0x0000555c24ad9cab in stop_all_threads () at ../../fixleaks/gdb/infrun.c:4373
  #13 0x0000555c24ada00f in stop_waiting (ecs=<optimized out>) at ../../fixleaks/gdb/infrun.c:7464
  #14 0x0000555c24adc401 in process_event_stop_test (ecs=ecs@entry=0x7ffc9402d9d0) at ../../fixleaks/gdb/infrun.c:6181
  ...
  (top-gdb) fr 12
  #12 0x0000555c24ad9cab in stop_all_threads () at ../../fixleaks/gdb/infrun.c:4373
  (top-gdb) p event_ptid
  $5 = {m_pid = 25419, m_lwp = 25427, m_tid = 0}
  (top-gdb) p ptid
  $6 = {m_pid = 0, m_lwp = 0, m_tid = 0}
  (top-gdb) p ws
  $7 = {kind = TARGET_WAITKIND_THREAD_EXITED, value = {integer = 0, sig = GDB_SIGNAL_0, related_pid = {m_pid = 0,
        m_lwp = 0, m_tid = 0}, execd_pathname = 0x0, syscall_number = 0}}
  (top-gdb)

The gdb.log corresponding to the above crash is:
  (gdb) PASS: gdb.threads/watchthreads-reorder.exp: reorder1: set debug infrun 1
  continue
  Continuing.
  infrun: clear_proceed_status_thread (Thread 0x7ffff7fcfb40 (LWP 25419))
  infrun: clear_proceed_status_thread (Thread 0x7ffff7310700 (LWP 25427))
  infrun: clear_proceed_status_thread (Thread 0x7ffff6b0f700 (LWP 25428))
  infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
  infrun: proceed: resuming Thread 0x7ffff7fcfb40 (LWP 25419)
  infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7fcfb40 (LWP 25419)] at 0x7ffff7344317
  infrun: infrun_async(1)
  infrun: prepare_to_wait
  infrun: proceed: resuming Thread 0x7ffff7310700 (LWP 25427)
  infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7310700 (LWP 25427)] at 0x5555555553d7
  infrun: prepare_to_wait
  infrun: proceed: resuming Thread 0x7ffff6b0f700 (LWP 25428)
  infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff6b0f700 (LWP 25428)] at 0x5555555554c8
  infrun: prepare_to_wait
  infrun: target_wait (-1.0.0, status) =
  infrun:   -1.0.0 [process -1],
  infrun:   status->kind = ignore
  infrun: TARGET_WAITKIND_IGNORE
  infrun: prepare_to_wait
  Joining the threads.
  [Thread 0x7ffff6b0f700 (LWP 25428) exited]
  infrun: target_wait (-1.0.0, status) =
  infrun:   -1.0.0 [process -1],
  infrun:   status->kind = ignore
  infrun: TARGET_WAITKIND_IGNORE
  infrun: prepare_to_wait
  infrun: target_wait (-1.0.0, status) =
  infrun:   25419.25419.0 [Thread 0x7ffff7fcfb40 (LWP 25419)],
  infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
  infrun: TARGET_WAITKIND_STOPPED
  infrun: stop_pc = 0x555555555e50
  infrun: context switch
  infrun: Switching context from Thread 0x7ffff6b0f700 (LWP 25428) to Thread 0x7ffff7fcfb40 (LWP 25419)
  infrun: BPSTAT_WHAT_STOP_NOISY
  infrun: stop_waiting
  infrun: stop_all_threads
  infrun: stop_all_threads, pass=0, iterations=0
  infrun:   Thread 0x7ffff7fcfb40 (LWP 25419) not executing
  infrun:   Thread 0x7ffff7310700 (LWP 25427) executing, need stop
  [Thread 0x7ffff7310700 (LWP 25427) exited]
  infrun: target_wait (-1.0.0, status) =
  infrun:   25419.25427.0 [LWP 25427],
  infrun:   status->kind = thread exited, status = 0
  infrun: infrun_async(0)
  ../../fixleaks/gdb/inferior.c:287: internal-error: inferior* find_inferior_pid(int): Assertion `pid != 0' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) FAIL: gdb.threads/watchthreads-reorder.exp: reorder1: continue to breakpoint: break-at-exit (GDB internal error)
  Resyncing due to internal error.
  n
  infrun: infrun_async(1)

  This is a bug, please report it.  For instructions, see:
  <http://www.gnu.org/software/gdb/bugs/>.

  infrun: infrun_async(0)
  ../../fixleaks/gdb/inferior.c:287: internal-error: inferior* find_inferior_pid(int): Assertion `pid != 0' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Create a core file of GDB? (y or n) y
antmak referenced this pull request in espressif/binutils-gdb Apr 11, 2019
One test in gdb.compile/compile.exp passes on one fedora builder,

 bt
 #0  0x00007ffff7ff43f6 in _gdb_expr (__regs=0x7ffff7ff2000) at gdb
 command line:1^M
 #1  <function called from gdb>^M
 #2  main () at /home/gdb-buildbot/fedora-x86-64-1/fedora-x86-64/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.compile/compile.c:106^M
 (gdb) PASS: gdb.compile/compile.exp: bt

but fails on my machine with gcc trunk,

 bt^M
 #0  _gdb_expr (__regs=0x7ffff7ff3000) at gdb command line:1^M
 #1  <function called from gdb>^M
 #2  main () at gdb/testsuite/gdb.compile/compile.c:106^M
 (gdb) FAIL: gdb.compile/compile.exp: bt

The test should be tweaked to match both cases (pc in the start of line
vs pc in the middle of line).  Note that I am not clear that why libcc1
emits debug info this way so that the address is in the middle of line.

gdb/testsuite:

2018-01-17  Yao Qi  <yao.qi@linaro.org>

	* gdb.compile/compile.exp: Match the address printed for
	frame in the output of command "bt".
antmak referenced this pull request in espressif/binutils-gdb Apr 11, 2019
I got some crashes while doing some work with dwarf2_per_objfile.  It
turns out that dwarf2_per_objfile_free is using the dwarf2_per_objfile
objects after their destructor has ran.

The easiest way to reproduce this is to run the inferior twice (do
"start" twice).  Currently, it goes unnoticed, but when I tried to
change all_comp_units and all_type_units to std::vectors, things started
crashing.

The dwarf2_per_objfile objects get destroyed here:

 #0  dwarf2_per_objfile::~dwarf2_per_objfile (this=0x35afe70, __in_chrg=<optimized out>) at /home/emaisin/src/binutils-gdb/gdb/dwarf2read.c:2422
 #1  0x0000000000833282 in dwarf2_free_objfile (objfile=0x356cff0) at /home/emaisin/src/binutils-gdb/gdb/dwarf2read.c:25363
 #2  0x0000000000699255 in elf_symfile_finish (objfile=0x356cff0) at /home/emaisin/src/binutils-gdb/gdb/elfread.c:1309
 #3  0x0000000000911ed3 in objfile::~objfile (this=0x356cff0, __in_chrg=<optimized out>) at /home/emaisin/src/binutils-gdb/gdb/objfiles.c:674

and just after that the dwarf2read per-objfile registry cleanup function
gets called:

 #0  dwarf2_per_objfile_free (objfile=0x356cff0, d=0x35afe70) at /home/emaisin/src/binutils-gdb/gdb/dwarf2read.c:25667
 ... registry boilerplate ...
 bminor#4  0x00000000009103ea in objfile_free_data (container=0x356cff0) at /home/emaisin/src/binutils-gdb/gdb/objfiles.c:61
 bminor#5  0x0000000000911ee2 in objfile::~objfile (this=0x356cff0, __in_chrg=<optimized out>) at /home/emaisin/src/binutils-gdb/gdb/objfiles.c:678

In dwarf2_per_objfile_free, we access fields of the dwarf2_per_objfile
object, which is invalid since its destructor has been executed.

This patch moves the content of dwarf2_per_objfile_free to the
destructor of dwarf2_per_objfile.  The call to
register_objfile_data_with_cleanup in _initialize_dwarf2_read can be
changed to the simpler register_objfile_data.

gdb/ChangeLog:

	* dwarf2read.c (free_dwo_files): Add forward-declaration.
	(dwarf2_per_objfile::~dwarf2_per_objfile): Move content from
	dwarf2_per_objfile_free here.
	(dwarf2_per_objfile_free): Remove.
	(_initialize_dwarf2_read): Don't register
	dwarf2_per_objfile_free as a registry cleanup.
antmak referenced this pull request in espressif/binutils-gdb Apr 11, 2019
When running the test gdb.dwarf2/dw2-bad-parameter-type.exp under
valgrind, I see the following issue reported (on x86-64 Fedora):

  (gdb) ptype f
  ==5203== Invalid read of size 1
  ==5203==    at 0x6931FE: process_die_scope::~process_die_scope() (dwarf2read.c:10642)
  ==5203==    by 0x66818F: process_die(die_info*, dwarf2_cu*) (dwarf2read.c:10664)
  ==5203==    by 0x66A01F: read_file_scope(die_info*, dwarf2_cu*) (dwarf2read.c:11650)
  ==5203==    by 0x667F2D: process_die(die_info*, dwarf2_cu*) (dwarf2read.c:10672)
  ==5203==    by 0x6677B6: process_full_comp_unit(dwarf2_per_cu_data*, language) (dwarf2read.c:10445)
  ==5203==    by 0x66657A: process_queue(dwarf2_per_objfile*) (dwarf2read.c:9945)
  ==5203==    by 0x6559B4: dw2_do_instantiate_symtab(dwarf2_per_cu_data*) (dwarf2read.c:3163)
  ==5203==    by 0x66683D: psymtab_to_symtab_1(partial_symtab*) (dwarf2read.c:10034)
  ==5203==    by 0x66622A: dwarf2_read_symtab(partial_symtab*, objfile*) (dwarf2read.c:9811)
  ==5203==    by 0x787984: psymtab_to_symtab(objfile*, partial_symtab*) (psymtab.c:792)
  ==5203==    by 0x786E3E: psym_lookup_symbol(objfile*, int, char const*, domain_enum_tag) (psymtab.c:522)
  ==5203==    by 0x804BD0: lookup_symbol_via_quick_fns(objfile*, int, char const*, domain_enum_tag) (symtab.c:2383)
  ==5203==  Address 0x147ed063 is 291 bytes inside a block of size 4,064 free'd
  ==5203==    at 0x4C2CD5A: free (vg_replace_malloc.c:530)
  ==5203==    by 0x444415: void xfree<void>(void*) (common-utils.h:60)
  ==5203==    by 0x9DA8C2: call_freefun (obstack.c:103)
  ==5203==    by 0x9DAD35: _obstack_free (obstack.c:280)
  ==5203==    by 0x44464C: auto_obstack::~auto_obstack() (gdb_obstack.h:73)
  ==5203==    by 0x68AFB0: dwarf2_cu::~dwarf2_cu() (dwarf2read.c:25080)
  ==5203==    by 0x68B204: free_one_cached_comp_unit(dwarf2_per_cu_data*) (dwarf2read.c:25174)
  ==5203==    by 0x66668C: dwarf2_release_queue(void*) (dwarf2read.c:9982)
  ==5203==    by 0x563A4C: do_my_cleanups(cleanup**, cleanup*) (cleanups.c:154)
  ==5203==    by 0x563AA7: do_cleanups(cleanup*) (cleanups.c:176)
  ==5203==    by 0x5646CE: throw_exception_cxx(gdb_exception) (common-exceptions.c:289)
  ==5203==    by 0x5647B7: throw_exception(gdb_exception) (common-exceptions.c:317)
  ==5203==  Block was alloc'd at
  ==5203==    at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
  ==5203==    by 0x564BE8: xmalloc (common-utils.c:44)
  ==5203==    by 0x9DA872: call_chunkfun (obstack.c:94)
  ==5203==    by 0x9DA935: _obstack_begin_worker (obstack.c:141)
  ==5203==    by 0x9DAA3C: _obstack_begin (obstack.c:164)
  ==5203==    by 0x4445E0: auto_obstack::auto_obstack() (gdb_obstack.h:70)
  ==5203==    by 0x68AE07: dwarf2_cu::dwarf2_cu(dwarf2_per_cu_data*) (dwarf2read.c:25073)
  ==5203==    by 0x661A8A: init_cutu_and_read_dies(dwarf2_per_cu_data*, abbrev_table*, int, int, void (*)(die_reader_specs const*, unsigned char const*, die_info*, int, void*), void*) (dwarf2read.c:7869)
  ==5203==    by 0x666A29: load_full_comp_unit(dwarf2_per_cu_data*, language) (dwarf2read.c:10108)
  ==5203==    by 0x655847: load_cu(dwarf2_per_cu_data*) (dwarf2read.c:3120)
  ==5203==    by 0x655928: dw2_do_instantiate_symtab(dwarf2_per_cu_data*) (dwarf2read.c:3148)
  ==5203==    by 0x66683D: psymtab_to_symtab_1(partial_symtab*) (dwarf2read.c:10034)

There's actually a series of three issues reported, but it turns out
they're all related, so we can consider on the first one.

The invalid read is triggered from a destructor which is being invoked
as part of a stack unwind after throwing an error.  At the time the
error is thrown, the stack looks like this:

    #0  0x00000000009f4ecd in __cxa_throw ()
    #1  0x0000000000564761 in throw_exception_cxx (exception=...) at ../../src/gdb/common/common-exceptions.c:303
    #2  0x00000000005647b8 in throw_exception (exception=...) at ../../src/gdb/common/common-exceptions.c:317
    #3  0x00000000005648ff in throw_it(return_reason, errors, const char *, typedef __va_list_tag __va_list_tag *) (reason=RETURN_ERROR,
        error=GENERIC_ERROR, fmt=0xb33020 "Dwarf Error: Cannot find DIE at 0x%x referenced from DIE at 0x%x [in module %s]",
        ap=0x7fff387f2d68) at ../../src/gdb/common/common-exceptions.c:373
    bminor#4  0x0000000000564929 in throw_verror (error=GENERIC_ERROR,
        fmt=0xb33020 "Dwarf Error: Cannot find DIE at 0x%x referenced from DIE at 0x%x [in module %s]", ap=0x7fff387f2d68)
        at ../../src/gdb/common/common-exceptions.c:379
    bminor#5  0x0000000000867be4 in verror (string=0xb33020 "Dwarf Error: Cannot find DIE at 0x%x referenced from DIE at 0x%x [in module %s]",
        args=0x7fff387f2d68) at ../../src/gdb/utils.c:251
    bminor#6  0x000000000056879d in error (fmt=0xb33020 "Dwarf Error: Cannot find DIE at 0x%x referenced from DIE at 0x%x [in module %s]")
        at ../../src/gdb/common/errors.c:43
    bminor#7  0x0000000000686875 in follow_die_ref (src_die=0x30bc8a0, attr=0x30bc8c8, ref_cu=0x7fff387f2ed0) at ../../src/gdb/dwarf2read.c:22969
    bminor#8  0x00000000006844cd in lookup_die_type (die=0x30bc8a0, attr=0x30bc8c8, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:21976
    bminor#9  0x0000000000683f27 in die_type (die=0x30bc8a0, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:21832
    bminor#10 0x0000000000679b39 in read_subroutine_type (die=0x30bc830, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:17343
    bminor#11 0x00000000006845fb in read_type_die_1 (die=0x30bc830, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:22035
    bminor#12 0x0000000000684576 in read_type_die (die=0x30bc830, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:22010
    bminor#13 0x000000000067003f in read_func_scope (die=0x30bc830, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:13822
    bminor#14 0x0000000000667f5e in process_die (die=0x30bc830, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:10679
    bminor#15 0x000000000066a020 in read_file_scope (die=0x30bc720, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:11650
    bminor#16 0x0000000000667f2e in process_die (die=0x30bc720, cu=0x30bc5d0) at ../../src/gdb/dwarf2read.c:10672
    #17 0x00000000006677b7 in process_full_comp_unit (per_cu=0x3089b80, pretend_language=language_minimal)
        at ../../src/gdb/dwarf2read.c:10445
    #18 0x000000000066657b in process_queue (dwarf2_per_objfile=0x30897d0) at ../../src/gdb/dwarf2read.c:9945
    #19 0x00000000006559b5 in dw2_do_instantiate_symtab (per_cu=0x3089b80) at ../../src/gdb/dwarf2read.c:3163
    #20 0x000000000066683e in psymtab_to_symtab_1 (pst=0x3089bd0) at ../../src/gdb/dwarf2read.c:10034
    #21 0x000000000066622b in dwarf2_read_symtab (self=0x3089bd0, objfile=0x3073f40) at ../../src/gdb/dwarf2read.c:9811
    #22 0x0000000000787985 in psymtab_to_symtab (objfile=0x3073f40, pst=0x3089bd0) at ../../src/gdb/psymtab.c:792
    #23 0x0000000000786e3f in psym_lookup_symbol (objfile=0x3073f40, block_index=1, name=0x30b2e30 "f", domain=VAR_DOMAIN)
        at ../../src/gdb/psymtab.c:522
    #24 0x0000000000804bd1 in lookup_symbol_via_quick_fns (objfile=0x3073f40, block_index=1, name=0x30b2e30 "f", domain=VAR_DOMAIN)
        at ../../src/gdb/symtab.c:2383
    #25 0x0000000000804fe4 in lookup_symbol_in_objfile (objfile=0x3073f40, block_index=1, name=0x30b2e30 "f", domain=VAR_DOMAIN)
        at ../../src/gdb/symtab.c:2558
    #26 0x0000000000805125 in lookup_static_symbol (name=0x30b2e30 "f", domain=VAR_DOMAIN) at ../../src/gdb/symtab.c:2595
    #27 0x0000000000804357 in lookup_symbol_aux (name=0x30b2e30 "f", match_type=symbol_name_match_type::FULL, block=0x0,
        domain=VAR_DOMAIN, language=language_c, is_a_field_of_this=0x0) at ../../src/gdb/symtab.c:2105
    #28 0x0000000000803ad9 in lookup_symbol_in_language (name=0x30b2e30 "f", block=0x0, domain=VAR_DOMAIN, lang=language_c,
        is_a_field_of_this=0x0) at ../../src/gdb/symtab.c:1887
    #29 0x0000000000803b53 in lookup_symbol (name=0x30b2e30 "f", block=0x0, domain=VAR_DOMAIN, is_a_field_of_this=0x0)
        at ../../src/gdb/symtab.c:1899
    #30 0x000000000053b246 in classify_name (par_state=0x7fff387f6090, block=0x0, is_quoted_name=false, is_after_structop=false)
        at ../../src/gdb/c-exp.y:2879
    #31 0x000000000053b7e9 in c_yylex () at ../../src/gdb/c-exp.y:3083
    #32 0x000000000053414a in c_yyparse () at c-exp.c:1903
    #33 0x000000000053c2e7 in c_parse (par_state=0x7fff387f6090) at ../../src/gdb/c-exp.y:3255
    #34 0x0000000000774a02 in parse_exp_in_context_1 (stringptr=0x7fff387f61c0, pc=0, block=0x0, comma=0, void_context_p=0, out_subexp=0x0)
        at ../../src/gdb/parse.c:1213
    #35 0x000000000077476a in parse_exp_in_context (stringptr=0x7fff387f61c0, pc=0, block=0x0, comma=0, void_context_p=0, out_subexp=0x0)
        at ../../src/gdb/parse.c:1115
    #36 0x0000000000774714 in parse_exp_1 (stringptr=0x7fff387f61c0, pc=0, block=0x0, comma=0) at ../../src/gdb/parse.c:1106
    #37 0x0000000000774c53 in parse_expression (string=0x27ff996 "f") at ../../src/gdb/parse.c:1253
    #38 0x0000000000861dc4 in whatis_exp (exp=0x27ff996 "f", show=1) at ../../src/gdb/typeprint.c:472
    #39 0x00000000008620d8 in ptype_command (type_name=0x27ff996 "f", from_tty=1) at ../../src/gdb/typeprint.c:561
    #40 0x000000000047430b in do_const_cfunc (c=0x3012010, args=0x27ff996 "f", from_tty=1) at ../../src/gdb/cli/cli-decode.c:106
    #41 0x000000000047715e in cmd_func (cmd=0x3012010, args=0x27ff996 "f", from_tty=1) at ../../src/gdb/cli/cli-decode.c:1886
    #42 0x00000000008431bb in execute_command (p=0x27ff996 "f", from_tty=1) at ../../src/gdb/top.c:630
    #43 0x00000000006bf946 in command_handler (command=0x27ff990 "ptype f") at ../../src/gdb/event-top.c:583
    #44 0x00000000006bfd12 in command_line_handler (rl=0x30bb3a0 "\240\305\v\003") at ../../src/gdb/event-top.c:774

The problem is that in `process_die` (frames 14 and 16) we create a
`process_die_scope` object, that takes a copy of the `struct
dwarf2_cu *` passed into the frame.  The destructor of the
`process_die_scope` dereferences the stored pointer.  This wouldn't be
an issue, except...

... in dw2_do_instantiate_symtab (frame 19) a clean up was registered that
clears the dwarf2_queue in case of an error.  Part of this clean up
involves deleting the `struct dwarf2_cu`s referenced from the queue..

The problem then, is that cleanups are processed at the site of the
throw, while, class destructors are invoked as we unwind their frame.
The result is that we process the frame 19 cleanup (and delete the
struct dwarf2_cu) before we process the destructors in frames 14 and 16.
When we do get back to frames 14 and 16 the objects being references
have already been deleted.

The solution is to remove the cleanup from dw2_do_instantiate_symtab, and
instead use a destructor to release the dwarf2_queue instead.  With this
patch in place, the valgrind errors are now resolved.

gdb/ChangeLog:

	* dwarf2read.c (dwarf2_release_queue): Delete function, move body
	into...
	(class dwarf2_queue_guard): ...the destructor of this new class.
	(dw2_do_instantiate_symtab): Create instance of the new class
	dwarf2_queue_guard, remove cleanup.
antmak referenced this pull request in espressif/binutils-gdb Apr 11, 2019
In https://sourceware.org/ml/gdb-patches/2017-06/msg00741.html,
Pedro asks:

> Doesn't the "info verbose on" bit affect frame filters too?

The answer is that yes, it could.  However, it's not completely
effective, because the C code can't guess how many frames might need
to be unwound to satisfy the request -- a frame filter will request as
many frames as it needs.

Also, I tried removing this code from backtrace, and I think the
result is better without it.  In particular, now the expansion line
occurs just before the frame that caused the expansion, like:

    (gdb) bt no-filters
    #0  0x00007ffff576cecd in poll () from /lib64/libc.so.6
    Reading in symbols for ../../binutils-gdb/gdb/event-loop.c...done.
    #1  0x00000000007ecc33 in gdb_wait_for_event (block=1)
	at ../../binutils-gdb/gdb/event-loop.c:772
    #2  0x00000000007ec006 in gdb_do_one_event ()
	at ../../binutils-gdb/gdb/event-loop.c:347
    #3  0x00000000007ec03e in start_event_loop ()
	at ../../binutils-gdb/gdb/event-loop.c:371
    Reading in symbols for ../../binutils-gdb/gdb/main.c...done.
    bminor#4  0x000000000086693d in captured_command_loop (
	Reading in symbols for ../../binutils-gdb/gdb/exceptions.c...done.
    data=0x0) at ../../binutils-gdb/gdb/main.c:325

So, I am proposing this patch to simply remove this code.

gdb/ChangeLog
2018-03-26  Tom Tromey  <tom@tromey.com>

	* stack.c (backtrace_command_1): Remove verbose code.
antmak referenced this pull request in espressif/binutils-gdb Apr 11, 2019
A future patch will propose making the remote target's target_ops be
heap-allocated (to make it possible to have multiple instances of
remote targets, for multiple simultaneous connections), and will
delete/destroy the remote target at target_close time.

That change trips on a latent problem, though.  File I/O handles
remain open even after the target is gone, with a dangling pointer to
a target that no longer exists.  This results in GDB crashing when it
calls the target_ops backend associated with the file handle:

 (gdb) Disconnect
 Ending remote debugging.
 * GDB crashes deferencing a dangling pointer

Backtrace:

   #0  0x00007f79338570a0 in main_arena () at /lib64/libc.so.6
   #1  0x0000000000858bfe in target_fileio_close(int, int*) (fd=1, target_errno=0x7ffe0499a4c8)
       at src/gdb/target.c:2980
   #2  0x00000000007088bd in gdb_bfd_iovec_fileio_close(bfd*, void*) (abfd=0x1a631b0, stream=0x223c9d0)
       at src/gdb/gdb_bfd.c:353
   #3  0x0000000000930906 in opncls_bclose (abfd=0x1a631b0) at src/bfd/opncls.c:528
   bminor#4  0x0000000000930cf9 in bfd_close_all_done (abfd=0x1a631b0) at src/bfd/opncls.c:768
   bminor#5  0x0000000000930cb3 in bfd_close (abfd=0x1a631b0) at src/bfd/opncls.c:735
   bminor#6  0x0000000000708dc5 in gdb_bfd_close_or_warn(bfd*) (abfd=0x1a631b0) at src/gdb/gdb_bfd.c:511
   bminor#7  0x00000000007091a2 in gdb_bfd_unref(bfd*) (abfd=0x1a631b0) at src/gdb/gdb_bfd.c:615
   bminor#8  0x000000000079ed8e in objfile::~objfile() (this=0x2154730, __in_chrg=<optimized out>)
       at src/gdb/objfiles.c:682
   bminor#9  0x000000000079fd1a in objfile_purge_solibs() () at src/gdb/objfiles.c:1065
   bminor#10 0x00000000008162ca in no_shared_libraries(char const*, int) (ignored=0x0, from_tty=1)
       at src/gdb/solib.c:1251
   bminor#11 0x000000000073b89b in disconnect_command(char const*, int) (args=0x0, from_tty=1)
       at src/gdb/infcmd.c:3035

This goes unnoticed in current master, because the current remote
target's target_ops is never destroyed nowadays, so we end up calling:

  remote_hostio_close -> remote_hostio_send_command

which gracefully fails with FILEIO_ENOSYS if remote_desc is NULL
(because the target is closed).

Fix this by invalidating a target's file I/O handles when the target
is closed.

With this change, remote_hostio_send_command no longer needs to handle the
case of being called with a closed remote target, originally added here:
<https://sourceware.org/ml/gdb-patches/2008-08/msg00359.html>.

gdb/ChangeLog:
2018-04-11  Pedro Alves  <palves@redhat.com>

	* target.c (fileio_fh_t::t): Add comment.
	(target_fileio_pwrite, target_fileio_pread, target_fileio_fstat)
	(target_fileio_close): Handle a NULL target.
	(invalidate_fileio_fh): New.
	(target_close): Call it.
	* remote.c (remote_hostio_send_command): No longer check whether
	remote_desc is open.
antmak referenced this pull request in espressif/binutils-gdb Apr 11, 2019
Since we use obstacks with objects that are not default constructible,
we sometimes need to manually call the constructor by hand using
placement new:

  foo *f = obstack_alloc (obstack, sizeof (foo));
  f = new (f) foo;

It's possible to use allocate_on_obstack instead, but there are types
that we sometimes want to allocate on an obstack, and sometimes on the
regular heap.  This patch introduces a utility to make this pattern
simpler if allocate_on_obstack is not an option:

  foo *f = obstack_new<foo> (obstack);

Right now there's only one usage (in tdesc_data_init).

To help catch places where we would forget to call new when allocating
such an object on an obstack, this patch also poisons some other methods
of allocating an instance of a type on an obstack:

  - OBSTACK_ZALLOC/OBSTACK_CALLOC
  - XOBNEW/XOBNEW
  - GDBARCH_OBSTACK_ZALLOC/GDBARCH_OBSTACK_CALLOC

Unfortunately, there's no way to catch wrong usages of obstack_alloc.

By pulling on that string though, it tripped on allocating struct
template_symbol using OBSTACK_ZALLOC.  The criterion currently used to
know whether it's safe to "malloc" an instance of a struct is whether it
is a POD.  Because it inherits from struct symbol, template_symbol is
not a POD.  This criterion is a bit too strict however, it should still
safe to allocate memory for a template_symbol and memset it to 0.  We
didn't use is_trivially_constructible as the criterion in the first
place only because it is not available in gcc < 5.  So here I considered
two alternatives:

1. Relax that criterion to use std::is_trivially_constructible and add a
   bit more glue code to make it work with gcc < 5
2. Continue pulling on the string and change how the symbol structures
   are allocated and initialized

I managed to do both, but I decided to go with #1 to keep this patch
simpler and more focused.  When building with a compiler that does not
have is_trivially_constructible, the check will just not be enforced.

gdb/ChangeLog:

	* common/traits.h (HAVE_IS_TRIVIALLY_COPYABLE): Define if
	compiler supports std::is_trivially_constructible.
	* common/poison.h: Include obstack.h.
	(IsMallocable): Define to is_trivially_constructible if the
	compiler supports it, define to true_type otherwise.
	(xobnew): New.
	(XOBNEW): Redefine.
	(xobnewvec): New.
	(XOBNEWVEC): Redefine.
	* gdb_obstack.h (obstack_zalloc): New.
	(OBSTACK_ZALLOC): Redefine.
	(obstack_calloc): New.
	(OBSTACK_CALLOC): Redefine.
	(obstack_new): New.
	* gdbarch.sh: Include gdb_obstack in gdbarch.h.
	(gdbarch_obstack): New declaration in gdbarch.h, definition in
	gdbarch.c.
	(GDBARCH_OBSTACK_CALLOC, GDBARCH_OBSTACK_ZALLOC): Use
	obstack_calloc/obstack_zalloc.
	(gdbarch_obstack_zalloc): Remove.
	* target-descriptions.c (tdesc_data_init): Use obstack_new.
antmak referenced this pull request in espressif/binutils-gdb Apr 11, 2019
Currently, gdb.gdb/selftest.exp fails if you build GDB with
optimization (-O2, etc.).

The reason is that after setting a breakpoint in captured_main, we
stop at:
 ...
 Breakpoint 1, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
 ...
while selftest_setup expects a stop at captured_main.

Here, captured_main_1 has been inlined into captured_main, and
captured_main has been inlined into gdb_main:

 ...
 $ nm ./build/gdb/gdb | egrep ' [tT] .*captured_main|gdb_main' | c++filt
 000000000061b950 T gdb_main(captured_main_args*)
 ...

Indeed, the two inlined functions show up in the backtrace:

 ...
 (gdb) bt
 #0  captured_main_1 (context=<optimized out>) at main.c:492
 #1  captured_main (data=<optimized out>) at main.c:1147
 #2  gdb_main (args=args@entry=0x7fffffffdb80) at main.c:1173
 #3  0x000000000040fea5 in main (argc=<optimized out>, argv=<optimized out>)
     at gdb.c:32
 ...

We're now stopping at captured_main_1 because commit ddfe970
("Don't elide all inlined frames") makes GDB present a stop at the
innermost inlined frame if the program stopped by a user breakpoint.

Now, the selftest.exp testcase explicitly asks to stop at
"captured_main", not "captured_main_1", so I'm thinking that it's
GDB'S behavior that should be improved.  That is what this commit
does, by only showing a stop at an inline frame if the user breakpoint
was set in that frame's block.

Before this commit:

 (top-gdb) b captured_main
 Breakpoint 1 at 0x792f99: file src/gdb/main.c, line 492.
 (top-gdb) r
 Starting program: build/gdb/gdb

 Breakpoint 1, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
 492       lim_at_start = (char *) sbrk (0);
 (top-gdb)

After this commit, we now instead get:

 (top-gdb) b captured_main
 Breakpoint 1 at 0x791339: file src/gdb/main.c, line 492.
 (top-gdb) r
 Starting program: build/gdb/gdb

 Breakpoint 1, captured_main (data=<optimized out>) at src/gdb/main.c:1147
 1147      captured_main_1 (context);
 (top-gdb)

and:

 (top-gdb) b captured_main_1
 Breakpoint 2 at 0x791339: file src/gdb/main.c, line 492.
 (top-gdb) r
 Starting program: build/gdb/gdb
 Breakpoint 2, captured_main_1 (context=<optimized out>) at src/gdb/main.c:492
 492       lim_at_start = (char *) sbrk (0);
 (top-gdb)

Note that both captured_main and captured_main_1 resolved to the same
address, 0x791339.  That is necessary to trigger the issue in
question.  The gdb.base/inline-break.exp testcase currently does not
exercise that, but the new test added by this commit does.  That new
test fails without the GDB fix and passes with the fix.  No
regressions on x86-64 GNU/Linux.

While at it, the THIS_PC comparison in stopped_by_user_bp_inline_frame
is basically a nop, so just remove it -- if a software or hardware
breakpoint explains the stop, then it must be that it was installed at
the current PC.

gdb/ChangeLog:
2018-06-19  Pedro Alves  <palves@redhat.com>

	* inline-frame.c (stopped_by_user_bp_inline_frame): Replace PC
	parameter with a block parameter.  Compare location's block symbol
	with the frame's block instead of addresses.
	(skip_inline_frames): Pass the current block instead of the
	frame's address.  Break out as soon as we determine the frame
	should not be skipped.

gdb/testsuite/ChangeLog:
2018-06-19  Pedro Alves  <palves@redhat.com>

	* gdb.opt/inline-break.c (func_inline_callee, func_inline_caller)
	(func_extern_caller): New.
	(main): Call func_extern_caller.
	* gdb.opt/inline-break.exp: Add tests for inline frame skipping
	logic change.
berenm pushed a commit that referenced this pull request Apr 19, 2019
When debugging any of the testcases added by this commit, which do a
vfork in a thread with "set follow-fork-mode child" + "set
detach-on-fork on", we run into this assertion:

...
src/gdb/nat/x86-linux-dregs.c:146: internal-error: \
  void x86_linux_update_debug_registers(lwp_info*): \
  Assertion `lwp_is_stopped (lwp)' failed.
...

The assert is caused by the following: the vfork-child exit or exec
event is handled by handle_vfork_child_exec_or_exit, which calls
target_detach to detach from the vfork parent.  During target_detach
we call linux_nat_target::detach, which:

#1 - stops all the threads
#2 - waits for all the threads to be stopped
#3 - detaches all the threads

However, during the second step we run into this code in
stop_wait_callback:

...
  /* If this is a vfork parent, bail out, it is not going to report
     any SIGSTOP until the vfork is done with.  */
  if (inf->vfork_child != NULL)
    return 0;
...

and we don't wait for the threads to be stopped, which results in this
assert in x86_linux_update_debug_registers triggering during the third
step:

...
  gdb_assert (lwp_is_stopped (lwp));
...

The fix is to reset the vfork parent's vfork_child field before
calling target_detach in handle_vfork_child_exec_or_exit.  There's
already similar code for the other paths handled by
handle_vfork_child_exec_or_exit, so this commit refactors the code a
bit so that all paths share the same code.

The new tests cover both a vfork child exiting, and a vfork child
execing, since both cases would trigger the assertion.

The new testcases also exercise following the vfork children with "set
detach-on-fork off", since it doesn't seem to be tested anywhere.

Tested on x86_64-linux, using native and native-gdbserver.

gdb/ChangeLog:
2019-04-18  Tom de Vries  <tdevries@suse.de>
	    Pedro Alves  <palves@redhat.com>

	PR gdb/24454
	* infrun.c (handle_vfork_child_exec_or_exit): Reset vfork parent's
	vfork_child field before calling target_detach.

gdb/testsuite/ChangeLog:
2019-04-18  Tom de Vries  <tdevries@suse.de>
	    Pedro Alves  <palves@redhat.com>

	PR gdb/24454
	* gdb.threads/vfork-follow-child-exec.c: New file.
	* gdb.threads/vfork-follow-child-exec.exp: New file.
	* gdb.threads/vfork-follow-child-exit.c: New file.
	* gdb.threads/vfork-follow-child-exit.exp: New file.
berenm pushed a commit that referenced this pull request May 9, 2019
- set print frame-arguements all
- an Ada function named pck.call_me calls a C function named break_me
- you put a breakpoint in break_me and the program reaches this
  breakpoint.

Now display the backtrace:

  (gdb) bt
  #0  break_me () at [...]
  #1  0x000000000040243e in pck.call_me (
      s={P_ARRAY = 0x7fffffffe21c, P_BOUNDS = 0x41e6e8}) at [...]

whereas we should expect:

  (gdb) bt
  #0  break_me () at [...]
  #1  0x000000000040243e in pck.call_me (s="test") at [...]

The problem is that GDB prints the S parameter in the pck.call_me Ada
function using the current language, so the C one, because the program
is stopped in a C function, whereas it should use the pck.call_me frame
one. This behavior is ok when user manually changes the language but it's
not the right one when language is auto.

This patch fixes this problem so now when using auto language, all Ada
frame arguments are printed using Ada like syntax when the frame is part
of Ada code, even if the program is stopped in a frame using a different
language.

If the user explicitly sets a language (using "set language ...") then
no change here, all the Ada frame arguments are printed using this
language.

gdb/ChangeLog:

    * ada-valprint.c (ada_val_print_gnat_array): Remove language
    parameter and use Ada language definition instead.
    (ada_val_print_ptr): Remove unused language parameter.
    (ada_val_print_num): Remove language parameter and use Ada language
    definition instead.
    (ada_val_print_enum, ada_val_print_flt): Remove unused language
    parameter.
    (ada_val_print_struct_union, ada_val_print_ref): Remove language
    parameter and use Ada language definition instead.
    (ada_val_print_1): Update all ada_val_print_xxx calls.
    Remove language parameter.
    (ada_val_print): Update ada_val_print_1 call.

gdb/testsuite/ChangeLog:

    * gdb.ada/frame_arg_lang.exp: New testcase.
    * gdb.ada/frame_arg_lang/bla.adb: New file.
    * gdb.ada/frame_arg_lang/pck.ads: New file.
    * gdb.ada/frame_arg_lang/pck.adb: New file.
    * gdb.ada/frame_arg_lang/foo.c: New file.

Tested on x86_64-linux, no regressions.
berenm pushed a commit that referenced this pull request May 18, 2019
When running gdb using AddressSanitizer, and loading a cc1plus binary built
with profiledbootstrap and -flto, we run into a heap-use-after-free error:
...
$ LD_PRELOAD=/usr/lib64/libasan.so.3 ./gdb -batch cc1plus
==26855==ERROR: AddressSanitizer: heap-use-after-free on address \
  0x62100ad8a8b0 at pc 0x7f13803cc9e3 bp 0x7ffe55b0d090 sp 0x7ffe55b0c840
READ of size 47 at 0x62100ad8a8b0 thread T0
    #0 0x7f13803cc9e2  (/usr/lib64/libasan.so.3+0x3e9e2)
    #1 0x5e7a0d in typename_concat gdb/dwarf2read.c:22661
    #2 0x5c6437 in partial_die_full_name gdb/dwarf2read.c:8876
    #3 0x5c6555 in add_partial_symbol gdb/dwarf2read.c:8893
    #4 0x5c6ecf in add_partial_subprogram gdb/dwarf2read.c:9156
    #5 0x5c5e90 in scan_partial_symbols gdb/dwarf2read.c:8668
    #6 0x5c6c0a in add_partial_namespace gdb/dwarf2read.c:9081
    #7 0x5c5f99 in scan_partial_symbols gdb/dwarf2read.c:8702
    #8 0x5c48b6 in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8056
    #9 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
    #10 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
    #11 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
    #12 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
    #13 0x612359 in read_psyms gdb/elfread.c:1311
    #14 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
    #15 0x867d7b in read_symbols gdb/symfile.c:821
    #16 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
    #17 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
    #18 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #19 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
                    enum_flags<symfile_add_flag>, std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
    #20 0x868b64 in symbol_file_add(char const*, \
                    enum_flags<symfile_add_flag>, \
		    std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>) gdb/symfile.c:1217
    #21 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
    #22 0x868bd0 in symbol_file_add_main(char const*, \
                    enum_flags<symfile_add_flag>) gdb/symfile.c:1231
    #23 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
    #24 0x71f10e in catch_command_errors gdb/main.c:372
    #25 0x71ff5f in captured_main_1 gdb/main.c:1043
    #26 0x72045d in captured_main gdb/main.c:1163
    #27 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
    #28 0x40fd7d in main gdb/gdb.c:32
    #29 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
    #30 0x40fc89 in _start (/data/gdb_versions/devel/build/gdb/gdb+0x40fc89)

0x62100ad8a8b0 is located 944 bytes inside of 4064-byte region \
  [0x62100ad8a500,0x62100ad8b4e0)
freed by thread T0 here:
    #0 0x7f13804523a0 in __interceptor_free (/usr/lib64/libasan.so.3+0xc43a0)
    #1 0x435e44 in xfree<void> gdb/common/common-utils.h:60
    #2 0xa82c25 in call_freefun libiberty/obstack.c:103
    #3 0xa83098 in _obstack_free libiberty/obstack.c:280
    #4 0x4367da in auto_obstack::~auto_obstack() gdb/gdb_obstack.h:101
    #5 0x5ed72c in dwarf2_cu::~dwarf2_cu() gdb/dwarf2read.c:25341
    #6 0x5fb5bb in std::default_delete<dwarf2_cu>::operator()(dwarf2_cu*) const \
                   /usr/include/c++/7/bits/unique_ptr.h:78
    #7 0x5f7334 in std::unique_ptr<dwarf2_cu, \
                   std::default_delete<dwarf2_cu> >::~unique_ptr() \
		   /usr/include/c++/7/bits/unique_ptr.h:268
    #8 0x5c3ce5 in init_cutu_and_read_dies gdb/dwarf2read.c:7624
    #9 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
    #10 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
    #11 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
    #12 0x612359 in read_psyms gdb/elfread.c:1311
    #13 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
    #14 0x867d7b in read_symbols gdb/symfile.c:821
    #15 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
    #16 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
    #17 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #18 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
                    enum_flags<symfile_add_flag>, std::vector<other_sections,  \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
    #19 0x868b64 in symbol_file_add(char const*, \
                    enum_flags<symfile_add_flag>, std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>) gdb/symfile.c:1217
    #20 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
    #21 0x868bd0 in symbol_file_add_main(char const*, \
                    enum_flags<symfile_add_flag>) gdb/symfile.c:1231
    #22 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
    #23 0x71f10e in catch_command_errors gdb/main.c:372
    #24 0x71ff5f in captured_main_1 gdb/main.c:1043
    #25 0x72045d in captured_main gdb/main.c:1163
    #26 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
    #27 0x40fd7d in main gdb/gdb.c:32
    #28 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)

previously allocated by thread T0 here:
    #0 0x7f13804526b8 in __interceptor_malloc (/usr/lib64/libasan.so.3+0xc46b8)
    #1 0x5114b5 in xmalloc gdb/common/common-utils.c:44
    #2 0xa82bd5 in call_chunkfun libiberty/obstack.c:94
    #3 0xa82eda in _obstack_newchunk libiberty/obstack.c:206
    #4 0x477310 in allocate_on_obstack::operator new(unsigned long, obstack*) \
                   gdb/gdb_obstack.h:117
    #5 0x5dea8c in load_partial_dies gdb/dwarf2read.c:18571
    #6 0x5c487f in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8054
    #7 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
    #8 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
    #9 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
    #10 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
    #11 0x612359 in read_psyms gdb/elfread.c:1311
    #12 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
    #13 0x867d7b in read_symbols gdb/symfile.c:821
    #14 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
    #15 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
    #16 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #17 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
                    enum_flags<symfile_add_flag>, \
		    std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
    #18 0x868b64 in symbol_file_add(char const*, enum_flags<symfile_add_flag>, \
                    std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>) gdb/symfile.c:1217
    #19 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
    #20 0x868bd0 in symbol_file_add_main(char const*, \
                    enum_flags<symfile_add_flag>) gdb/symfile.c:1231
    #21 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
    #22 0x71f10e in catch_command_errors gdb/main.c:372
    #23 0x71ff5f in captured_main_1 gdb/main.c:1043
    #24 0x72045d in captured_main gdb/main.c:1163
    #25 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
    #26 0x40fd7d in main gdb/gdb.c:32
    #27 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
...

This error happens as follows.

The function find_partial_die has a cu argument, but returns a pdi which may
or may not be from that cu:
...
/* Find a partial DIE at OFFSET, which may or may not be in CU,
   except in the case of .debug_types DIEs which do not reference
   outside their CU (they do however referencing other types via
   DW_FORM_ref_sig8).  */

static struct partial_die_info *
find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
...

So the pdi returned by find_partial_die here in partial_die_parent_scope may
be from another cu:
...
partial_die_parent_scope (struct partial_die_info *pdi,
                          struct dwarf2_cu *cu)
{
  const char *grandparent_scope;
  struct partial_die_info *parent, *real_pdi;

  /* We need to look at our parent DIE; if we have a DW_AT_specification,
     then this means the parent of the specification DIE.  */

  real_pdi = pdi;
  while (real_pdi->has_specification)
    real_pdi = find_partial_die (real_pdi->spec_offset,
                                 real_pdi->spec_is_dwz, cu);

  parent = real_pdi->die_parent;
...
in which case both real_pdi and parent will be not from cu, but from another
one, say cu2.

Subsequently, cu's comp_unit_obstack is used to set parent->scope:
...
        parent->scope = typename_concat (&cu->comp_unit_obstack,
                                         grandparent_scope,
                                         parent->name, 0, cu);
...

So, we use cu->comp_unit_obstack to assign a value to the scope field of
a pdi belonging to cu2, and when cu is deleted, the scope field points to a
freed value.

Fix this by making find_partial_die return the cu corresponding to the
returned pdi, and handling this at the call sites.

Tested on x86_64-linux.

gdb/ChangeLog:

2019-05-17  Tom de Vries  <tdevries@suse.de>

	PR gdb/24094
	* dwarf2read.c (struct cu_partial_die_info): New struct.
	(find_partial_die): Return cu_partial_die_info.
	(partial_die_parent_scope, guess_partial_die_structure_name)
	(partial_die_info::fixup): Handle new return type of find_partial_die.
berenm pushed a commit that referenced this pull request Jun 11, 2019
When compiling gdb with '-lasan -fsanitizer=address' and running tests with
'export ASAN_OPTIONS="detect_leaks=0:alloc_dealloc_mismatch=0"', I run into:
...
ERROR: GDB process no longer exists
UNRESOLVED: gdb.linespec/cpls-abi-tag.exp: \
  test_abi_tag: completion: at tag: tab complete "b test_abi_tag_function[abi:"
...

In more detail:
...
==3637==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address \
  0x7fff5952bbdd at pc 0x000000fe5c57 bp 0x7fff5952af30 sp 0x7fff5952af28
READ of size 1 at 0x7fff5952bbdd thread T0
    #0 0xfe5c56 in linespec_lexer_lex_string src/gdb/linespec.c:727
    #1 0xfe7473 in linespec_lexer_lex_one src/gdb/linespec.c:946
    #2 0xfe799d in linespec_lexer_consume_token src/gdb/linespec.c:982
    #3 0xff446d in parse_linespec src/gdb/linespec.c:2564
    #4 0xff78be in linespec_complete(completion_tracker&, char const*, \
                   symbol_name_match_type) src/gdb/linespec.c:2961
    #5 0xb9299c in complete_address_and_linespec_locations \
                   src/gdb/completer.c:573
    #6 0xb93e90 in location_completer(cmd_list_element*, completion_tracker&, \
                   char const*, char const*) src/gdb/completer.c:919
    #7 0xb940c5 in location_completer_handle_brkchars src/gdb/completer.c:956
    #8 0xb957ec in complete_line_internal_normal_command \
                   src/gdb/completer.c:1208
    #9 0xb96507 in complete_line_internal_1 src/gdb/completer.c:1430
    #10 0xb965c2 in complete_line_internal src/gdb/completer.c:1449
    #11 0xb98630 in gdb_completion_word_break_characters_throw \
                    src/gdb/completer.c:1862
    #12 0xb98838 in gdb_completion_word_break_characters() \
                    src/gdb/completer.c:1897
    #13 0x16c6362 in _rl_find_completion_word src/readline/complete.c:943
    #14 0x16ca8d0 in rl_complete_internal src/readline/complete.c:1843
    #15 0x16c460c in rl_complete src/readline/complete.c:408
    #16 0x16b3368 in _rl_dispatch_subseq src/readline/readline.c:774
    #17 0x16b3092 in _rl_dispatch src/readline/readline.c:724
    #18 0x16b2939 in readline_internal_char src/readline/readline.c:552
    #19 0x16f1fb0 in rl_callback_read_char src/readline/callback.c:201
    #20 0xddc5a1 in gdb_rl_callback_read_char_wrapper_noexcept \
                    src/gdb/event-top.c:175
    #21 0xddc773 in gdb_rl_callback_read_char_wrapper src/gdb/event-top.c:192
    #22 0xddd9f5 in stdin_event_handler(int, void*) src/gdb/event-top.c:514
    #23 0xdd7d8f in handle_file_event src/gdb/event-loop.c:731
    #24 0xdd8607 in gdb_wait_for_event src/gdb/event-loop.c:857
    #25 0xdd629c in gdb_do_one_event() src/gdb/event-loop.c:321
    #26 0xdd6344 in start_event_loop() src/gdb/event-loop.c:370
    #27 0x10a7715 in captured_command_loop src/gdb/main.c:331
    #28 0x10aa548 in captured_main src/gdb/main.c:1173
    #29 0x10aa5d8 in gdb_main(captured_main_args*) src/gdb/main.c:1188
    #30 0x87bd35 in main src/gdb/gdb.c:32
    #31 0x7fb0364c6f89 in __libc_start_main (/lib64/libc.so.6+0x20f89)
    #32 0x87bb49 in _start (build/gdb/gdb+0x87bb49)

Address 0x7fff5952bbdd is located in stack of thread T0 at offset 557 in frame
    #0 0xb93702 in location_completer(cmd_list_element*, completion_tracker&, \
                   char const*, char const*) src/gdb/completer.c:831

  This frame has 4 object(s):
    [32, 40) 'copy'
    [96, 104) 'location'
    [160, 168) 'text'
    [224, 256) 'completion_info' <== Memory access at offset 557 overflows \
                                    this variable
HINT: this may be a false positive if your program uses some custom stack \
      unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: dynamic-stack-buffer-overflow \
         src/gdb/linespec.c:727 in linespec_lexer_lex_string
Shadow bytes around the buggy address:
  0x10006b29d720: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006b29d730: 00 00 00 00 00 00 f1 f1 f1 f1 00 f2 f2 f2 f2 f2
  0x10006b29d740: f2 f2 00 f2 f2 f2 f2 f2 f2 f2 00 f2 f2 f2 f2 f2
  0x10006b29d750: f2 f2 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00
  0x10006b29d760: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10006b29d770: 00 00 00 00 ca ca ca ca 00 00 00[05]cb cb cb cb
  0x10006b29d780: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x10006b29d790: 00 f2 f2 f2 f2 f2 f2 f2 00 f2 f2 f2 f3 f3 f3 f3
  0x10006b29d7a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006b29d7b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006b29d7c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3637==ABORTING
...

The problem happens in linespec_lexer_lex_string when lexing
"b test_abi_tag_function[abi:\0" (using a notation where we make the implicit
terminating \0 explicit).

We arrrive here with (PARSER_STREAM (parser)) == ":\0":
...
             /* Do not tokenize ABI tags such as "[abi:cxx11]".  */
             else if (PARSER_STREAM (parser) - start > 4
                      && startswith (PARSER_STREAM (parser) - 4, "[abi"))
               ++(PARSER_STREAM (parser));
...
and consume ':', after which we end up here and consume '\0':
...
         /* Advance the stream.  */
         ++(PARSER_STREAM (parser));
...
after which (PARSER_STREAM (parser)) points past the end of the string.

Fix this by removing the first "++(PARSER_STREAM (parser))", and add an assert
to the second one to detect moving past the end-of-string.

Build and tested on x86_64-linux.

gdb/ChangeLog:

2019-06-10  Tom de Vries  <tdevries@suse.de>

	PR gdb/24611
	* linespec.c (linespec_lexer_lex_string): Remove incorrect
	"++(PARSER_STREAM (parser))" for "[abi"-prefixed colon.  Add assert.
berenm pushed a commit that referenced this pull request Jun 11, 2019
When compiling gdb with '-lasan -fsanitizer=address' and running tests with:
- export ASAN_OPTIONS="detect_leaks=0:alloc_dealloc_mismatch=0",
- target board cc-with-gdb-index,
- the "[gdb/testsuite] Fix gdb.base/break-probes.exp with native-gdbserver"
  commit reverted to avoid running into PR24617,
we get with gdb.arch/amd64-init-x87-values.exp:
...
==31229==ERROR: AddressSanitizer: heap-buffer-overflow on address \
  0x62500098c93c at pc 0x000000bcc748 bp 0x7ffe39487660 sp 0x7ffe39487658
READ of size 1 at 0x62500098c93c thread T0
    #0 0xbcc747 in cp_find_first_component_aux src/gdb/cp-support.c:999
    #1 0xbcc6e9 in cp_find_first_component(char const*) \
                   src/gdb/cp-support.c:977
    #2 0xcc2cf3 in mapped_index_base::build_name_components() \
                   src/gdb/dwarf2read.c:4499
    #3 0xcc3322 in dw2_expand_symtabs_matching_symbol src/gdb/dwarf2read.c:4552
    #4 0xcc817f in dw2_expand_symtabs_matching src/gdb/dwarf2read.c:5228
    #5 0xfe8f48 in iterate_over_all_matching_symtabs src/gdb/linespec.c:1147
    #6 0x1003506 in add_matching_symbols_to_info src/gdb/linespec.c:4413
    #7 0xffe21b in find_function_symbols src/gdb/linespec.c:3886
    #8 0xffe4a2 in find_linespec_symbols src/gdb/linespec.c:3914
    #9 0xfee3ad in linespec_parse_basic src/gdb/linespec.c:1865
    #10 0xff5128 in parse_linespec src/gdb/linespec.c:2655
    #11 0xff8872 in event_location_to_sals src/gdb/linespec.c:3150
    #12 0xff90a8 in decode_line_full(event_location const*, int, \
                    program_space*, symtab*, int, linespec_result*, \
		    char const*, char const*) src/gdb/linespec.c:3230
    #13 0x9ce449 in parse_breakpoint_sals src/gdb/breakpoint.c:9057
    #14 0x9ea022 in create_sals_from_location_default src/gdb/breakpoint.c:13708
    #15 0x9e2c1f in bkpt_create_sals_from_location src/gdb/breakpoint.c:12514
    #16 0x9cff06 in create_breakpoint(gdbarch*, event_location const*, \
                    char const*, int, char const*, int, int, bptype, int, \
		    auto_boolean, breakpoint_ops const*, int, int, int, \
		    unsigned int) src/gdb/breakpoint.c:9238
    #17 0x9d114a in break_command_1 src/gdb/breakpoint.c:9402
    #18 0x9d1b60 in break_command(char const*, int) src/gdb/breakpoint.c:9473
    #19 0xac96aa in do_const_cfunc src/gdb/cli/cli-decode.c:106
    #20 0xad0e5a in cmd_func(cmd_list_element*, char const*, int) \
                    src/gdb/cli/cli-decode.c:1892
    #21 0x15226f6 in execute_command(char const*, int) src/gdb/top.c:630
    #22 0xddde37 in command_handler(char const*) src/gdb/event-top.c:586
    #23 0xdde7c1 in command_line_handler(std::unique_ptr<char, \
                    gdb::xfree_deleter<char> >&&) src/gdb/event-top.c:773
    #24 0xddc9e8 in gdb_rl_callback_handler src/gdb/event-top.c:217
    #25 0x16f2198 in rl_callback_read_char src/readline/callback.c:220
    #26 0xddc5a1 in gdb_rl_callback_read_char_wrapper_noexcept \
                    src/gdb/event-top.c:175
    #27 0xddc773 in gdb_rl_callback_read_char_wrapper src/gdb/event-top.c:192
    #28 0xddd9f5 in stdin_event_handler(int, void*) src/gdb/event-top.c:514
    #29 0xdd7d8f in handle_file_event src/gdb/event-loop.c:731
    #30 0xdd8607 in gdb_wait_for_event src/gdb/event-loop.c:857
    #31 0xdd629c in gdb_do_one_event() src/gdb/event-loop.c:321
    #32 0xdd6344 in start_event_loop() src/gdb/event-loop.c:370
    #33 0x10a7715 in captured_command_loop src/gdb/main.c:331
    #34 0x10aa548 in captured_main src/gdb/main.c:1173
    #35 0x10aa5d8 in gdb_main(captured_main_args*) src/gdb/main.c:1188
    #36 0x87bd35 in main src/gdb/gdb.c:32
    #37 0x7f16e1434f89 in __libc_start_main (/lib64/libc.so.6+0x20f89)
    #38 0x87bb49 in _start (build/gdb/gdb+0x87bb49)

0x62500098c93c is located 0 bytes to the right of 8252-byte region \
  [0x62500098a900,0x62500098c93c)
allocated by thread T0 here:
    #0 0x7f16e359a600 in malloc (/usr/lib64/libasan.so.5+0xeb600)
    #1 0x1742ddf in bfd_malloc src/bfd/libbfd.c:275
    #2 0x1738824 in bfd_get_full_section_contents src/bfd/compress.c:253
    #3 0xe30044 in gdb_bfd_map_section(bfd_section*, unsigned long*) \
                   src/gdb/gdb_bfd.c:704
    #4 0xcb56bf in dwarf2_read_section(objfile*, dwarf2_section_info*) \
                   src/gdb/dwarf2read.c:2539
    #5 0xd5bcd0 in get_gdb_index_contents_from_section<dwarf2_per_objfile> \
                   src/gdb/dwarf2read.c:6217
    #6 0xd7fc7d in gdb::function_view<gdb::array_view<unsigned char const> \
                   (...) const src/gdb/common/function-view.h:284
    #7 0xd7fddd in gdb::function_view<gdb::array_view<unsigned char const> \
                   (...) src/gdb/common/function-view.h:278
    #8 0xd730cf in gdb::function_view<gdb::array_view<unsigned char const> \
                   (...) const src/gdb/common/function-view.h:247
    #9 0xcbc7ee in dwarf2_read_gdb_index src/gdb/dwarf2read.c:3582
    #10 0xcce731 in dwarf2_initialize_objfile(objfile*, dw_index_kind*) \
                    src/gdb/dwarf2read.c:6297
    #11 0xdb88c4 in elf_symfile_read src/gdb/elfread.c:1256
    #12 0x141262a in read_symbols src/gdb/symfile.c:798
    #13 0x14140a7 in syms_from_objfile_1 src/gdb/symfile.c:1000
    #14 0x1414393 in syms_from_objfile src/gdb/symfile.c:1017
    #15 0x1414fb7 in symbol_file_add_with_addrs src/gdb/symfile.c:1124
    #16 0x14159b7 in symbol_file_add_from_bfd(bfd*, char const*, \
                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
	             std::allocator<other_sections> >*, \
		     enum_flags<objfile_flag>, objfile*) src/gdb/symfile.c:1203
    #17 0x1415b6c in symbol_file_add(char const*,
                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
		     std::allocator<other_sections> >*, \
		     enum_flags<objfile_flag>) src/gdb/symfile.c:1216
    #18 0x1415f2f in symbol_file_add_main_1 src/gdb/symfile.c:1240
    #19 0x1418599 in symbol_file_command(char const*, int) \
                     src/gdb/symfile.c:1675
    #20 0xde2fa6 in file_command src/gdb/exec.c:433
    #21 0xac96aa in do_const_cfunc src/gdb/cli/cli-decode.c:106
    #22 0xad0e5a in cmd_func(cmd_list_element*, char const*, int) \
                    src/gdb/cli/cli-decode.c:1892
    #23 0x15226f6 in execute_command(char const*, int) src/gdb/top.c:630
    #24 0xddde37 in command_handler(char const*) src/gdb/event-top.c:586
    #25 0xdde7c1 in command_line_handler(std::unique_ptr<char, \
                    gdb::xfree_deleter<char> >&&) src/gdb/event-top.c:773
    #26 0xddc9e8 in gdb_rl_callback_handler src/gdb/event-top.c:217
    #27 0x16f2198 in rl_callback_read_char src/readline/callback.c:220
    #28 0xddc5a1 in gdb_rl_callback_read_char_wrapper_noexcept \
                    src/gdb/event-top.c:175
    #29 0xddc773 in gdb_rl_callback_read_char_wrapper src/gdb/event-top.c:192

SUMMARY: AddressSanitizer: heap-buffer-overflow src/gdb/cp-support.c:999 in \
  cp_find_first_component_aux
Shadow bytes around the buggy address:
  0x0c4a801298d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a801298e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a801298f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80129900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80129910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4a80129920: 00 00 00 00 00 00 00[04]fa fa fa fa fa fa fa fa
  0x0c4a80129930: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129940: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==31229==ABORTING
...

The problem happens as follows.

The executable amd64-init-x87-values gets an index (due to target board
cc-with-gdb-index), which looks as follows:
...
Hex dump of section '.gdb_index':
  0x00000000 08000000 18000000 28000000 28000000 ........(...(...
  0x00000010 3c000000 3c200000 00000000 00000000 <...< ..........
  0x00000020 2e000000 00000000 d4004000 00000000 ..........@.....
  0x00000030 db004000 00000000 00000000 00000000 ..@.............
  0x00000040 00000000 00000000 00000000 00000000 ................
  0x00000050 00000000 00000000 00000000 00000000 ................
  ... more zeroes ...
  0x00002010 00000000 00000000 00000000 00000000 ................
  0x00002020 00000000 00000000 00000000 00000000 ................
  0x00002030 00000000 00000000 00000000          ............
...

The structure of this index is:
...
header       : [0x0, 0x18)     : size 0x18
culist       : [0x18 ,0x28)    : size 0x10
typesculist  : [0x28, 0x28)    : size 0x0
adress area  : [0x28, 0x3c)    : size 0x14
symbol table : [0x3c, 0x203c)  : size 0x2000
constant pool: [0x203c, 0x203c): size 0x0
EOF          : 0x203c
...

Note that the symbol table consists entirely of empty slots (where an empty
slot is a pair of 32-bit zeroes), and that the constant pool is empty.

The problem happens here in mapped_index_base::build_name_components:
...
  auto count = this->symbol_name_count ();
  for (offset_type idx = 0; idx < count; idx++)
    {
      if (this->symbol_name_slot_invalid (idx))
	continue;

      const char *name = this->symbol_name_at (idx);
...
when accessing the slot at idx == 0 in the symbol table,
symbol_name_slot_invalid returns false so we calculate name, which is
calculated using 'constant_pool + symbol_table[idx].name', which means we get
name == constant_pool.  And given that the constant pool is empty, name now
points past the memory allocated for the index, and when we access name[0] for
the first time in cp_find_first_component_aux, we run into the
heap-buffer-overflow.

Fix this by fixing the definition of symbol_name_slot_invalid:
...
-    return bucket.name == 0 && bucket.vec;
+    return bucket.name == 0 && bucket.vec == 0;
...

Tested on x86_64-linux.

gdb/ChangeLog:

2019-06-10  Tom de Vries  <tdevries@suse.de>

	PR gdb/24618
	* dwarf2read.c (struct mapped_index::symbol_name_slot_invalid): Make
	sure an empty slot (defined by a 32-bit zero pair) is recognized as
	invalid.
berenm pushed a commit that referenced this pull request Jun 18, 2019
When compiling gdb with '-lasan -fsanitizer=address' and running tests with:
- export ASAN_OPTIONS="detect_leaks=0:alloc_dealloc_mismatch=0", and
- a target board using local-board.exp, which sets sysroot to ""
we run into a heap-buffer-overflow in child_path for f.i. gdb.arch/amd64-byte:
...
==3997==ERROR: AddressSanitizer: heap-buffer-overflow on address \
  0x60200002abcf at pc 0x5602acdf6872 bp 0x7ffe5237a090 sp 0x7ffe5237a080
READ of size 1 at 0x60200002abcf thread T0
    #0 0x5602acdf6871 in child_path(char const*, char const*) \
                      gdb/common/pathstuff.c:161
    #1 0x5602adb06587 in find_separate_debug_file gdb/symfile.c:1483
    #2 0x5602adb06f2f in find_separate_debug_file_by_debuglink[abi:cxx11](...) \
                      gdb/symfile.c:1563
    #3 0x5602ad13b743 in elf_symfile_read gdb/elfread.c:1293
    #4 0x5602adb01cfa in read_symbols gdb/symfile.c:798
    #5 0x5602adb03769 in syms_from_objfile_1 gdb/symfile.c:1000
    #6 0x5602adb039d0 in syms_from_objfile gdb/symfile.c:1017
    #7 0x5602adb04551 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #8 0x5602adb04ebf in symbol_file_add_from_bfd(...) gdb/symfile.c:1204
    #9 0x5602ada5a78d in solib_read_symbols(...) gdb/solib.c:695
    #10 0x5602ada5bdae in solib_add(char const*, int, int) gdb/solib.c:1004
    #11 0x5602ada49bcd in enable_break gdb/solib-svr4.c:2394
    #12 0x5602ada4dae9 in svr4_solib_create_inferior_hook gdb/solib-svr4.c:3028
    #13 0x5602ada5d4f1 in solib_create_inferior_hook(int) gdb/solib.c:1215
    #14 0x5602ad347f66 in post_create_inferior(target_ops*, int) \
                          gdb/infcmd.c:467
    #15 0x5602ad348b3c in run_command_1 gdb/infcmd.c:663
    #16 0x5602ad348e55 in run_command gdb/infcmd.c:686
    #17 0x5602acd7d32b in do_const_cfunc gdb/cli/cli-decode.c:106
    #18 0x5602acd84bfe in cmd_func(cmd_list_element*, char const*, int) \
                          gdb/cli/cli-decode.c:1892
    #19 0x5602adc62a90 in execute_command(char const*, int) gdb/top.c:630
    #20 0x5602ad5053e6 in catch_command_errors gdb/main.c:372
    #21 0x5602ad507eb1 in captured_main_1 gdb/main.c:1138
    #22 0x5602ad5081ec in captured_main gdb/main.c:1163
    #23 0x5602ad508281 in gdb_main(captured_main_args*) gdb/main.c:1188
    #24 0x5602ac9ddc3a in main gdb/gdb.c:32
    #25 0x7f582b56eb96 in __libc_start_main \
                       (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #26 0x5602ac9dda09 in _start \
                       (/home/smarchi/build/binutils-gdb/gdb/gdb+0x19a2a09)

0x60200002abcf is located 1 bytes to the left of 1-byte region \
  [0x60200002abd0,0x60200002abd1)
allocated by thread T0 here:
    #0 0x7f582e0e4b50 in __interceptor_malloc \
                      (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
    #1 0x5602acdd3656 in xmalloc gdb/common/common-utils.c:44
    #2 0x5602aefe17d1 in xstrdup libiberty/xstrdup.c:34
    #3 0x5602acdf61f6 in gdb_realpath(char const*) gdb/common/pathstuff.c:80
    #4 0x5602adb06278 in find_separate_debug_file gdb/symfile.c:1444
    #5 0x5602adb06f2f in find_separate_debug_file_by_debuglink[abi:cxx11](...) \
                      gdb/symfile.c:1563
    #6 0x5602ad13b743 in elf_symfile_read gdb/elfread.c:1293
    #7 0x5602adb01cfa in read_symbols gdb/symfile.c:798
    #8 0x5602adb03769 in syms_from_objfile_1 gdb/symfile.c:1000
    #9 0x5602adb039d0 in syms_from_objfile gdb/symfile.c:1017
    #10 0x5602adb04551 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #11 0x5602adb04ebf in symbol_file_add_from_bfd(...) gdb/solib.c:695
    #13 0x5602ada5bdae in solib_add(char const*, int, int) gdb/solib.c:1004
    #14 0x5602ada49bcd in enable_break gdb/solib-svr4.c:2394
    #15 0x5602ada4dae9 in svr4_solib_create_inferior_hook gdb/solib-svr4.c:3028
    #16 0x5602ada5d4f1 in solib_create_inferior_hook(int) gdb/solib.c:1215
    #17 0x5602ad347f66 in post_create_inferior(target_ops*, int) \
                       gdb/infcmd.c:467
    #18 0x5602ad348b3c in run_command_1 gdb/infcmd.c:663
    #19 0x5602ad348e55 in run_command gdb/infcmd.c:686
    #20 0x5602acd7d32b in do_const_cfunc gdb/cli/cli-decode.c:106
    #21 0x5602acd84bfe in cmd_func(cmd_list_element*, char const*, int) \
                       gdb/cli/cli-decode.c:1892
    #22 0x5602adc62a90 in execute_command(char const*, int) gdb/top.c:630
    #23 0x5602ad5053e6 in catch_command_errors gdb/main.c:372
    #24 0x5602ad507eb1 in captured_main_1 gdb/main.c:1138
    #25 0x5602ad5081ec in captured_main gdb/main.c:1163
    #26 0x5602ad508281 in gdb_main(captured_main_args*) gdb/main.c:1188
    #27 0x5602ac9ddc3a in main gdb/gdb.c:32
    #28 0x7f582b56eb96 in __libc_start_main \
                       (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

SUMMARY: AddressSanitizer: heap-buffer-overflow gdb/common/pathstuff.c:161 \
  in child_path(char const*, char const*)
Shadow bytes around the buggy address:
  0x0c047fffd520: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fa
  0x0c047fffd530: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
  0x0c047fffd540: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
  0x0c047fffd550: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fa
  0x0c047fffd560: fa fa fd fa fa fa fd fa fa fa fd fa fa fa 00 00
=>0x0c047fffd570: fa fa 07 fa fa fa 00 fa fa[fa]01 fa fa fa fa fa
  0x0c047fffd580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fffd590: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fffd5a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fffd5b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fffd5c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3997==ABORTING
...

The direct cause is that child_path gets called with parent == "", so this
test:
...
  if (IS_DIR_SEPARATOR (parent[parent_len - 1]))
...
accesses parent[-1].

[ There is an open discussion (1) about whether an empty sysroot should indeed
be represented internally as "".  But this patch focuses on fixing the
heap-buffer-overflow without any redesign. ]

Fix this by guarding the test with 'parent_len > 0'.

Note that the fix makes child_path behave the same for:
- parent == "/" && child == "/foo" (returns "foo")
- parent == "" and child == "/foo" (returns "foo").

Build and reg-tested on x86_64-linux.

(1) https://sourceware.org/ml/gdb-patches/2019-05/msg00193.html

gdb/ChangeLog:

2019-06-17  Tom de Vries  <tdevries@suse.de>

	PR gdb/24617
	* common/pathstuff.c (child_path): Make sure parent_len > 0 before
	accessing parent[parent_len - 1].
berenm pushed a commit that referenced this pull request Jul 13, 2019
When:
- building trunk gdb with '-fsanitize=address -lasan',
- running gdb tests with "export ASAN_OPTIONS=detect_leaks=0",
I run into a heap-buffer-overflow failure for
gdb.base/utf8-identifiers.exp.

In more detail, the libasan error report looks like this:
...
=================================================================
==22340==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x619000054a80 at pc 0x7fcd0306b4c9 bp 0x7fffb1a8d880 sp 0x7fffb1a8d030
READ of size 32766 at 0x619000054a80 thread T0
    #0 0x7fcd0306b4c8  (/usr/lib64/libasan.so.4+0xae4c8)
    #1 0x15f12a1 in update_line
/data/gdb_versions/devel/src/readline/display.c:1377
    #2 0x15f03cb in rl_redisplay
/data/gdb_versions/devel/src/readline/display.c:1204
    #3 0x15bf932 in readline_internal_setup
/data/gdb_versions/devel/src/readline/readline.c:394
    #4 0x15fe723 in _rl_callback_newline
/data/gdb_versions/devel/src/readline/callback.c:89
    #5 0x15fe7ef in rl_callback_handler_install
/data/gdb_versions/devel/src/readline/callback.c:102
    #6 0xd7bce6 in gdb_rl_callback_handler_install(char const*)
/data/gdb_versions/devel/src/gdb/event-top.c:319
    #7 0xd7c0c6 in display_gdb_prompt(char const*)
/data/gdb_versions/devel/src/gdb/event-top.c:409
    #8 0xd7d6c1 in command_line_handler(std::unique_ptr<char,
gdb::xfree_deleter<char> >&&)
/data/gdb_versions/devel/src/gdb/event-top.c:776
    #9 0xd7b92a in gdb_rl_callback_handler
/data/gdb_versions/devel/src/gdb/event-top.c:217
    #10 0x15ff479 in rl_callback_read_char
/data/gdb_versions/devel/src/readline/callback.c:220
    #11 0xd7b4d5 in gdb_rl_callback_read_char_wrapper_noexcept
/data/gdb_versions/devel/src/gdb/event-top.c:175
    #12 0xd7b6b5 in gdb_rl_callback_read_char_wrapper
/data/gdb_versions/devel/src/gdb/event-top.c:192
    #13 0xd7c8aa in stdin_event_handler(int, void*)
/data/gdb_versions/devel/src/gdb/event-top.c:514
    #14 0xd76ca7 in handle_file_event
/data/gdb_versions/devel/src/gdb/event-loop.c:731
    #15 0xd7751f in gdb_wait_for_event
/data/gdb_versions/devel/src/gdb/event-loop.c:857
    #16 0xd7547e in gdb_do_one_event()
/data/gdb_versions/devel/src/gdb/event-loop.c:321
    #17 0xd75526 in start_event_loop()
/data/gdb_versions/devel/src/gdb/event-loop.c:370
    #18 0x101b04c in captured_command_loop
/data/gdb_versions/devel/src/gdb/main.c:331
    #19 0x101de73 in captured_main
/data/gdb_versions/devel/src/gdb/main.c:1173
    #20 0x101df03 in gdb_main(captured_main_args*)
/data/gdb_versions/devel/src/gdb/main.c:1188
    #21 0x872dba in main /data/gdb_versions/devel/src/gdb/gdb.c:32
    #22 0x7fcd00f2ff49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
    #23 0x872bc9 in _start (/data/gdb_versions/devel/build/gdb/gdb+0x872bc9)

0x619000054a80 is located 0 bytes to the right of 1024-byte region
[0x619000054680,0x619000054a80)
allocated by thread T0 here:
    #0 0x7fcd03099510 in malloc (/usr/lib64/libasan.so.4+0xdc510)
    #1 0xae0078 in xmalloc
/data/gdb_versions/devel/src/gdb/common/common-utils.c:44
    #2 0x15eaccb in init_line_structures
/data/gdb_versions/devel/src/readline/display.c:458
    #3 0x15eb4d8 in rl_redisplay
/data/gdb_versions/devel/src/readline/display.c:526
    #4 0x15bf932 in readline_internal_setup
/data/gdb_versions/devel/src/readline/readline.c:394
    #5 0x15fe723 in _rl_callback_newline
/data/gdb_versions/devel/src/readline/callback.c:89
    #6 0x15fe7ef in rl_callback_handler_install
/data/gdb_versions/devel/src/readline/callback.c:102
    #7 0xd7bce6 in gdb_rl_callback_handler_install(char const*)
/data/gdb_versions/devel/src/gdb/event-top.c:319
    #8 0xd7c0c6 in display_gdb_prompt(char const*)
/data/gdb_versions/devel/src/gdb/event-top.c:409
    #9 0xaa041b in cli_interp_base::pre_command_loop()
/data/gdb_versions/devel/src/gdb/cli/cli-interp.c:286
    #10 0xf5342a in interp_pre_command_loop(interp*)
/data/gdb_versions/devel/src/gdb/interps.c:320
    #11 0x101b047 in captured_command_loop
/data/gdb_versions/devel/src/gdb/main.c:328
    #12 0x101de73 in captured_main
/data/gdb_versions/devel/src/gdb/main.c:1173
    #13 0x101df03 in gdb_main(captured_main_args*)
/data/gdb_versions/devel/src/gdb/main.c:1188
    #14 0x872dba in main /data/gdb_versions/devel/src/gdb/gdb.c:32
    #15 0x7fcd00f2ff49 in __libc_start_main (/lib64/libc.so.6+0x20f49)

SUMMARY: AddressSanitizer: heap-buffer-overflow
(/usr/lib64/libasan.so.4+0xae4c8)
Shadow bytes around the buggy address:
  0x0c3280002900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280002910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280002920: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280002930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280002940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3280002950:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3280002960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3280002970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280002980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280002990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c32800029a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==22340==ABORTING
...

I've written an assert in rl_redisplay that formulates the error condition:
...
@@ -1387,6 +1389,10 @@ rl_redisplay (void)
          cpos_adjusted = 0;
+         assert (last_lmargin + (_rl_screenwidth + visible_wrap_offset)
+                 <= line_size);
+         assert (lmargin + (_rl_screenwidth + (lmargin ? 0 : wrap_offset))
+                 <= line_size);
          update_line (&visible_line[last_lmargin],
                       &invisible_line[lmargin],
                       0,
                       _rl_screenwidth + visible_wrap_offset,
                       _rl_screenwidth + (lmargin ? 0 : wrap_offset),
                       0);
...
which triggers without needing the address sanitizer (or even an executable),
like this:
...
$ TERM=dumb gdb -q -ex "set width 0"
gdb: src/display.c:1393: rl_redisplay: Assertion
`last_lmargin + (_rl_screenwidth + visible_wrap_offset) <= line_size'
failed.
Aborted (core dumped)
...

The basic problem is this: visible_line and invisible_line have length
line_size, but the update_line call assumes that line_size is at least
_rl_screenwidth + 1.  Executing "set width 0" sets _rl_screenwidth to 32766 but
doesn't affect line_size, which is initialized to 1024.

Fix this by ensuring in init_line_structures and rl_redisplay that line_size
is at least _rl_screenwidth + 1.

Tested on x86_64-linux.

Reviewed by readline maintainer (
https://sourceware.org/ml/gdb-patches/2019-05/msg00566.html ).

readline/ChangeLog.gdb:

2019-07-12  Tom de Vries  <tdevries@suse.de>
	    Chet Ramey  <chet.ramey@case.edu>

	PR cli/24514
	* readline/display.c (init_line_structures, rl_redisplay): Ensure
	line_size is at	least _rl_screenwidth + 1.
berenm pushed a commit that referenced this pull request Jul 28, 2019
… blocks

The discussion on gdb-patches which led to this patch may be found
here:

https://www.sourceware.org/ml/gdb-patches/2019-05/msg00018.html

Here's a brief synopsis/analysis:

Eli Zaretskii, while debugging a Windows emacs executable, found
that functions comprised of more than one (non-contiguous)
address range were not being displayed correctly in a backtrace.  This
is the example that Eli provided:

  (gdb) bt
  #0  0x76a63227 in KERNELBASE!DebugBreak ()
     from C:\Windows\syswow64\KernelBase.dll
  #1  0x012e7b89 in emacs_abort () at w32fns.c:10768
  #2  0x012e1f3b in print_vectorlike.cold () at print.c:1824
  #3  0x011d2dec in print_object (obj=<optimized out>, printcharfun=XIL(0),
      escapeflag=true) at print.c:2150

The function print_vectorlike consists of two address ranges, one of
which contains "cold" code which is expected to not execute very often.
There is a minimal symbol, print_vectorlike.cold.65, which is the address
of the "cold" range.

GDB is prefering this minsym over the the name provided by the
DWARF info due to some really old code in GDB which handles
"certain pathological cases".  This comment reads as follows:

      /* In certain pathological cases, the symtabs give the wrong
	 function (when we are in the first function in a file which
	 is compiled without debugging symbols, the previous function
	 is compiled with debugging symbols, and the "foo.o" symbol
	 that is supposed to tell us where the file with debugging
	 symbols ends has been truncated by ar because it is longer
	 than 15 characters).  This also occurs if the user uses asm()
	 to create a function but not stabs for it (in a file compiled
	 with -g).

	 So look in the minimal symbol tables as well, and if it comes
	 up with a larger address for the function use that instead.
	 I don't think this can ever cause any problems; there
	 shouldn't be any minimal symbols in the middle of a function;
	 if this is ever changed many parts of GDB will need to be
	 changed (and we'll create a find_pc_minimal_function or some
	 such).  */

In an earlier version of this patch, I had left the code for the
pathological case intact, but those who reviwed that patch recommended
removing it.  So that's what I've done - I've removed it.

gdb/ChangeLog:

	* stack.c (find_frame_funname): Remove code which preferred
	minsym over symtab sym in "certain pathological cases".
berenm pushed a commit that referenced this pull request Jul 28, 2019
In the course of revising the test case for
gdb.dwarf2/dw2-ranges-func.exp, I added a new .c file which would
cause the "cold" range to be at a higher address than the rest of the
function.  In these tests, the range in question isn't really cold in
the sense that a compiler has determined that it'll be executed less
frequently.  Instead, it's simply the range that does not include the
entry pc.  These tests are intended to mimic the output of such a
compiler, so I'll continue to refer to this range as "cold" in the
following discussion.

The original test case had only tested a cold range placed
at lower addresses than the rest of the function.  During testing of the
new code where the cold range was placed at higher addresses, I found
that I could produce the following backtrace:

    (gdb) bt
    #0  0x0000000000401138 in baz ()
	at dw2-ranges-func-hi-cold.c:72
    #1  0x0000000000401131 in foo_cold ()
	at dw2-ranges-func-hi-cold.c:64
    #2  0x000000000040111e in foo ()
	at dw2-ranges-func-hi-cold.c:50
    #3  0x0000000000401144 in main ()
	at dw2-ranges-func-hi-cold.c:78

This is correct, except that we'd like to see foo() listed instead
of foo_cold().  (I handle that problem in another patch.)

Now look at what happens for a similar backtrace where the cold range
is at a lower address than the foo's entry pc:

    (gdb) bt
    #0  0x000000000040110a in baz ()
	at dw2-ranges-func-lo-cold.c:48
    #1  0x0000000000401116 in foo ()
	at dw2-ranges-func-lo-cold.c:54
    #2  0x00007fffffffd4c0 in ?? ()
    #3  0x0000000000401138 in foo ()
	at dw2-ranges-func-lo-cold.c:70

Note that the backtrace doesn't go all the way back to main().  Moreover,
frame #2 is messed up.

I had seen this behavior when I had worked on the non-contiguous
address problem last year.  At the time I convinced myself that the
mangled backtrace was "okay" since we're doing strange things with
the DWARF assembler.  We're taking a function called foo_cold (though
it was originally called foo_low - my recent changes to the test case
changed the name) and via the magic of the DWARF assembler, we're
combining it into a separate (non-contiguous) range for foo.  Thus,
it was a surprise to me when I got a good and complete backtrace when
the cold symbol is placed at an address that's greater than entry pc.

The function dwarf2_frame_cache (in dwarf2-frame.c) is making this
call:

    if (get_frame_func_if_available (this_frame, &entry_pc)) ...

If that call succeeds (returns a true value), the FDE is then
processed up to the entry pc.  It doesn't make sense to do this,
however, when the FDE in question does not contain the entry pc.  This
can happen when the function in question is comprised of more than one
(non-contiguous) address range.

My fix is to add some comparisons to the test above to ensure that
ENTRY_PC is within the address range covered by the FDE.

gdb/ChangeLog:

	* dwarf2-frame.c (dwarf2_frame_cache): Don't decode FDE instructions
	for entry pc when entry pc is out of range for that FDE.
Luffy-tsai pushed a commit to Luffy-tsai/binutils-gdb that referenced this pull request Jan 15, 2025
…read call

Commit 7fcdec0 ("GDB: Use gdb::array_view for buffers used in
register reading and unwinding") introduces a regression in
gdb.base/jit-reader.exp:

    $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.base/jit-reader/jit-reader -ex 'jit-reader-load /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-reader/jit-reader.so' -ex r -batch

    This GDB supports auto-downloading debuginfo from the following URLs:
      <https://debuginfod.archlinux.org>
    Enable debuginfod for this session? (y or [n]) [answered N; input not from terminal]
    Debuginfod has been disabled.
    To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".

    Program received signal SIGTRAP, Trace/breakpoint trap.
    Recursive internal problem.

The "Recusive internal problem" part is not good, but it's not the point
of this patch.  It still means we hit an internal error.

The stack trace is:

    #0  internal_error_loc (file=0x55555ebefb20 "/home/simark/src/binutils-gdb/gdb/frame.c", line=1207, fmt=0x55555ebef500 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:53
    bminor#1  0x0000555561604d83 in frame_register_unwind (next_frame=..., regnum=16, optimizedp=0x7ffff12e5a20, unavailablep=0x7ffff12e5a30, lvalp=0x7ffff12e5a40, addrp=0x7ffff12e5a60, realnump=0x7ffff12e5a50, buffer=...)
        at /home/simark/src/binutils-gdb/gdb/frame.c:1207
    bminor#2  0x0000555561608334 in deprecated_frame_register_read (frame=..., regnum=16, myaddr=...) at /home/simark/src/binutils-gdb/gdb/frame.c:1496
    bminor#3  0x0000555561a74259 in jit_unwind_reg_get_impl (cb=0x7ffff1049ca0, regnum=16) at /home/simark/src/binutils-gdb/gdb/jit.c:988
    bminor#4  0x00007fffd26e634e in read_register (callbacks=0x7ffff1049ca0, dw_reg=16, value=0x7fffffffb4c8) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:100
    bminor#5  0x00007fffd26e645f in unwind_frame (self=0x50400000ac10, cbs=0x7ffff1049ca0) at /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.c:143
    bminor#6  0x0000555561a74a12 in jit_frame_sniffer (self=0x55556374d040 <jit_frame_unwind>, this_frame=..., cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/jit.c:1042
    bminor#7  0x00005555615f499e in frame_unwind_try_unwinder (this_frame=..., this_cache=0x5210002905f8, unwinder=0x55556374d040 <jit_frame_unwind>) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:138
    bminor#8  0x00005555615f512c in frame_unwind_find_by_frame (this_frame=..., this_cache=0x5210002905f8) at /home/simark/src/binutils-gdb/gdb/frame-unwind.c:209
    bminor#9  0x00005555616178d0 in get_frame_type (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2996
    bminor#10 0x000055556282db03 in do_print_frame_info (uiout=0x511000027500, fp_opts=..., frame=..., print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at /home/simark/src/binutils-gdb/gdb/stack.c:1033

The problem is that function `jit_unwind_reg_get_impl` passes field
`gdb_reg_value::value`, a gdb_byte array of 1 element (used as a
flexible array member), as the array view parameter of
`deprecated_frame_register_read`.  This results in an array view of size
1.  The assertion in `frame_register_unwind` that verifies the passed in
buffer is larger enough to hold the unwound register value then fails.

Fix this by explicitly creating an array view of the right size.

Change-Id: Ie170da438ec9085863e7be8b455a067b531635dc
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
MingcongBai pushed a commit to AOSC-Tracking/binutils-gdb that referenced this pull request Jan 29, 2025
This resolves the following memory leak reported by ASAN:

Direct leak of 17 byte(s) in 1 object(s) allocated from:
    #0 0x3ffb32fbb1d in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    bminor#1 0x2aa149861cf in xmalloc ../../libiberty/xmalloc.c:149
    bminor#2 0x2aa149868ff in xstrdup ../../libiberty/xstrdup.c:34
    bminor#3 0x2aa1312391f in s390_machinemode ../../gas/config/tc-s390.c:2241
    bminor#4 0x2aa130ddc7b in read_a_source_file ../../gas/read.c:1293
    bminor#5 0x2aa1304f7bf in perform_an_assembly_pass ../../gas/as.c:1223
    bminor#6 0x2aa1304f7bf in main ../../gas/as.c:1436
    bminor#7 0x3ffb282be35 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    bminor#8 0x3ffb282bf33 in __libc_start_main_impl ../csu/libc-start.c:360
    bminor#9 0x2aa1305758f  (/home/jremus/git/binutils/build-asan/gas/as-new+0x2d5758f) (BuildId: ...)

gas/
	* config/tc-s390.c (s390_machinemode): Free mode_string before
	returning.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
MingcongBai pushed a commit to AOSC-Tracking/binutils-gdb that referenced this pull request Jan 29, 2025
Simplify the .machine directive parsing logic, so that cpu_string is
always xstrdup'd and can therefore always be xfree'd before returning
to the caller.

This resolves the following memory leak reported by ASAN:

Direct leak of 13 byte(s) in 3 object(s) allocated from:
    #0 0x3ff8aafbb1d in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    bminor#1 0x2aa338861cf in xmalloc ../../libiberty/xmalloc.c:149
    bminor#2 0x2aa338868ff in xstrdup ../../libiberty/xstrdup.c:34
    bminor#3 0x2aa320253cb in s390_machine ../../gas/config/tc-s390.c:2172
    bminor#4 0x2aa31fddc7b in read_a_source_file ../../gas/read.c:1293
    bminor#5 0x2aa31f4f7bf in perform_an_assembly_pass ../../gas/as.c:1223
    bminor#6 0x2aa31f4f7bf in main ../../gas/as.c:1436
    bminor#7 0x3ff8a02be35 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    bminor#8 0x3ff8a02bf33 in __libc_start_main_impl ../csu/libc-start.c:360
    bminor#9 0x2aa31f5758f  (/home/jremus/git/binutils/build-asan/gas/as-new+0x2d5758f) (BuildId: ...)

While at it add tests with double quoted .machine
"<cpu>[+<extension>...]" values.

gas/
	* config/tc-s390.c (s390_machine): Simplify parsing and free
	cpu_string before returning.

gas/testsuite/
	* gas/s390/machine-parsing-1.l: Add tests with double quoted
	values.
	* gas/s390/machine-parsing-1.s: Likewise.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
mikpe referenced this pull request in mikpe/binutils-gdb Feb 13, 2025
On ubuntu systems with libc debug info available (libc6-dbg), I see the
following failures for the gdb.base/corefile-exec-context.exp testcase:

    show args
    Argument list to give program being debugged when it is started is "aaaaa bbbbb ccccc ddddd e\ e\ e\ e\ e".
    (gdb) PASS: gdb.base/corefile-exec-context.exp: show args
    up
    #1  __pthread_kill_internal (signo=6, threadid=133859295332160) at ./nptl/pthread_kill.c:78
    78	in ./nptl/pthread_kill.c
    (gdb) FAIL: gdb.base/corefile-exec-context.exp: move up to main

This failures is because the pattern used to parse the output of `up`
is not expecting what is seen when debugging information is present for
those frames.

This patch adjusts the pattern to allow both possible outputs.

Tested on ubuntu-22.04 and ubuntu24.04 with libc6-dbg installed for gdb
build with --with-separate-debug-dir=/usr/lib/debug.

Change-Id: I217d4b20006d0ecdb4b7a71eeb8d01597ec5ac63
Approved-By: Tom Tromey <tom@tromey.com>
mikpe referenced this pull request in mikpe/binutils-gdb Feb 13, 2025
Recent work in the TUI has improved GDB's use of the curses
wnoutrefresh and doupdate mechanism, which improves performance by
batching together updates and then doing a single set of writes to the
screen when doupdate is finally called.

The tui_batch_rendering type is a RAII class which, in its destructor,
calls doupdate to send the batched updates to the screen.

However, if there is no tui_batch_rendering active on the call stack
then any wnoutrefresh calls will remain batched but undisplayed until
the next time doupdate happens to be called.

This problem can be seen in PR gdb/32623.  When an inferior is started
the 'Starting program' message is not immediately displayed to the
user.

The 'Starting program' message originates from run_command_1 in
infcmd.c, the message is sent to the current_uiout, which will be the
TUI ui_out.  After the message is sent, ui_out::flush() is called,
here's the backtrace when that happens:

  #0  tui_file::flush (this=0x36e4ab0) at ../../src/gdb/tui/tui-file.c:42
  #1  0x0000000001004f4b in pager_file::flush (this=0x36d35f0) at ../../src/gdb/utils.c:1531
  #2  0x0000000001004f71 in gdb_flush (stream=0x36d35f0) at ../../src/gdb/utils.c:1539
  #3  0x00000000006975ab in cli_ui_out::do_flush (this=0x35a50b0) at ../../src/gdb/cli-out.c:250
  bminor#4  0x00000000009fd1f9 in ui_out::flush (this=0x35a50b0) at ../../src/gdb/ui-out.h:263
  bminor#5  0x00000000009f56ad in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at ../../src/gdb/infcmd.c:449
  bminor#6  0x00000000009f599a in run_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:511

And if we check out tui_file::flush (tui-file.c) we can see that this
just calls tui_win_info::refresh_window(), which in turn, just uses
wnoutrefresh to batch any pending output.

The problem is that, in the above backtrace, there is no
tui_batch_rendering active, and so there will be no doupdate call to
flush the output to the screen.

We could add a tui_batch_rendering into tui_file::flush.  And
tui_file::write.  And tui_file::puts .....

... but that all seems a bit unnecessary.  Instead, I propose that
tui_win_info::refresh_window() should be changed.  If suppress_output
is true (i.e. a tui_batch_rendering is active) then we should continue
to call wnoutrefresh().  But if suppress_output is false, meaning that
no tui_batch_rendering is in place, then we should call wrefresh(),
which immediately writes the output to the screen.

Testing but PR gdb/32623 was a little involved.  We need to 'run' the
inferior and check for the 'Starting program' message.  But DejaGNUU
can only check for the message once it knows the message should have
appeared.  But, as the bug is that output is not displayed, we don't
have any output hints that the inferior is started yet...

In the end, I have the inferior create a file in the test's output
directory.  Now DejaGNU can send the 'run' command, and wait for the
file to appear.  Once that happens, we know that the 'Starting
program' message should have appeared.

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

Approved-By: Tom Tromey <tom@tromey.com>
mikpe referenced this pull request in mikpe/binutils-gdb Feb 13, 2025
I noticed that attempting to select a tail-call frame using 'frame
function NAME' wouldn't work:

  (gdb) bt
  #0  func_that_never_returns () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:49
  #1  0x0000000000401183 in func_that_tail_calls () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:59
  #2  0x00000000004011a5 in main () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:70
  (gdb) frame function func_that_tail_calls
  No frame for function "func_that_tail_calls".
  (gdb) up
  #1  0x0000000000401183 in func_that_tail_calls () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:59
  59	  func_that_never_returns ();
  (gdb) disassemble
  Dump of assembler code for function func_that_tail_calls:
     0x000000000040117a <+0>:	push   %rbp
     0x000000000040117b <+1>:	mov    %rsp,%rbp
     0x000000000040117e <+4>:	call   0x40116c <func_that_never_returns>
  End of assembler dump.
  (gdb)

The problem is that the 'function' mechanism uses get_frame_pc() and
then compares the address returned with the bounds of the function
we're looking for.

So in this case, the bounds of func_that_tail_calls are 0x40117a to
0x401183, with 0x401183 being the first address _after_ the function.

However, because func_that_tail_calls ends in a tail call, then the
get_frame_pc() is 0x401183, the first address after the function.  As
a result, GDB fails to realise that frame #1 is inside the function
we're looking for, and the lookup fails.

The fix is to use get_frame_address_in_block, which will return an
adjusted address, in this case, 0x401182, which is within the function
bounds.  Now the lookup works:

  (gdb) frame function func_that_tail_calls
  #1  0x0000000000401183 in func_that_tail_calls () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:59
  59	  func_that_never_returns ();
  (gdb)

I've extended the gdb.base/frame-selection.exp test to cover this
case.
wataash pushed a commit to wataash/binutils-gdb that referenced this pull request Mar 19, 2025
Consider the test-case sources main.c and foo.c:

  $ cat main.c
  extern int foo (void);

  int
  main (void)
  {
    return foo ();
  }
  $ cat foo.c
  extern int foo (void);

  int
  foo (void)
  {
    return 0;
  }

and main.c compiled with debug info, and foo.c without:

  $ gcc -g main.c -c
  $ gcc foo.c -c
  $ gcc -g main.o foo.o

In TUI mode, if we run to foo:

  $ gdb -q a.out -tui -ex "b foo" -ex run

it gets us "[ No Source Available ]":

  ┌─main.c─────────────────────────────────────────┐
  │                                                │
  │                                                │
  │                                                │
  │            [ No Source Available ]             │
  │                                                │
  │                                                │
  └────────────────────────────────────────────────┘
  (src) In: foo                  L??   PC: 0x400566
  ...
  Breakpoint 1, 0x0000000000400566 in foo ()
  (gdb)

But after resizing (pressing ctrl-<minus> in the gnome-terminal), we
get instead the source for main.c:

  ┌─main.c─────────────────────────────────────────┐
  │        3 int                                   │
  │        4 main (void)                           │
  │        5 {                                     │
  │        6   return foo ();                      │
  │        7 }                                     │
  │                                                │
  │                                                │
  └────────────────────────────────────────────────┘
  (src) In: foo                   L??   PC: 0x400566
  ...
  Breakpoint 1, 0x0000000000400566 in foo ()
  (gdb)

which is inappropriate because we're stopped in function foo, which is
not in main.c.

The problem is that, when the window is resized, GDB ends up calling
tui_source_window_base::rerender.  The rerender function has three
cases, one for when the window already has some source code
content (which is not the case here), a case for when the inferior is
active, and we have a selected frame (which is the case that applies
here), and a final case for when the inferior is not running.

For the case which we end up in, the source code window has no
content, but the inferior is running, so we have a selected frame, GDB
calls the get_current_source_symtab_and_line() function to get the
symtab_and_line for the current location.

The get_current_source_symtab_and_line() will actually return the last
recorded symtab and line location, not the current symtab and line
location.

What this means, is that, if the current location has no debug
information, get_current_source_symtab_and_line() will return any
previously recorded location, or failing that, the default (main)
location.

This behaviour of get_current_source_symtab_and_line() also causes
problems for the 'list' command.  Consider this pure CLI session:

  (gdb) break foo
  Breakpoint 1 at 0x40110a
  (gdb) run
  Starting program: /tmp/a.out

  Breakpoint 1, 0x000000000040110a in foo ()
  (gdb) list
  1	extern int foo (void);
  2
  3	int
  4	main (void)
  5	{
  6	  return foo ();
  7	}
  (gdb) list .
  Insufficient debug info for showing source lines at current PC (0x40110a).
  (gdb)

However, if we look at how GDB's TUI updates the source window during
a normal stop, we see that GDB does a better job of displaying the
expected contents.  Going back to our original example, when we start
GDB with:

  $ gdb -q a.out -tui -ex "b foo" -ex run

we do get the "[ No Source Available ]" message as expected.  Why is
that?

The answer is that, in this case GDB uses tui_show_frame_info to
update the source window, tui_show_frame_info is called each time a
prompt is displayed, like this:

  #0  tui_show_frame_info (fi=...) at ../../src/gdb/tui/tui-status.c:269
  bminor#1  0x0000000000f55975 in tui_refresh_frame_and_register_information () at ../../src/gdb/tui/tui-hooks.c:118
  bminor#2  0x0000000000f55ae8 in tui_before_prompt (current_gdb_prompt=0x31ef930 <top_prompt+16> "(gdb) ") at ../../src/gdb/tui/tui-hooks.c:165
  bminor#3  0x000000000090ea45 in std::_Function_handler<void(char const*), void (*)(char const*)>::_M_invoke (__functor=..., __args#0=@0x7ffc955106b0: 0x31ef930 <top_prompt+16> "(gdb) ") at /usr/include/c++/9/bits/std_function.h:300
  bminor#4  0x00000000009020df in std::function<void(char const*)>::operator() (this=0x5281260, __args#0=0x31ef930 <top_prompt+16> "(gdb) ") at /usr/include/c++/9/bits/std_function.h:688
  bminor#5  0x0000000000901c35 in gdb::observers::observable<char const*>::notify (this=0x31dda00 <gdb::observers::before_prompt>, args#0=0x31ef930 <top_prompt+16> "(gdb) ") at ../../src/gdb/../gdbsupport/observable.h:166
  bminor#6  0x00000000008ffed8 in notify_before_prompt (prompt=0x31ef930 <top_prompt+16> "(gdb) ") at ../../src/gdb/event-top.c:518
  bminor#7  0x00000000008fff08 in top_level_prompt () at ../../src/gdb/event-top.c:534
  bminor#8  0x00000000008ffdeb in display_gdb_prompt (new_prompt=0x0) at ../../src/gdb/event-top.c:487

If we look at how tui_show_frame_info figures out what source to
display, it doesn't use get_current_source_symtab_and_line(), instead,
it finds a symtab_and_line directly from a frame_info_pt.  This means
we are not dependent on get_current_source_symtab_and_line() returning
the current location (which it does not).

I propose that we change tui_source_window_base::rerender() so that,
for the case we are discussing here (the inferior has a selected
frame, but the source window has no contents), we move away from using
get_current_source_symtab_and_line(), and instead use find_frame_sal
instead, like tui_show_frame_info does.

This means that we will always use the inferior's current location.

Tested on x86_64-linux.

Reviewed-By: Tom de Vries <tdevries@suse.de>
Reported-By: Andrew Burgess <aburgess@redhat.com>
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32614
wataash pushed a commit to wataash/binutils-gdb that referenced this pull request Mar 19, 2025
…get_file_names

PR 32742 shows this failing:

    $ make check TESTS="gdb.ada/access_to_unbounded_array.exp" RUNTESTFLAGS="--target_board=fission"
    Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.ada/access_to_unbounded_array.exp ...
    FAIL: gdb.ada/access_to_unbounded_array.exp: scenario=all: gdb_breakpoint: set breakpoint at foo.adb:23 (GDB internal error)

Or, interactively:

    $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.ada/access_to_unbounded_array/foo-all -ex 'b foo.adb:23' -batch
    /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:19567: internal-error: set_lang: Assertion `old_value == language_unknown || old_value == language_minimal || old_value == lang' failed.

The symptom is that for a given dwarf2_per_cu, the language gets set
twice.  First, set to `language_ada`, and then, to `language_minimal`.
It's unexpected for the language of a CU to get changed like this.

The CU at offset 0x0 in the main file looks like:

    0x00000000: Compile Unit: length = 0x00000030, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x00000034)

    0x0000000b: DW_TAG_compile_unit
                  DW_AT_low_pc [DW_FORM_addr]       (0x000000000000339a)
                  DW_AT_high_pc [DW_FORM_data8]     (0x0000000000000432)
                  DW_AT_stmt_list [DW_FORM_sec_offset]      (0x00000000)
                  DW_AT_GNU_dwo_name [DW_FORM_strp] ("b~foo.dwo")
                  DW_AT_comp_dir [DW_FORM_strp]     ("/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.ada/access_to_unbounded_array")
                  DW_AT_GNU_pubnames [DW_FORM_flag_present] (true)
                  DW_AT_GNU_addr_base [DW_FORM_sec_offset]  (0x00000000)
                  DW_AT_GNU_dwo_id [DW_FORM_data8]  (0x277aee54e7bd47f7)

This refers to the DWO file b~foo.dwo, whose top-level DIE is:

    .debug_info.dwo contents:
    0x00000000: Compile Unit: length = 0x00000b63, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x00000b67)

    0x0000000b: DW_TAG_compile_unit
                  DW_AT_producer [DW_FORM_GNU_str_index]    ("GNU Ada 14.2.1 20250207 -fgnat-encodings=minimal -gdwarf-4 -fdebug-types-section -fuse-ld=gold -gnatA -gnatWb -gnatiw -gdwarf-4 -gsplit-dwarf -ggnu-pubnames -gnatws -mtune=generic -march=x86-64")
                  DW_AT_language [DW_FORM_data1]    (DW_LANG_Ada95)
                  DW_AT_name [DW_FORM_GNU_str_index]        ("/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.ada/access_to_unbounded_array/b~foo.adb")
                  DW_AT_comp_dir [DW_FORM_GNU_str_index]    ("/home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.ada/access_to_unbounded_array")
                  DW_AT_GNU_dwo_id [DW_FORM_data8]  (0xdbeffefab180a2cb)

The thing to note is that the language attribute is only present in the
DIE in the DWO file, not on the DIE in the main file.

The first time the language gets set is here:

    #0  dwarf2_per_cu::set_lang (this=0x50f0000044b0, lang=language_ada, dw_lang=DW_LANG_Ada95) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:20788
    bminor#1  0x0000555561666af6 in cutu_reader::prepare_one_comp_unit (this=0x7ffff10bf2b0, cu=0x51700008e000, pretend_language=language_minimal) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:21029
    bminor#2  0x000055556159f740 in cutu_reader::cutu_reader (this=0x7ffff10bf2b0, this_cu=0x50f0000044b0, per_objfile=0x516000066080, abbrev_table=0x510000004640, existing_cu=0x0, skip_partial=false, pretend_language=language_minimal, cache=0x7ffff11b95e0) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:3371
    bminor#3  0x00005555615a547a in process_psymtab_comp_unit (this_cu=0x50f0000044b0, per_objfile=0x516000066080, storage=0x7ffff11b95e0) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:3799
    bminor#4  0x00005555615a9292 in cooked_index_worker_debug_info::process_cus (this=0x51700008dc80, task_number=0, first=std::unique_ptr<dwarf2_per_cu> = {...}, end=std::unique_ptr<dwarf2_per_cu> = {...}) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:4122

In this code path (particularly this specific cutu_reader constructir),
the work is done to find and read the DWO file.  So the language is
properly identifier as language_ada, all good so far.

The second time the language gets set is:

    #0  dwarf2_per_cu::set_lang (this=0x50f0000044b0, lang=language_minimal, dw_lang=0) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:20788
    bminor#1  0x0000555561666af6 in cutu_reader::prepare_one_comp_unit (this=0x7ffff0f42730, cu=0x517000091b80, pretend_language=language_minimal) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:21029
    bminor#2  0x00005555615a1822 in cutu_reader::cutu_reader (this=0x7ffff0f42730, this_cu=0x50f0000044b0, per_objfile=0x516000066080, pretend_language=language_minimal, parent_cu=0x0, dwo_file=0x0) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:3464
    bminor#3  0x000055556158c850 in dw2_get_file_names (this_cu=0x50f0000044b0, per_objfile=0x516000066080) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1956
    bminor#4  0x000055556158f4f5 in dw_expand_symtabs_matching_file_matcher (per_objfile=0x516000066080, file_matcher=...) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:2157
    bminor#5  0x00005555616329e2 in cooked_index_functions::expand_symtabs_matching (this=0x50200002ab50, objfile=0x516000065780, file_matcher=..., lookup_name=0x0, symbol_matcher=..., expansion_notify=..., search_flags=..., domain=..., lang_matcher=...) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:15912
    bminor#6  0x0000555562ca8a14 in objfile::map_symtabs_matching_filename (this=0x516000065780, name=0x50200002ad90 "break pck.adb", real_path=0x0, callback=...) at /home/smarchi/src/binutils-gdb/gdb/symfile-debug.c:207
    bminor#7  0x0000555562d68775 in iterate_over_symtabs (pspace=0x513000005600, name=0x50200002ad90 "break pck.adb", callback=...) at /home/smarchi/src/binutils-gdb/gdb/symtab.c:727

Here, we use the other cutu_reader constructor, the one that does not
look up the DWO file for the passed CU.  If a DWO file exists for this
CU, the caller is expected to pass it as a parameter.  That cutu_reader
constructor also ends up setting the language of the CU.  But because it
didn't read the DWO file, it didn't figure out the language is
language_ada, so it tries to set the language to the default,
language_minimal.

A question is: why do we end up trying to set the CU's language is this
context.  This is completely unrelated to what we're trying to do, that
is get the file names from the line table.  Setting the language is a
side-effect of just constructing a cutu_reader, which we need to look up
attributes in dw2_get_file_names_reader.  There are probably some
cleanups to be done here, to avoid doing useless work like looking up
and setting the CU's language when all we need is an object to help
reading the DIEs and attributes.  But that is future work.

The same cutu_reader constructor is used in
`dwarf2_per_cu::ensure_lang`.  Since this is the version of cutu_reader
that does not look up the DWO file, it will conclude that the language
is language_minimal and set that as the CU's language.  In other words,
`dwarf2_per_cu::ensure_lang` will get the language wrong, pretty ironic.

Fix this by using the other cutu_reader constructor in those two spots.
Pass `per_objfile->get_cu (this_cu)`, as the `existing_cu` parameter.  I
think this is necessary, because that constructor has an assert to check
that if `existing_cu` is nullptr, then there must not be an existing
`dwarf2_cu` in the per_objfile.

To avoid getting things wrong like this, I think that the second
cutu_reader constructor should be reserved for the spots that do pass a
non-nullptr dwo_file.  The only spot at the moment in
create_cus_hash_table, where we read multiple units from the same DWO
file.  In this context, I guess it makes sense for efficiency to get the
dwo_file once and pass it down to cutu_reader.  For that constructor,
make the parameters non-optional, add "non-nullptr" asserts, and update
the code to assume the passed values are not nullptr.

What I don't know is if this change is problematic thread-wise, if the
functions I have modified to use the other cutu_reader constructor can
be called concurrently in worker threads.  If so, I think it would be
problematic.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32742
Change-Id: I980d16875b9a43ab90e251504714d0d41165c7c8
Approved-By: Tom Tromey <tom@tromey.com>
wataash pushed a commit to wataash/binutils-gdb that referenced this pull request Mar 19, 2025
…_all_symtabs

Commit 2920415 ("gdb/dwarf: use ranged for loop in some spots")
broke some tests notably gdb.base/maint.exp with the fission board.

    $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.base/maint/maint -ex start -ex "maint expand-sym" -batch
    ...
    Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc48, envp=0x7fffffffdc58) at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/break.c:43
    43          if (argc == 12345) {  /* an unlikely value < 2^16, in case uninited */ /* set breakpoint 6 here */
    /usr/include/c++/14.2.1/debug/safe_iterator.h:392:
    In function:
        gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>&
        gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>::operator++()
        [with _Iterator = gnu_cxx::
        normal_iterator<std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>*,
        std::vector<std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>,
        std::allocator<std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter> > >
        >; _Sequence = std::debug::vector<std::unique_ptr<dwarf2_per_cu,
        dwarf2_per_cu_deleter> >; _Category = std::forward_iterator_tag]

    Error: attempt to increment a singular iterator.

Note that this is caught because I build with -D_GLIBCXX_DEBUG=1.
Otherwise, it might crash more randomly, or just not crash at all (but
still be buggy).

While iterating on the all_units vector, some type units get added
there:

    #0  add_type_unit (per_bfd=0x51b000044b80, section=0x50e0000c2280, sect_off=0, length=74, sig=4367013491293299229) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:2576
    bminor#1  0x00005555618a3a40 in lookup_dwo_signatured_type (cu=0x51700009b580, sig=4367013491293299229) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:2664
    bminor#2  0x00005555618ee176 in queue_and_load_dwo_tu (dwo_unit=0x521000120e00, cu=0x51700009b580) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:8329
    bminor#3  0x00005555618eeafe in queue_and_load_all_dwo_tus (cu=0x51700009b580) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:8366
    bminor#4  0x00005555618966a6 in dw2_do_instantiate_symtab (per_cu=0x50f0000043c0, per_objfile=0x516000065a80, skip_partial=true) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1695
    bminor#5  0x00005555618968d4 in dw2_instantiate_symtab (per_cu=0x50f0000043c0, per_objfile=0x516000065a80, skip_partial=true) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1719
    bminor#6  0x000055556189ac3f in dwarf2_base_index_functions::expand_all_symtabs (this=0x502000024390, objfile=0x516000065780) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1977

This invalidates the iterator in
dwarf2_base_index_functions::expand_all_symtabs, which is caught by the
libstdc++ debug mode.

I'm not entirely sure that it is correct to append type units from dwo
files to the all_units vector like this.  The
dwarf2_find_containing_comp_unit function expects a precise ordering of
the elements of the all_units vector, to be able to do a binary search.
Appending a type unit at the end at this point certainly doesn't respect
that ordering.

For now I'd just like to undo the regression.  Do that by using
all_units_range in the ranged for loop.  I will keep in mind to
investigate whether this insertion of type units in all_units after the
fact really makes sense or not.

Change-Id: Iec131e59281cf2dbd12d3f3d163b59018fdc54da
wataash pushed a commit to wataash/binutils-gdb that referenced this pull request Mar 19, 2025
On Debian 12, with gcc 12 and ld 2.40, I get some failures when running:

    $ make check TESTS="gdb.base/style.exp" RUNTESTFLAGS="--target_board=fission"

I think I stumble on this bug [1], preventing the test from doing
anything that requires expanding the compilation unit:

    $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.base/style/style
    Reading symbols from testsuite/outputs/gdb.base/style/style...
    (gdb) p main
    DW_FORM_strp pointing outside of .debug_str section [in module /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/style/style]
    (gdb)

The error is thrown here:

    #0  0x00007ffff693f0a1 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6
    bminor#1  0x0000555569ce6852 in throw_it(return_reason, errors, const char *, typedef __va_list_tag __va_list_tag *) (reason=RETURN_ERROR, error=GENERIC_ERROR, fmt=0x555562a9fc40 "%s pointing outside of %s section [in module %s]", ap=0x7fffffff8df0) at /home/smarchi/src/binutils-gdb/gdbsupport/common-exceptions.cc:203
    bminor#2  0x0000555569ce690f in throw_verror (error=GENERIC_ERROR, fmt=0x555562a9fc40 "%s pointing outside of %s section [in module %s]", ap=0x7fffffff8df0) at /home/smarchi/src/binutils-gdb/gdbsupport/common-exceptions.cc:211
    bminor#3  0x000055556879c0cb in verror (string=0x555562a9fc40 "%s pointing outside of %s section [in module %s]", args=0x7fffffff8df0) at /home/smarchi/src/binutils-gdb/gdb/utils.c:193
    bminor#4  0x0000555569cfa88d in error (fmt=0x555562a9fc40 "%s pointing outside of %s section [in module %s]") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:45
    bminor#5  0x000055556667dbff in dwarf2_section_info::read_string (this=0x61b000042a08, objfile=0x616000055e80, str_offset=262811, form_name=0x555562886b40 "DW_FORM_strp") at /home/smarchi/src/binutils-gdb/gdb/dwarf2/section.c:211
    bminor#6  0x00005555662486b7 in dwarf_decode_macro_bytes (per_objfile=0x616000056180, builder=0x614000006040, abfd=0x6120000f4b40, mac_ptr=0x60300004f5be "", mac_end=0x60300004f5bb "\002\004", current_file=0x62100007ad70, lh=0x60f000028bd0, section=0x61700008ba78, section_is_gnu=1, section_is_dwz=0, offset_size=4, str_section=0x61700008bac8, str_offsets_section=0x61700008baf0, str_offsets_base=std::optional<unsigned long> = {...}, include_hash=..., cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/macro.c:511
    bminor#7  0x000055556624af0e in dwarf_decode_macros (per_objfile=0x616000056180, builder=0x614000006040, section=0x61700008ba78, lh=0x60f000028bd0, offset_size=4, offset=0, str_section=0x61700008bac8, str_offsets_section=0x61700008baf0, str_offsets_base=std::optional<unsigned long> = {...}, section_is_gnu=1, cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/macro.c:934
    bminor#8  0x000055556642cb82 in dwarf_decode_macros (cu=0x61700008b600, offset=0, section_is_gnu=1) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:19435
    bminor#9  0x000055556639bd12 in read_file_scope (die=0x6210000885c0, cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:6366
    bminor#10 0x0000555566392d99 in process_die (die=0x6210000885c0, cu=0x61700008b600) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:5310
    bminor#11 0x0000555566390d72 in process_full_comp_unit (cu=0x61700008b600, pretend_language=language_minimal) at /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:5075

The exception is then only caught at the event-loop level
(start_event_loop), causing the whole debug info reading process to be
aborted.  I think it's a little harsh, considering that a lot of things
could work even if we failed to read macro information.

Catch the exception inside read_file_scope, print the exception, and
carry on.  We could go even more fine-grained: if reading the string for
one macro definition fails, we could continue reading the macro
information.  Perhaps it's just that one macro definition that is
broken.  However, I don't need this level of granularity, so I haven't
attempted this.  Also, my experience is that macro reading fails when
the compiler or linker has a bug, in which case pretty much everything
is messed up.

With this patch, it now looks like:

    $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.base/style/style
    Reading symbols from testsuite/outputs/gdb.base/style/style...
    (gdb) p main
    While reading section .debug_macro.dwo: DW_FORM_strp pointing outside of .debug_str section [in module /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/style/style]
    $1 = {int (int, char **)} 0x684 <main>
    (gdb)

In the test I am investigating (gdb.base/style.exp with the fission
board), it allows more tests to run:

    -# of expected passes           107
    -# of unexpected failures       17
    +# of expected passes           448
    +# of unexpected failures       19

Of course, we still see the error about the macro information, and some
macro-related tests still fail (those would be kfailed ideally), but
many tests that are not macro-dependent now pass.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111409

Change-Id: I0bdb01f153eff23c63c96ce3f41114bb027e5796
Approved-By: Tom Tromey <tom@tromey.com>
theverygaming pushed a commit to theverygaming/binutils-gdb that referenced this pull request Mar 28, 2025
Currently GDB's source cache doesn't track whether the entries within
the cache are styled or not.  This is pretty much fine, the assumption
is that any time we are fetching source code, we do so in order to
print it to the terminal, so where possible we always want styling
applied, and if styling is not applied, then it is because that file
cannot be styled for some reason.

Changes to 'set style enabled' cause the source cache to be flushed,
so future calls to fetch source code will regenerate the cache entries
with styling enabled or not as appropriate.

But this all assumes that styling is either on or off, and that
switching between these two states isn't done very often.

However, the Python API allows for individual commands to be executed
with styling turned off via gdb.execute().  See commit:

  commit e5348a7
  Date:   Thu Feb 13 15:39:31 2025 +0000

      gdb/python: new styling argument to gdb.execute

Currently the source cache doesn't handle this case. Consider this:

  (gdb) list main
  ... snip, styled source code displayed here ...
  (gdb) python gdb.execute("list main", True, False, False)
  ... snip, styled source code is still shown here ...

In the second case, the final `False` passed to gdb.execute() is
asking for unstyled output.

The problem is that, `get_source_lines` calls `ensure` to prime the
cache for the file in question, then `extract_lines` just pulls the
lines of interest from the cached contents.

In `ensure`, if there is a cache entry for the desired filename, then
that is considered good enough.  There is no consideration about
whether the cache entry is styled or not.

This commit aims to fix this, after this commit, the `ensure` function
will make sure that the cache entry used by `get_source_lines` is
styled correctly.

I think there are two approaches I could take:

  1. Allow multiple cache entries for a single file, a styled, and
     non-styled entry.  The `ensure` function would then place the
     correct cache entry into the last position so that
     `get_source_lines` would use the correct entry, or

  2. Have `ensure` recalculate entries if the required styling mode is
     different to the styling mode of the current entry.

Approach bminor#1 is better if we are rapidly switching between styling
modes, while bminor#2 might be better if we want to keep more files in the
cache and we only rarely switch styling modes.

In the end I chose approach bminor#2, but the good thing is that the changes
are all contained within the `ensure` function.  If in the future we
wanted to change to strategy bminor#1, this could be done transparently to
the rest of GDB.

So after this commit, the `ensure` function checks if styling is
currently possible or not.  If it is not, and the current entry is
styled, then the current entry only is dropped from the cache, and a
new, unstyled entry is created.  Likewise, if the current entry is
non-styled, but styling is required, we drop one entry and
recalculate.

With this change in place, I have updated set_style_enabled (in
cli/cli-style.c) so the source cache is no longer flushed when the
style settings are changed, the source cache will automatically handle
changes to the style settings now.

This problem was discovered in PR gdb/32676.

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

Approved-By: Tom Tromey <tom@tromey.com>
theverygaming pushed a commit to theverygaming/binutils-gdb that referenced this pull request Mar 28, 2025
Modifying inline-frame-cycle-unwind.exp to use `bt -no-filters` produces
the following incorrect backtrace:

  #0  inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:49
  bminor#1  normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
  bminor#2  0x000055555555517f in inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:50
  bminor#3  normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
  (gdb) FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1

The expected output, which we get with `bt`, is:

  #0  inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:49
  bminor#1  normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
  (gdb) PASS: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1

The cycle checking in `get_prev_frame_maybe_check_cycle` relies on newer
frame ids having already been computed and stashed.  Unlike other
frames, frame #0's id does not get computed immediately.

The test passes with `bt` because when applying python frame filters,
the call to `bootstrap_python_frame_filters` happens to compute the id
of frame #0.  When `get_prev_frame_maybe_check_cycle` later tries to
stash frame bminor#2's id, the cycle is detected.

The test fails with `bt -no-filters` because frame #0's id has not been
stashed by the time `get_prev_frame_maybe_check_cycle` tries to stash
frame bminor#2's id which succeeds and the cycle is only detected later when
trying to stash frame bminor#4's id.  Doing `stepi` after the incorrect
backtrace would then trigger an assertion failure when trying to stash
frame #0's id because it is a duplicate of bminor#2's already stashed id.

In `get_prev_frame_always_1`, if this_frame is inline frame 0, then
compute and stash its frame id before returning the previous frame.
This ensures that the id of inline frame 0 has been stashed before
`get_prev_frame_maybe_check_cycle` is called on older frames.

The test case has been updated to run both `bt` and `bt -no-filters`.

Co-authored-by: Andrew Burgess <aburgess@redhat.com>
mikpe referenced this pull request in mikpe/binutils-gdb Mar 30, 2025
Consider this backtrace within GDB:

  #0  notify_breakpoint_modified (b=0x57d31d0) at ../../src/gdb/breakpoint.c:1083
  #1  0x00000000005b6406 in breakpoint_set_commands (b=0x57d31d0, commands=...) at ../../src/gdb/breakpoint.c:1523
  #2  0x00000000005c8c63 in update_dprintf_command_list (b=0x57d31d0) at ../../src/gdb/breakpoint.c:8641
  #3  0x00000000005d3c4e in dprintf_breakpoint::re_set (this=0x57d31d0) at ../../src/gdb/breakpoint.c:12476
  bminor#4  0x00000000005d6347 in breakpoint_re_set () at ../../src/gdb/breakpoint.c:13298

Whenever breakpoint_re_set is called we re-build the commands that the
dprintf b/p will execute and store these into the breakpoint.  The
commands are re-built in update_dprintf_command_list and stored into
the breakpoint object in breakpoint_set_commands.

Now sometimes these commands can change, dprintf_breakpoint::re_set
explains one case where this can occur, and I'm sure there must be
others.  But in most cases the commands we recalculate will not
change.  This means that the breakpoint modified event which is
emitted from breakpoint_set_commands is redundant.

This commit aims to eliminate the redundant breakpoint modified events
for dprintf breakpoints.  This is done by adding a commands_equal call
to the start of breakpoint_set_commands.

The commands_equal function is a new function which compares two
command_line objects and returns true if they are identical.  Using
this function we can check if the new commands passed to
breakpoint_set_commands are identical to the breakpoint's existing
commands.  If the new commands are equal then we don't need to change
anything on the new breakpoint, and the breakpoint modified event can
be skipped.

The test for this commit stops at a dlopen() call in the inferior,
sets up a dprintf breakpoint, then uses 'next' to step over the
dlopen() call.  When the library loads GDB call breakpoint_re_set,
which calls dprintf_breakpoint::re_set.  But in this case we don't
expect the calculated command string to change, so we don't expect to
see the breakpoint modified event.
mikpe referenced this pull request in mikpe/binutils-gdb Apr 21, 2025
…uence_raw

The Linaro CI reported a regression [1] in test-case
gdb.base/step-over-syscall.exp due to commit 674d485 ("[gdb/testsuite] Fix
gdb.base/step-over-syscall.exp with glibc 2.41").

Investigation shows that it's a progression in the sense that the test-case
fails at a later point than before.

The cause for the test-case failure is that an atomic sequence
ldaex/adds/strex is not skipped over when instruction stepping, leading to a
hang (in the sense of not being able to instruction-step out of the loop
containing the atomic sequence).

The arm target does have support for recognizing atomic sequences, but it
fails because it doesn't recognize the ldaex insn.

Fix this by:
- adding a new function ldaex_p which recognizes ldaex instructions, based
  on information found in opcodes/arm-dis.c, and
- using ldaex_p in thumb_deal_with_atomic_sequence_raw.

I was not able to reproduce the failure in its original setting, but I
was able to do so using a test.c:
...
static void exit (int status) {
  while (1)
    ;
}
void _start (void) {
  int a = 0;
  __atomic_fetch_add (&a, 1, __ATOMIC_ACQUIRE);
  exit (0);
}
...
compiled like this:
...
$ gcc test.c -march=armv8-a -mfloat-abi=soft -nostdlib -static
...
giving this atomic sequence of 32-bit Thumb-2 instructions:
...
   100ce:       e8d3 1fef       ldaex   r1, [r3]
   100d2:       f101 0101       add.w   r1, r1, #1
   100d6:       e843 1200       strex   r2, r1, [r3]
...

Without the fix, after 100 stepi's we're still in _start (and likewise with
10.000 stepi's):
...
$ gdb -q -batch a.out -ex 'display /i $pc' -ex starti -ex "stepi 100"
  ...
0x000100dc in _start ()
1: x/i $pc
=> 0x100dc <_start+26>:	bne.n	0x100ce <_start+12>
...
but with the fix we've managed to progress to exit:
...
$ gdb -q -batch a.out -ex 'display /i $pc' -ex starti -ex "stepi 100"
  ...
0x000100c0 in exit ()
1: x/i $pc
=> 0x100c0 <exit+8>:	b.n	0x100c0 <exit+8>
...

Having addressed the "-mthumb" case, do we need a similar fix for "-marm"?

Adding "-marm" in the compilation line mentioned above gives the following
atomic sequence:
...
   100e4:       e1931e9f        ldaex   r1, [r3]
   100e8:       e2811001        add     r1, r1, #1
   100ec:       e1832f91        strex   r2, r1, [r3]
...
and gdb already recognizes it as such because of this statement:
...
  if ((insn & 0xff9000f0) != 0xe1900090)
    return {};
...

The trouble with this statement is that it requires knowledge of arm
instruction encoding to understand which cases it does and doesn't cover.

Note that the corresponding comment only mentions ldrex:
...
  /* Assume all atomic sequences start with a ldrex{,b,h,d} instruction. ... */
...
but evidently at least some forms of ldaex are also detected.

So, also use ldaex_p in arm_deal_with_atomic_sequence_raw.  This may or may
not be redundant, but at least ldaex_p is explicit and precise about what it
supports.

Likewise for stlex (generated when using __ATOMIC_RELEASE instead of
__ATOMIC_ACQUIRE in the example above).

Tested in arm-linux chroot on aarch64-linux.

Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Co-Authored-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>

PR tdep/32796
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32796

[1] https://linaro.atlassian.net/browse/GNU-1541
9xbt pushed a commit to 9xbt/x86_64-bentobox-elf-binutils that referenced this pull request May 8, 2025
When building with gcc, with flags -gdwarf-5, -gsplit-dwarf and
-fdebug-types-section, the resulting .dwo files contain multiple
.debug_info.dwo sections.  One for each type unit and one for the
compile unit.  This is correct, as per DWARF 5, section F.2.3 ("Contents
of the Split DWARF Object Files"):

    The split DWARF object files each contain the following sections:

        ...
        .debug_info.dwo (for the compilation unit)
        .debug_info.dwo (one COMDAT section for each type unit)
	...

GDB currently assumes that there is a single .debug_info.dwo section,
causing unpredictable behavior.  For example, sometimes this crash:

    ==81781==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x508000007a71 at pc 0x58704d32a59c bp 0x7ffc0acc0bb0 sp 0x7ffc0acc0ba0
    READ of size 1 at 0x508000007a71 thread T0
        #0 0x58704d32a59b in bfd_getl32 /home/smarchi/src/binutils-gdb/bfd/libbfd.c:846
        bminor#1 0x58704ae62dce in read_initial_length(bfd*, unsigned char const*, unsigned int*, bool) /home/smarchi/src/binutils-gdb/gdb/dwarf2/leb.c:92
        bminor#2 0x58704aaf76bf in read_comp_unit_head(comp_unit_head*, unsigned char const*, dwarf2_section_info*, rcuh_kind) /home/smarchi/src/binutils-gdb/gdb/dwarf2/comp-unit-head.c:47
        bminor#3 0x58704aaf8f97 in read_and_check_comp_unit_head(dwarf2_per_objfile*, comp_unit_head*, dwarf2_section_info*, dwarf2_section_info*, unsigned char const*, rcuh_kind) /home/smarchi/src/binutils-gdb/gdb/dwarf2/comp-unit-head.c:193
        bminor#4 0x58704b022908 in create_dwo_unit_hash_tables /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:6233
        bminor#5 0x58704b0334a5 in open_and_init_dwo_file /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:7588
        bminor#6 0x58704b03965a in lookup_dwo_cutu /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:7935
        bminor#7 0x58704b03a5b1 in lookup_dwo_comp_unit /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:8009
        bminor#8 0x58704aff5b70 in lookup_dwo_unit /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:2802

The first time that locate_dwo_sections gets called for a
.debug_info.dwo section, dwo_sections::info gets initialized properly.
The second time it gets called for a .debug_info.dwo section, the size
field in dwo_sections::info gets overwritten with the size of the second
section.  But the buffer remains pointing to the contents of the first
section, because the section is already "read in".  So the size does not
match the buffer.  And even if it did, we would only keep the
information about one .debug_info.dwo, out of the many.

First, add an assert in locate_dwo_sections to make sure we don't
try to fill in a dwo section info twice.  Add the assert to other
functions with the same pattern, while at it.

Then, change dwo_sections::info to be a vector of sections (just like we
do for type sections).  Update locate_dwo_sections to append to that
vector when seeing a new .debug_info.dwo section.  Update
open_and_init_dwo_file to read the units from each section.

The problem can be observed by running some tests with the
dwarf5-fission-debug-types target board.  For example,
gdb.base/condbreak.exp crashes (with the ASan failure shown above)
before the patch and passes after).

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119766

Change-Id: Iedf275768b6057dee4b1542396714f3d89903cf3
Reviewed-By: Tom de Vries <tdevries@suse.de>
9xbt pushed a commit to 9xbt/x86_64-bentobox-elf-binutils that referenced this pull request May 8, 2025
On arm-linux, with test-case gdb.python/py-missing-objfile.exp I get:
...
(gdb) whatis global_exec_var^M
type = volatile exec_type^M
(gdb) FAIL: $exp: initial sanity check: whatis global_exec_var
...
instead of the expected "type = volatile struct exec_type".

The problem is that the current language is ASM instead of C, because the
inner frame at the point of the core dump has language ASM:
...
 #0  __libc_do_syscall () at libc-do-syscall.S:47
 bminor#1  0xf7882920 in __pthread_kill_implementation () at pthread_kill.c:43
 bminor#2  0xf784df22 in __GI_raise (sig=sig@entry=6) at raise.c:26
 bminor#3  0xf783f03e in __GI_abort () at abort.c:73
 bminor#4  0x009b0538 in dump_core () at py-missing-objfile.c:34
 bminor#5  0x009b0598 in main () at py-missing-objfile.c:46
...

Fix this by manually setting the language to C.

Tested on arm-linux and x86_64-linux.

PR testsuite/32445
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32445
9xbt pushed a commit to 9xbt/x86_64-bentobox-elf-binutils that referenced this pull request May 8, 2025
With the test-case contained in the patch, and gdb build with
-fsanitize=address we get:
...
==23678==ERROR: AddressSanitizer: heap-buffer-overflow ...^M
READ of size 1 at 0x6020000c30dc thread T3^[[1m^[[0m^M
ptype global_var^M
    #0 0x2c6a40b in bfd_getl32 bfd/libbfd.c:846^M
    bminor#1 0x168f96c in read_str_index gdb/dwarf2/read.c:15349^M
...

The executable contains an out-of-bounds DW_FORM_strx attribute:
...
$ readelf -wi $exec
<2eb>   DW_AT_name        :readelf: Warning: string index of 1 converts to \
  an offset of 0xc which is too big for section .debug_str
 (indexed string: 0x1): <string index too big>
...
and read_str_index doesn't check for this:
...
  info_ptr = (str_offsets_section->buffer
	      + str_offsets_base
	      + str_index * offset_size);
   if (offset_size == 4)
     str_offset = bfd_get_32 (abfd, info_ptr);
...
and consequently reads out-of-bounds.

Fix this in read_str_index by checking for the out-of-bounds condition and
throwing a DWARF error:
...
(gdb) ptype global_var
DWARF Error: Offset from DW_FORM_GNU_str_index or DW_FORM_strx pointing \
  outside of .debug_str_offsets section in CU at offset 0x2d7 \
  [in module dw-form-strx-out-of-bounds]
No symbol "global_var" in current context.
(gdb)
...

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>
mikpe referenced this pull request in mikpe/binutils-gdb May 11, 2025
The fix for PR22988 in 2018 added a new operand AARCH64_OPND_SVE_ADDR_R
to support implicit XZR offsets, but this fix had several flaws that
meant it accepted several invalid addressing modes:

1. The base register type wasn't properly checked when the optional
register offset was omitted.  This meant that
  ldff1b {z1.s}, p1/z,[z1.d]
was parsed as if it were
  ldff1b z1.d, p1/z, [x1.d, xzr].

2. The explicit offset parsing didn't include a shift type, so the new
operand would incorrectly parse
  ldff1h{z0.s}, p0/z, [x0, x0]
as if it were
  ldff1h{z0.s}, p0/z, [x0, x0, lsl #1].

3. Regardless of the above correctness issues, support for implicit
offsets should have been added by amending the operands in the existing
opcode table entries, instead of adding new duplicate table entires.

Issue 1 can be fixed by using an "if" instead of an "else if" in
parse_operands, while issue 2 can be fixed by failing when the first
condition is false.  This patch applies just these two fixes, leaving
issue 3 to be addressed in a subsequent more invasive patch.

The instructions removed from the test sme-5.d are architecturally
invalid. The new tests cover all of the affected ldff1 variants; the
issue also affected SME ZA ld1*/st1* instructions using the same operand
type.
MBeijer pushed a commit to AmigaPorts/binutils-gdb that referenced this pull request Jun 1, 2025
mikpe referenced this pull request in mikpe/binutils-gdb Jun 9, 2025
I decided to try to build and test gdb on Windows.

I found a page on the wiki [1] suggesting three ways of building gdb:
- MinGW,
- MinGW on Cygwin, and
- Cygwin.

I picked Cygwin, because I've used it before (though not recently).

I managed to install Cygwin and sufficient packages to build gdb and start the
testsuite.

However, testsuite progress ground to a halt at gdb.base/branch-to-self.exp.
[ AFAICT, similar problems reported here [2]. ]

I managed to reproduce this hang by running just the test-case.

I attempted to kill the hanging processes by:
- first killing the inferior process, using the cygwin "kill -9" command, and
- then killing the gdb process, likewise.

But the gdb process remained, and I had to point-and-click my way through task
manager to actually kill the gdb process.

I investigated this by attaching to the hanging gdb process.  Looking at the
main thread, I saw it was stopped in a call to WaitForSingleObject, with
the dwMilliseconds parameter set to INFINITE.

The backtrace in more detail:
...
(gdb) bt
 #0  0x00007fff196fc044 in ntdll!ZwWaitForSingleObject () from
     /cygdrive/c/windows/SYSTEM32/ntdll.dll
 #1  0x00007fff16bbcdcf in WaitForSingleObjectEx () from
     /cygdrive/c/windows/System32/KERNELBASE.dll
 #2  0x0000000100998065 in wait_for_single (handle=0x1b8, howlong=4294967295) at
     gdb/windows-nat.c:435
 #3  0x0000000100999aa7 in
     windows_nat_target::do_synchronously(gdb::function_view<bool ()>)
       (this=this@entry=0xa001c6fe0, func=...) at gdb/windows-nat.c:487
 bminor#4  0x000000010099a7fb in windows_nat_target::wait_for_debug_event_main_thread
     (event=<optimized out>, this=0xa001c6fe0)
     at gdb/../gdbsupport/function-view.h:296
 bminor#5  windows_nat_target::kill (this=0xa001c6fe0) at gdb/windows-nat.c:2917
 bminor#6  0x00000001008f2f86 in target_kill () at gdb/target.c:901
 bminor#7  0x000000010091fc46 in kill_or_detach (from_tty=0, inf=0xa000577d0)
     at gdb/top.c:1658
 bminor#8  quit_force (exit_arg=<optimized out>, from_tty=from_tty@entry=0)
     at gdb/top.c:1759
 bminor#9  0x00000001004f9ea8 in quit_command (args=args@entry=0x0,
     from_tty=from_tty@entry=0) at gdb/cli/cli-cmds.c:483
 bminor#10 0x000000010091c6d0 in quit_cover () at gdb/top.c:295
 bminor#11 0x00000001005e3d8a in async_disconnect (arg=<optimized out>)
     at gdb/event-top.c:1496
 bminor#12 0x0000000100499c45 in invoke_async_signal_handlers ()
     at gdb/async-event.c:233
 bminor#13 0x0000000100eb23d6 in gdb_do_one_event (mstimeout=mstimeout@entry=-1)
     at gdbsupport/event-loop.cc:198
 bminor#14 0x00000001006df94a in interp::do_one_event (mstimeout=-1,
     this=<optimized out>) at gdb/interps.h:87
 bminor#15 start_event_loop () at gdb/main.c:402
 bminor#16 captured_command_loop () at gdb/main.c:466
 #17 0x00000001006e2865 in captured_main (data=0x7ffffcba0) at gdb/main.c:1346
 #18 gdb_main (args=args@entry=0x7ffffcc10) at gdb/main.c:1365
 #19 0x0000000100f98c70 in main (argc=10, argv=0xa000129f0) at gdb/gdb.c:38
...

In the docs [3], I read that using an INFINITE argument to WaitForSingleObject
might cause a system deadlock.

This prompted me to try this simple change in wait_for_single:
...
   while (true)
     {
-      DWORD r = WaitForSingleObject (handle, howlong);
+      DWORD r = WaitForSingleObject (handle,
+                                     howlong == INFINITE ? 100 : howlong);
+      if (howlong == INFINITE && r == WAIT_TIMEOUT)
+        continue;
...
with the timeout of 0.1 second estimated to be:
- small enough for gdb to feel reactive, and
- big enough not to consume too much cpu cycles with looping.

And indeed, the test-case, while still failing, now finishes in ~50 seconds.

While there may be an underlying bug that triggers this behaviour, the failure
mode is so severe that I consider it a bug in itself.

Fix this by avoiding calling WaitForSingleObject with INFINITE argument.

Tested on x86_64-cygwin, by running the testsuite past the test-case.

Approved-By: Pedro Alves <pedro@palves.net>

PR tdep/32894
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32894

[1] https://sourceware.org/gdb/wiki/BuildingOnWindows
[2] https://sourceware.org/pipermail/gdb-patches/2025-May/217949.html
[3] https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject
jiapei100 pushed a commit to LongerVisionRobot/binutils-gdb that referenced this pull request Jun 20, 2025
Running gdbserver under Valgrind I get:

  ==26925== Conditional jump or move depends on uninitialised value(s)
  ==26925==    at 0x473E7F: i387_cache_to_xsave(regcache*, void*) (i387-fp.c:579)
  ==26925==    by 0x46E3ED: x86_fill_xstateregset(regcache*, void*) (linux-x86-low.c:418)
  ==26925==    by 0x45E747: regsets_store_inferior_registers(regsets_info*, regcache*) (linux-low.c:5456)
  ==26925==    by 0x45EEF8: linux_store_registers(regcache*, int) (linux-low.c:5731)
  ==26925==    by 0x426441: regcache_invalidate_thread(thread_info*) (regcache.c:89)
  ==26925==    by 0x45CCAF: linux_resume_one_lwp_throw(lwp_info*, int, int, siginfo_t*) (linux-low.c:4447)
  ==26925==    by 0x45CE2A: linux_resume_one_lwp(lwp_info*, int, int, siginfo_t*) (linux-low.c:4519)
  ==26925==    by 0x45E17C: proceed_one_lwp(thread_info*, lwp_info*) (linux-low.c:5216)
  ==26925==    by 0x45DC81: linux_resume_one_thread(thread_info*, bool) (linux-low.c:5031)
  ==26925==    by 0x45DD34: linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)bminor#2}::operator()(thread_info*) const (linux-low.c:5095)
  ==26925==    by 0x462907: void for_each_thread<linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)bminor#2}>(linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)bminor#2}) (gdbthread.h:150)
  ==26925==    by 0x45DE62: linux_resume(thread_resume*, unsigned long) (linux-low.c:5093)
  ==26925==
  ==26925== Conditional jump or move depends on uninitialised value(s)
  ==26925==    at 0x473EBD: i387_cache_to_xsave(regcache*, void*) (i387-fp.c:586)
  ==26925==    by 0x46E3ED: x86_fill_xstateregset(regcache*, void*) (linux-x86-low.c:418)
  ==26925==    by 0x45E747: regsets_store_inferior_registers(regsets_info*, regcache*) (linux-low.c:5456)
  ==26925==    by 0x45EEF8: linux_store_registers(regcache*, int) (linux-low.c:5731)
  ==26925==    by 0x426441: regcache_invalidate_thread(thread_info*) (regcache.c:89)
  ==26925==    by 0x45CCAF: linux_resume_one_lwp_throw(lwp_info*, int, int, siginfo_t*) (linux-low.c:4447)
  ==26925==    by 0x45CE2A: linux_resume_one_lwp(lwp_info*, int, int, siginfo_t*) (linux-low.c:4519)
  ==26925==    by 0x45E17C: proceed_one_lwp(thread_info*, lwp_info*) (linux-low.c:5216)
  ==26925==    by 0x45DC81: linux_resume_one_thread(thread_info*, bool) (linux-low.c:5031)
  ==26925==    by 0x45DD34: linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)bminor#2}::operator()(thread_info*) const (linux-low.c:5095)
  ==26925==    by 0x462907: void for_each_thread<linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)bminor#2}>(linux_resume(thread_resume*, unsigned long)::{lambda(thread_info*)bminor#2}) (gdbthread.h:150)
  ==26925==    by 0x45DE62: linux_resume(thread_resume*, unsigned long) (linux-low.c:5093)

The problem is a type/width mismatch in code like this, in
gdbserver/i387-fp.c:

  /* Some registers are 16-bit.  */
  collect_register_by_name (regcache, "fctrl", &val);
  fp->fctrl = val;

In the above code:

 bminor#1 - 'val' is a 64-bit unsigned long.

 bminor#2 - "fctrl" is 32-bit in the register cache, thus half of 'val' is
      left uninitialized by collect_register_by_name, which works with
      an untyped raw buffer output (i.e., void*).

 bminor#3 - fp->fctrl is an unsigned short (16-bit).  For some such
      registers we're masking off the uninitialized bits with 0xffff,
      but not in all cases.

We end up in such a fragile situation because
collect_registers_by_name works with an untyped output buffer pointer,
making it easy to pass a pointer to a variable of the wrong size.

Fix this by using regcache_raw_get_unsigned instead (actually a new
regcache_raw_get_unsigned_by_name wrapper), which always returns a
zero-extended ULONGEST register value.  It ends up simplifying the
i387-tdep.c code a bit, even.

gdb/gdbserver/ChangeLog:
2018-07-11  Pedro Alves  <palves@redhat.com>

	* i387-fp.c (i387_cache_to_fsave, cache_to_fxsave)
	(i387_cache_to_xsave): Use regcache_raw_get_unsigned_by_name
	instead of collect_register_by_name.
	* regcache.c (regcache_raw_get_unsigned_by_name): New.
	* regcache.h (regcache_raw_get_unsigned_by_name): New.
jiapei100 pushed a commit to LongerVisionRobot/binutils-gdb that referenced this pull request Jun 20, 2025
… gdb/23377)

This fixes a gdb.base/multi-forks.exp regression with GDBserver.

Git commit f2ffa92 ("gdb: Eliminate the 'stop_pc' global") caused
the regression by exposing a latent bug in gdbserver.

The bug is that GDBserver's implementation of the D;PID packet
incorrectly assumes that the selected thread points to the process
being detached.  This happens via the any_persistent_commands call,
which calls current_process:

  (gdb) bt
  #0  0x000000000040a57e in internal_error(char const*, int, char const*, ...)
  (file=0x4a53c0 "src/gdb/gdbserver/inferiors.c", line=212, fmt=0x4a539e "%s:
  Assertion `%s' failed.") at src/gdb/gdbserver/../common/errors.c:54
  bminor#1  0x0000000000420acf in current_process() () at
  src/gdb/gdbserver/inferiors.c:212
  bminor#2  0x00000000004226a0 in any_persistent_commands() () at
  gdb/gdbserver/mem-break.c:308
  bminor#3  0x000000000042cb43 in handle_detach(char*) (own_buf=0x6f0280 "D;62ea") at
  src/gdb/gdbserver/server.c:1210
  bminor#4  0x0000000000433af3 in process_serial_event() () at
  src/gdb/gdbserver/server.c:4055
  bminor#5  0x0000000000434878 in handle_serial_event(int, void*) (err=0,
  client_data=0x0)

The "eliminate stop_pc" commit exposes the problem because before that
commit, GDB's switch_to_thread always read the newly-selected thread's
PC, and that would end up forcing GDBserver's selected thread to
change accordingly as side effect.  After that commit, GDB no longer
reads the thread's PC, and GDBserver does not switch the thread.

Fix this by removing the assumption from GDBserver.

gdb/gdbserver/ChangeLog:
2018-07-11  Pedro Alves  <palves@redhat.com>

	PR gdb/23377
	* mem-break.c (any_persistent_commands): Add process_info
	parameter and use it instead of relying on the current process.
	Change return type to bool.
	* mem-break.h (any_persistent_commands): Add process_info
	parameter and change return type to bool.
	* server.c (handle_detach): Remove require_running_or_return call.
	Look up the process_info for the process we're about to detach.
	If not found, return back error to GDB.  Adjust
	any_persistent_commands call to pass down a process pointer.
jiapei100 pushed a commit to LongerVisionRobot/binutils-gdb that referenced this pull request Jun 20, 2025
…b/23379)

This commit fixes a 8.1->8.2 regression exposed by
gdb.python/py-evthreads.exp when testing with
--target_board=native-gdbserver.

gdb.log shows:

  src/gdb/thread.c:93: internal-error: thread_info* inferior_thread(): Assertion `tp' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) FAIL: gdb.python/py-evthreads.exp: run to breakpoint 1 (GDB internal error)

A backtrace shows (frames bminor#2 and bminor#10 highlighted) that the assertion
fails when GDB is setting up the connection to the remote target, in
non-stop mode:

  #0  0x0000000000622ff0 in internal_error(char const*, int, char const*, ...) (file=0xc1ad98 "src/gdb/thread.c", line=93, fmt=0xc1ad20 "%s: Assertion `%s' failed.") at src/gdb/common/errors.c:54
  bminor#1  0x000000000089567e in inferior_thread() () at src/gdb/thread.c:93
= bminor#2  0x00000000004da91d in get_event_thread() () at src/gdb/python/py-threadevent.c:38
  bminor#3  0x00000000004da9b7 in create_thread_event_object(_typeobject*, _object*) (py_type=0x11574c0 <continue_event_object_type>, thread=0x0)
      at src/gdb/python/py-threadevent.c:60
  bminor#4  0x00000000004bf6fe in create_continue_event_object() () at src/gdb/python/py-continueevent.c:27
  bminor#5  0x00000000004bf738 in emit_continue_event(ptid_t) (ptid=...) at src/gdb/python/py-continueevent.c:40
  bminor#6  0x00000000004c7d47 in python_on_resume(ptid_t) (ptid=...) at src/gdb/python/py-inferior.c:108
  bminor#7  0x0000000000485bfb in std::_Function_handler<void (ptid_t), void (*)(ptid_t)>::_M_invoke(std::_Any_data const&, ptid_t&&) (__functor=..., __args#0=...) at /usr/include/c++/7/bits/std_function.h:316
  bminor#8  0x000000000089b416 in std::function<void (ptid_t)>::operator()(ptid_t) const (this=0x12aa600, __args#0=...)
      at /usr/include/c++/7/bits/std_function.h:706
  bminor#9  0x000000000089aa0e in gdb::observers::observable<ptid_t>::notify(ptid_t) const (this=0x118a7a0 <gdb::observers::target_resumed>, args#0=...)
      at src/gdb/common/observable.h:106
= bminor#10 0x0000000000896fbe in set_running(ptid_t, int) (ptid=..., running=1) at src/gdb/thread.c:880
  bminor#11 0x00000000007f750f in remote_target::remote_add_thread(ptid_t, bool, bool) (this=0x12c5440, ptid=..., running=true, executing=true) at src/gdb/remote.c:2434
  bminor#12 0x00000000007f779d in remote_target::remote_notice_new_inferior(ptid_t, int) (this=0x12c5440, currthread=..., executing=1)
      at src/gdb/remote.c:2515
  bminor#13 0x00000000007f9c44 in remote_target::update_thread_list() (this=0x12c5440) at src/gdb/remote.c:3831
  bminor#14 0x00000000007fb922 in remote_target::start_remote(int, int) (this=0x12c5440, from_tty=0, extended_p=0)
      at src/gdb/remote.c:4655
  bminor#15 0x00000000007fd102 in remote_target::open_1(char const*, int, int) (name=0x1a4f45e "localhost:2346", from_tty=0, extended_p=0)
      at src/gdb/remote.c:5638
  bminor#16 0x00000000007fbec1 in remote_target::open(char const*, int) (name=0x1a4f45e "localhost:2346", from_tty=0)
      at src/gdb/remote.c:4862

So on frame bminor#10, we're marking a newly-discovered thread as running,
and that causes the Python API to emit a gdb.ContinueEvent.
gdb.ContinueEvent is a gdb.ThreadEvent, and as such includes the event
thread as the "inferior_thread" attribute.  The problem is that when
we get to frame bminor#3/bminor#4, we lost all references to the thread that is
being marked as running.  create_continue_event_object assumes that it
is the current thread, which is not true in this case.

Fix this by passing down the right thread in
create_continue_event_object.  Also remove
create_thread_event_object's default argument and have the only other
caller left pass down the right thread explicitly too.

gdb/ChangeLog:
2018-08-24  Pedro Alves  <palves@redhat.com>
	    Simon Marchi  <simon.marchi@ericsson.com>

	PR gdb/23379
	* python/py-continueevent.c: Include "gdbthread.h".
	(create_continue_event_object): Add intro comment.  Add 'ptid'
	parameter.  Use it to find thread to pass to
	create_thread_event_object.
	(emit_continue_event): Pass PTID down to
	create_continue_event_object.
	* python/py-event.h (py_get_event_thread): Declare.
	(create_thread_event_object): Remove default from 'thread'
	parameter.
	* python/py-stopevent.c (create_stop_event_object): Use
	py_get_event_thread.
	* python/py-threadevent.c (get_event_thread): Rename to ...
	(py_get_event_thread): ... this, make extern, add 'ptid' parameter
	and use it to find the thread.
	(create_thread_event_object): Assert that THREAD isn't null.
	Don't find the event thread here.
jiapei100 pushed a commit to LongerVisionRobot/binutils-gdb that referenced this pull request Jun 20, 2025
This change adds an optional output parameter BLOCK to
find_pc_partial_function.  If BLOCK is non-null, then *BLOCK will be
set to the address of the block corresponding to the function symbol
if such a symbol was found during lookup.  Otherwise it's set to the
NULL value.  Callers may wish to use the block information to
determine whether the block contains any non-contiguous ranges.  The
caller may also iterate over or examine those ranges.

When I first started looking at the broken stepping behavior associated
with functions w/ non-contiguous ranges, I found that I could "fix"
the problem by disabling the find_pc_partial_function cache.  It would
sometimes happen that the PC passed in would be between the low and
high cache values, but would be in some other function that happens to
be placed in between the ranges for the cached function.  This caused
incorrect values to be returned.

So dealing with this cache turns out to be very important for fixing
this problem.  I explored three different ways of dealing with the
cache.

My first approach was to clear the cache when a block was encountered
with more than one range.  This would cause the non-cache pathway to
be executed on the next call to find_pc_partial_function.

Another approach, which I suspect is slightly faster, checks to see
whether the PC is within one of the ranges associated with the cached
block.  If so, then the cached values can be used.  It falls back to
the original behavior if there is no cached block.

The current approach, suggested by Simon Marchi, is to restrict the
low/high pc values recorded for the cache to the beginning and end of
the range containing the PC value under consideration.  This allows us
to retain the simple (and fast) test for determining whether the
memoized (cached) values apply to the PC passed to
find_pc_partial_function.

Another choice that had to be made regards setting *ADDRESS and
*ENDADDR.  There are three possibilities which might make sense:

1) *ADDRESS and *ENDADDR represent the lowest and highest address
   of the function.
2) *ADDRESS and *ENDADDR are set to the start and end address of
   the range containing the entry pc.
3) *ADDRESS and *ENDADDR are set to the start and end address of
   the range in which PC is found.

An earlier version of this patch implemented option bminor#1.  I found out
that it's not very useful though and, in fact, returns results that
are incorrect when used in the context of determining the start and
end of the function for doing prologue analysis.  While debugging a
function in which the entry pc was in the second range (of a function
containing two non-contiguous ranges), I noticed that
amd64_skip_prologue called find_pc_partial_function - the returned
start address was set to the beginning of the first range.  This is
incorrect for this function.  What was also interesting was that this
first invocation of find_pc_partial_function correctly set the cache
for the PC on which it had been invoked, but a slightly later call
from skip_prologue_using_sal could not use this cached value because
it was now being used to lookup the very lowest address of the
function - which is in a range not containing the entry pc.

Option bminor#2 is attractive as it would provide a desirable result
when used in the context of prologue analysis.  However, many callers,
including some which do prologue analysis want the condition
*ADDRESS <= PC < *ENDADDR to hold.  This will not be the case when
find_pc_partial_function is called on a PC that's in a non-entry-pc
range.  A later patch to this series adds
find_function_entry_range_from_pc as a wrapper of
find_pc_partial_function.

Option bminor#3 causes the *ADDRESS <= PC < *ENDADDR property to hold.  If
find_pc_partial_function is called with a PC that's within entry pc's
range, then it will correctly return the limits of that range.  So, if
the result of a minsym search is passed to find_pc_partial_function
to find the limits, then correct results will be achieved.  Returned
limits (for prologue analysis) won't be correct when PC is within some
other (non-entry-pc) range.  I don't yet know how big of a problem
this might be; I'm guessing that it won't be a serious problem - if a
compiler generates functions which have non-contiguous ranges, then it
also probably generates DWARF2 CFI which makes a lot of the old
prologue analysis moot.

I've implemented option bminor#3 for this version of the patch.  I don't see
any regressions for x86-64.  Moreover, I don't expect to see
regressions for other targets either simply because
find_pc_partial_function behaves the same as it did before for the
contiguous address range case.  That said, there may be some
adjustments needed if GDB encounters a function requiring prologue
analysis which occupies non-contiguous ranges.

gdb/ChangeLog:

	* symtab.h (find_pc_partial_function): Add new parameter `block'.
	* blockframe.c (cache_pc_function_block): New static global.
	(clear_pc_function_cache): Clear cache_pc_function_block.
	(find_pc_partial_function): Move comment to symtab.h.  Add
	support for non-contiguous blocks.
mikpe referenced this pull request in mikpe/binutils-gdb Jul 4, 2025
When running gdb.base/foll-fork-syscall.exp with a GDB built with UBSan,
I get:

    /home/simark/src/binutils-gdb/gdb/linux-nat.c:1906:28: runtime error: load of value 3200171710, which is not a valid value for type 'target_waitkind'
    ERROR: GDB process no longer exists
    GDB process exited with wait status 3026417 exp9 0 1
    UNRESOLVED: gdb.base/foll-fork-syscall.exp: follow-fork-mode=child: detach-on-fork=on: test_catch_syscall: continue to breakpoint after fork

The error happens here:

    #0  __sanitizer::Die () at /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_termination.cpp:50
    #1  0x00007ffff600d8dd in __ubsan::__ubsan_handle_load_invalid_value_abort (Data=<optimized out>, Val=<optimized out>) at /usr/src/debug/gcc/gcc/libsanitizer/ubsan/ubsan_handlers.cpp:551
    #2  0x00005555636d37b6 in linux_handle_syscall_trap (lp=0x7cdff1eb1b00, stopping=0) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:1906
    #3  0x00005555636e0991 in linux_nat_filter_event (lwpid=3030627, status=1407) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:3044
    bminor#4  0x00005555636e407f in linux_nat_wait_1 (ptid=..., ourstatus=0x7bfff0d6cf18, target_options=...) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:3381
    bminor#5  0x00005555636e7795 in linux_nat_target::wait (this=0x5555704d35e0 <the_amd64_linux_nat_target>, ptid=..., ourstatus=0x7bfff0d6cf18, target_options=...) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:3607
    bminor#6  0x000055556378fad2 in thread_db_target::wait (this=0x55556af42980 <the_thread_db_target>, ptid=..., ourstatus=0x7bfff0d6cf18, options=...) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1398
    bminor#7  0x0000555564811327 in target_wait (ptid=..., status=0x7bfff0d6cf18, options=...) at /home/simark/src/binutils-gdb/gdb/target.c:2593

I believe the problem is that lwp_info::syscall_state is never
initialized.  Fix that by initializing it with TARGET_WAITKIND_IGNORE.
This is the value we use elsewhere when resetting this field to mean
"not stopped at a syscall".

Change-Id: I5b76c63d1466d6e63448fced03305fd5ca8294eb
Approved-By: Tom Tromey <tom@tromey.com>
MingcongBai pushed a commit to AOSC-Tracking/binutils-gdb that referenced this pull request Jul 31, 2025
Recent work in the TUI has improved GDB's use of the curses
wnoutrefresh and doupdate mechanism, which improves performance by
batching together updates and then doing a single set of writes to the
screen when doupdate is finally called.

The tui_batch_rendering type is a RAII class which, in its destructor,
calls doupdate to send the batched updates to the screen.

However, if there is no tui_batch_rendering active on the call stack
then any wnoutrefresh calls will remain batched but undisplayed until
the next time doupdate happens to be called.

This problem can be seen in PR gdb/32623.  When an inferior is started
the 'Starting program' message is not immediately displayed to the
user.

The 'Starting program' message originates from run_command_1 in
infcmd.c, the message is sent to the current_uiout, which will be the
TUI ui_out.  After the message is sent, ui_out::flush() is called,
here's the backtrace when that happens:

  #0  tui_file::flush (this=0x36e4ab0) at ../../src/gdb/tui/tui-file.c:42
  bminor#1  0x0000000001004f4b in pager_file::flush (this=0x36d35f0) at ../../src/gdb/utils.c:1531
  bminor#2  0x0000000001004f71 in gdb_flush (stream=0x36d35f0) at ../../src/gdb/utils.c:1539
  bminor#3  0x00000000006975ab in cli_ui_out::do_flush (this=0x35a50b0) at ../../src/gdb/cli-out.c:250
  bminor#4  0x00000000009fd1f9 in ui_out::flush (this=0x35a50b0) at ../../src/gdb/ui-out.h:263
  bminor#5  0x00000000009f56ad in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at ../../src/gdb/infcmd.c:449
  bminor#6  0x00000000009f599a in run_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:511

And if we check out tui_file::flush (tui-file.c) we can see that this
just calls tui_win_info::refresh_window(), which in turn, just uses
wnoutrefresh to batch any pending output.

The problem is that, in the above backtrace, there is no
tui_batch_rendering active, and so there will be no doupdate call to
flush the output to the screen.

We could add a tui_batch_rendering into tui_file::flush.  And
tui_file::write.  And tui_file::puts .....

... but that all seems a bit unnecessary.  Instead, I propose that
tui_win_info::refresh_window() should be changed.  If suppress_output
is true (i.e. a tui_batch_rendering is active) then we should continue
to call wnoutrefresh().  But if suppress_output is false, meaning that
no tui_batch_rendering is in place, then we should call wrefresh(),
which immediately writes the output to the screen.

Testing but PR gdb/32623 was a little involved.  We need to 'run' the
inferior and check for the 'Starting program' message.  But DejaGNUU
can only check for the message once it knows the message should have
appeared.  But, as the bug is that output is not displayed, we don't
have any output hints that the inferior is started yet...

In the end, I have the inferior create a file in the test's output
directory.  Now DejaGNU can send the 'run' command, and wait for the
file to appear.  Once that happens, we know that the 'Starting
program' message should have appeared.

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

Approved-By: Tom Tromey <tom@tromey.com>
MingcongBai pushed a commit to AOSC-Tracking/binutils-gdb that referenced this pull request Jul 31, 2025
Modifying inline-frame-cycle-unwind.exp to use `bt -no-filters` produces
the following incorrect backtrace:

  #0  inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:49
  bminor#1  normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
  bminor#2  0x000055555555517f in inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:50
  bminor#3  normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
  (gdb) FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1

The expected output, which we get with `bt`, is:

  #0  inline_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:49
  bminor#1  normal_func () at .../gdb/gdb/testsuite/gdb.base/inline-frame-cycle-unwind.c:32
  Backtrace stopped: previous frame identical to this frame (corrupt stack?)
  (gdb) PASS: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1

The cycle checking in `get_prev_frame_maybe_check_cycle` relies on newer
frame ids having already been computed and stashed.  Unlike other
frames, frame #0's id does not get computed immediately.

The test passes with `bt` because when applying python frame filters,
the call to `bootstrap_python_frame_filters` happens to compute the id
of frame #0.  When `get_prev_frame_maybe_check_cycle` later tries to
stash frame bminor#2's id, the cycle is detected.

The test fails with `bt -no-filters` because frame #0's id has not been
stashed by the time `get_prev_frame_maybe_check_cycle` tries to stash
frame bminor#2's id which succeeds and the cycle is only detected later when
trying to stash frame bminor#4's id.  Doing `stepi` after the incorrect
backtrace would then trigger an assertion failure when trying to stash
frame #0's id because it is a duplicate of bminor#2's already stashed id.

In `get_prev_frame_always_1`, if this_frame is inline frame 0, then
compute and stash its frame id before returning the previous frame.
This ensures that the id of inline frame 0 has been stashed before
`get_prev_frame_maybe_check_cycle` is called on older frames.

The test case has been updated to run both `bt` and `bt -no-filters`.

Co-authored-by: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32757
seantywork pushed a commit to seantywork/binutils-gdb that referenced this pull request Aug 25, 2025
When running a program that uses multiple linker namespaces, I get
something like:

    $ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-ns-ids -ex "tb 50" -ex r -ex "info shared" -batch
    ...
    From                To                  NS Syms Read   Shared Object Library
    0x00007ffff7fc6000  0x00007ffff7fff000  0  Yes         /lib64/ld-linux-x86-64.so.2
    0x00007ffff7e93000  0x00007ffff7f8b000  0  Yes         /usr/lib/libm.so.6
    0x00007ffff7ca3000  0x00007ffff7e93000  0  Yes         /usr/lib/libc.so.6
    0x00007ffff7fb7000  0x00007ffff7fbc000  1  Yes         /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
    0x00007ffff7b77000  0x00007ffff7c6f000  1  Yes         /usr/lib/libm.so.6
    0x00007ffff7987000  0x00007ffff7b77000  1  Yes         /usr/lib/libc.so.6
    0x00007ffff7fc6000  0x00007ffff7fff000  1  Yes         /usr/lib/ld-linux-x86-64.so.2
    0x00007ffff7fb2000  0x00007ffff7fb7000  2  Yes         /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
    0x00007ffff788f000  0x00007ffff7987000  2  Yes         /usr/lib/libm.so.6
    0x00007ffff769f000  0x00007ffff788f000  2  Yes         /usr/lib/libc.so.6
    0x00007ffff7fc6000  0x00007ffff7fff000  1! Yes         /usr/lib/ld-linux-x86-64.so.2
    0x00007ffff7fad000  0x00007ffff7fb2000  3  Yes         /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/dlmopen-ns-ids/dlmopen-lib.so
    0x00007ffff75a7000  0x00007ffff769f000  3  Yes         /usr/lib/libm.so.6
    0x00007ffff73b7000  0x00007ffff75a7000  3  Yes         /usr/lib/libc.so.6
    0x00007ffff7fc6000  0x00007ffff7fff000  1! Yes         /usr/lib/ld-linux-x86-64.so.2

Some namespace IDs for the dynamic linker entries (ld-linux) are wrong
(I placed a ! next to those that are wrong).

The dynamic linker is special: it is loaded only once (notice how all
ld-linux entries have the same addresses), but it is visible in all
namespaces.  It is therefore listed separately in all namespaces.

The problem happens like this:

 - for each solib, print_solib_list_table calls solib_ops::find_solib_ns
   to get the namespace ID to print
 - svr4_solib_ops::find_solib_ns calls find_debug_base_for_solib
 - find_debug_base_for_solib iterates on the list of solibs in all
   namespaces, looking for a match for the given solib.  For this, it
   uses svr4_same, which compares two SOs by name and low address.
   Because there are entries for the dynamic linker in all namespaces,
   with the same low address, find_debug_base_for_solib is unable to
   distinguish them, and sometimes returns the wrong namespace.

To fix this, save in lm_info_svr4 the debug base address that this
lm/solib comes from, as a way to distinguish two solibs that would be
otherwise identical.

The code changes are:

 - Add a constructor to lm_info_svr4 accepting the debug base.  Update
   all callers, which sometimes requires passing down the debug base.
 - Modify find_debug_base_for_solib to return the debug base directly
   from lm_info_svr4.
 - Modify svr4_same to consider the debug base value of the two
   libraries before saying they are the same.  While at it, move the
   address checks before the name check, since they are likely less
   expensive to do.
 - Modify svr4_solib_ops::default_debug_base to update the debug base of
   existing solibs when the default debug base becomes known.

I found the last point to be necessary, because when running an
inferior, we list the shared libraries very early (before the first
instruction):

    #0  svr4_solib_ops::current_sos (this=0x7c1ff1e09710)
    bminor#1  0x00005555643c774e in update_solib_list (from_tty=0)
    bminor#2  0x00005555643ca377 in solib_add (pattern=0x0, from_tty=0, readsyms=1)
    bminor#3  0x0000555564335585 in svr4_solib_ops::enable_break (this=0x7c1ff1e09710, info=0x7d2ff1de8c40, from_tty=0)
    bminor#4  0x000055556433c85c in svr4_solib_ops::create_inferior_hook (this=0x7c1ff1e09710, from_tty=0)
    bminor#5  0x00005555643d22cb in solib_create_inferior_hook (from_tty=0)
    bminor#6  0x000055556337071b in post_create_inferior (from_tty=0, set_pspace_solib_ops=true)
    bminor#7  0x00005555633726a2 in run_command_1 (args=0x0, from_tty=0, run_how=RUN_NORMAL)
    bminor#8  0x0000555563372b35 in run_command (args=0x0, from_tty=0)

At this point, the dynamic linker hasn't yet filled the DT_DEBUG slot,
which normally points at the base of r_debug.  Since we're unable to
list shared libraries at this point, we go through
svr4_solib_ops::default_sos, which creates an solib entry for the
dynamic linker.  At this point, we have no choice but to create it with
a debug base of 0 (or some other value that indicates "unknown").  If we
left it as-is, then it would later not be recognized to be part of any
existing namespace and that would cause problems down the line.

With this change, the namespaces of the dynamic linker become correct.

I was not sure if the code in library_list_start_library was conflating
debug base and lmid.  The documentation says this about the "lmid" field
in the response of a qxfer:libraries-svr4:read packet:

    lmid, which is an identifier for a linker namespace, such as the
    memory address of the r_debug object that contains this namespace’s
    load map or the namespace identifier returned by dlinfo (3).

When I read "lmid", I typically think about "the namespace identifier
returned by dlinfo (3)".  In library_list_start_library, we use the
value of the "lmid" attribute as the debug base address.  This is the
case even before this patch, since we do:

      solist = &list->solib_lists[lmid];

The key for the solib_lists map is documented as being the debug base
address.  In practice, GDBserver uses the debug base address for the
"lmid" field, so we're good for now.

If the remote side instead used "the namespace identifier returned by
dlinfo (3)" (which in practice with glibc are sequential integers
starting at 0), I think we would be mostly fine.  If we use the qxfer
packet to read the libraries, we normally won't use the namespace base
address to do any memory reads, as all the information comes from the
XML.  There might be some problems however because we treat the
namespace 0 specially, for instance in
svr4_solib_ops::update_incremental.  In that case, we might need a
different way of indicating that the remote side does not give namespace
information than using namespace 0.  This is just a thought for the
future.

I improved the existing test gdb.base/dlmopen-ns-ids.exp to verify that
"info sharedlibrary" does not show duplicate libraries, duplicate
meaning same address range, namespace and name.

Change-Id: I84467c6abf4e0109b1c53a86ef688b934e8eff99
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
jdbren pushed a commit to jdbren/binutils-gdb that referenced this pull request Aug 31, 2025
(1) Description of Problem:

When debugging the following code, the execution result of
the backtrace command is incorrect.

$ cat test.S
	.text
	.globl	fun1
	.type	fun1, @function
fun1:
	or	$r12,$r0,$r0
	or	$r4,$r12,$r0
	jr	$r1
	.globl	fun
	.type	fun, @function
fun:
	addi.d	$r3,$r3,-16
	st.d	$r1,$r3,8
	bl	fun1
	or	$r12,$r4,$r0
	or	$r4,$r12,$r0
	ld.d	$r1,$r3,8
	addi.d	$r3,$r3,16
	jr	$r1
	.globl	main
	.type	main, @function
main:
	addi.d	$r3,$r3,-16
	st.d	$r1,$r3,8
	bl	fun
	nop
	ld.d	$r1,$r3,8
	addi.d	$r3,$r3,16
	jr	$r1
$ gcc test.S -o test
$ gdb test
...
(gdb) b fun1
Breakpoint 1 at 0x748
(gdb) r
Breakpoint 1, 0x0000555555554748 in fun1 ()
(gdb) bt
  #0  0x0000555555554748 in fun1 ()
  bminor#1  0x0000555555554758 in fun ()
  bminor#2  0x0000555555554758 in fun ()
  bminor#3  0x0000555555554758 in fun ()
....
--Type <RET> for more, q to quit, c to continue without paging

(2) Root Cause Analysis:

The return address of fun() in r1(ra) is saved on the stack:

	addi.d	$r3,$r3,-16
	st.d	$r1,$r3,8

The bl instruction in fun () will call the fun1 () and save
the value of pc+4 to r1(ra).

	bl	fun1
	or	$r12,$r4,$r0

Because registers such as fp and ra saved in the stack of the sub-function
are not recorded in current code. When trace back fun() to main(), the pc
of the previous frame to be read from ra register instead of the saved location
on the stack. At this time, the value of ra register in fun() is already the
address of the next instruction after the bl. So it is impossible to trace
back to the main().

(3) Solution:

Record the location of ra, fp, s0 to s8 on the stack to ensure the correct
execution of backtrace.

(4) Test:

$ gdb test
...
(gdb) b fun1
Breakpoint 1 at 0x748
(gdb) r
Breakpoint 1, 0x0000555555554748 in fun1 ()
(gdb) bt
  #0  0x0000555555554748 in fun1 ()
  bminor#1  0x0000555555554758 in fun ()
  bminor#2  0x0000555555554778 in main ()

Signed-off-by: Hui Li <lihui@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
jdbren pushed a commit to jdbren/binutils-gdb that referenced this pull request Aug 31, 2025
For background, see this thread:

  https://inbox.sourceware.org/gdb-patches/20250612144607.27507-1-tdevries@suse.de

Tom describes the issue clearly in the above thread, here's what he
said:

  Once in a while, when running test-case gdb.base/bp-cmds-continue-ctrl-c.exp,
  I run into:
  ...
  Breakpoint 2, foo () at bp-cmds-continue-ctrl-c.c:23^M
  23        usleep (100);^M
  ^CFAIL: $exp: run: stop with control-c (unexpected) (timeout)
  FAIL: $exp: run: stop with control-c
  ...

  This is PR python/32167, observed both on x86_64-linux and powerpc64le-linux.

  This is not a timeout due to accidental slowness, gdb actually hangs.

  The backtrace at the hang is (on cfarm120 running AlmaLinux 9.6):
  ...
  (gdb) bt
   #0  0x00007fffbca9dd94 in __lll_lock_wait () from
       /lib64/glibc-hwcaps/power10/libc.so.6
   bminor#1  0x00007fffbcaa6ddc in pthread_mutex_lock@@GLIBC_2.17 () from
       /lib64/glibc-hwcaps/power10/libc.so.6
   bminor#2  0x000000001067aee8 in __gthread_mutex_lock ()
       at /usr/include/c++/11/ppc64le-redhat-linux/bits/gthr-default.h:749
   bminor#3  0x000000001067afc8 in __gthread_recursive_mutex_lock ()
       at /usr/include/c++/11/ppc64le-redhat-linux/bits/gthr-default.h:811
   bminor#4  0x000000001067b0d4 in std::recursive_mutex::lock ()
       at /usr/include/c++/11/mutex:108
   bminor#5  0x000000001067b380 in std::lock_guard<std::recursive_mutex>::lock_guard ()
       at /usr/include/c++/11/bits/std_mutex.h:229
   bminor#6  0x0000000010679d3c in set_quit_flag () at gdb/extension.c:865
   bminor#7  0x000000001066b6dc in handle_sigint () at gdb/event-top.c:1264
   bminor#8  0x00000000109e3b3c in handler_wrapper () at gdb/posix-hdep.c:70
   bminor#9  <signal handler called>
   bminor#10 0x00007fffbcaa6d14 in pthread_mutex_lock@@GLIBC_2.17 () from
       /lib64/glibc-hwcaps/power10/libc.so.6
   bminor#11 0x000000001067aee8 in __gthread_mutex_lock ()
       at /usr/include/c++/11/ppc64le-redhat-linux/bits/gthr-default.h:749
   bminor#12 0x000000001067afc8 in __gthread_recursive_mutex_lock ()
       at /usr/include/c++/11/ppc64le-redhat-linux/bits/gthr-default.h:811
   bminor#13 0x000000001067b0d4 in std::recursive_mutex::lock ()
       at /usr/include/c++/11/mutex:108
   bminor#14 0x000000001067b380 in std::lock_guard<std::recursive_mutex>::lock_guard ()
       at /usr/include/c++/11/bits/std_mutex.h:229
   bminor#15 0x00000000106799cc in set_active_ext_lang ()
       at gdb/extension.c:775
   bminor#16 0x0000000010b287ac in gdbpy_enter::gdbpy_enter ()
       at gdb/python/python.c:232
   #17 0x0000000010a8e3f8 in bpfinishpy_handle_stop ()
       at gdb/python/py-finishbreakpoint.c:414
  ...

  What happens here is the following:
  - the gdbpy_enter constructor attempts to set the current extension language
    to python using set_active_ext_lang
  - set_active_ext_lang attempts to lock ext_lang_mutex
  - while doing so, it is interrupted by sigint_wrapper (the SIGINT handler),
    handling a SIGINT
  - sigint_wrapper calls handle_sigint, which calls set_quit_flag, which also
    tries to lock ext_lang_mutex
  - since std::recursive_mutex::lock is not async-signal-safe, things go wrong,
    resulting in a hang.

  The hang bisects to commit 8bb8f83 ("Fix gdb.interrupt race"), which
  introduced the lock, making PR python/32167 a regression since gdb 15.1.

  Commit 8bb8f83 fixes PR dap/31263, a race reported by ThreadSanitizer:
  ...
  WARNING: ThreadSanitizer: data race (pid=615372)

    Read of size 1 at 0x00000328064c by thread T19:
      #0 set_active_ext_lang(extension_language_defn const*) gdb/extension.c:755
      bminor#1 scoped_disable_cooperative_sigint_handling::scoped_disable_cooperative_sigint_handling()
         gdb/extension.c:697
      bminor#2 gdbpy_interrupt gdb/python/python.c:1106
      bminor#3 cfunction_vectorcall_NOARGS <null>

    Previous write of size 1 at 0x00000328064c by main thread:
      #0 scoped_disable_cooperative_sigint_handling::scoped_disable_cooperative_sigint_handling()
         gdb/extension.c:704
      bminor#1 fetch_inferior_event() gdb/infrun.c:4591
      ...

    Location is global 'cooperative_sigint_handling_disabled' of size 1 at 0x00000328064c

    ...

  SUMMARY: ThreadSanitizer: data race gdb/extension.c:755 in \
    set_active_ext_lang(extension_language_defn const*)
  ...

  The problem here is that gdb.interrupt is called from a worker thread, and its
  implementation, gdbpy_interrupt races with the main thread on some variable.

The fix presented here is based on the fix that Tom proposed, but
fills in the missing Mingw support.

The problem is basically split into two: hosts that support unix like
signals, and Mingw, which doesn't support signals.

For signal supporting hosts, I've adopted the approach that Tom
suggests, gdbpy_interrupt uses kill() to send SIGINT to the GDB
process.  This is then handled in the main thread as if the user had
pressed Ctrl+C.  For these hosts no locking is required, so the
existing lock is removed.  However, everywhere the lock currently
exists I've added an assert:

    gdb_assert (is_main_thread ());

If this assert ever triggers then we're setting or reading the quit
flag on a worker thread, this will be a problem without the mutex.

For Mingw, the current mutex is retained.  This is fine as there are
no signals, so no chance of the mutex acquisition being interrupted by
a signal, and so, deadlock shouldn't be an issue.

To manage the complexity of when we need an assert, and when we need
the mutex, I've created 'struct ext_lang_guard', which can be used as
a RAII object.  This object either performs the assertion check, or
acquires the mutex, depending on the host.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32167
Co-Authored-By: Tom de Vries <tdevries@suse.de>
Approved-By: Tom Tromey <tom@tromey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant