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

Tom de Vries <[email protected]> Fri, 31 Jul 2026 15:27:41 +0200
Newsgroups gmane.comp.gdb.patches
Message-ID <[email protected]>
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