Re: [PATCH] x86: Warn .insn instruction with length > 15 bytes

[Jan Beulich <jbeulich@suse.com>]

On 07.02.2024 16:24, H.J. Lu wrote:
> On Tue, Feb 6, 2024 at 11:51 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 06.02.2024 19:06, H.J. Lu wrote:
>>> On Tue, Feb 6, 2024 at 9:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 06.02.2024 17:28, H.J. Lu wrote:
>>>>> With as_bad, assembler will continue to assemble, just not generate
>>>>> an object file.   We ran into this with APX.  Not everyone checks
>>>>> assembler warnings closely.  It led to mysterious crashes.   I am
>>>>> not against it if someone else implements an assembler option to
>>>>> turn this error into a warning.
>>>>
>>>> But it should be the other way around: The compiler could pass an
>>>> option to promote the (default) warning to an error. And if you
>>>> don#t pay attention to warning for assembly files, you could pass
>>>> the same option as well. Without harming anyone else with anything
>>>
>>> People who use/need instructions > 15 bytes belong to a very small
>>> minority.  If they want to do it, they can use .insn or use binutlls 2.41
>>> or older.  The default assembler isn't for them.
>>
>> No, staying on an old assembler isn't viable. And minority or not, you
>> have to face it: In the present discussion it is you who represents a
>> minority. As such I'm even inclined to suggest that your earlier patch
>> wants reverting, on the basis that it was put in despite there being
>> disagreement. Unless you soon come forward with an incremental change
>> undoing at least the worst of its effects ...
> 
> Please tell me exactly which projects are negatively impacted by
> disallowing > 15 byte instructions.

I already told you: I'm using such in testing of my personal disassembler
library.

>>>> that has worked before.
>>>
>>> The reason that we didn't run into the size limit before is that normal
>>> instruction usages never exceed the 15 byte limit before APX.
>>
>> Didn't I earlier give examples of "normal instructions" that have this
>> issue? I don't see anything "not normal" in insns using e.g. segment or
>> address size overrides.
>>
>> The impression I'm getting is that the problem originally was of no
>> interest to you because initially it affected AMD-specific insns only.
>> And the subsequent appearance of the same issue in HLE insns then was
>> mentally put off by you for being "too exotic" (and I partly agree
>> that _there_ one may indeed consider the example contrived). You only
>> started caring when it was an Intel extension which was noticeably
>> affected. Yet that's not the position you ought to take as a binutils
>> maintainer, imo.
> 
> Assembler should generate decodable binaries for normal instructions
> even with a warning.

See Michael's earlier response as to the wide variety of meanings of
"decodable". I'm actually of the opinion that objdump testing could also
do with a testcase as outlined above, and objdump could further do with
properly decoding such an encoding, while at the same time clearly
signaling that this doesn't look like an instruction that would
successfully execute. This is one of the many shortcomings of objdump
which keep me preferring my own disassembler for day-to-day work. And
while I'm striving to improve objdump, some of the issues would
apparently require an almost complete re-write, which I'm afraid is out
of scope for me.

Jan