Re: [PATCH] break gdb build on 32-bit host with ADI support

[Pedro Alves <palves@redhat.com>]

On 08/24/2017 09:27 PM, Wei-min Pan wrote:

>>>> Use paddress instead?  Also spaces around '*' and after the cast.
>>> Where is paddress defined? I tried casting to "uint64" which yields to
>>> "unsigned long" on a 64-bit host and didn't bode well with %llx.
>>   $ grep paddress *.h
>>   utils.h:extern const char *paddress (struct gdbarch *gdbarch,
>> CORE_ADDR addr);
> 
> On a 64-bit host:
> ...
> sparc64-tdep.c: In function 'void do_assign(CORE_ADDR, size_t, int)':
> sparc64-tdep.c:449:56: error: expected ')' before 'vaddr'
>      error(_("No ADI information at 0x%llx"), (paddress)vaddr);
>                                                         ^~~~~
> sparc64-tdep.c:449:61: error: format '%llx' expects argument of type
> 'long long
> unsigned int', but argument 2 has type 'const char* (*)(gdbarch*,
> CORE_ADDR) {ak
> a const char* (*)(gdbarch*, long unsigned int)}' [-Werror=format=]
>      error(_("No ADI information at 0x%llx"), (paddress)vaddr);
>                                                              ^
> cc1plus: all warnings being treated as errors
> make: *** [sparc64-tdep.o] Error 1

Of course.  paddress is a function.  See any of the other ~400 uses
in the tree.  Something like:

 error(_("No ADI information at %s"), paddress (gdbarch, vaddr));

>>>> looks suspiciously bogus to me.  Consider a 32-bit host
>>>> remote/cross debugging a SPARC64 target machine.  Also consider
>>>> a Win64-hosted GDB.
>>> Good point. Changing it to:
>>>
>>>        return ((long long)(((long long)addr << ast.nbits) >>
>>> ast.nbits));
>>>
>>> Thanks.
>> Still looks odd to me.
>> Why are you shifting signed types, for instance?
>> Any why do you need the casts in the first place, BTW?
> 
> We need to sign extend to get a normalized address, based on the
> definitions in ADI space:
> 
>         ADI versioned address - a VA with ADI bits (63-60) set
>         Normalized address - a VA with bit 59 sign extended into ADI bits

OK, but then please do it with valid/portable C/C++ code, with
the usual masking and xoring.
Left shifting signed integer values is undefined C++11.
Right shifting a signed integer is implementation defined.

Thanks,
Pedro Alves