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

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Feb 8, 2024 at 12:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 07.02.2024 18:03, H.J. Lu wrote:
> > On Wed, Feb 7, 2024 at 8:59 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 07.02.2024 17:53, H.J. Lu wrote:
> >>> 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.
> >>
> >> I has been a warning until 2.42, which you've regressed for my use case.
> >> This is why I expect you to at least soften the regression, in allowing
> >> people like me to simply add a command line option to the gas invocations.
> >
> > So this is for your personal use case.  If you asked nicely, I might have
> > considered spending my time on this.
>
> Well, to be blunt: I would have asked more nicely if you hadn't overridden
> my concern. The more that this isn't the first time that you went ahead
> with changes without having reached consensus.

Well, you disagreed on my change to make the long instruction an error
by default.  For most x86 binutils developers, it is a non-starter.

> >> Plus, as you have learnt from Michael's responses, I'm not the only one
> >> to think that this diagnostic ought to continue to be a warning by
> >> default.
> >
> > I can also tell you that there are other binutils developers who want to
> > treat this as an error.   Should I ask them for their opinions?
>
> From further reactions I can see that I suddenly moved to the minority. So

You shouldn't be surprised.

> be it then, so long as a future patch to allow this diagnostic to be
> converted to a warning won't be blocked.
>

Thank you for your work and feedback.  I am not against it if someone
else adds an option to change the error to warning as long as it isn't
the default.

-- 
H.J.