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

[Sunil Pandey <skpgkp2@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2531 bytes --]

On Tue, Feb 6, 2024 at 8:30 AM H.J. Lu <hjl.tools@gmail.com> wrote:

> On Tue, Feb 6, 2024 at 7:48 AM Michael Matz <matz@suse.de> wrote:
> >
> > Hello,
> >
> > On Tue, 6 Feb 2024, H.J. Lu wrote:
> >
> > > > > > > It is an error on both Intel and AMD processors.   There is no
> > > > > > > valid reason not to be an error at the moment.
> > > > > >
> > > > > > Jan gave you one.  I would prefer for the assembler to not be
> anally
> > > > >
> > > > > That is not a valid reason.
> > > >
> > > > Yes it is.
> > > >
> > > > > There is no such processor in the foreseeable future.
> > > >
> > > > Doesn't matter.
> > > >
> > >
> > > When a warning is given, a decodable instruction should still
> > > be generated.
> >
> > A software library can decode such instruction just fine.  A human as
> well.
> >
> > > Assembler shouldn't generate something which
> > > can't be decoded by default.
> >
> > That's the important words: "by default".  Here's the documentation for
> > as_bad and as_warn:
> >
> >    as_bad() is used to mark errors that result in what we
> >    presume to be a useless object file.  Say, we ignored
> >    something that might have been vital.
> >    ...
> >
> >    as_warn() is used when we have an error from which we
> >    have a plausible error recovery.  eg, masking the top
> >    bits of a constant that is longer than will fit in the
> >    destination.  In this case we will continue to assemble
> >    the source, although we may have made a bad assumption,
> >    and we will produce an object file and return normal exit
> >    status (ie, no error).
> >    ...
> >
> > It's obvious to me that just continuing to assemble the over-long
> > instruction is a "plausible error recovery".  It's even more plausible
> > than "masking the top bits of a constant".  Certainly an object file
> > containing a byte sequence correspending to the overlong instruction is
> > not "useless".
>
> 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.
>
>

IMO, most people check error at first glance not warning.

Making it error by default with proper error message will

save precious debugging time for assembler users, especially

if it can avoid mysterious crashes down the road.