Re: [PATCH] Powerpc, fix for test gdb.base/unwind-on-each-insn.exp

[Guinevere Larsen <blarsen@redhat.com>]

On 08/11/2023 12:59, Luis Machado wrote:
> Hi Carl,
>
> Is the message body referring to a distinct patch than the one attached? The subject mentions
> gdb.base/unwind-on-each-insn.exp, but the patch deals with gdb.ada/inline-section-gc.exp.
I read through, and I think carl just used the wrong test name on the 
email body/subject. The point of gdb.base/unwind-on-each-insn.exp is 
completely different.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> On 11/7/23 21:29, Carl Love wrote:
>> GDB maintainers, Luis, Tom:
>>
>> Here is the patch to fix test gdb.base/unwind-on-each-insn.exp on
>> PowerPC as discussed on IRC.  Per that discussion with Tom and Luis,
>> the point of the test is to look for an error where a breakpoint in an
>> inlined ada function was reported as being set in multiple places.
>> There should only be one location reported for the test and the
>> breakpoint address should not be at address 0x0. The test also fails on
>> aarch64 but passes on X86-64.  The issue is the location of the
>> inserted breakpoint in function callee may be reported as being in file
>> callee.adb or in file caller.adb.  The location reported by the ada
>> compiler for inlined functions seems to be a function of either the ada
>> compiler version or target dependent.
>>
>> The following patch will accept the reported breakpoint location as
>> being correctly set in either file callee.adb or caller.adb.  In either
>> case the address of the breakpoint must not be zero.  The test checks
>> that the file line number matches the requested line number in file
>> calleeadb or one less if the reported location is in caller.adb.  The
>> key thing is we want to make sure we have a reasonable line number and
>> the breakpoint address is not zero.
>>
>> The patch fixes the single test failure on PowerPC.  It does not
>> introduce any additional errors on the X86-84 platform on which it was
>> tested.
>>
>> Please let me know if the patch looks OK for gdb mainline.  Thanks.
>>
>>                  Carl
>>
>> -------------------------
>> Fix the gdb.ada/inline-section-gc.exp test
>>
>> The original intention of the test appears to be checking to make sure
>> setting a breakpoint in an inlined function didn't set multiple breakpoints
>> where one of them was at address 0.
>>
>> The gdb.ada/inline-section-gc.exp test may pass or fail depending on the
>> version of gnat.  Per the discussion on IRC, the ada inlining appears to
>> have some target dependencies.  In this test there are two functions,
>> callee and caller. Function calee is inlined into caller.  The test sets
>> a breakpoint in function callee.  The reported location where the breakpoint
>> is set may be at the requested location in callee or the location in caller
>> after callee has been inlined.  The test needs to accept either location as
>> correct provided the breakpoint address is not zero.
>>
>> This patch checks to see if the reported breakpoint is in function callee
>> or function caller and fails if the breakpoint address is 0x0.  The line
>> number where the breakpoint is set will match the requested line if the
>> breakpoint location is reported is callee.adb.  If the reported file is
>> caller.adb, the line number is one less.  The difference is a function of
>> the source code.  The key thing is the line number should be reasonable.
>>
>> This patch fixes the single regression failure for the test on PowerPC.
>> It does not introduce any failures on X86-64.
>> ---
>>   gdb/testsuite/gdb.ada/inline-section-gc.exp | 23 ++++++++++++++++++---
>>   1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.ada/inline-section-gc.exp b/gdb/testsuite/gdb.ada/inline-section-gc.exp
>> index b707335eb04..1f5dabc1896 100644
>> --- a/gdb/testsuite/gdb.ada/inline-section-gc.exp
>> +++ b/gdb/testsuite/gdb.ada/inline-section-gc.exp
>> @@ -34,8 +34,25 @@ if {[gdb_compile_ada "${srcfile}" "${binfile}" executable $options] != ""} {
>>   
>>   clean_restart ${testfile}
>>   
>> -set bp_location [gdb_get_line_number "BREAK" ${testdir}/callee.adb]
>> +
>> +# Depending on the version of gnat, the location of the set breakpoint may
>> +# be reported as being at the requested location in file callee.adb or in
>> +# file caller.adb where the callee function was inlined.  Either way, only
>> +# on breakpoint should be reported and it's address should not be at 0x0.
>> +# If the breakpoint is reported in caller, then the line number happens to
>> +# be one less the the requested line number.
>> +set bp_location1 [gdb_get_line_number "BREAK" ${testdir}/callee.adb]
>> +set bp_location2 [expr $bp_location1 - 1]
>> +set test "break callee.adb:$bp_location1"
>> +set message "Breakpoint set"
>> +
>>   # The bug here was that gdb would set a breakpoint with two locations,
>>   # one of them at 0x0.
>> -gdb_test "break callee.adb:$bp_location" \
>> -    "Breakpoint $decimal at $hex: file .*callee.adb, line $bp_location."
>> +gdb_test_multiple $test $message {
>> +    -re "Breakpoint $decimal at $hex: file .*callee.adb, line $bp_location1." {
>> +	pass $test
>> +    }
>> +    -re "Breakpoint $decimal at $hex: file .*caller.adb, line $bp_location2." {
>> +	pass $test
>> +    }
>> +}