[PATCH] x86: Add .nop directive to assembler

classic Classic list List threaded Threaded
47 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[PATCH] x86: Add .nop directive to assembler

H.J. Lu-30
On Mon, Feb 12, 2018 at 7:48 AM, Maciej W. Rozycki <[hidden email]> wrote:

> On Mon, 12 Feb 2018, H.J. Lu wrote:
>
>> My implementation uses the existing relaxation frame work.
>> When we are processing .nop, we don't know exactly how big the
>> the NOP size will be.  We allocate a frag with the maximum size
>> and set the exact size after relaxation.  After relaxation is done,
>> all frags are converted to rs_fill.  We can add rs_fill_nop to
>> support arbitrary .nop directive size. But I don't know if it is
>> necessary.
>
>  Right, so this is needed for argument expressions using forward
> references.  Understood and accepted.  Thank your for patience.
>
Implement the '.nop SIZE[, LIMIT]' directive for x86 assembler.  This
directive emits SIZE bytes filled with 'NOP' instructions.  SIZE is
absolute expression, which must be between 0 and 512.  LIMIT specifies
the size limit of a single 'NOP' instruction.  If the comma and LIMIT
are omitted, LIMIT is assumed to the maximum supported size of a single
'NOP' instruction.  The valid values of LIMIT are between 1 and 8 for
16-bit mode, between 1 and 10 for 32-bit mode, between 1 and 11 for
64-bit mode.  This directive is only allowed in text sections.

This is implemented by adding a relax state, rs_space_nop, to enum
_relax_state, which is similar to rs_space, but it fills with NOPs,
instead of a single byte.  A pseudo relocation, BFD_RELOC_NOP_DIRECTIVE,
is added to fix up frag data with the proper number of NOPs.  The new
rs_space_nop state is processed only when TARGET_USE_NOP_DIRECTIVE is
defined.  To enable .nop directive, a target backend should

1. Define TARGET_USE_NOP_DIRECTIVE.
2. Create a rs_space_nop frag for .nop directive.
3. Update md_convert_frag to create a fixup with BFD_RELOC_NOP_DIRECTIVE
for rs_space_nop frag.
4. Update md_apply_fix to process fixup with BFD_RELOC_NOP_DIRECTIVE.

OK for master?

Andrew, please test my current users/hjl/nop branch.

Thanks.


--
H.J.

0001-x86-Add-.nop-directive-to-assembler.patch (59K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Jan Beulich-2
>>> On 12.02.18 at 20:38, <[hidden email]> wrote:
> On Mon, Feb 12, 2018 at 7:48 AM, Maciej W. Rozycki <[hidden email]> wrote:
>> On Mon, 12 Feb 2018, H.J. Lu wrote:
>>
>>> My implementation uses the existing relaxation frame work.
>>> When we are processing .nop, we don't know exactly how big the
>>> the NOP size will be.  We allocate a frag with the maximum size
>>> and set the exact size after relaxation.  After relaxation is done,
>>> all frags are converted to rs_fill.  We can add rs_fill_nop to
>>> support arbitrary .nop directive size. But I don't know if it is
>>> necessary.
>>
>>  Right, so this is needed for argument expressions using forward
>> references.  Understood and accepted.  Thank your for patience.
>>
>
> Implement the '.nop SIZE[, LIMIT]' directive for x86 assembler.  This
> directive emits SIZE bytes filled with 'NOP' instructions.  SIZE is
> absolute expression, which must be between 0 and 512.  LIMIT specifies
> the size limit of a single 'NOP' instruction.  If the comma and LIMIT
> are omitted, LIMIT is assumed to the maximum supported size of a single
> 'NOP' instruction.  The valid values of LIMIT are between 1 and 8 for
> 16-bit mode, between 1 and 10 for 32-bit mode, between 1 and 11 for
> 64-bit mode.  This directive is only allowed in text sections.

Did you consider generalizing .skip instead (e.g. by allowing its
FILL to be "@NOP" alongside an absolute expression)? I have
to admit that adding a new directive looks a little odd to me
when all you want is some more flexibility with an existing one.

Also I'm not sure I really follow what the upper bounds for
LIMIT in the different modes are being derived from. Without
a comment next to the patterns that's going to remain guesswork
forever. For example, why would

static const unsigned char alt64_12[] =
  {0x67,0x66,0x2e,0x40,0x0f,0x1f,0x84,0x00,0x00,0x00,0x00,0x00};

not be a possibility? Or, like at least AMD suggests, multiple 0x66
prefixes?

> This is implemented by adding a relax state, rs_space_nop, to enum
> _relax_state, which is similar to rs_space, but it fills with NOPs,
> instead of a single byte.  A pseudo relocation, BFD_RELOC_NOP_DIRECTIVE,
> is added to fix up frag data with the proper number of NOPs.  The new
> rs_space_nop state is processed only when TARGET_USE_NOP_DIRECTIVE is
> defined.

In your earlier reply to Maciej didn't you indicate you restrict
LIMIT only because you don't want to go through the
complexity of introducing a new relaxation state?

>  To enable .nop directive, a target backend should
>
> 1. Define TARGET_USE_NOP_DIRECTIVE.

With this, why is the new directive being added to i386's
md_pseudo_table[], instead of the arch independent one in
read.c?

> 2. Create a rs_space_nop frag for .nop directive.
> 3. Update md_convert_frag to create a fixup with BFD_RELOC_NOP_DIRECTIVE
> for rs_space_nop frag.
> 4. Update md_apply_fix to process fixup with BFD_RELOC_NOP_DIRECTIVE.
>
> OK for master?
>
> Andrew, please test my current users/hjl/nop branch.

On the original thread Andrew had indicated that producing a single
byte NOP at the end of a sequence of NOPs is undesirable. Your
i386_output_nops() appears to do just that, however.

Additionally - what's wrong with emitting NOPs to a non-executable
section?

Finally, would you mind making the diagnostic complaining about too
large a LIMIT also report the upper bound (not the least because - as
per above remark - this may change over time)?

Jan

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

H.J. Lu-30
On Tue, Feb 13, 2018 at 12:50 AM, Jan Beulich <[hidden email]> wrote:

>>>> On 12.02.18 at 20:38, <[hidden email]> wrote:
>> On Mon, Feb 12, 2018 at 7:48 AM, Maciej W. Rozycki <[hidden email]> wrote:
>>> On Mon, 12 Feb 2018, H.J. Lu wrote:
>>>
>>>> My implementation uses the existing relaxation frame work.
>>>> When we are processing .nop, we don't know exactly how big the
>>>> the NOP size will be.  We allocate a frag with the maximum size
>>>> and set the exact size after relaxation.  After relaxation is done,
>>>> all frags are converted to rs_fill.  We can add rs_fill_nop to
>>>> support arbitrary .nop directive size. But I don't know if it is
>>>> necessary.
>>>
>>>  Right, so this is needed for argument expressions using forward
>>> references.  Understood and accepted.  Thank your for patience.
>>>
>>
>> Implement the '.nop SIZE[, LIMIT]' directive for x86 assembler.  This
>> directive emits SIZE bytes filled with 'NOP' instructions.  SIZE is
>> absolute expression, which must be between 0 and 512.  LIMIT specifies
>> the size limit of a single 'NOP' instruction.  If the comma and LIMIT
>> are omitted, LIMIT is assumed to the maximum supported size of a single
>> 'NOP' instruction.  The valid values of LIMIT are between 1 and 8 for
>> 16-bit mode, between 1 and 10 for 32-bit mode, between 1 and 11 for
>> 64-bit mode.  This directive is only allowed in text sections.
>
> Did you consider generalizing .skip instead (e.g. by allowing its
> FILL to be "@NOP" alongside an absolute expression)? I have
> to admit that adding a new directive looks a little odd to me
> when all you want is some more flexibility with an existing one.
This requires much bigger changes.  Also I like ".not SIZE, LIMIT".
But if everyone agrees that we should extend .skip, we can do that.
This may also remove the size limit.

> Also I'm not sure I really follow what the upper bounds for
> LIMIT in the different modes are being derived from. Without
> a comment next to the patterns that's going to remain guesswork
> forever. For example, why would

I just reused what i386 has for aligning code.

> static const unsigned char alt64_12[] =
>   {0x67,0x66,0x2e,0x40,0x0f,0x1f,0x84,0x00,0x00,0x00,0x00,0x00};
>
> not be a possibility? Or, like at least AMD suggests, multiple 0x66
> prefixes?

We can update NOP padding, independent of .nop directive.

>> This is implemented by adding a relax state, rs_space_nop, to enum
>> _relax_state, which is similar to rs_space, but it fills with NOPs,
>> instead of a single byte.  A pseudo relocation, BFD_RELOC_NOP_DIRECTIVE,
>> is added to fix up frag data with the proper number of NOPs.  The new
>> rs_space_nop state is processed only when TARGET_USE_NOP_DIRECTIVE is
>> defined.
>
> In your earlier reply to Maciej didn't you indicate you restrict
> LIMIT only because you don't want to go through the
> complexity of introducing a new relaxation state?
It turned that I need a new relaxation state to cover branches
which need its own relaxation.  But I keep the generic change
to minimum.

>>  To enable .nop directive, a target backend should
>>
>> 1. Define TARGET_USE_NOP_DIRECTIVE.
>
> With this, why is the new directive being added to i386's
> md_pseudo_table[], instead of the arch independent one in
> read.c?

This requires generic a s_nop function.  I don't think i386 s_nop
is suitable as a generic implementation.

>> 2. Create a rs_space_nop frag for .nop directive.
>> 3. Update md_convert_frag to create a fixup with BFD_RELOC_NOP_DIRECTIVE
>> for rs_space_nop frag.
>> 4. Update md_apply_fix to process fixup with BFD_RELOC_NOP_DIRECTIVE.
>>
>> OK for master?
>>
>> Andrew, please test my current users/hjl/nop branch.
>
> On the original thread Andrew had indicated that producing a single
> byte NOP at the end of a sequence of NOPs is undesirable. Your
> i386_output_nops() appears to do just that, however.
386_output_nops is extracted out of i386_align_code.  We can always
improve it.

> Additionally - what's wrong with emitting NOPs to a non-executable
> section?

Removed.

> Finally, would you mind making the diagnostic complaining about too
> large a LIMIT also report the upper bound (not the least because - as
> per above remark - this may change over time)?
>

Done.

Here is the updated patch.  OK for master?

Thanks.

--
H.J.

0001-x86-Add-.nop-directive-to-assembler.patch (59K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Michael Matz
Hi,

On Wed, 14 Feb 2018, H.J. Lu wrote:

> > Did you consider generalizing .skip instead (e.g. by allowing its FILL
> > to be "@NOP" alongside an absolute expression)? I have to admit that
> > adding a new directive looks a little odd to me when all you want is
> > some more flexibility with an existing one.
>
> This requires much bigger changes.  Also I like ".not SIZE, LIMIT". But
> if everyone agrees that we should extend .skip, we can do that. This may
> also remove the size limit.

If I may chime in, it didn't occur to me before Jan mentioned it, but yes,
I also think extending .fill feels more natural.


Ciao,
Michael.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

H.J. Lu-30
On Wed, Feb 14, 2018 at 7:55 AM, Michael Matz <[hidden email]> wrote:

> Hi,
>
> On Wed, 14 Feb 2018, H.J. Lu wrote:
>
>> > Did you consider generalizing .skip instead (e.g. by allowing its FILL
>> > to be "@NOP" alongside an absolute expression)? I have to admit that
>> > adding a new directive looks a little odd to me when all you want is
>> > some more flexibility with an existing one.
>>
>> This requires much bigger changes.  Also I like ".not SIZE, LIMIT". But
>> if everyone agrees that we should extend .skip, we can do that. This may
>> also remove the size limit.
>
> If I may chime in, it didn't occur to me before Jan mentioned it, but yes,
> I also think extending .fill feels more natural.
>

No, .fill isn't what we want.  We need to extend

.skip SIZE , FILL

So what should FILL look like?

BTW, to implement that, we need to extend both rs_fill and rs_space relax
states.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Michael Matz
Hi,

On Wed, 14 Feb 2018, H.J. Lu wrote:

> >> This requires much bigger changes.  Also I like ".not SIZE, LIMIT".
> >> But if everyone agrees that we should extend .skip, we can do that.
> >> This may also remove the size limit.
> >
> > If I may chime in, it didn't occur to me before Jan mentioned it, but yes,
> > I also think extending .fill feels more natural.
> >
>
> No, .fill isn't what we want.

I fat-fingered that one, yes I meant .fill.

> We need to extend
>
> .skip SIZE , FILL
>
> So what should FILL look like?

@nop, $nop, %nop?  (I think only 'nop' without prefix would interact with
uses in macros?  The '@' prefix resembles a prefix for relocations, so
that might be the natural choice).  Perhaps also 'nop' should be 'anynop'
or so, to indicate it's not just the single-byte 0x90 filler).


Ciao,
Michael.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Andrew Cooper
In reply to this post by H.J. Lu-30
On 14/02/18 14:03, H.J. Lu wrote:
> On Tue, Feb 13, 2018 at 12:50 AM, Jan Beulich <[hidden email]> wrote:
>
>> Finally, would you mind making the diagnostic complaining about too
>> large a LIMIT also report the upper bound (not the least because - as
>> per above remark - this may change over time)?
>>
> Done.
>
> Here is the updated patch.  OK for master?

Sorry for the delay.  This patch does work correctly.

As for exactly how this functionality gets implemented, I am not fussed
at all.  I'll defer that to the binutils community - its very easy for
me to tweak my patch series to start using it when the eventual
implementation is decided upon.

~Andrew
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Jan Beulich-2
In reply to this post by Michael Matz
>>> On 14.02.18 at 17:48, <[hidden email]> wrote:
> On Wed, 14 Feb 2018, H.J. Lu wrote:
>> >> This requires much bigger changes.  Also I like ".not SIZE, LIMIT".
>> >> But if everyone agrees that we should extend .skip, we can do that.
>> >> This may also remove the size limit.
>> >
>> > If I may chime in, it didn't occur to me before Jan mentioned it, but yes,
>> > I also think extending .fill feels more natural.
>> >
>>
>> No, .fill isn't what we want.
>
> I fat-fingered that one, yes I meant .fill.

.fill again? Didn't you mean to write .skip this time?

>> We need to extend
>>
>> .skip SIZE , FILL
>>
>> So what should FILL look like?
>
> @nop, $nop, %nop?  (I think only 'nop' without prefix would interact with
> uses in macros?  The '@' prefix resembles a prefix for relocations, so
> that might be the natural choice).  Perhaps also 'nop' should be 'anynop'
> or so, to indicate it's not just the single-byte 0x90 filler).

Having thought about this some more - since currently FILL is
expected to be an expression, none of the above is really suitable.
How about "nop" (including the quotes) instead?

Jan

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Michael Matz
Hi,

On Thu, 15 Feb 2018, Jan Beulich wrote:

> >> > I also think extending .fill feels more natural.
> >> >
> >>
> >> No, .fill isn't what we want.
> >
> > I fat-fingered that one, yes I meant .fill.
>
> .fill again? Didn't you mean to write .skip this time?

Sigh.  It seems my fingers are unusually fat in this thread.  .skip,
dammit! .skip, .skip, .skip!

> Having thought about this some more - since currently FILL is expected
> to be an expression, none of the above is really suitable.

Well, that could be easily rectified.  Something along the lines of:

diff --git a/gas/read.c b/gas/read.c
index 7bf52f1..8730318 100644
--- a/gas/read.c
+++ b/gas/read.c
@@ -3326,6 +3326,7 @@ s_space (int mult)
   char *stop = NULL;
   char stopc = 0;
   int bytes;
+  bfd_boolean do_nops = FALSE;
 
 #ifdef md_flush_pending_output
   md_flush_pending_output ();
@@ -3386,7 +3387,12 @@ s_space (int mult)
   if (*input_line_pointer == ',')
     {
       ++input_line_pointer;
-      expression (&val);
+      SKIP_WHITESPACE ();
+      if (!strncasecmp (input_line_pointer, "@NOP", 4)
+         && (input_line_pointer[4] == ' ' || !input_line_pointer[4]))
+       do_nops = TRUE, input_line_pointer += 4;
+      else
+       expression (&val);
     }
   else
     {

> How about "nop" (including the quotes) instead?

Also possible, yeah.


Ciao,
Michael.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Jan Beulich-2
>>> On 15.02.18 at 16:02, <[hidden email]> wrote:
> Hi,
>
> On Thu, 15 Feb 2018, Jan Beulich wrote:
>
>> >> > I also think extending .fill feels more natural.
>> >> >
>> >>
>> >> No, .fill isn't what we want.
>> >
>> > I fat-fingered that one, yes I meant .fill.
>>
>> .fill again? Didn't you mean to write .skip this time?
>
> Sigh.  It seems my fingers are unusually fat in this thread.  .skip,
> dammit! .skip, .skip, .skip!
>
>> Having thought about this some more - since currently FILL is expected
>> to be an expression, none of the above is really suitable.
>
> Well, that could be easily rectified.  Something along the lines of:

I didn't make the remark because of foreseeing any issues with
coding this up, but because @NOP (or any of your other
suggestions) could actually be a valid symbol name, which could be
valid to use here even if it's not part of any expression in case it's
an absolute one (e.g. an equate). The leading @ is undesirable
anyway because of ARM's use of it as a comment char.

Jan

> diff --git a/gas/read.c b/gas/read.c
> index 7bf52f1..8730318 100644
> --- a/gas/read.c
> +++ b/gas/read.c
> @@ -3326,6 +3326,7 @@ s_space (int mult)
>    char *stop = NULL;
>    char stopc = 0;
>    int bytes;
> +  bfd_boolean do_nops = FALSE;
>  
>  #ifdef md_flush_pending_output
>    md_flush_pending_output ();
> @@ -3386,7 +3387,12 @@ s_space (int mult)
>    if (*input_line_pointer == ',')
>      {
>        ++input_line_pointer;
> -      expression (&val);
> +      SKIP_WHITESPACE ();
> +      if (!strncasecmp (input_line_pointer, "@NOP", 4)
> +         && (input_line_pointer[4] == ' ' || !input_line_pointer[4]))
> +       do_nops = TRUE, input_line_pointer += 4;
> +      else
> +       expression (&val);
>      }
>    else
>      {
>
>> How about "nop" (including the quotes) instead?
>
> Also possible, yeah.
>
>
> Ciao,
> Michael.



Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Michael Matz
Hi,

On Thu, 15 Feb 2018, Jan Beulich wrote:

> >> to be an expression, none of the above is really suitable.
> >
> > Well, that could be easily rectified.  Something along the lines of:
>
> I didn't make the remark because of foreseeing any issues with
> coding this up, but because @NOP (or any of your other
> suggestions) could actually be a valid symbol name, which could be
> valid to use here even if it's not part of any expression in case it's
> an absolute one (e.g. an equate).

Well, then I don't see other ways of special casing something that also is
a valid expression right now, like your string suggestion.  The problem of
course being that this would reinterpret currently valid (though
strange) directives into something else.  Right now
  .skip 10, "foo"
is equivalent to
  .skip 10, foo
and results in 10 one-byte relocs against symbol 'foo'.  We'd reinterpret
this, which may be fine, but needs to be a conscious decision.

> The leading @ is undesirable anyway because of ARM's use of it as a
> comment char.

Sure, other prefix chars could be used, e.g. one of the binary operators
in expressions.  But in light of the above it might not be such a bright
idea to extend .skip after all.


Ciao,
Michael.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Jan Beulich-2
>>> On 15.02.18 at 16:50, <[hidden email]> wrote:
> Hi,
>
> On Thu, 15 Feb 2018, Jan Beulich wrote:
>
>> >> to be an expression, none of the above is really suitable.
>> >
>> > Well, that could be easily rectified.  Something along the lines of:
>>
>> I didn't make the remark because of foreseeing any issues with
>> coding this up, but because @NOP (or any of your other
>> suggestions) could actually be a valid symbol name, which could be
>> valid to use here even if it's not part of any expression in case it's
>> an absolute one (e.g. an equate).
>
> Well, then I don't see other ways of special casing something that also is
> a valid expression right now, like your string suggestion.  The problem of
> course being that this would reinterpret currently valid (though
> strange) directives into something else.  Right now
>   .skip 10, "foo"
> is equivalent to
>   .skip 10, foo
> and results in 10 one-byte relocs against symbol 'foo'.  We'd reinterpret
> this, which may be fine, but needs to be a conscious decision.

Oh, right, symbol names may be quoted now.

>> The leading @ is undesirable anyway because of ARM's use of it as a
>> comment char.
>
> Sure, other prefix chars could be used, e.g. one of the binary operators
> in expressions.  But in light of the above it might not be such a bright
> idea to extend .skip after all.

Well, if we can fine a universally usable escape character, things
ought to be fine this way. Apart from binary operator chars which
aren't also unary operator ones, \ would come to mind.

Jan

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

H.J. Lu-30
On Thu, Feb 15, 2018 at 8:08 AM, Jan Beulich <[hidden email]> wrote:

>>>> On 15.02.18 at 16:50, <[hidden email]> wrote:
>> Hi,
>>
>> On Thu, 15 Feb 2018, Jan Beulich wrote:
>>
>>> >> to be an expression, none of the above is really suitable.
>>> >
>>> > Well, that could be easily rectified.  Something along the lines of:
>>>
>>> I didn't make the remark because of foreseeing any issues with
>>> coding this up, but because @NOP (or any of your other
>>> suggestions) could actually be a valid symbol name, which could be
>>> valid to use here even if it's not part of any expression in case it's
>>> an absolute one (e.g. an equate).
>>
>> Well, then I don't see other ways of special casing something that also is
>> a valid expression right now, like your string suggestion.  The problem of
>> course being that this would reinterpret currently valid (though
>> strange) directives into something else.  Right now
>>   .skip 10, "foo"
>> is equivalent to
>>   .skip 10, foo
>> and results in 10 one-byte relocs against symbol 'foo'.  We'd reinterpret
>> this, which may be fine, but needs to be a conscious decision.
>
> Oh, right, symbol names may be quoted now.
>
>>> The leading @ is undesirable anyway because of ARM's use of it as a
>>> comment char.
>>
>> Sure, other prefix chars could be used, e.g. one of the binary operators
>> in expressions.  But in light of the above it might not be such a bright
>> idea to extend .skip after all.
>
> Well, if we can fine a universally usable escape character, things
> ought to be fine this way. Apart from binary operator chars which
> aren't also unary operator ones, \ would come to mind.

I think we should simply go with

.nop SIZE


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

H.J. Lu-30
On Thu, Feb 15, 2018 at 9:11 AM, H.J. Lu <[hidden email]> wrote:

> On Thu, Feb 15, 2018 at 8:08 AM, Jan Beulich <[hidden email]> wrote:
>>>>> On 15.02.18 at 16:50, <[hidden email]> wrote:
>>> Hi,
>>>
>>> On Thu, 15 Feb 2018, Jan Beulich wrote:
>>>
>>>> >> to be an expression, none of the above is really suitable.
>>>> >
>>>> > Well, that could be easily rectified.  Something along the lines of:
>>>>
>>>> I didn't make the remark because of foreseeing any issues with
>>>> coding this up, but because @NOP (or any of your other
>>>> suggestions) could actually be a valid symbol name, which could be
>>>> valid to use here even if it's not part of any expression in case it's
>>>> an absolute one (e.g. an equate).
>>>
>>> Well, then I don't see other ways of special casing something that also is
>>> a valid expression right now, like your string suggestion.  The problem of
>>> course being that this would reinterpret currently valid (though
>>> strange) directives into something else.  Right now
>>>   .skip 10, "foo"
>>> is equivalent to
>>>   .skip 10, foo
>>> and results in 10 one-byte relocs against symbol 'foo'.  We'd reinterpret
>>> this, which may be fine, but needs to be a conscious decision.
>>
>> Oh, right, symbol names may be quoted now.
>>
>>>> The leading @ is undesirable anyway because of ARM's use of it as a
>>>> comment char.
>>>
>>> Sure, other prefix chars could be used, e.g. one of the binary operators
>>> in expressions.  But in light of the above it might not be such a bright
>>> idea to extend .skip after all.
>>
>> Well, if we can fine a universally usable escape character, things
>> ought to be fine this way. Apart from binary operator chars which
>> aren't also unary operator ones, \ would come to mind.
>
> I think we should simply go with
>
> .nop SIZE
>
Here is a patch for

'.nop SIZE[, CONTROL]'

OK for master?

Andrew, please try users/hjl/nop/master branch at

https://github.com/hjl-tools/binutils-gdb

There is no usage change.

--
H.J.

0001-Add-.nop-assembler-directive.patch (57K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Michael Matz
Hi,

On Thu, 15 Feb 2018, H.J. Lu wrote:

> OK for master?

@@ -4523,6 +4523,7 @@ Some machine configurations provide additional directives.
 * Sleb128::                    @code{.sleb128 @var{expressions}}
 @ifclear no-space-dir
 * Space::                       @code{.space @var{size} , @var{fill}}
+* Nop::                         @code{.nop @var{size}[, @var{limit}]}
 @end ifclear
 @ifset have-stabs
 * Stab::                        @code{.stabd, .stabn, .stabs}

You probably don't want to have the Nop entry conditional on no-space-dir.
The index is also supposed to be sorted alphabetical.  And the description
of above item still says "limit" instead of "control".

@@ -450,14 +453,18 @@ cvt_frag_to_fill (segT sec ATTRIBUTE_UNUSED, fragS *fragP)
       if (fragP->fr_offset < 0)
        {
          as_bad_where (fragP->fr_file, fragP->fr_line,
-                       _("attempt to .org/.space backwards? (%ld)"),
+                       _("attempt to .org/.space/,nop backwards? (%ld)"),

".nop", not ",nop".


Ciao,
Michael.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

H.J. Lu-30
On Fri, Feb 16, 2018 at 5:58 AM, Michael Matz <[hidden email]> wrote:

> Hi,
>
> On Thu, 15 Feb 2018, H.J. Lu wrote:
>
>> OK for master?
>
> @@ -4523,6 +4523,7 @@ Some machine configurations provide additional directives.
>  * Sleb128::                    @code{.sleb128 @var{expressions}}
>  @ifclear no-space-dir
>  * Space::                       @code{.space @var{size} , @var{fill}}
> +* Nop::                         @code{.nop @var{size}[, @var{limit}]}
>  @end ifclear
>  @ifset have-stabs
>  * Stab::                        @code{.stabd, .stabn, .stabs}
>
> You probably don't want to have the Nop entry conditional on no-space-dir.
> The index is also supposed to be sorted alphabetical.  And the description
> of above item still says "limit" instead of "control".
>
> @@ -450,14 +453,18 @@ cvt_frag_to_fill (segT sec ATTRIBUTE_UNUSED, fragS *fragP)
>        if (fragP->fr_offset < 0)
>         {
>           as_bad_where (fragP->fr_file, fragP->fr_line,
> -                       _("attempt to .org/.space backwards? (%ld)"),
> +                       _("attempt to .org/.space/,nop backwards? (%ld)"),
>
> ".nop", not ",nop".
>
All fixed.  Here is the updated patch.  OK for master?


--
H.J.

0001-Add-.nop-assembler-directive.patch (57K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Maciej W. Rozycki-2
On Fri, 16 Feb 2018, H.J. Lu wrote:

> All fixed.  Here is the updated patch.  OK for master?

 I skimmed over briefly only.  I think the convention for the manual is to
use the generic term `no-op instruction' rather than `@code{NOP}'.  

 Firstly, not all targets we support have an actual NOP mnemonic.  And
even with x86 the encodings you choose are not true architectural NOP
instructions, but just machine code encodings which happen to execute
quickly and with no visible effects (and e.g. for MIPS `addiu $0, $3, -1'
yields no observable effect, however nobody calls that `nop'; there are
millions of such MIPS machine code encodings).

  Maciej
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

H.J. Lu-30
On Fri, Feb 16, 2018 at 7:41 AM, Maciej W. Rozycki <[hidden email]> wrote:

> On Fri, 16 Feb 2018, H.J. Lu wrote:
>
>> All fixed.  Here is the updated patch.  OK for master?
>
>  I skimmed over briefly only.  I think the convention for the manual is to
> use the generic term `no-op instruction' rather than `@code{NOP}'.
>
>  Firstly, not all targets we support have an actual NOP mnemonic.  And
> even with x86 the encodings you choose are not true architectural NOP
> instructions, but just machine code encodings which happen to execute
> quickly and with no visible effects (and e.g. for MIPS `addiu $0, $3, -1'
> yields no observable effect, however nobody calls that `nop'; there are
> millions of such MIPS machine code encodings).
Good points.  Here is the updated patch.   OK for master?

Thanks.

--
H.J.

0001-Add-.nop-assembler-directive.patch (57K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Andrew Cooper
On 16/02/18 16:07, H.J. Lu wrote:

> On Fri, Feb 16, 2018 at 7:41 AM, Maciej W. Rozycki <[hidden email]> wrote:
>> On Fri, 16 Feb 2018, H.J. Lu wrote:
>>
>>> All fixed.  Here is the updated patch.  OK for master?
>>  I skimmed over briefly only.  I think the convention for the manual is to
>> use the generic term `no-op instruction' rather than `@code{NOP}'.
>>
>>  Firstly, not all targets we support have an actual NOP mnemonic.  And
>> even with x86 the encodings you choose are not true architectural NOP
>> instructions, but just machine code encodings which happen to execute
>> quickly and with no visible effects (and e.g. for MIPS `addiu $0, $3, -1'
>> yields no observable effect, however nobody calls that `nop'; there are
>> millions of such MIPS machine code encodings).
> Good points.  Here is the updated patch.   OK for master?

I've tested this latest patch, and everything looks ok.

~Andrew
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86: Add .nop directive to assembler

Alan Modra-3
In reply to this post by H.J. Lu-30
On Fri, Feb 16, 2018 at 08:07:50AM -0800, H.J. Lu wrote:
> Good points.  Here is the updated patch.   OK for master?

OK, except for one quibble with the default md_generate_nops.  I think
that should error rather than filling with zeros (which is likely to
be an illegal instruction or trap).

--
Alan Modra
Australia Development Lab, IBM
123