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

Craig Blackmore <[email protected]> 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 <[email protected]>
> ---
>  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