[ was: Re: [PATCH][gdb/testsuite] Rewrite gdb.base/store.exp into .s test ]

On 29-08-19 20:07, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2019-08-29 19:47:12 +0200]:
> 
>> Hi,
>>
>> The test-case gdb.base/store.exp fails with gcc 7.4.0:
>> ...
>> nr of unexpected failures        27
>> ...
>> and with gcc 4.8.5:
>> ...
>> nr of unexpected failures        9
>> ...
>>
>> The test-case relies on "the compiler taking heed of requests for values to be
>> stored in registers", which appearantly isn't the case anymore for
>> modern gcc.
> 
> Could you expand on this a little more.  I took a quick look and it
> appears more that variables just have missing location information.

You're right, I jumped to a conclusion here, sorry for not being more
careful.

> Sure the test marks the variables with the 'register' keyword, but
> surely GDB should still pass the test even if the variable is placed
> on the stack?
> 

Agreed (and indeed, not obeing the 'register' hint, emulated by "#define
register" makes the test pass).

>>
>> Fix this by changing this into an assembly file test-case, and generating the
>> assembly file using gcc 4.2.1.
>>
>> Tested on x86_64-linux.
>>
>> OK for trunk?
> 
> No.  What about architectures other than x86-64?
> 
> I took a quick look, and maybe I missed something, but I don't think
> that there are any architecture specific assembler tests in gdb.base/
> and I don't think we should be adding any.
> 
> Maybe we should add a version of this test into gdb.arch along with
> the assembler file for x86-64.
> 
> On a slightly different note, I've run into this test before, and I'm
> pretty sure that the test is broken, it's been a while since I dug
> into this but consider these snippets:
> 
>     ...
> 
>     float
>     add_float (register float u, register float v)
>     {
>       return u + v;
>     }
> 
>     ...
> 
>     wack_float (register float u, register float v)
>     {
>       register float l = u, r = v;
>       l = add_float (l, r);
>       return l + r;
>     }
> 
>     ...
> 
> Part of the test involves breaking on 'add_float' then going 'up' to
> 'wack_float' and printing 'l'.  GDB expects an answer.
> 
> My problem with this is that on many architectures, even at
> optimisation level 0 'l' is dead, or at least non-recoverable at the
> point of the call to add_float due to being placed in a caller saved
> argument register.
> 

I've investigated the FAILs related to the wack_float function, and the
test-case expects to access and modify l, but it can't because there's
no DW_AT_location for l, which AFAIU is valid behaviour of gcc for a
register variable at -O0.

So, ISTM the FAILs need to be fixed by marking the failing tests as
unsupported, in case l shows up as <optimized out>.

> Anyway, I agree with you that this test is in need of some clean up,
> I'm just not convinced on this approach yet.
> 

Better like this?

Thanks,
- Tom