Re: [PATCH] Add missing null pointer check in get_sal_arch

[Andrew Burgess <aburgess@redhat.com>]

Andrew Burgess <aburgess@redhat.com> writes:

> Craig Blackmore <craig.blackmore@embecosm.com> writes:
>
>> This fixes a GDB crash when trying to set a breakpoint on a function in
>> an ELF where there is both no .text section and the first section within
>> the ELF is not allocatable.
>
> This tells us WHAT happened, but not WHY.  We understand the input as
> you gave a description of the ELF, and you explained the end result, a
> crash.  But it would be really useful if you could fill in the middle
> bit.  Why does the objfile end up as NULL?
>
> When a fix is "add a NULL pointer check" my immediate question is:
> should the pointer even be NULL?  Maybe there's a better fix elsewhere
> in GDB which prevents the pointer from ever becoming NULL.  The goal of
> the "middle bit" that I asked for above is to convince the reviewers
> that NULL is a valid possibility and that a NULL check should be added.
>
> This commit from April seems like it might be in a similar area of GDB:
>
>   commit cd289df068e39683576f95907b5dd06ae3e4e254
>   Date:   Wed Apr 15 10:43:31 2026 +0100
>
>     gdb: don't use .text as default entry point section
>
> and might be worth a read.

I looked at this a bit more and `init_objfile_sect_indices` ends with
this code:

  for (i = 0; i < objfile->section_offsets.size (); i++)
    {
      if (objfile->section_offsets[i] != 0)
	{
	  break;
	}
    }
  if (i == objfile->section_offsets.size ())
    {
      if (objfile->sect_index_text == -1)
	objfile->sect_index_text = 0;
      if (objfile->sect_index_data == -1)
	objfile->sect_index_data = 0;
      if (objfile->sect_index_bss == -1)
	objfile->sect_index_bss = 0;
      if (objfile->sect_index_rodata == -1)
	objfile->sect_index_rodata = 0;
    }

With the idea being that if every section has a relocation offset of
zero then we can just point at any section.  That's fine as far as the
actual relocation offset is concerned, but sect_index_text is also used
to find an objfile, and in this case, we need to point to an actual
allocatable section.

Maybe we should rewrite the 'if (objfile->sect_index_text == -1)' case
so instead of always selecting index 0 we select the first allocatable
and executable section?  I had a go at this, see the patch below, and
your test case still passes.

I also wondered if we should be adding an assert to catch this
problematic case earlier on?  In
buildsym_compunit::finish_block_internal where we do:

      symbol->set_section_index (SECT_OFF_TEXT (m_objfile));

this seems to be the first point where we could spot the problem maybe
as this is where the offset to the wrong section is used for a symbol.
Maybe here, or close to here, we could have an assert that the symbol
has a valid objfile?  I haven't exactly figured this bit out, but could
be something to investigate.

Anyway, let me know what you think of this alternative approach.

Thanks,
Andrew

---

commit 12f7a855e97e6f4623607b2a503952ff8a7bb5c3
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Jul 29 18:21:12 2026 +0100

    WIP: possible alternative

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7df63856278..ca600a845e5 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7764,7 +7764,7 @@ set_breakpoint_location_function (struct bp_location *loc)
 struct gdbarch *
 get_sal_arch (struct symtab_and_line sal)
 {
-  if (sal.section != nullptr && sal.section->objfile != nullptr)
+  if (sal.section != nullptr)
     return sal.section->objfile->arch ();
   if (sal.symtab != nullptr)
     return sal.symtab->compunit ().objfile ()->arch ();
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 017f7a49d8d..ee1c40dada9 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -102,6 +102,8 @@ static int simple_overlay_update_1 (struct obj_section *);
 
 static void symfile_find_segment_sections (struct objfile *objfile);
 
+static int symfile_default_text_sect_index (objfile *objfile);
+
 /* Map from a BFD flavour to the corresponding sym_fns instance.  On
    gdb startup, each object file reader calls add_symtab_fns() to
    register information on each format it is prepared to read.  */
@@ -300,7 +302,7 @@ init_objfile_sect_indices (struct objfile *objfile)
   if (i == objfile->section_offsets.size ())
     {
       if (objfile->sect_index_text == -1)
-	objfile->sect_index_text = 0;
+	objfile->sect_index_text = symfile_default_text_sect_index (objfile);
       if (objfile->sect_index_data == -1)
 	objfile->sect_index_data = 0;
       if (objfile->sect_index_bss == -1)
@@ -3706,6 +3708,48 @@ symfile_find_segment_sections (struct objfile *objfile)
     }
 }
 
+/* Return the section index of a section in OBJFILE which can act as
+   the default text section.
+
+   This returns the first allocatable and executable section, or the
+   first allocatable section if no section is marked executable.
+
+   As an absolute fallback, 0 is returned.  */
+
+static int
+symfile_default_text_sect_index (objfile *objfile)
+{
+  gdb_assert (objfile->sect_index_text == -1);
+
+  bfd *abfd = objfile->obfd.get ();
+
+  int first_allocatable_section_index = -1;
+
+  for (asection *sect = abfd->sections; sect != nullptr; sect = sect->next)
+    {
+      /* Skip non-allocatable sections.  */
+      if ((bfd_section_flags (sect) & SEC_ALLOC) == 0)
+	continue;
+
+      /* Record the first allocatable section.  */
+      if (first_allocatable_section_index == -1)
+	first_allocatable_section_index = sect->index;
+
+      /* Return the first allocatable code section found.  */
+      if ((bfd_section_flags (sect) & SEC_CODE) == SEC_CODE)
+	return sect->index;
+    }
+
+  /* We didn't even find an allocatable section.  Return 0, but this
+     is likely going to cause issues if (somehow) there are any debug
+     symbols in OBJFILE as those symbols will end up with a NULL
+     objfile pointer.  */
+  if (first_allocatable_section_index == -1)
+    return 0;
+
+  return first_allocatable_section_index;
+}
+
 /* Listen for free_objfile events.  */
 
 static void