Add missing null pointer check in get_sal_arch

  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.

Co-authored-by: Simon Cook <simon.cook@embecosm.com>
---
 gdb/breakpoint.c                        |  2 +-
 gdb/testsuite/gdb.base/bp-non-alloc.c   | 21 +++++++++++++++
 gdb/testsuite/gdb.base/bp-non-alloc.exp | 36 +++++++++++++++++++++++++
 gdb/testsuite/gdb.base/bp-non-alloc.ld  | 35 ++++++++++++++++++++++++
 4 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/bp-non-alloc.c
 create mode 100644 gdb/testsuite/gdb.base/bp-non-alloc.exp
 create mode 100644 gdb/testsuite/gdb.base/bp-non-alloc.ld
  

On 7/29/26 5:06 PM, Craig Blackmore wrote:

Hi,

thanks for the patch.

Here are some comments on the test-case.

> new file mode 100644
> index 00000000000..7758a591fa7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-non-alloc.exp
> @@ -0,0 +1,36 @@
> +# Copyright (C) 2026 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.
> +
> +# For an ELF that has no section called ".text" and the first section is
> +# non-alloc, test that a breakpoint can be set on a function.  This previously
> +# caused GDB to crash due to a missing null pointer check.
> +
> +require is_elf_target
> +
> +global srcdir
> +global subdir
> +

These can be dropped, they are only necessary inside a proc.

> +standard_testfile
> +
> +set linker_script $srcdir/$subdir/$testfile.ld
> +
> +set options "debug ldscript=-Wl,-T${linker_script}"
> +if {[build_executable "failed to prepare" $testfile $srcfile $options]} {
> +    return -1
> +}
> +
> +clean_restart $testfile
> +

The clean_restart can be dropped if you use prepare_for_testing instead 
of build_executable.

> +gdb_test "break main" "Breakpoint .* at .*"

You could also use "gdb_breakpoint main -message".

At this point, I wouldn't mind a comment pointing out that we don't run 
to main.  I tried it out, and ran into a SIGSEGV in the inferior.  I'm 
assuming that's expected:
...
$ readelf -h outputs/gdb.base/bp-non-alloc/bp-non-alloc | grep Entry
   Entry point address:               0x0
...

So, perhaps something like:
...
# The executable doesn't support actually running, so we don't run to
# main here.
...

Thanks,
- Tom
  

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.
>
> Co-authored-by: Simon Cook <simon.cook@embecosm.com>
> ---
>  gdb/breakpoint.c                        |  2 +-
>  gdb/testsuite/gdb.base/bp-non-alloc.c   | 21 +++++++++++++++
>  gdb/testsuite/gdb.base/bp-non-alloc.exp | 36 +++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/bp-non-alloc.ld  | 35 ++++++++++++++++++++++++
>  4 files changed, 93 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.base/bp-non-alloc.c
>  create mode 100644 gdb/testsuite/gdb.base/bp-non-alloc.exp
>  create mode 100644 gdb/testsuite/gdb.base/bp-non-alloc.ld
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index ca600a845e5..7df63856278 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)
> +  if (sal.section != nullptr && sal.section->objfile != nullptr)
>      return sal.section->objfile->arch ();
>    if (sal.symtab != nullptr)
>      return sal.symtab->compunit ().objfile ()->arch ();
> diff --git a/gdb/testsuite/gdb.base/bp-non-alloc.c b/gdb/testsuite/gdb.base/bp-non-alloc.c
> new file mode 100644
> index 00000000000..2eb9f523887
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-non-alloc.c
> @@ -0,0 +1,21 @@
> +/* Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int main ()
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/bp-non-alloc.exp b/gdb/testsuite/gdb.base/bp-non-alloc.exp
> new file mode 100644
> index 00000000000..7758a591fa7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-non-alloc.exp
> @@ -0,0 +1,36 @@
> +# Copyright (C) 2026 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# For an ELF that has no section called ".text" and the first section is
> +# non-alloc, test that a breakpoint can be set on a function.  This previously
> +# caused GDB to crash due to a missing null pointer check.
> +
> +require is_elf_target
> +
> +global srcdir
> +global subdir
> +
> +standard_testfile
> +
> +set linker_script $srcdir/$subdir/$testfile.ld

Isn't a test with a custom linker script, especially one that's placing
text sections going to be architecture specific?  I would have expected
this to be a gdb.arch/ test?

If this does work more widely then maybe a list of the targets it has
been confirmed to work on would be good.

You can push a `try` branch to sourceware into your username namespace,
e.g. I could push to 'aburgess/try-my-awesome-fix' and the sourceware CI
will spot this branch, run its tests, and email you the results.  The
key is the 'try-' part of the branch name.  Though thinking about it,
I'm not sure if it runs all tests, or just a subset, I guess you'd have
to "try" it and find out.

Thanks,
Andrew


> +
> +set options "debug ldscript=-Wl,-T${linker_script}"
> +if {[build_executable "failed to prepare" $testfile $srcfile $options]} {
> +    return -1
> +}
> +
> +clean_restart $testfile
> +
> +gdb_test "break main" "Breakpoint .* at .*"
> diff --git a/gdb/testsuite/gdb.base/bp-non-alloc.ld b/gdb/testsuite/gdb.base/bp-non-alloc.ld
> new file mode 100644
> index 00000000000..6a8ad57af18
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-non-alloc.ld
> @@ -0,0 +1,35 @@
> +/* Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +   This linker script is used to produce an executable that starts with a
> +   non-allocatable section and does not contain a `.text` section.  */
> +
> +MEMORY
> +{
> +  DATA (rw) : ORIGIN = 0x8000000, LENGTH = 0x10000
> +  TEXT (rx) : ORIGIN = LENGTH (DATA), LENGTH = 0x10000
> +}
> +
> +SECTIONS
> +{
> +  .my_non_alloc_sec (INFO) : { . = . + 0x10; }
> +  .text.all : { *(.text) } > TEXT
> +  .data : { *(.data) } > DATA
> +  _edata = .;
> +  .bss : { *(.bss) } > DATA
> +  _end = .;
> +}
> -- 
> 2.43.0
  

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.

> diff --git a/gdb/testsuite/gdb.base/bp-non-alloc.exp b/gdb/testsuite/gdb.base/bp-non-alloc.exp
> new file mode 100644
> index 00000000000..7758a591fa7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-non-alloc.exp
> @@ -0,0 +1,36 @@
> +# Copyright (C) 2026 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# For an ELF that has no section called ".text" and the first section is
> +# non-alloc, test that a breakpoint can be set on a function.  This previously
> +# caused GDB to crash due to a missing null pointer check.
> +
> +require is_elf_target
> +
> +global srcdir
> +global subdir
> +
> +standard_testfile
> +
> +set linker_script $srcdir/$subdir/$testfile.ld
> +
> +set options "debug ldscript=-Wl,-T${linker_script}"
> +if {[build_executable "failed to prepare" $testfile $srcfile $options]} {
> +    return -1

The '-1' here can be dropped.

> +}
> +
> +clean_restart $testfile
> +
> +gdb_test "break main" "Breakpoint .* at .*"
> diff --git a/gdb/testsuite/gdb.base/bp-non-alloc.ld b/gdb/testsuite/gdb.base/bp-non-alloc.ld
> new file mode 100644
> index 00000000000..6a8ad57af18
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-non-alloc.ld
> @@ -0,0 +1,35 @@
> +/* Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +   This linker script is used to produce an executable that starts with a
> +   non-allocatable section and does not contain a `.text` section.  */
> +
> +MEMORY
> +{
> +  DATA (rw) : ORIGIN = 0x8000000, LENGTH = 0x10000
> +  TEXT (rx) : ORIGIN = LENGTH (DATA), LENGTH = 0x10000

This seems a little strange.  The ORIGIN of TET will be set to 0x10000
will it not?  Which places TEXT before DATA.  Now there's nothing wrong
with that at all, but the ordering here seems weird.  And also having
the ORIGIN of TEXT depend on a LENGTH when it's going to be placed
earlier in memory seems unnecessary, you'd be better just saying
'ORIGIN=0x10000' if that's what you mean.

But maybe you actually meant something different?

Thanks,
Andrew


> +}
> +
> +SECTIONS
> +{
> +  .my_non_alloc_sec (INFO) : { . = . + 0x10; }
> +  .text.all : { *(.text) } > TEXT
> +  .data : { *(.data) } > DATA
> +  _edata = .;
> +  .bss : { *(.bss) } > DATA
> +  _end = .;
> +}
> -- 
> 2.43.0
  

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