[PATCH 0/2] Two minor Windows changes

[PATCH 0/2] Two minor Windows changes

[Tom Tromey]

Here are a couple of Windows-related patches that came up when I was
digging through the gdb printf code.

The first removes some dead code, then further cleans up the function
it is in.

The second is a minor change to use warning().

I tested these on Windows using the AdaCore internal test suite.  I
don't know whether these code paths are actually exercised there,
though (almost certainly not the warning path).

Tom

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

[Tom Tromey]

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(-)

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
-- 
2.31.1

[PATCH 2/2] Use warning in windows-nat error messages

[Tom Tromey]

A warning in windows-nat.c can be converted to use the warning
function.  As a side effect, this arranges for the output to be sent
to gdb_stderr.
---
 gdb/windows-nat.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 03447e0922a..47760b7b8b8 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1905,11 +1905,8 @@ windows_nat_target::attach (const char *args, int from_tty)
   pid = parse_pid_to_attach (args);
 
   if (set_process_privilege (SE_DEBUG_NAME, TRUE) < 0)
-    {
-      printf_unfiltered ("Warning: Failed to get SE_DEBUG_NAME privilege\n");
-      printf_unfiltered ("This can cause attach to "
-			 "fail on Windows NT/2K/XP\n");
-    }
+    warning ("Failed to get SE_DEBUG_NAME privilege\n"
+	     "This can cause attach to fail on Windows NT/2K/XP");
 
   windows_init_thread_list ();
   ok = DebugActiveProcess (pid);
-- 
2.31.1

Re: [PATCH 0/2] Two minor Windows changes

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Wed,  5 Jan 2022 13:44:39 -0700
> 
> Here are a couple of Windows-related patches that came up when I was
> digging through the gdb printf code.
> 
> The first removes some dead code, then further cleans up the function
> it is in.
> 
> The second is a minor change to use warning().
> 
> I tested these on Windows using the AdaCore internal test suite.  I
> don't know whether these code paths are actually exercised there,
> though (almost certainly not the warning path).

Thanks, this LGTM.