Re: [PATCH] gdb/testsuite: rework bp-cond-failure to not depend on inlining

Guinevere Larsen
2024-09-20T16:21:09Z

On 9/20/24 12:58 PM, Keith Seitz wrote:
> On 9/20/24 8:56 AM, Guinevere Larsen wrote:
>> On 9/20/24 12:35 PM, Keith Seitz wrote:
>>>
>>> For the casual visitor to this file that has often been burned by
>>> hard-coded line numbers or locations, it would, at least, save me some
>>> grief.
>> At the top of the exp file, the comment that explains the purpose of
>> the test ends with the following:
>>
>> # We check that the correct breakpoint number appears in the error
>> # message, and that the error is reported at the correct source
>> # location.
>>
>> Would you still like me to add the comment in context, or do you
>> think that is enough?
>
> Okay, fully optional now, but please consider adding a comment referring
> to that explanation, at least. Again, I missed it entirely. :-)

I've given some more thought and I think a couple of comments could help
anyway. Here's my suggestion, thought?

diff --git a/gdb/testsuite/gdb.base/bp-cond-failure.exp
b/gdb/testsuite/gdb.base/bp-cond-failure.exp
index 403e7db9032..b4c046c4bd6 100644
--- a/gdb/testsuite/gdb.base/bp-cond-failure.exp
+++ b/gdb/testsuite/gdb.base/bp-cond-failure.exp
@@ -57,9 +57,18 @@ proc run_test { cond_eval access_type bpexpr nloc } {

# Setup the conditional breakpoint and record its number.
gdb_breakpoint "${bpexpr} if (*(${access_type} *) 0) == 0"
+
+    # This test aims to test that GDB displays the correct breakpoint
number
+    # and location when there is an error testing a breakpoint condition,
+    # so it is important to hardcode the breakpoint number into the regex,
+    # along with the location, if applicable.
set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*"]

if { $nloc > 1 } {
+       # We hardcode location 2 because, for some reason, Clang will always
+       # order the debug information so we hit the second location.  For
+       # simplicity the .c is ordered in such a way that GCC will also
order
+       # the debug info to have us land on location 2.
gdb_test "continue" \
[multi_line \
"Continuing\\." \

--
Cheers,
Guinevere Larsen
She/Her/Hers

>
> Keith
>