Re: [PATCH] Always fetch Ada "main" name from the executable

Tom Tromey <[email protected]> writes:

> The gdb.ada/file-then-restart.exp test was failing with gnat-llvm.  I
> tracked this down to the "main" name not being stored in a readonly
> section, meaning that the code in ada_main_name using trust_readonly
> did not work.
>
> However, it seems to me that gdb should always prefer the data from
> the executable in this particular case.  So, rather than relying on
> trust_readonly, this patch changes gdb to do this directly.

I was pointed at this:

  https://sourceware.org/pipermail/bunsen/2026q3/001484.html

It's an AI/LLM code review of this patch.  The review in this case seems
to be totally bogus, everything it is commenting on is either fine, or
is part of the documented API of the function.

However, I asked Claude to review the patch and it did highlight one
issue.  It's mostly theoretical, but fixing it is trivial, so we might
as well.

Let me know what you think.

Thanks,
Andrew

---

commit 5990efb77092a3a02330260b88899d5a8b15bca7
Author: Andrew Burgess <[email protected]>
Date:   Wed Aug 5 17:23:38 2026 +0100

    gdb/ada: avoid rereading stale main name data in edge case
    
    The commit:
    
      commit 8eafbbc74748e499ec785f78858687bd7ea79005
      Date:   Wed Jul 29 12:40:03 2026 -0600
    
        Always fetch Ada "main" name from the executable
    
    changes ada_main_name to use section_table_xfer_memory_partial.  This
    introduced a highly unlikely, but theoretical bug where stale buffer
    data could cause GDB to find an invalid name for "main".
    
    Looking at ada_main_name (in ada-lang.c), the steps to reproduce the
    bug are:
    
      1. Debug a program that causes the static buffer main_program_name
         to have some content written to it.  For the sake of this bug
         let's assume the main name is "xxxxxxxxxx", the main_program_name
         buffer will contain 10 'x' characters, a null byte, then whatever
         happened to be in the section after that.
    
      2. A new executable is loaded into GDB and ada_main_name is called
         again.
    
      3. For whatever reason the new executable is maybe not correct.  The
         ADA_MAIN_PROGRAM_SYMBOL_NAME symbol points to an address 5 bytes
         before the end of a section.  None of these 5 bytes are a null
         bytes.  Let's assume these 5 bytes are "aaaaa".
    
      4. The section_table_xfer_memory_partial call will try to read up to
         1024 bytes, but as there are only 5 bytes left in the section,
         only 5 will be read.  This leaves the main_program_name buffer
         containing "aaaaaxxxxx" followed by a null character byte.
    
      5. GDB returns this merged string as the result from ada_main_name.
    
    Now given this depends on the second executable being broken, we maybe
    don't really care too much, however, fixing this is pretty easy.
    
    The current code already checks:
    
      && (strnlen ((char *) main_program_name, sizeof (main_program_name))
          < sizeof (main_program_name))
    
    This ensures that there's a string with a null byte contained within
    the buffer, but makes the assumption that we always read
    sizeof (main_program_name) bytes from the section.
    
    But we know how many bytes were read, that's the value in XFERRED.
    What we really want to ask is: was there a null terminated string
    within the bytes that we just read.  This is:
    
      && (strnlen ((char *) main_program_name, xferred) < xferred)
    
    Given how simple this fix is, let's make it.

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 906c5cd3465..174e04af04c 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -806,8 +806,7 @@ ada_main_name ()
 					      sections)
 	   == TARGET_XFER_OK)
 	  && xferred > 0
-	  && (strnlen ((char *) main_program_name, sizeof (main_program_name))
-	      < sizeof (main_program_name)))
+	  && (strnlen ((char *) main_program_name, xferred) < xferred))
 	return (char *) main_program_name;
     }