Re: [PATCH 3/3] opcodes/i386: partially implement disassembler style support

[Jan Beulich <jbeulich@suse.com>]

On 17.02.2022 23:37, Andrew Burgess wrote:
> Jan Beulich via Binutils <binutils@sourceware.org> writes:
>> On 17.02.2022 17:15, Andrew Burgess wrote:
>>> Jan Beulich via Binutils <binutils@sourceware.org> writes:
>>>> On 16.02.2022 21:53, Andrew Burgess via Binutils wrote:
>>>>> +	      (*ins->info->fprintf_styled_func)
>>>>> +		(ins->info->stream, dis_style_text, " ");
>>>>> +	      (*ins->info->fprintf_styled_func)
>>>>> +		(ins->info->stream, dis_style_immediate, "0x%x",
>>>>> +		 (unsigned int) priv.the_buffer[0]);
>>>>
>>>> I wonder if the naming (dis_style_immediate) isn't misleading. As per
>>>> the comment next to its definition it really appears to mean any kind
>>>> of number (like is the case here), not just immediate operands of
>>>> instructions. Hence maybe dis_style_number (as replacement for or in
>>>> addition to dis_style_immediate)?
>>>
>>> You mentioned this before in the previous thread, and I didn't really
>>> understand then either.
>>>
>>> Can you give an example of something that's a number, but not an
>>> immediate?  e.g. I wonder (given the instruction/directive distinction
>>> you draw above), I wonder if you're conserned about: '.byte 0x4', maybe
>>> you don't like referring to this 0x4 here as an immediate?
>>
>> Well, an operand to a directive for example is not an immediate imo,
>> yes. A "load offset" (as your comment calls it) may also not be an
>> immediate. E.g. in x86 memory access instructions:
>>
>> 	mov	0x10(%rbx), %eax
>>
>> the 0x10 isn't an immediate, but a displacement. The difference may
>> be more relevant in something like
>>
>> 	mov	$0, 0x10(%rbx)
>>
>> where the $0 is an immediate operand, but the 0x10 isn't (and you
>> wouldn't want to mix the two).
>>
>> From that comment it's not clear to me where else you would think
>> "immediate" applies (or not), but in RISC-V's
>>
>> 	lw	x0, 0x10(x0)
>>
>> I wouldn't consider the 0x10 an immediate either, albeit this may
>> be a result of my x86 bias.
> 
> I wonder if there's a name we could come up with that would allow me to
> classify the '$0' and '0x10' (in your example above) as the same style?
> 
> I've kind-of lost the thread a bit, but maybe that's what the 'number'
> you suggested original was for?  If I replaced dis_style_immediate with
> dis_style_number, and just replaced thoughout, would that be less
> problematic?

Yes, "number" was meant to be a possible replacement. Whether it's
helpful to style all forms of numbers the same is questionable
though. Note that to me in "$0" the '$' would not be covered by
"number" then, while it would be covered by "immediate".

> Another possibility would be to have some aliases either in the original
> enum, as in:
> 
>   dis_style_displacement = dis_style_immediate,
> 
> or even at the top of i386-dis.c, as in:
> 
>   #define dis_style_displacement dis_style_immediate

But that's wrong. I'm not primarily after the naming in the sources,
but after the output not showing distinct things as similar.

> I really think we should avoid adding too many distinct styles if we
> can.  My concern is less about disassembler users handling the different
> styles, and more about consistency between the disassemblers.  I figure
> it's easier to be consistent if we only have a small number of styles.
> If displacement is a different style to other immediate (yes, I'm still
> going to call numbers in instruction immediates!) then we end up with
> some architectures going one way, and others another.

I agree with the goal of limiting the number of styles. But instead
of marking distinct items with similar non-basic (text) styles, I'd
rather see items left alone then. IOW with dis_style_immediate I'd
prefer (in the x86 example) to see only true immediate insn operands
be tagged that way, and all other numbers to remain dis_style_text.

Jan