diff mbox series

[v2,2/5] Handle bad alloc handling in gdb_bfd_open

Message ID 20241011143544.15400-2-tdevries@suse.de
State Committed
Headers
Series [v2,1/5] Don't create registry keys in destructor |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries Oct. 11, 2024, 2:35 p.m. UTC 
  Say we simulate a bad alloc in gdb_bfd_init_data:
...
+  {
+    static bool throw_bad_alloc = true;
+    if (throw_bad_alloc)
+      {
+       throw_bad_alloc = false;
+
+       va_list dummy;
+       throw gdb_quit_bad_alloc (gdb_exception_quit ("bad alloc", dummy));
+      }
+  }
   gdata = new gdb_bfd_data (abfd, st);
...

That works out fine for doing "file a.out" once:
...
$ gdb -q -batch -ex "file a.out"
bad alloc
$
...
but doing so twice get us:
...
$ gdb -q -batch -ex "file a.out" -ex "file a.out"
bad alloc

Fatal signal: Segmentation fault
----- Backtrace -----
0x5183f7 gdb_internal_backtrace_1
        /home/vries/gdb/src/gdb/bt-utils.c:121
0x5183f7 _Z22gdb_internal_backtracev
        /home/vries/gdb/src/gdb/bt-utils.c:167
0x62329b handle_fatal_signal
        /home/vries/gdb/src/gdb/event-top.c:917
0x6233ef handle_sigsegv
        /home/vries/gdb/src/gdb/event-top.c:990
0xfffeffba483f ???
0x65554c eq_bfd
        /home/vries/gdb/src/gdb/gdb_bfd.c:231
0xeaca77 htab_find_with_hash
        /home/vries/gdb/src/libiberty/hashtab.c:597
0x657487 _Z12gdb_bfd_openPKcS0_ib
        /home/vries/gdb/src/gdb/gdb_bfd.c:580
0x6272d7 _Z16exec_file_attachPKci
        /home/vries/gdb/src/gdb/exec.c:451
0x627e67 exec_file_command
        /home/vries/gdb/src/gdb/exec.c:550
0x627f23 file_command
        /home/vries/gdb/src/gdb/exec.c:565
Segmentation fault (core dumped)
$
...

The problem is in gdb_bfd_open, where we insert abfd into gdb_bfd_cache:
...
  if (bfd_sharing)
    {
      slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
      gdb_assert (!*slot);
      *slot = abfd;
    }

  gdb_bfd_init_data (abfd, &st);
...
while the bad alloc means that gdb_bfd_init_data is interrupted and abfd is
not properly initialized.

Fix this by reversing the order, inserting abfd into gdb_bfd_cache only after
a successful call to gdb_bfd_init_data, such that we get:
...
$ gdb -q -batch -ex "file a.out" -ex "file a.out"
bad alloc
$
...

Tested on aarch64-linux.
---
 gdb/gdb_bfd.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Tom Tromey Oct. 17, 2024, 7:13 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Fix this by reversing the order, inserting abfd into gdb_bfd_cache only after
Tom> a successful call to gdb_bfd_init_data, such that we get:
Tom> ...
Tom> $ gdb -q -batch -ex "file a.out" -ex "file a.out"
Tom> bad alloc

I believe this is fine.

Approved-By: Tom Tromey <tom@tromey.com>

Tom
diff mbox series

Patch

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 0854d571ecf..b142f985dcd 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -587,6 +587,14 @@  gdb_bfd_open (const char *name, const char *target, int fd,
 			  host_address_to_string (abfd),
 			  bfd_get_filename (abfd));
 
+  /* It's important to pass the already-computed stat info here,
+     rather than, say, calling gdb_bfd_ref_ptr::new_reference.  BFD by
+     default will "stat" the file each time bfd_get_mtime is called --
+     and since we will enter it into the hash table using this
+     mtime, if the file changed at the wrong moment, the race would
+     lead to a hash table corruption.  */
+  gdb_bfd_init_data (abfd, &st);
+
   if (bfd_sharing)
     {
       slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
@@ -594,13 +602,6 @@  gdb_bfd_open (const char *name, const char *target, int fd,
       *slot = abfd;
     }
 
-  /* It's important to pass the already-computed stat info here,
-     rather than, say, calling gdb_bfd_ref_ptr::new_reference.  BFD by
-     default will "stat" the file each time bfd_get_mtime is called --
-     and since we already entered it into the hash table using this
-     mtime, if the file changed at the wrong moment, the race would
-     lead to a hash table corruption.  */
-  gdb_bfd_init_data (abfd, &st);
   return gdb_bfd_ref_ptr (abfd);
 }