[PATCH] Add missing null pointer check in get_sal_arch

[PATCH] Add missing null pointer check in get_sal_arch

[Craig Blackmore]

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

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

[Tom de Vries]

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

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

[Andrew Burgess]

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

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

[Andrew Burgess]

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

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

[Andrew Burgess]

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

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

[Craig Blackmore]

Hi Tom,

Thanks for the review. I will include these cleanups and the suggested
comment in an updated test case which I will post shortly.

Craig

On 31/07/2026 14:27, Tom de Vries wrote:
> 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

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

[Craig Blackmore]

[-- Attachment #1: Type: text/plain, Size: 6313 bytes --]

Hi Andrew,

Thanks for the reviews.

On 31/07/2026 14:54, Andrew Burgess wrote:
> 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?
I have moved the test to gdb.arch.

> If this does work more widely then maybe a list of the targets it has
> been confirmed to work on would be good.
I have updated the test to work more widely and so far confirmed it
works on arm, riscv and x86_64.  The patch below contains the updated
test only - I will discuss how to proceed with the fix in my next reply.

The test is now linked with -nostdlib to avoid linking library code that
references specific symbols that would usually be defined in the linker
script.  I have updated the linker script to ensure there are three LOAD
segments, as at least three are needed to reproduce the issue.

I have also included the cleanups suggested by you and Tom de Vries and
fixed the strangely defined memory regions in the linker script.

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

I would like to try this but I don't have gdb write permission. I know I
need to send an email to admin-requests. Would you be willing to approve
me having write after approval access please?

Thanks,
Craig

---

commit 00c611b0c754a03c426af0e8476ee1e9347919e3
Author: Craig Blackmore<craig.blackmore@embecosm.com>
Date:   Thu Aug 20 16:47:52 2026 +0100

     [WIP] Updated testcase
     
     This is the test case only.  It does not include either fix.

diff --git a/gdb/testsuite/gdb.arch/bp-non-alloc.c b/gdb/testsuite/gdb.arch/bp-non-alloc.c
new file mode 100644
index 00000000000..2eb9f523887
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/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.arch/bp-non-alloc.exp b/gdb/testsuite/gdb.arch/bp-non-alloc.exp
new file mode 100644
index 00000000000..79f29dbb1da
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/bp-non-alloc.exp
@@ -0,0 +1,40 @@
+# 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/>.
+
+# Test that a breakpoint can be set on a function in an ELF file that has all
+# of the following properties:
+#   1. There is no section called ".text"
+#   2. The first section is non-allocatable
+#   3. There are three LOAD segments
+# This previously caused GDB to segfault due to attempting to dereference a null
+# objfile within a sal section.
+
+# This test has a custom linker script so may not work on all targets.  It has
+# been confirmed to work on arm-none-eabi, riscv64-unknown-elf and
+# x64_64-linux-gnu.
+require {is_any_target "arm*-*-*" "riscv*-*-*" "x86_64*-*-*"} is_elf_target
+
+standard_testfile
+
+set linker_script $srcdir/$subdir/$testfile.ld
+
+set options "debug ldscript=-Wl,-T${linker_script} ldflags=-nostdlib"
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile $options]} {
+    return
+}
+
+# The executable does not support actually running, so we do not run to main
+# here.
+gdb_breakpoint main -message
diff --git a/gdb/testsuite/gdb.arch/bp-non-alloc.ld b/gdb/testsuite/gdb.arch/bp-non-alloc.ld
new file mode 100644
index 00000000000..73801e9f865
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/bp-non-alloc.ld
@@ -0,0 +1,42 @@
+/* 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, does not contain a `.text` section and has three
+   LOAD segments.  */
+
+MEMORY
+{
+  TEXT (rx) : ORIGIN = 0x80000000, LENGTH = 0x10000
+  DATA (rw) : ORIGIN = 0x80010000, LENGTH = 0x10000
+}
+
+PHDRS
+{
+  text PT_LOAD ;
+  data1 PT_LOAD ;
+  data2 PT_LOAD ;
+}
+
+SECTIONS
+{
+  . = SIZEOF_HEADERS;
+  .my_non_alloc_sec (INFO) : { . = . + 0x10; }
+  .text.all : { *(.text) } > TEXT :text
+  .data : { *(.data) } > DATA :data1
+  .bss : { *(.bss) } > DATA :data2
+}

[-- Attachment #2: Type: text/html, Size: 7420 bytes --]

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

[Craig Blackmore]

[-- Attachment #1: Type: text/plain, Size: 3406 bytes --]

Hi Andrew,

On 31/07/2026 16:27, Andrew Burgess wrote:
> 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 for looking into this and sharing your analysis and alternative
fix. I much prefer your fix as it addresses the root cause and it works
on both my original test and the updated test that I just posted.

As you've written the fix, would you prefer to add my test to your patch
or for me to combine everything into a new submission?

Thanks,
Craig

[-- Attachment #2: Type: text/html, Size: 4588 bytes --]