public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: binutils@sourceware.org, Michael Matz <matz@suse.de>
Subject: Re: [PATCH] x86: Warn .insn instruction with length > 15 bytes
Date: Wed, 7 Feb 2024 08:53:52 -0800	[thread overview]
Message-ID: <CAMe9rOrvKd=pcemPz1=BAj35SO=q2CvvuZkA0JbgdsKXmohZag@mail.gmail.com> (raw)
In-Reply-To: <93ef6fde-42aa-485a-8b22-f2c1f1d52efa@suse.com>

On Wed, Feb 7, 2024 at 7:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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.

So, it is only you.  You can either use .insn or add a switch to turn
this error to warning.

> >>>> 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



-- 
H.J.

  reply	other threads:[~2024-02-07 16:54 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-05 20:00 H.J. Lu
2024-02-06  8:19 ` Jan Beulich
2024-02-06 11:36   ` H.J. Lu
2024-02-06 11:41     ` Jan Beulich
2024-02-06 12:26       ` H.J. Lu
2024-02-06 14:43         ` Michael Matz
2024-02-06 14:49           ` H.J. Lu
2024-02-06 15:04             ` Michael Matz
2024-02-06 15:34               ` H.J. Lu
2024-02-06 15:48                 ` Michael Matz
2024-02-06 16:28                   ` H.J. Lu
2024-02-06 17:05                     ` Jan Beulich
2024-02-06 18:06                       ` H.J. Lu
2024-02-07  7:51                         ` Jan Beulich
2024-02-07 15:24                           ` H.J. Lu
2024-02-07 15:32                             ` Jan Beulich
2024-02-07 16:53                               ` H.J. Lu [this message]
2024-02-07 16:59                                 ` Jan Beulich
2024-02-07 17:03                                   ` H.J. Lu
2024-02-08  4:09                                     ` Jiang, Haochen
2024-02-08  4:47                                       ` Hongyu Wang
2024-02-08  8:15                                         ` Jan Beulich
2024-02-08  8:23                                     ` Jan Beulich
2024-02-08 11:38                                       ` H.J. Lu
2024-02-08  6:26                     ` Sunil Pandey
2024-02-08  6:41                   ` Cui, Lili
2024-02-08  8:18                     ` Jan Beulich
2024-02-08 11:31                       ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMe9rOrvKd=pcemPz1=BAj35SO=q2CvvuZkA0JbgdsKXmohZag@mail.gmail.com' \
    --to=hjl.tools@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=jbeulich@suse.com \
    --cc=matz@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).