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

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-08-25 12:18, Pedro Alves wrote:
> $subject should be "unbreak", I hope.  :-)
> 
> On 08/25/2017 02:33 AM, Weimin Pan wrote:
> 
>> @@ -240,7 +242,7 @@ adi_normalize_address (CORE_ADDR addr)
>>    adi_stat_t ast = get_adi_info (ptid_get_pid (inferior_ptid));
>> 
>>    if (ast.nbits)
>> -    return ((CORE_ADDR)(((long)addr << ast.nbits) >> ast.nbits));
>> +    return (addr & ((1 << ast.nbits) - 1)) ^ (addr & (-1 >> 
>> ast.nbits));
> 
> How did you test this?  Doesn't look right to me.
> Also, "-1 >>" is still an implementation-defined signed
> right shift.
> 
> I _think_ this is what you want:
> 
>   /* Clear upper bits.  */
>   addr &= ((uint64_t) -1) >> ast.nbits;
> 
>   /* Sign extend.  */
>   CORE_ADDR signbit = (uint64_t) 1 << (64 - ast.nbits - 1);
>   return (addr ^ signbit) - signbit;
> 
> I.e., with ast.nbits == 4:
> 
>  before: ffffffffffffffff
>  after:  ffffffffffffffff
> 
>  before: f7ffffffffffffff
>  after:  07ffffffffffffff

Since it's not trivial, I would suggest putting this operation in its 
own function and adding a unit-test for it.

Simon