gdb/testsuite: fix gdb.multi/inferior-specific-bp.exp
Checks
Commit Message
A recent commit, "16a6f7d2ee3 gdb: avoid breakpoint::clear_locations
calls in update_breakpoint_locations", started checking if GDB correctly
relocates a breakpoint from inferior 1's declaration of the function
"bar" to inferior 2's declaration.
Unfortunately, inferior 2 never calls bar in its regular execution, and
because of that, clang would optimize that whole function away, making
it so there is no location for the breakpoint to be relocated to.
This commit changes the .c file so that the function is not optimized
away and the test fully passes with clang. It is important to actually
call bar instead of using __attribute__((used)) because the latter
causes the breakpoint locations to be inverted, 3.1 belongs to inferior
2 and 3.2 belongs to inferior 1, which will cause an unrelated failure.
---
gdb/testsuite/gdb.multi/inferior-specific-bp-2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
Guinevere Larsen <guinevere@redhat.com> writes:
> A recent commit, "16a6f7d2ee3 gdb: avoid breakpoint::clear_locations
> calls in update_breakpoint_locations", started checking if GDB correctly
> relocates a breakpoint from inferior 1's declaration of the function
> "bar" to inferior 2's declaration.
>
> Unfortunately, inferior 2 never calls bar in its regular execution, and
> because of that, clang would optimize that whole function away, making
> it so there is no location for the breakpoint to be relocated to.
>
> This commit changes the .c file so that the function is not optimized
> away and the test fully passes with clang. It is important to actually
> call bar instead of using __attribute__((used)) because the latter
> causes the breakpoint locations to be inverted, 3.1 belongs to inferior
> 2 and 3.2 belongs to inferior 1, which will cause an unrelated failure.
I suspect this location ordering issue might crop up from time to time.
Locations are ordered by address with no consideration for which
inferior they are in.
In this test we compile two different source files, how their functions
are laid out will determine which order the locations appear in.
If we have a solution which reliably gives one ordering for now then
that's fine. But it's possible that at some point we maybe have to
check for either ordering in tests like this.... but not today
thankfully!
> ---
> gdb/testsuite/gdb.multi/inferior-specific-bp-2.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c b/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c
> index bde6fbf8224..3a45c21034f 100644
> --- a/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c
> +++ b/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c
> @@ -30,7 +30,8 @@ main (void)
> {
> int ret = baz ();
> stop_breakpt ();
> - return ret;
> + /* We have to call bar here, otherwise it might be optimized away. */
> + return ret + bar ();
> }
LGTM.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> static int
> --
> 2.47.0
On 10/14/24 11:52 AM, Andrew Burgess wrote:
> Guinevere Larsen <guinevere@redhat.com> writes:
>
>> A recent commit, "16a6f7d2ee3 gdb: avoid breakpoint::clear_locations
>> calls in update_breakpoint_locations", started checking if GDB correctly
>> relocates a breakpoint from inferior 1's declaration of the function
>> "bar" to inferior 2's declaration.
>>
>> Unfortunately, inferior 2 never calls bar in its regular execution, and
>> because of that, clang would optimize that whole function away, making
>> it so there is no location for the breakpoint to be relocated to.
>>
>> This commit changes the .c file so that the function is not optimized
>> away and the test fully passes with clang. It is important to actually
>> call bar instead of using __attribute__((used)) because the latter
>> causes the breakpoint locations to be inverted, 3.1 belongs to inferior
>> 2 and 3.2 belongs to inferior 1, which will cause an unrelated failure.
> I suspect this location ordering issue might crop up from time to time.
> Locations are ordered by address with no consideration for which
> inferior they are in.
>
> In this test we compile two different source files, how their functions
> are laid out will determine which order the locations appear in.
>
> If we have a solution which reliably gives one ordering for now then
> that's fine. But it's possible that at some point we maybe have to
> check for either ordering in tests like this.... but not today
> thankfully!
Yeah, we should probably have a generic gdb_test_info_breakpoints or
something, which could check those in a order-independent way, but since
I had a simple fix for this one and for the other time I ran into this,
it felt like over-engineering. Probably third time will be the charm :)
>
>> ---
>> gdb/testsuite/gdb.multi/inferior-specific-bp-2.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c b/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c
>> index bde6fbf8224..3a45c21034f 100644
>> --- a/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c
>> +++ b/gdb/testsuite/gdb.multi/inferior-specific-bp-2.c
>> @@ -30,7 +30,8 @@ main (void)
>> {
>> int ret = baz ();
>> stop_breakpt ();
>> - return ret;
>> + /* We have to call bar here, otherwise it might be optimized away. */
>> + return ret + bar ();
>> }
> LGTM.
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks, pushed!
Guinevere Larsen <guinevere@redhat.com> writes:
> On 10/14/24 11:52 AM, Andrew Burgess wrote:
>> Guinevere Larsen <guinevere@redhat.com> writes:
>>
>>> A recent commit, "16a6f7d2ee3 gdb: avoid breakpoint::clear_locations
>>> calls in update_breakpoint_locations", started checking if GDB correctly
>>> relocates a breakpoint from inferior 1's declaration of the function
>>> "bar" to inferior 2's declaration.
>>>
>>> Unfortunately, inferior 2 never calls bar in its regular execution, and
>>> because of that, clang would optimize that whole function away, making
>>> it so there is no location for the breakpoint to be relocated to.
>>>
>>> This commit changes the .c file so that the function is not optimized
>>> away and the test fully passes with clang. It is important to actually
>>> call bar instead of using __attribute__((used)) because the latter
>>> causes the breakpoint locations to be inverted, 3.1 belongs to inferior
>>> 2 and 3.2 belongs to inferior 1, which will cause an unrelated failure.
>> I suspect this location ordering issue might crop up from time to time.
>> Locations are ordered by address with no consideration for which
>> inferior they are in.
>>
>> In this test we compile two different source files, how their functions
>> are laid out will determine which order the locations appear in.
>>
>> If we have a solution which reliably gives one ordering for now then
>> that's fine. But it's possible that at some point we maybe have to
>> check for either ordering in tests like this.... but not today
>> thankfully!
> Yeah, we should probably have a generic gdb_test_info_breakpoints or
> something, which could check those in a order-independent way, but since
> I had a simple fix for this one and for the other time I ran into this,
> it felt like over-engineering. Probably third time will be the charm
> :)
Oh for sure it would be OTT in this case. But eventually we'll hit a
case where one compiled places the code in one order, while another
places the code in a different order, and so the locations will be
switched. But we can cross that bridge when we come to it :) No point
going looking for problems...
Thanks,
Andrew
@@ -30,7 +30,8 @@ main (void)
{
int ret = baz ();
stop_breakpt ();
- return ret;
+ /* We have to call bar here, otherwise it might be optimized away. */
+ return ret + bar ();
}
static int