Re: [PATCH 3/3] gdb/amd-dbgapi: add displaced stepping support

[Simon Marchi <simon.marchi@polymtl.ca>]

>> diff --git a/gdb/testsuite/gdb.rocm/displaced-stepping.exp b/gdb/testsuite/gdb.rocm/displaced-stepping.exp
>> new file mode 100644
>> index 000000000000..6b26e1adfd7a
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.rocm/displaced-stepping.exp
>> @@ -0,0 +1,54 @@
>> +# Copyright 2024 Free Software Foundation, Inc.
> 
> Should probably 2025.

Fixed.

>> +
>> +# 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 displaced stepping on AMD GPUs.
>> +
>> +load_lib rocm.exp
>> +
>> +standard_testfile simple.cpp
> 
> I though the usage in GDB testsuite was to not share the .cpp across multiple testcases.  Should there be a displaced-stepping.cpp?

For "hello world" style programs (which this one is) I don't really
mind.  But I will add displaced-stepping.cpp.

> 
>> +
>> +require allow_hipcc_tests
>> +
>> +# Since GDB doesn't yet understand DWARF expressions generated by the HIP
>> +# compiler, purposefully generate the binary without debug info.
>> +if {[build_executable "failed to prepare" $testfile $srcfile {hip}]} {
>> +    return
>> +}
>> +
>> +proc do_test {} {
>> +    clean_restart $::binfile
>> +
>> +    with_rocm_gpu_lock {
>> +       if ![runto_main] {
>> +           return
>> +       }
>> +
>> +       gdb_test "with breakpoint pending on -- break do_an_addition" \
>> +           "Breakpoint $::decimal \\(do_an_addition\\) pending."
>> +
>> +       gdb_test "continue" \
>> +           "Thread $::decimal hit Breakpoint $::decimal, $::hex in do_an_addition.*"
>> +
>> +       gdb_test_no_output "set debug displaced on"
>> +
>> +       gdb_test "stepi" "displaced_step_prepare_throw: prepared successfully.*$::hex in do_an_addition.*"
> 
> Just to reduce the noise in the gdb.log file when running the continue, this could be
> 
> gdb_test "with debug displaced on -- stepi" ...

Good idea, I never remember to use "with".  Done.

> 
>> +
>> +       gdb_test "continue" \
>> +           "Inferior 1 .* exited normally.*" \
>> +           "continue to end"
>> +    }
>> +}
>> +
>> +do_test
>> -- 
>> 2.47.1
>>
> 
> With those minor comments addressed, and given that it only touches amdgpu related code,
> Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)

Thanks, added.

Simon