Re: [PATCH] [gdb/tdep] Assume epilogue unwind info is valid unless gcc < 4.5.0

[Tom de Vries <tdevries@suse.de>]

On 1/27/23 22:13, Tom de Vries via Gdb-patches wrote:
> On 1/21/23 18:48, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries via Gdb-patches 
>>>>>>> <gdb-patches@sourceware.org> writes:
>>
>> Tom> Fix these two issues by reversing the burden of proof:
>> Tom> - currently we assume epilogue unwind info is invalid unless we 
>> can proof that
>> Tom>   gcc >= 4.5.0.
>> Tom> - instead, assume epilogue unwind info is valid unless we can 
>> proof that
>> Tom>   gcc < 4.5.0.
>>
>> FWIW this approach makes sense to me.
>>
> 
> OK, then changing RFC -> PATCH.
> 
>> It's pretty lame that there's no way to detect this failure from the
>> frame section -- it can't be producer-sniffed and the augmentation
>> strings can't really be changed.
>>
>> gcc 4.5 was released in 2010, and so it's not like we're inconveniencing
>> a lot of users.  If needed I guess we could add a user setting to switch
>> this behavior back on.
>>
>> Note there is a similar issue for the prologue, see:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=25696
>> https://sourceware.org/bugzilla/show_bug.cgi?id=17265
>> https://sourceware.org/bugzilla/show_bug.cgi?id=21470
>>
>> Also worth seeing the hilarious:
>>
>> https://github.com/rust-lang/rust/issues/41252#issuecomment-293676579
>>
>> I think that in this area we should assume the debug info is correct,
>> and keep a list of known-bad producers rather than assuming the debug
>> info is wrong and having a list of known-good ones.
>>
>> Tom> +  if (/* In absence of producer information, optimistically 
>> assume that we're
>> Tom> +     not dealing with gcc < 4.5.0.  */
>>
>> This placement of the comment is pretty weird, it seems fine to just
>> stick it before the 'if'.
>>
> 
> Done.
> 
>> Tom> +      if (cu->producer == nullptr)
>> Tom> +    /* In absence of producer information, optimistically assume 
>> that we're
>> Tom> +       not dealing with gcc < 4.5.0.  */
>> Tom> +    cust->set_epilogue_unwind_valid (true);
>> Tom> +      if (!producer_is_gcc (cu->producer, nullptr, nullptr))
>>
>> Normally if there is a comment and a line of code as the consequence of
>> an 'if', we put them both in a block.
>>
> 
> Done.
> 
>> Anyway I was also thinking that the second one should say 'else if'.
> 
> True, thanks for catching that, also done.
> 
> I've also added a test-case, for the amd64-tdep.c change. I could make 
> another one for the i386-tdep.c change, and/or one for the dwarf/read.c 
> change, but I'm not sure that's worth the trouble.
> 

Posted v2 here: 
https://sourceware.org/pipermail/gdb-patches/2023-February/197011.html

Thanks,
- Tom