Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Bruno Larsen <blarsen@redhat.com>]

On 6/9/22 15:25, Pedro Alves wrote:
> On 2022-06-09 17:27, Bruno Larsen wrote:
>>
>> On 6/8/22 12:37, Pedro Alves wrote:
>>> On 2022-05-26 16:10, Bruno Larsen via Gdb-patches wrote:
>>>> When running gdb.base/skip-solib.exp, the backtrace tests could fail if
>>>> the compiler did not emit epilogue information for trivial epilogues,
>>>
>>> I found "emit epilogue information" very confusing FWIW.  When I read that the first time,
>>> I thought this was talking about unwind info for the prologue after the stack is destroyed,
>>> and I wondered why not fix clang instead.
>>>
>>> I think the following would be more accurate:
>>>
>>>    "fail with compilers that associate prologue insns with the function's last
>>>     statement line instead of the function's closing brace".
>>
>> Yeah, I think this is a better explanation, but s/prologue/epilogue, right?
> 
> Right.  That was a typo.  I meant:
> 
>    "fail with compilers that associate epilogue insns with the function's last
>     statement line instead of the function's closing brace".
> 
>> I tried to make this concept more specific by saying that clang doesn't emit epilogue information, Have I misunderstood or misnamed it?
> 
> To me "epilogue information" sounds like some special DWARF information specific to epilogues.
> It reminds of me of special epilogue unwinders that GDB has, like amd64_epilogue_frame_unwind,
> i386_epilogue_frame_unwind, rs6000_epilogue_frame_unwind, etc.  By not saying "line" anywhere,
> I have nothing to connect "epilogue information" to "line info".  Once you make the connection,
> you understand that there's nothing special about "epilogue information", it's just that clang
> and gcc choose different lines numbers for the epilogue instructions.

Ah, I see! Then  yes, I can change it to what you suggested. Just wanted to confirm my understanding :)

> 
> 
>>>> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
>>>> index f6e9926b66c..327ea676140 100644
>>>> --- a/gdb/testsuite/gdb.base/skip-inline.exp
>>>> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
>>>> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
>>>>    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>>>>    gdb_test "step" ".*" "step into baz, since foo will be skipped"
>>>>    gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
>>>> -gdb_test "step" ".*" "step in the baz"
>>>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
>>>> -gdb_test "step" ".*" "step back to main"
>>>> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"
>>>
>>> FWIW, I'd remove the "regexp" from the function's name, just call it gdb_step_until.
>>
>> Alright, but this sounds more like a review of the first patch than this one.
> 
> It only stood out to me when I saw it in use, as in the function name is long
> and regexp stuck out like a sore thumb here.  :-)
> 
>>> Or even with a simple "just main" program, like so:
>>>
>>>    (gdb) list
>>>    1       int
>>>    2       main ()
>>>    3       {
>>>    4         return 0;
>>>    5       }
>>>
>>> we get this with gcc:
>>>
>>>    (gdb) info line 4
>>>    Line 4 of "main.c" starts at address 0x1131 <main+8> and ends at 0x1136 <main+13>.
>>>    (gdb) info line 5
>>>    Line 5 of "main.c" starts at address 0x1136 <main+13> and ends at 0x1138.
>>>    (gdb)
>>>
>>> and this with clang:
>>>
>>>    (gdb) info line 4
>>>    Line 4 of "main.c" starts at address 0x40111d <main+13> and ends at 0x40111f.
>>>    (gdb) info line 5
>>>    Line number 5 is out of range for "main.c".
>>>    (gdb)
>>>
>>> I think that we can use that to write a caching proc like:
>>>
>>>    # Return true if the compiler emits line information associating prologue insns with
>>>    # the function's closing brace.  Return false if not, meaning the prologue
>>>    # associates prologue instructions with function's last line with a statement.
>>>
>>>    gdb_caching_proc have_prologue_line_info {
>>>        use gdb_simple_compile the simple main program from above
>>>              use "info line 5", and return false if we get "out of range", otherwise return true.
>>>    }
>>>
>>
>> This sounds like a good idea, I would only change the default behavior to returning false, unless we saw specifically "start at address.*and ends at".
> 
> ...
> 
>> This would make GDB changes more pronounced, as GCC would start having different results.
> 
> Note sure what you mean here.
> 

I meant that if something changes in GDB and the epilogue detection fails, making gcc's test result different would be easier to detect than making clang's results different.

Cheers!
Bruno Larsen