public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Commit: Gas: Add .nop directive.
@ 2020-09-14 15:13 Nick Clifton
  2020-09-14 15:27 ` Jose E. Marchesi
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Nick Clifton @ 2020-09-14 15:13 UTC (permalink / raw)
  To: binutils

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

Hi Guys,

  I am checking in the attached patch to add a ".nop" directive to the
  assembler.  This directive creates a single no-op instruction.  It is
  similar to the already existing ".nops" directive, apart from two
  important differences:

    1. It is implemented for all architectures, not just x86.
    2. The instruction it generates counts as a real instruction for the
       purposes of DWARF line number table generation.

  Fact 1 is important as it allows the directive to be used to generate
  architecture neutral test cases containing real instructions.

  Tested with a large variety of different targets.

Cheers
  Nick

gas/ChangeLog
2020-09-14  Nick Clifton  <nickc@redhat.com>

	* read.c (s_nop): New function.  Handles the .nop directive.
	(potable): Add entry for "nop".
	(s_nops): Code tidy.
	* read.h (s_nop): Add prototype.
	* config/tc-bpf.h (md_single_noop_insn): Define.
	* config/tc-mmix.h (md_single_noop_insn): Define.
	* config/tc-or1k.h (md_single_noop_insn): Define.
	* config/tc-s12z.c (md_assemble): Preserve the input line pointer,
	rather than corrupting it.
	* write.c (relax_segment): Update error message regarding
	non-absolute values passed to .fill and .nops.
	* NEWS: Mention the new directive.
	* doc/as.texi: Document the new directive.
	* doc/internals.texi: Document the new internal macros used to
	implement the new directive.
	* testsuite/gas/all/nop.s: New test.
	* testsuite/gas/all/nop.d: New test control file.
	* testsuite/gas/all/gas.exp: Run the new test.
	* testsuite/gas/elf/dwarf-5-nop-for-line-table.s: New test.
	* testsuite/gas/elf/dwarf-5-nop-for-line-table.d: New test
	control file.
	* testsuite/gas/elf/elf.exp: Run the new test.
	* testsuite/gas/i386/space1.l: Adjust expected output.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gas-nop-directive.patch --]
[-- Type: text/x-patch, Size: 12849 bytes --]

diff --git a/gas/NEWS b/gas/NEWS
index 66afd0357b..d709edf9b5 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -1,5 +1,9 @@
 -*- text -*-
 
+* Added a .nop directive to generate a single no-op instruction in a target
+  neutral manner.  This instruction does have an effect on DWARF line number
+  generation, if that is active.
+
 * Removed --reduce-memory-overheads and --hash-size as gas now
   uses hash tables that can be expand and shrink automatically.
 
diff --git a/gas/config/tc-bpf.h b/gas/config/tc-bpf.h
index 5765833997..cb02d6c133 100644
--- a/gas/config/tc-bpf.h
+++ b/gas/config/tc-bpf.h
@@ -48,3 +48,4 @@
 /* Values passed to md_apply_fix don't include the symbol value.  */
 #define MD_APPLY_SYM_VALUE(FIX) 0
 
+#define md_single_noop_insn "mov %r1,%r1"
diff --git a/gas/config/tc-mmix.h b/gas/config/tc-mmix.h
index f7a54c1de2..2d8e226dc8 100644
--- a/gas/config/tc-mmix.h
+++ b/gas/config/tc-mmix.h
@@ -228,3 +228,5 @@ extern void mmix_md_do_align (int, char *, int, int);
 
 /* MMIX has global register symbols.  */
 #define TC_GLOBAL_REGISTER_SYMBOL_OK
+
+#define md_single_noop_insn "set $0, $0"
diff --git a/gas/config/tc-or1k.h b/gas/config/tc-or1k.h
index 0242dd480f..b9aa00cb9a 100644
--- a/gas/config/tc-or1k.h
+++ b/gas/config/tc-or1k.h
@@ -74,3 +74,5 @@ void or1k_elf_final_processing (void);
 #define tc_cfi_frame_initial_instructions \
     or1k_cfi_frame_initial_instructions
 extern void or1k_cfi_frame_initial_instructions (void);
+
+#define md_single_noop_insn "l.nop"
diff --git a/gas/config/tc-s12z.c b/gas/config/tc-s12z.c
index d89fb0c21f..c79d2f43f1 100644
--- a/gas/config/tc-s12z.c
+++ b/gas/config/tc-s12z.c
@@ -3807,6 +3807,7 @@ md_assemble (char *str)
       return;
     }
 
+  char * saved_ilp = input_line_pointer;
   input_line_pointer = skip_whites (op_end);
 
   size_t i;
@@ -3816,15 +3817,17 @@ md_assemble (char *str)
       if (0 == strcmp (name, opc->name))
 	{
 	  if (opc->parse_operands (opc))
-	    return;
+	    {
+	      input_line_pointer = saved_ilp;
+	      return;
+	    }
 	  continue;
 	}
     }
 
   as_bad (_("Invalid instruction: \"%s\""), str);
   as_bad (_("First invalid token: \"%s\""), fail_line_pointer);
-  while (*input_line_pointer++)
-    ;
+  input_line_pointer = saved_ilp;
 }
 
 \f
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index f2a0314310..b88c1f9997 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -4446,6 +4446,7 @@ Some machine configurations provide additional directives.
 * MRI::				@code{.mri @var{val}}
 * Noaltmacro::                  @code{.noaltmacro}
 * Nolist::                      @code{.nolist}
+* Nop::                         @code{.nop}
 * Nops::                        @code{.nops @var{size}[, @var{control}]}
 * Octa::                        @code{.octa @var{bignums}}
 * Offset::			@code{.offset @var{loc}}
@@ -6157,22 +6158,31 @@ internal counter (which is zero initially).   @code{.list} increments the
 counter, and @code{.nolist} decrements it.  Assembly listings are
 generated whenever the counter is greater than zero.
 
+@node Nop
+@section @code{.nop}
+
+@cindex @code{nop} directive
+@cindex filling memory with no-op instructions
+This directive emits a single no-op instruction.  It is provided on all
+architectures, allowing the creation of architecture neutral tests involving
+actual code.  The size of the generated instruction is target specific.  The
+instruction does affect the generation of DWARF debug line information.
+
 @node Nops
 @section @code{.nops @var{size}[, @var{control}]}
 
 @cindex @code{nops} directive
 @cindex filling memory with no-op instructions
-This directive emits @var{size} bytes filled with no-op instructions.
-@var{size} is absolute expression, which must be a positve value.
-@var{control} controls how no-op instructions should be generated.  If
-the comma and @var{control} are omitted, @var{control} is assumed to be
-zero.
-
-Note: For Intel 80386 and AMD x86-64 targets, @var{control} specifies
-the size limit of a no-op instruction.  The valid values of @var{control}
-are between 0 and 4 in 16-bit mode, between 0 and 7 when tuning for
-older processors in 32-bit mode, between 0 and 11 in 64-bit mode or when
-tuning for newer processors in 32-bit mode.  When 0 is used, the no-op
+This directive emits no-op instructions.  It is specific to the Intel 80386 and
+AMD x86-64 targets.  It takes a @var{size} argument and generates @var{size}
+bytes of no-op instructions.  @var{size} must be absolute and positive.  These
+bytes do not affect the generation of DWARF debug line information.
+
+The optional @var{control} argument specifies a size limit for a single no-op
+instruction.  If not provided then a value of 0 is assumed.  The valid values
+of @var{control} are between 0 and 4 in 16-bit mode, between 0 and 7 when
+tuning for older processors in 32-bit mode, between 0 and 11 in 64-bit mode or
+when tuning for newer processors in 32-bit mode.  When 0 is used, the no-op
 instruction size limit is set to the maximum supported size.
 
 @node Octa
diff --git a/gas/doc/internals.texi b/gas/doc/internals.texi
index a690d78772..8afa283dbd 100644
--- a/gas/doc/internals.texi
+++ b/gas/doc/internals.texi
@@ -1547,6 +1547,16 @@ The function should return the debug format that is preferred by the CPU
 backend.  This format will be used when generating assembler specific debug
 information.
 
+@item md_emit_single_noop_insn
+@itemx md_single_noop_insn
+These macro facilitate the @var{.nop} directive.  If defined the
+@var{md_emit_single_noop_insn) macro provides code to insert a single no-op
+instruction into the output stream.  If this involves calling @var{md_assemble}
+with a fixed string then the alternative macro @var{md_single_noop_insn} can be
+defined, specifying the string to pass.  If neither of these macros are defined
+then the @var{.nop} directive will call @var{md_assemble} with the string
+@option{nop}.
+
 @item md_allow_local_subtract (@var{left}, @var{right}, @var{section})
 If defined, GAS will call this macro when evaluating an expression which is the
 difference of two symbols defined in the same section.  It takes three
diff --git a/gas/read.c b/gas/read.c
index cd06ea51d9..97a9e66e6f 100644
--- a/gas/read.c
+++ b/gas/read.c
@@ -465,6 +465,7 @@ static const pseudo_typeS potable[] = {
   {"noformat", s_ignore, 0},
   {"nolist", listing_list, 0},	/* Turn listing off.  */
   {"nopage", listing_nopage, 0},
+  {"nop", s_nop, 0},
   {"nops", s_nops, 0},
   {"octa", cons, 16},
   {"offset", s_struct, 0},
@@ -3502,6 +3503,38 @@ s_space (int mult)
     mri_comment_end (stop, stopc);
 }
 
+void
+s_nop (int ignore ATTRIBUTE_UNUSED)
+{
+#ifdef md_flush_pending_output
+  md_flush_pending_output ();
+#endif
+
+#ifdef md_cons_align
+  md_cons_align (1);
+#endif
+
+  SKIP_WHITESPACE ();
+  demand_empty_rest_of_line ();
+
+#ifdef md_emit_single_noop
+  md_emit_single_noop;
+#else
+  char * nop;
+
+#ifndef md_single_noop_insn
+#define md_single_noop_insn "nop"
+#endif
+  /* md_assemble might modify its argument, so
+     we must pass it a string that is writeable.  */
+  if (asprintf (&nop, "%s", md_single_noop_insn) < 0)
+    as_fatal ("%s", xstrerror (errno));
+
+  md_assemble (nop);
+  free (nop);
+#endif
+}
+
 void
 s_nops (int ignore ATTRIBUTE_UNUSED)
 {
@@ -3516,8 +3549,12 @@ s_nops (int ignore ATTRIBUTE_UNUSED)
   md_cons_align (1);
 #endif
 
+  SKIP_WHITESPACE ();
   expression (&exp);
+  /* Note - this expression is tested for an absolute value in
+     write.c:relax_segment().  */
 
+  SKIP_WHITESPACE ();
   if (*input_line_pointer == ',')
     {
       ++input_line_pointer;
@@ -3529,29 +3566,30 @@ s_nops (int ignore ATTRIBUTE_UNUSED)
       val.X_add_number = 0;
     }
 
-  if (val.X_op == O_constant)
+  if (val.X_op != O_constant)
     {
-      if (val.X_add_number < 0)
-	{
-	  as_warn (_("negative nop control byte, ignored"));
-	  val.X_add_number = 0;
-	}
-
-      if (!need_pass_2)
-	{
-	  /* Store the no-op instruction control byte in the first byte
-	     of frag.  */
-	  char *p;
-	  symbolS *sym = make_expr_symbol (&exp);
-	  p = frag_var (rs_space_nop, 1, 1, (relax_substateT) 0,
-			sym, (offsetT) 0, (char *) 0);
-	  *p = val.X_add_number;
-	}
+      as_bad (_("unsupported variable nop control in .nops directive"));
+      val.X_op = O_constant;
+      val.X_add_number = 0;
+    }
+  else if (val.X_add_number < 0)
+    {
+      as_warn (_("negative nop control byte, ignored"));
+      val.X_add_number = 0;
     }
-  else
-    as_bad (_("unsupported variable nop control in .nops directive"));
 
   demand_empty_rest_of_line ();
+
+  if (need_pass_2)
+    /* Ignore this directive if we are going to perform a second pass.  */
+    return;
+
+  /* Store the no-op instruction control byte in the first byte of frag.  */
+  char *p;
+  symbolS *sym = make_expr_symbol (&exp);
+  p = frag_var (rs_space_nop, 1, 1, (relax_substateT) 0,
+		sym, (offsetT) 0, (char *) 0);
+  *p = val.X_add_number;
 }
 
 /* This is like s_space, but the value is a floating point number with
diff --git a/gas/read.h b/gas/read.h
index ffcdbb69a7..b2f0d363b1 100644
--- a/gas/read.h
+++ b/gas/read.h
@@ -207,6 +207,7 @@ extern void s_purgem (int);
 extern void s_rept (int);
 extern void s_set (int);
 extern void s_space (int mult);
+extern void s_nop (int);
 extern void s_nops (int);
 extern void s_stab (int what);
 extern void s_struct (int);
diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index a0158f38dd..af9cb61e3f 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -443,6 +443,7 @@ dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/err-*.s $srcdir/$subdir/warn
 dg-finish
 
 # Set $nop_type appropriately to indicate the NOP instruction mnemonic.
+# Note - this code is made obsolete by the new .nops pseudo-op.
 switch -glob $target_triplet {
     bpf-*-* {
         set nop_type 6
@@ -470,3 +471,5 @@ run_dump_test "org-6"
 run_dump_test "fill-1"
 
 gas_test "pr23938.s" "" "" ".xstabs"
+
+run_dump_test "nop"
diff --git a/gas/testsuite/gas/elf/elf.exp b/gas/testsuite/gas/elf/elf.exp
index 8520421ba3..135ade24ec 100644
--- a/gas/testsuite/gas/elf/elf.exp
+++ b/gas/testsuite/gas/elf/elf.exp
@@ -279,6 +279,7 @@ if { [is_elf_format] } then {
     run_dump_test "dwarf-5-file0" $dump_opts
     run_dump_test "dwarf-4-cu" $dump_opts
     run_dump_test "dwarf-5-cu" $dump_opts
+    run_dump_test "dwarf-5-nop-for-line-table" $dump_opts
     run_dump_test "pr25917"
     run_dump_test "bss"
     run_dump_test "bad-bss"
diff --git a/gas/testsuite/gas/i386/space1.l b/gas/testsuite/gas/i386/space1.l
index 5b0053ee37..bf446fa9b1 100644
--- a/gas/testsuite/gas/i386/space1.l
+++ b/gas/testsuite/gas/i386/space1.l
@@ -1,9 +1,9 @@
 .*: Assembler messages:
-.*:2: Error: .space specifies non-absolute value
-.*:3: Error: .space specifies non-absolute value
-.*:4: Error: .space specifies non-absolute value
-.*:5: Error: .space specifies non-absolute value
-.*:6: Error: .space specifies non-absolute value
+.*:2: Error: .space, .nops or .fill specifies non-absolute value
+.*:3: Error: .space, .nops or .fill specifies non-absolute value
+.*:4: Error: .space, .nops or .fill specifies non-absolute value
+.*:5: Error: .space, .nops or .fill specifies non-absolute value
+.*:6: Error: .space, .nops or .fill specifies non-absolute value
 GAS LISTING .*
 
 
diff --git a/gas/write.c b/gas/write.c
index 0b43063bba..054f27987d 100644
--- a/gas/write.c
+++ b/gas/write.c
@@ -3017,7 +3017,7 @@ relax_segment (struct frag *segment_frag_root, segT segment, int pass)
 			|| ! S_IS_DEFINED (symbolP))
 		      {
 			as_bad_where (fragP->fr_file, fragP->fr_line,
-				      _(".space specifies non-absolute value"));
+				      _(".space, .nops or .fill specifies non-absolute value"));
 			/* Prevent repeat of this error message.  */
 			fragP->fr_symbol = 0;
 		      }
--- /dev/null	2020-09-14 07:47:08.880437847 +0100
+++ gas/testsuite/gas/all/nop.s	2020-09-14 14:46:15.295140328 +0100
@@ -0,0 +1,2 @@
+	.text
+	.nop
--- /dev/null	2020-09-14 07:47:08.880437847 +0100
+++ gas/testsuite/gas/all/nop.d	2020-09-14 12:18:19.109379452 +0100
@@ -0,0 +1,8 @@
+#objdump: -s -j .text -j "\$TEXT\$"
+#name: Generate NOPs in an architecture neutral manner
+
+.*: +file format .*
+
+Contents of section (\.text|\$TEXT\$):
+ [^ ]* .*
+#pass
--- /dev/null	2020-09-14 07:47:08.880437847 +0100
+++ gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.s	2020-09-14 14:45:35.392250338 +0100
@@ -0,0 +1,3 @@
+	.text
+	.nop
+	.nop
--- /dev/null	2020-09-14 07:47:08.880437847 +0100
+++ gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.d	2020-09-14 14:49:21.514626936 +0100
@@ -0,0 +1,12 @@
+#as: --gdwarf-5
+#name: Check line table is produced with .nops
+#readelf: -wL
+
+#...
+Contents of the .debug_line section:
+
+CU: .*
+File name.*
+#...
+.*[ 	]+[1-8][ 	]+0.*
+#pass

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-14 15:13 Commit: Gas: Add .nop directive Nick Clifton
@ 2020-09-14 15:27 ` Jose E. Marchesi
  2020-09-15 11:13   ` Nick Clifton
  2020-09-14 15:38 ` Andreas Schwab
  2020-09-30 18:18 ` Maciej W. Rozycki
  2 siblings, 1 reply; 10+ messages in thread
From: Jose E. Marchesi @ 2020-09-14 15:27 UTC (permalink / raw)
  To: Nick Clifton via Binutils


Hi Nick.

>   I am checking in the attached patch to add a ".nop" directive to the
>   assembler.  This directive creates a single no-op instruction.  It is
>   similar to the already existing ".nops" directive, apart from two
>   important differences:
>
>     1. It is implemented for all architectures, not just x86.
>     2. The instruction it generates counts as a real instruction for the
>        purposes of DWARF line number table generation.
>
>   Fact 1 is important as it allows the directive to be used to generate
>   architecture neutral test cases containing real instructions.
>
>   Tested with a large variety of different targets.

Very nice!

In the same spirit, it would be awesome to be able to generate
architecture-neutral data directives as well, for DWARF and CTF and the
like :)

> diff --git a/gas/config/tc-bpf.h b/gas/config/tc-bpf.h
> index 5765833997..cb02d6c133 100644
> --- a/gas/config/tc-bpf.h
> +++ b/gas/config/tc-bpf.h
> @@ -48,3 +48,4 @@
>  /* Values passed to md_apply_fix don't include the symbol value.  */
>  #define MD_APPLY_SYM_VALUE(FIX) 0
>  
> +#define md_single_noop_insn "mov %r1,%r1"

For BPF we have this in GCC:

(define_insn "nop"
  [(const_int 0)]
  ""
  "mov\t%%r0,%%r0"
  [(set_attr "type" "alu")])

I think it would be good to settle on the same way to encode nops in
both GCC and binutils.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-14 15:13 Commit: Gas: Add .nop directive Nick Clifton
  2020-09-14 15:27 ` Jose E. Marchesi
@ 2020-09-14 15:38 ` Andreas Schwab
  2020-09-14 16:02   ` Nick Clifton
  2020-09-30 18:18 ` Maciej W. Rozycki
  2 siblings, 1 reply; 10+ messages in thread
From: Andreas Schwab @ 2020-09-14 15:38 UTC (permalink / raw)
  To: Nick Clifton via Binutils

This fails on ia64:

/home/andreas/src/binutils/binutils/gas/testsuite/gas/all/nop.s: Assembler messages:
/home/andreas/src/binutils/binutils/gas/testsuite/gas/all/nop.s:2: Error: Wrong number of input operands
failed with: </home/andreas/src/binutils/binutils/gas/testsuite/gas/all/nop.s: Assembler messages:
/home/andreas/src/binutils/binutils/gas/testsuite/gas/all/nop.s:2: Error: Wrong number of input operands>, no expected output
FAIL: Generate NOPs in an architecture neutral manner

/home/andreas/src/binutils/binutils/gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.s: Assembler messages:
/home/andreas/src/binutils/binutils/gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.s:2: Error: Wrong number of input operands
/home/andreas/src/binutils/binutils/gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.s:3: Error: Wrong number of input operands
failed with: </home/andreas/src/binutils/binutils/gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.s: Assembler messages:
/home/andreas/src/binutils/binutils/gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.s:2: Error: Wrong number of input operands
/home/andreas/src/binutils/binutils/gas/testsuite/gas/elf/dwarf-5-nop-for-line-table.s:3: Error: Wrong number of input operands>, no expected output
FAIL: Check line table is produced with .nops

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-14 15:38 ` Andreas Schwab
@ 2020-09-14 16:02   ` Nick Clifton
  0 siblings, 0 replies; 10+ messages in thread
From: Nick Clifton @ 2020-09-14 16:02 UTC (permalink / raw)
  To: Andreas Schwab, Nick Clifton via Binutils

Hi Andreas,

> This fails on ia64:

Ah - sorry - that architecture is now obsolete, so I did not think that anyone would notice...

I have checked in a quick fix add IA64 support for the .nop directive.

Cheers
  Nick


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-14 15:27 ` Jose E. Marchesi
@ 2020-09-15 11:13   ` Nick Clifton
  2020-09-15 11:57     ` Jose E. Marchesi
  0 siblings, 1 reply; 10+ messages in thread
From: Nick Clifton @ 2020-09-15 11:13 UTC (permalink / raw)
  To: Jose E. Marchesi, Nick Clifton via Binutils

Hi Jose,


> In the same spirit, it would be awesome to be able to generate
> architecture-neutral data directives as well, for DWARF and CTF and the
> like :)

What about ".dc.b <val>" ?  Does that not work for all targets ?

>> +#define md_single_noop_insn "mov %r1,%r1"
> 
> For BPF we have this in GCC:
> 
> (define_insn "nop"
>   [(const_int 0)]
>   ""
>   "mov\t%%r0,%%r0"

> I think it would be good to settle on the same way to encode nops in
> both GCC and binutils.

Actually David Faust just posted a patch to change the instruction to "ja 0"
as this is what the kernel expects.  I have applied his patch to gas.  Maybe
gcc should be updated as well ?

Cheers
  Nick



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-15 11:13   ` Nick Clifton
@ 2020-09-15 11:57     ` Jose E. Marchesi
  0 siblings, 0 replies; 10+ messages in thread
From: Jose E. Marchesi @ 2020-09-15 11:57 UTC (permalink / raw)
  To: Nick Clifton; +Cc: Nick Clifton via Binutils, David Faust


>> In the same spirit, it would be awesome to be able to generate
>> architecture-neutral data directives as well, for DWARF and CTF and the
>> like :)
>
> What about ".dc.b <val>" ?  Does that not work for all targets ?

Maybe it does.

We found problems while trying to have .s files in CTF tests in ld,
generated with a patched GCC: the data directives emitted by the
compiler are architecture-specific.  I think the GDB testsuite ships
with a DWARF assembler because of the same reason.

So I guess the problem is more a compiler one... i.e. what data
directives it uses.

>>> +#define md_single_noop_insn "mov %r1,%r1"
>> 
>> For BPF we have this in GCC:
>> 
>> (define_insn "nop"
>>   [(const_int 0)]
>>   ""
>>   "mov\t%%r0,%%r0"
>
>> I think it would be good to settle on the same way to encode nops in
>> both GCC and binutils.
>
> Actually David Faust just posted a patch to change the instruction to "ja 0"
> as this is what the kernel expects.  I have applied his patch to gas.  Maybe
> gcc should be updated as well ?

Yeah, actually it was your patch that prompted us to revisit the NOP
situation in BPF, and we found out the kernel actually expects a
particular encoding for them.  So thanks for that :)

We fixed GCC as well.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-14 15:13 Commit: Gas: Add .nop directive Nick Clifton
  2020-09-14 15:27 ` Jose E. Marchesi
  2020-09-14 15:38 ` Andreas Schwab
@ 2020-09-30 18:18 ` Maciej W. Rozycki
  2 siblings, 0 replies; 10+ messages in thread
From: Maciej W. Rozycki @ 2020-09-30 18:18 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

On Mon, 14 Sep 2020, Nick Clifton via Binutils wrote:

>   Fact 1 is important as it allows the directive to be used to generate
>   architecture neutral test cases containing real instructions.

 Indeed, and I bet some of our test cases can be cleaned up to use the new 
pseudo-op now.  ISTR making such test cases myself.  Thanks for the idea 
and implementation.

  Maciej

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-14 16:31   ` Nick Clifton
@ 2020-09-15  9:53     ` Nick Clifton
  0 siblings, 0 replies; 10+ messages in thread
From: Nick Clifton @ 2020-09-15  9:53 UTC (permalink / raw)
  To: binutils

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

Hi Guys,

  The code to implement the .nop directive calls the md_assemble() function 
  with an allocated string, rather than input_line_pointer.  This is supposed
  to work, but there are some targets that assume that md_assemble()'s 
  argument is input_line_pointer, and that they are free to update it.  Even 
  worse some of them assign input_line_pointer to the string argument and
  leave it there.  So when s_nop() finishes and frees the string pointer,
  input_line_pointer is left pointing at freed memory.

  At first I though that it was just the S12Z port that behaved in this way
  but now I have found that the MN10300 port does too.  There may be others
  as well, since the symptom is hard to trace.  So I am applying the attached
  patch to fix s_nop() so that it explicitly preserves input_line_pointer
  instead.

Cheers
  Nick

gas/ChangeLog
2020-09-15  Nick Clifton  <nickc@redhat.com>

	* read.c (s_nop): Preserve the input_line_pointer around the call
	to md_assemble.
	* config/tc-s12z.c (md_assemble): Revert previous delta.

[-- Attachment #2: gas-nop-directive.patch.2 --]
[-- Type: application/x-troff-man, Size: 1383 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
  2020-09-14 16:04 ` Michael Morrell
@ 2020-09-14 16:31   ` Nick Clifton
  2020-09-15  9:53     ` Nick Clifton
  0 siblings, 1 reply; 10+ messages in thread
From: Nick Clifton @ 2020-09-14 16:31 UTC (permalink / raw)
  To: morrell, binutils

Hi Michael,

>  Once this is in, I hope this code from gcc/configure can be removed:

That would be a nice thing to do, but I am not sure that it will work.  Since
gcc still needs to remain compatible with older versions of the binutils I
think that code will need to remain.

What could happen though is that the test could be taught about the new directive
and if it is supported, use it.  This would mean that future new architectures
would not have to have any code added here, the test should just work.

I am contemplating backporting the patch to the 2.35 branch and then triggering
a 2.35.1 point release.  (Actually there are a few more DWARF-5 patches that need
to go in, and then it might be a good time for a point release).  So in theory
the test in gcc could check for binutils 2.35.1 or later, and if present, just
use the .nop directive, without having to worry about anything target specific.

Cheers
  Nick







^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Commit: Gas: Add .nop directive.
       [not found] <mailman.151032.1600097231.8982.binutils@sourceware.org>
@ 2020-09-14 16:04 ` Michael Morrell
  2020-09-14 16:31   ` Nick Clifton
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Morrell @ 2020-09-14 16:04 UTC (permalink / raw)
  To: binutils

 Once this is in, I hope this code from gcc/configure can be removed:

# ??? Not all targets support dwarf2 debug_line, even within a version
# of gas. Moreover, we need to emit a valid instruction to trigger any
# info to the output file. So, as supported targets are added to gas 2.11,
# add some instruction here to (also) show we expect this might work.
# ??? Once 2.11 is released, probably need to add first known working
# version to the per-target configury.
case "$cpu_type" in
 aarch64 | alpha | arc | arm | avr | bfin | cris | csky | i386 | m32c | m68k \
 | microblaze | mips | nds32 | nios2 | pa | riscv | rs6000 | score | sparc | spu \
 | tilegx | tilepro | visium | xstormy16 | xtensa)
 insn="nop"
 ;;
 ia64 | s390)
 insn="nop 0"
 ;;
 mmix)
 insn="swym 0"
 ;;
esac

You currently have to update this for every new port to be able to check for dwarf2_debug_line support (even if the port has a "nop" instruction).

 Michael

On Monday, September 14, 2020, 08:27:15 AM PDT, Nick Clifton wrote:
 
Hi Guys,

 I am checking in the attached patch to add a ".nop" directive to the
 assembler. This directive creates a single no-op instruction. It is
 similar to the already existing ".nops" directive, apart from two
 important differences:

 1. It is implemented for all architectures, not just x86.
 2. The instruction it generates counts as a real instruction for the
 purposes of DWARF line number table generation.

 Fact 1 is important as it allows the directive to be used to generate
 architecture neutral test cases containing real instructions.

 Tested with a large variety of different targets.

Cheers
 Nick  

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-09-30 18:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 15:13 Commit: Gas: Add .nop directive Nick Clifton
2020-09-14 15:27 ` Jose E. Marchesi
2020-09-15 11:13   ` Nick Clifton
2020-09-15 11:57     ` Jose E. Marchesi
2020-09-14 15:38 ` Andreas Schwab
2020-09-14 16:02   ` Nick Clifton
2020-09-30 18:18 ` Maciej W. Rozycki
     [not found] <mailman.151032.1600097231.8982.binutils@sourceware.org>
2020-09-14 16:04 ` Michael Morrell
2020-09-14 16:31   ` Nick Clifton
2020-09-15  9:53     ` Nick Clifton

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