offset_in_range() question

Jan Beulich jbeulich@suse.com
Mon Jul 13 14:43:18 GMT 2020


On 13.07.2020 15:11, H.J. Lu wrote:
> On Mon, Jul 13, 2020 at 5:50 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> H.J.,
>>
>> the dependency on presence of an address size override, when the
>> function is also used on immediate operands, struck me as being
>> a possible source for problems. On 2009-09-15 you've made two
>> changes to the construct in question, but I wonder whether even
>> back then this code wasn't already dead. When, after quite a bit
>> of playing, I couldn't come up with any immediate that this
>> would go wrong on, I thought I'll give a try to removing that
>> code altogether. And indeed - no fallout. Looking more closely
>> then suggested that respective logic in optimize_imm() and
>> optimize_disp() are already arranging for values to never need
>> further massaging here.
>>
>> Do you agree that the code could be removed (see patch below in
>> case of any uncertainty about what block of code I mean), or are
>> you aware of things that might break this way?
>>
>> As an aside, I don't think the handling of out-of-range
>> immediates is quite correct, but I'll get to this in more detail
>> after thinking some more on possible solutions.
>>
>> Jan
>>
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -2539,14 +2539,6 @@ offset_in_range (offsetT val, int size)
>>      default: abort ();
>>      }
>>
>> -#ifdef BFD64
>> -  /* If BFD64, sign extend val for 32bit address mode.  */
>> -  if (flag_code != CODE_64BIT
>> -      || i.prefix[ADDR_PREFIX])
>> -    if ((val & ~(((addressT) 2 << 31) - 1)) == 0)
>> -      val = (val ^ ((addressT) 1 << 31)) - ((addressT) 1 << 31);
>> -#endif
>> -
> 
> This code came from
> 
> commit 3e73aa7c956514ce5bd5fa6320fb239229ac8a7b
> Author: Jan Hubicka <jh@suse.cz>
> Date:   Wed Dec 20 13:24:13 2000 +0000
> 
>             * tc-i386.h (i386_target_format): Define even for ELFs.
> 
> My changes only enabled it when BFD64 is defined.  If this code
> dead, please replace it with abort for now.

I guess I don't understand: There's no condition to abort() on right
now. The code I'm proposing to delete simply does nothing useful. Or
do you mean to turn the assignment into an != to control when to
abort()?

Doing so would undermine the main purpose of deleting this code: Its
dependency on address prefix presence is what needs to go away.

Jan


More information about the Binutils mailing list