[1/2] Clean up some dead code in windows-tdep.c

Message ID 20220105204441.3088372-2-tom@tromey.com
State New
Headers show
Series
  • Two minor Windows changes
Related show

Commit Message

Tom Tromey Jan. 5, 2022, 8:44 p.m.
windows-tdep.c checks the result of xmalloc, which isn't necessary.  I
initially removed this dead check, but then went a bit further and
modified the code so that some "goto"s and explicit memory management
could be removed.  Then, I added a couple of missing bounds checks.

I believe this also fixes a possible bug with a missing 0-termination
of a string.  I am not certain, but that is why I think the existing
code allocates a buffer that is 1 byte too long -- but then it fails
to set this byte to 0.
---
 gdb/windows-tdep.c | 44 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

-- 
2.31.1

Patch

diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index 616890493d4..78984d65fd6 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -1112,54 +1112,50 @@  core_process_module_section (bfd *abfd, asection *sect, void *obj)
   size_t module_name_offset;
   CORE_ADDR base_addr;
 
-  gdb_byte *buf = NULL;
-
   if (!startswith (sect->name, ".module"))
     return;
 
-  buf = (gdb_byte *) xmalloc (bfd_section_size (sect) + 1);
-  if (!buf)
-    {
-      printf_unfiltered ("memory allocation failed for %s\n", sect->name);
-      goto out;
-    }
+  gdb::byte_vector buf (bfd_section_size (sect) + 1);
   if (!bfd_get_section_contents (abfd, sect,
-				 buf, 0, bfd_section_size (sect)))
-    goto out;
-
-
+				 buf.data (), 0, bfd_section_size (sect)))
+    return;
+  /* We're going to treat part of the buffer as a string, so make sure
+     it is NUL-terminated.  */
+  buf.back () = 0;
 
   /* A DWORD (data_type) followed by struct windows_core_module_info.  */
-  data_type = extract_unsigned_integer (buf, 4, byte_order);
+  if (bfd_section_size (sect) < 4)
+    return;
+  data_type = extract_unsigned_integer (buf.data (), 4, byte_order);
 
   if (data_type == NOTE_INFO_MODULE)
     {
-      base_addr = extract_unsigned_integer (buf + 4, 4, byte_order);
-      module_name_size = extract_unsigned_integer (buf + 8, 4, byte_order);
       module_name_offset = 12;
+      if (bfd_section_size (sect) < module_name_offset)
+	return;
+      base_addr = extract_unsigned_integer (&buf[4], 4, byte_order);
+      module_name_size = extract_unsigned_integer (&buf[8], 4, byte_order);
     }
   else if (data_type == NOTE_INFO_MODULE64)
     {
-      base_addr = extract_unsigned_integer (buf + 4, 8, byte_order);
-      module_name_size = extract_unsigned_integer (buf + 12, 4, byte_order);
       module_name_offset = 16;
+      if (bfd_section_size (sect) < module_name_offset)
+	return;
+      base_addr = extract_unsigned_integer (&buf[4], 8, byte_order);
+      module_name_size = extract_unsigned_integer (&buf[12], 4, byte_order);
     }
   else
-    goto out;
+    return;
 
   if (module_name_offset + module_name_size > bfd_section_size (sect))
-    goto out;
-  module_name = (char *) buf + module_name_offset;
+    return;
+  module_name = (char *) buf.data () + module_name_offset;
 
   /* The first module is the .exe itself.  */
   if (data->module_count != 0)
     windows_xfer_shared_library (module_name, base_addr,
 				 NULL, data->gdbarch, data->obstack);
   data->module_count++;
-
-out:
-  xfree (buf);
-  return;
 }
 
 ULONGEST