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

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

[H.J. Lu]

Change .insn instruction with length > 15 bytes from error to warning.

	PR gas/31323
	* config/tc-i386.c (output_insn): Issue a warning when .insn
	instruction length exceeds the limit of 15 bytes.
	* testsuite/gas/i386/oversized64.s: Add a test for .insn
	* testsuite/gas/i386/oversized64.l: Updated.
---
 gas/config/tc-i386.c                 | 10 ++++++++--
 gas/testsuite/gas/i386/oversized64.l |  8 ++++++++
 gas/testsuite/gas/i386/oversized64.s |  3 +++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 8f3a1b6f686..20d17c954f8 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -11780,8 +11780,14 @@ output_insn (const struct last_insn *last_insn)
 	{
 	  j = encoding_length (insn_start_frag, insn_start_off, frag_more (0));
 	  if (j > 15)
-	    as_bad (_("instruction length of %u bytes exceeds the limit of 15"),
-		    j);
+	    {
+	      if (i.tm.mnem_off == MN__insn)
+		as_warn (_("instruction length of %u bytes exceeds the limit of 15"),
+			j);
+	      else
+		as_bad (_("instruction length of %u bytes exceeds the limit of 15"),
+			j);
+	    }
 	  else if (fragP)
 	    {
 	      /* NB: Don't add prefix with GOTPC relocation since
diff --git a/gas/testsuite/gas/i386/oversized64.l b/gas/testsuite/gas/i386/oversized64.l
index ac32c4d8139..f709456c5e5 100644
--- a/gas/testsuite/gas/i386/oversized64.l
+++ b/gas/testsuite/gas/i386/oversized64.l
@@ -4,6 +4,7 @@
 .*:7: Error: instruction length.*
 .*:9: Error: instruction length.*
 .*:10: Error: instruction length.*
+.*:13: Warning: instruction length.*
 GAS LISTING .*
 
 
@@ -37,4 +38,11 @@ GAS LISTING .*
 [ 	]*10[ 	]+48812CC5[ 	]*
 [ 	]*10[ 	]+00000000[ 	]*
 [ 	]*10[ 	]+44332211[ 	]*
+[ 	]*11[ 	]+
+[ 	]*12[ 	]+\.att_syntax prefix
+[ 	]*13[ 	]+\?\?\?\? 6762F1FC[ 	]+.insn EVEX.L0.0f 12/0, \$0x11223344,\(,%eax,8\),%rax
+\*\*\*\*  Warning: instruction length of 16 bytes exceeds the limit of 15
+[ 	]*13[ 	]+080C04C5[ 	]*
+[ 	]*13[ 	]+00000000[ 	]*
+[ 	]*13[ 	]+44332211[ 	]*
 #pass
diff --git a/gas/testsuite/gas/i386/oversized64.s b/gas/testsuite/gas/i386/oversized64.s
index 9db09af3f15..77a08a9c533 100644
--- a/gas/testsuite/gas/i386/oversized64.s
+++ b/gas/testsuite/gas/i386/oversized64.s
@@ -8,3 +8,6 @@ long64:
 
 	xacquire lock add qword ptr gs:[eax*8], 0x11223344
 	xrelease lock sub qword ptr gs:[eax*8], 0x11223344
+
+	.att_syntax prefix
+	.insn EVEX.L0.0f 12/0, $0x11223344,(,%eax,8),%rax
-- 
2.43.0

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

[Jan Beulich]

On 05.02.2024 21:00, H.J. Lu wrote:
> Change .insn instruction with length > 15 bytes from error to warning.

Thanks for doing this. FTAOD though - it addresses only half of my
concern. Besides .insn (where the concern was of general nature) I
also use

	bextr	eax, fs:[eax*4], 0x11223344
	xacquire lock add qword ptr gs:[eax*8], 0x11223344

in testing of my own disassembler library. I expect to continue to
be able to avoid using .insn (and even more so .byte) when assembling
this code. IOW there will still need to be a way to also override
the defaulting to as_bad() when not using .insn.

> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -11780,8 +11780,14 @@ output_insn (const struct last_insn *last_insn)
>  	{
>  	  j = encoding_length (insn_start_frag, insn_start_off, frag_more (0));
>  	  if (j > 15)
> -	    as_bad (_("instruction length of %u bytes exceeds the limit of 15"),
> -		    j);
> +	    {
> +	      if (i.tm.mnem_off == MN__insn)

Please don't open-code dot_insn().

Jan

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

[H.J. Lu]

On Tue, Feb 6, 2024 at 12:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 05.02.2024 21:00, H.J. Lu wrote:
> > Change .insn instruction with length > 15 bytes from error to warning.
>
> Thanks for doing this. FTAOD though - it addresses only half of my
> concern. Besides .insn (where the concern was of general nature) I
> also use
>
>         bextr   eax, fs:[eax*4], 0x11223344
>         xacquire lock add qword ptr gs:[eax*8], 0x11223344
>
> in testing of my own disassembler library. I expect to continue to
> be able to avoid using .insn (and even more so .byte) when assembling
> this code. IOW there will still need to be a way to also override
> the defaulting to as_bad() when not using .insn.

We issue a warning when something is wrong in input, but still manage
to generate an instruction.   This is an error case.

> > --- a/gas/config/tc-i386.c
> > +++ b/gas/config/tc-i386.c
> > @@ -11780,8 +11780,14 @@ output_insn (const struct last_insn *last_insn)
> >       {
> >         j = encoding_length (insn_start_frag, insn_start_off, frag_more (0));
> >         if (j > 15)
> > -         as_bad (_("instruction length of %u bytes exceeds the limit of 15"),
> > -                 j);
> > +         {
> > +           if (i.tm.mnem_off == MN__insn)
>
> Please don't open-code dot_insn().
>

Fixed in v2.

-- 
H.J.

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

[Jan Beulich]

On 06.02.2024 12:36, H.J. Lu wrote:
> On Tue, Feb 6, 2024 at 12:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.02.2024 21:00, H.J. Lu wrote:
>>> Change .insn instruction with length > 15 bytes from error to warning.
>>
>> Thanks for doing this. FTAOD though - it addresses only half of my
>> concern. Besides .insn (where the concern was of general nature) I
>> also use
>>
>>         bextr   eax, fs:[eax*4], 0x11223344
>>         xacquire lock add qword ptr gs:[eax*8], 0x11223344
>>
>> in testing of my own disassembler library. I expect to continue to
>> be able to avoid using .insn (and even more so .byte) when assembling
>> this code. IOW there will still need to be a way to also override
>> the defaulting to as_bad() when not using .insn.
> 
> We issue a warning when something is wrong in input, but still manage
> to generate an instruction.   This is an error case.

I disagree. It was a warning until you changed it (without me really
agreeing), and some hypothetical vendor could come and lift the limit,
on precisely the basis that there are legal encodings exceeding it. I'm
okay if this can be forced to be an error, but I continue to think that
it ought to be a warning only by default.

Jan

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

[H.J. Lu]

On Tue, Feb 6, 2024 at 3:41 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 06.02.2024 12:36, H.J. Lu wrote:
> > On Tue, Feb 6, 2024 at 12:19 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 05.02.2024 21:00, H.J. Lu wrote:
> >>> Change .insn instruction with length > 15 bytes from error to warning.
> >>
> >> Thanks for doing this. FTAOD though - it addresses only half of my
> >> concern. Besides .insn (where the concern was of general nature) I
> >> also use
> >>
> >>         bextr   eax, fs:[eax*4], 0x11223344
> >>         xacquire lock add qword ptr gs:[eax*8], 0x11223344
> >>
> >> in testing of my own disassembler library. I expect to continue to
> >> be able to avoid using .insn (and even more so .byte) when assembling
> >> this code. IOW there will still need to be a way to also override
> >> the defaulting to as_bad() when not using .insn.
> >
> > We issue a warning when something is wrong in input, but still manage
> > to generate an instruction.   This is an error case.
>
> I disagree. It was a warning until you changed it (without me really
> agreeing), and some hypothetical vendor could come and lift the limit,
> on precisely the basis that there are legal encodings exceeding it. I'm
> okay if this can be forced to be an error, but I continue to think that
> it ought to be a warning only by default.
>

It is an error on both Intel and AMD processors.   There is no
valid reason not to be an error at the moment.  We can change
it when there is such a vendor in the future.

-- 
H.J.

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

[Michael Matz]

Hello,

On Tue, 6 Feb 2024, H.J. Lu wrote:

> > > We issue a warning when something is wrong in input, but still manage
> > > to generate an instruction.   This is an error case.
> >
> > I disagree. It was a warning until you changed it (without me really
> > agreeing), and some hypothetical vendor could come and lift the limit,
> > on precisely the basis that there are legal encodings exceeding it. I'm
> > okay if this can be forced to be an error, but I continue to think that
> > it ought to be a warning only by default.
> >
> 
> 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 
retentive.  An error that can be disabled is completely enough for this.

(I would wish for a general demote-all-errors-to-warnings option, like we 
already have a promote-all-warnings-to-errors; but lacking this an option 
to merely disable "errors" for avoiding specific random hardware behaviour 
would be fine as well)


Ciao,
Michael.

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

[H.J. Lu]

On Tue, Feb 6, 2024 at 6:43 AM Michael Matz <matz@suse.de> wrote:
>
> Hello,
>
> On Tue, 6 Feb 2024, H.J. Lu wrote:
>
> > > > We issue a warning when something is wrong in input, but still manage
> > > > to generate an instruction.   This is an error case.
> > >
> > > I disagree. It was a warning until you changed it (without me really
> > > agreeing), and some hypothetical vendor could come and lift the limit,
> > > on precisely the basis that there are legal encodings exceeding it. I'm
> > > okay if this can be forced to be an error, but I continue to think that
> > > it ought to be a warning only by default.
> > >
> >
> > 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.  There is no such processor in the foreseeable
future.

> retentive.  An error that can be disabled is completely enough for this.
>
> (I would wish for a general demote-all-errors-to-warnings option, like we
> already have a promote-all-warnings-to-errors; but lacking this an option
> to merely disable "errors" for avoiding specific random hardware behaviour
> would be fine as well)
>
>
> Ciao,
> Michael.



-- 
H.J.

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

[Michael Matz]

> > > > I disagree. It was a warning until you changed it (without me really
> > > > agreeing), and some hypothetical vendor could come and lift the limit,
> > > > on precisely the basis that there are legal encodings exceeding it. I'm
> > > > okay if this can be forced to be an error, but I continue to think that
> > > > it ought to be a warning only by default.
> > > >
> > >
> > > 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.


Ciao,
Michael.

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

[H.J. Lu]

On Tue, Feb 6, 2024 at 7:04 AM Michael Matz <matz@suse.de> wrote:
>
> > > > > I disagree. It was a warning until you changed it (without me really
> > > > > agreeing), and some hypothetical vendor could come and lift the limit,
> > > > > on precisely the basis that there are legal encodings exceeding it. I'm
> > > > > okay if this can be forced to be an error, but I continue to think that
> > > > > it ought to be a warning only by default.
> > > > >
> > > >
> > > > 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.  Assembler shouldn't generate something which
can't be decoded by default.

-- 
H.J.

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

[Michael Matz]

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

I understand why you want to give an error by default, even though I 
disagree with even that (in my book only a warning is justified).  But 
ruling out that this can be demoted to a warning, possibly with an option, 
is not in line with my expectation of how the GNU assembler should work 
and has traditionally worked.


Ciao,
Michael.

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

[H.J. Lu]

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.

> I understand why you want to give an error by default, even though I
> disagree with even that (in my book only a warning is justified).  But
> ruling out that this can be demoted to a warning, possibly with an option,
> is not in line with my expectation of how the GNU assembler should work
> and has traditionally worked.
>
>
> Ciao,
> Michael.



-- 
H.J.

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

[Jan Beulich]

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
that has worked before.

Jan

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

[H.J. Lu]

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.

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


-- 
H.J.

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

[Jan Beulich]

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

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

Jan

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

[H.J. Lu]

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.

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

-- 
H.J.

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

[Jan Beulich]

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

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

[H.J. Lu]

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.

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

[Jan Beulich]

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.

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.

Jan

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

[H.J. Lu]

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.

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

-- 
H.J.

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

[Jiang, Haochen]

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

IMO, since 15 bytes is a golden rule, generating code for >15 byte should
be treated as wrong and throw an error.

Especially when there are more and more prefixes, users gets more chance
to generate too long code and get wrong result when they even don't know
why if they don't care about warning. It is unacceptable.

We need to first meet the scenario for those users who will not dig into
everything. Therefore, warning is not enough.

For those who need more bytes, we could assume that they are quite familiar
with assembler, an option to lower to warning for them is ok

Thx,
Haochen

> 
> 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?
> 
> --
> H.J.

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

[Hongyu Wang]

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

We have encountered such issue when running APX workloads with SDE.
The over-long instruction became either illegal or wrong instruction
when executing the program, then it crashes after long time running.
This definitely increases much effort for runtime diagnostics.
As the over-long instruction would actually break the program, say the
object crashes or returns with abnormal exit status, the default
should be an error for assembler and we can add an option to lower the
error to warning.

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

[Sunil Pandey]

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

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

[Cui, Lili]

> > 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".
> 
> I understand why you want to give an error by default, even though I disagree
> with even that (in my book only a warning is justified).  But ruling out that this
> can be demoted to a warning, possibly with an option, is not in line with my
> expectation of how the GNU assembler should work and has traditionally
> worked.
> 

It is a hardware limitation. Once an incorrect command occurs, it will be difficult to locate. This is a critical issue and should be reported as an error, especially with the addition of APX our prefixes are becoming more complex and diverse, it's time to make a change.

Thx,
Lili.

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

[Jan Beulich]

On 08.02.2024 05:47, Hongyu Wang wrote:
>>    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".
> 
> We have encountered such issue when running APX workloads with SDE.
> The over-long instruction became either illegal or wrong instruction
> when executing the program, then it crashes after long time running.
> This definitely increases much effort for runtime diagnostics.

Well, that's what you get for not paying attention to diagnostics.
They aren't emitted just for fun.

Jan

> As the over-long instruction would actually break the program, say the
> object crashes or returns with abnormal exit status, the default
> should be an error for assembler and we can add an option to lower the
> error to warning.

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

[Jan Beulich]

On 08.02.2024 07:41, Cui, Lili wrote:
>>> 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".
>>
>> I understand why you want to give an error by default, even though I disagree
>> with even that (in my book only a warning is justified).  But ruling out that this
>> can be demoted to a warning, possibly with an option, is not in line with my
>> expectation of how the GNU assembler should work and has traditionally
>> worked.
>>
> 
> It is a hardware limitation. Once an incorrect command occurs, it will be difficult to locate. This is a critical issue and should be reported as an error, especially with the addition of APX our prefixes are becoming more complex and diverse, it's time to make a change.

I find this (including H.J.'s) position odd: Until me introducing the
warning, no-one cared at all. Presumably because, as indicated before,
Intel ISA extensions weren't really affected. Now suddenly everyone's
calling for an even stronger diagnostic. IOW if there is a concern now,
the same concern should have been there many years earlier.

Jan

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

[Jan Beulich]

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.

>> 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
be it then, so long as a future patch to allow this diagnostic to be
converted to a warning won't be blocked.

Jan

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

[H.J. Lu]

On Thu, Feb 8, 2024 at 12:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.02.2024 07:41, Cui, Lili wrote:
> >>> 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".
> >>
> >> I understand why you want to give an error by default, even though I disagree
> >> with even that (in my book only a warning is justified).  But ruling out that this
> >> can be demoted to a warning, possibly with an option, is not in line with my
> >> expectation of how the GNU assembler should work and has traditionally
> >> worked.
> >>
> >
> > It is a hardware limitation. Once an incorrect command occurs, it will be difficult to locate. This is a critical issue and should be reported as an error, especially with the addition of APX our prefixes are becoming more complex and diverse, it's time to make a change.
>
> I find this (including H.J.'s) position odd: Until me introducing the
> warning, no-one cared at all. Presumably because, as indicated before,
> Intel ISA extensions weren't really affected. Now suddenly everyone's
> calling for an even stronger diagnostic. IOW if there is a concern now,
> the same concern should have been there many years earlier.
>

Before APX, this isn't a concern since GCC won't generate
such long instructions.   After we got reports from people using
APX GCC on real applications, it raised the alarm and changed
my opinion.

-- 
H.J.

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

[H.J. Lu]

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.