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

[Craig Blackmore <craig.blackmore@embecosm.com>]

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