Re: [PATCH ver 2] Fix the gdb.ada/inline-section-gc.exp test

Luis Machado
2023-11-08T16:58:14Z

Hi Carl,

This fixes the FAIL's I had been seeing on aarch64-linux. Thanks!

Some nits...

On 11/8/23 16:22, Carl Love wrote:
> GDB maintainers, Luis, Tom:
>
> Version 2, fixed the name of the test the patch fixes in the email.
> Mixed it up with another patch I am working on sorry about that.
>
> Here is the patch to fix test gdb.ada/inline-section-gc.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.
s/on breakpoint/one breakpoint?

s/and it's/and its?

> +# 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
> + }
> +}

Otherwise looks good.

Reviewed-by: <luis.machado@arm.com>