public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gas: xtensa: fix incorrect code generated with auto litpools
@ 2017-04-05 21:20 Max Filippov
  2017-04-10 12:15 ` Nick Clifton
  0 siblings, 1 reply; 2+ messages in thread
From: Max Filippov @ 2017-04-05 21:20 UTC (permalink / raw)
  To: binutils
  Cc: Sterling Augustine, David Weatherford, Marc Gauthier,
	linux-xtensa, Max Filippov

Assembling xtensa code w/o implicit (entry instruction) or explicit
(.literal_position directive) literal pool positioning hint with
--auto-litpools option may generate incorrect code when first literal is
emitted before the first automatic literal pool. In that case literal
pool without skipping jump is created right before the literal,
potentially allowing program flow to enter literal pool.

This bug is triggered e.g. by the following xtensa softfloat code from
libgcc built for call0 ABI:

  #ifdef L_negdf2

          .align  4
          .global __negdf2
          .type   __negdf2, @function
  __negdf2:
          leaf_entry sp, 16
          movi    a4, 0x80000000
          xor     xh, xh, a4
          leaf_return

  #endif /* L_negdf2 */

It turns into the following binary:

  Contents of section .text:
   0000 00000080 41ffff40 33300df0           ....A..@30..

  Disassembly of section .text:

  00000000 <__negdf2>:
     0:   000000          ill
     3:   ff4180          excw
                          4: R_XTENSA_SLOT0_OP    .text
     6:   ff              .byte 0xff
     7:   303340          xor     a3, a3, a4
     a:   f00d            ret.n

and it manifests itself as an illegal instruction exception happening
in __negdf2.

gas/
2017-03-31  Max Filippov  <jcmvbkbc@gmail.com>

	* config/tc-xtensa.c (xtensa_maybe_create_literal_pool_frag):
	Initialize lps->frag_count with auto_litpool_limit.
	(xg_promote_candidate_litpool): New function.
	(xtensa_move_literals): Extract candidate litpool promotion code
	into separate function. Call it for all possible found
	candidates.
	(xtensa_switch_to_literal_fragment): Drop 'recursive' flag and
	call to xtensa_mark_literal_pool_location that it guards.
	Replace it with call to xtensa_maybe_create_literal_pool_frag.
	Initialize pool_location with created literal pool candidate.
	* testsuite/gas/xtensa/all.exp: Add new tests.
	* testsuite/gas/xtensa/auto-litpools-first1.d: New test results.
	* testsuite/gas/xtensa/auto-litpools-first1.s: New test.
	* testsuite/gas/xtensa/auto-litpools-first2.d: New test results.
	* testsuite/gas/xtensa/auto-litpools-first2.s: New test.
	* testsuite/gas/xtensa/auto-litpools.d: Fix offsets changed due
	to additional jump instruction.
---
 gas/config/tc-xtensa.c                          | 66 ++++++++++++++-----------
 gas/testsuite/gas/xtensa/all.exp                |  2 +
 gas/testsuite/gas/xtensa/auto-litpools-first1.d | 12 +++++
 gas/testsuite/gas/xtensa/auto-litpools-first1.s |  3 ++
 gas/testsuite/gas/xtensa/auto-litpools-first2.d | 15 ++++++
 gas/testsuite/gas/xtensa/auto-litpools-first2.s |  3 ++
 gas/testsuite/gas/xtensa/auto-litpools.d        |  6 +--
 7 files changed, 75 insertions(+), 32 deletions(-)
 create mode 100644 gas/testsuite/gas/xtensa/auto-litpools-first1.d
 create mode 100644 gas/testsuite/gas/xtensa/auto-litpools-first1.s
 create mode 100644 gas/testsuite/gas/xtensa/auto-litpools-first2.d
 create mode 100644 gas/testsuite/gas/xtensa/auto-litpools-first2.s

diff --git a/gas/config/tc-xtensa.c b/gas/config/tc-xtensa.c
index c45c70d..e1efaae 100644
--- a/gas/config/tc-xtensa.c
+++ b/gas/config/tc-xtensa.c
@@ -7547,6 +7547,10 @@ xtensa_maybe_create_literal_pool_frag (bfd_boolean create,
       lps->seg = now_seg;
       lps->frag_list.next = &lps->frag_list;
       lps->frag_list.prev = &lps->frag_list;
+      /* Put candidate literal pool at the beginning of every section,
+         so that even when section starts with literal load there's a
+	 literal pool available.  */
+      lps->frag_count = auto_litpool_limit;
     }
 
   lps->frag_count++;
@@ -11035,6 +11039,30 @@ xtensa_move_seg_list_to_beginning (seg_list *head)
 static void mark_literal_frags (seg_list *);
 
 static void
+xg_promote_candidate_litpool (struct litpool_seg *lps,
+			      struct litpool_frag *lp)
+{
+  fragS *poolbeg;
+  fragS *poolend;
+  symbolS *lsym;
+  char label[10 + 2 * sizeof (fragS *)];
+
+  poolbeg = lp->fragP;
+  lp->priority = 1;
+  poolbeg->fr_subtype = RELAX_LITERAL_POOL_BEGIN;
+  poolend = poolbeg->fr_next;
+  gas_assert (poolend->fr_type == rs_machine_dependent &&
+	      poolend->fr_subtype == RELAX_LITERAL_POOL_END);
+  /* Create a local symbol pointing to the
+     end of the pool.  */
+  sprintf (label, ".L0_LT_%p", poolbeg);
+  lsym = (symbolS *)local_symbol_make (label, lps->seg,
+				       0, poolend);
+  poolbeg->fr_symbol = lsym;
+  /* Rest is done in xtensa_relax_frag.  */
+}
+
+static void
 xtensa_move_literals (void)
 {
   seg_list *segment;
@@ -11121,27 +11149,17 @@ xtensa_move_literals (void)
 				      /* This is still a "candidate" but the next one
 				         will be too far away, so revert to the nearest
 					 one, convert it and add the jump around.  */
-				      fragS *poolbeg;
-				      fragS *poolend;
-				      symbolS *lsym;
-				      char label[10 + 2 * sizeof (fragS *)];
 				      lp = lpf->prev;
-				      poolbeg = lp->fragP;
-				      lp->priority = 1;
-				      poolbeg->fr_subtype = RELAX_LITERAL_POOL_BEGIN;
-				      poolend = poolbeg->fr_next;
-				      gas_assert (poolend->fr_type == rs_machine_dependent &&
-						  poolend->fr_subtype == RELAX_LITERAL_POOL_END);
-				      /* Create a local symbol pointing to the
-				         end of the pool.  */
-				      sprintf (label, ".L0_LT_%p", poolbeg);
-				      lsym = (symbolS *)local_symbol_make (label, lps->seg,
-									   0, poolend);
-				      poolbeg->fr_symbol = lsym;
-				      /* Rest is done in xtensa_relax_frag.  */
+				      break;
 				    }
 				}
 			    }
+
+			  /* Convert candidate and add the jump around.  */
+			  if (lp->fragP->fr_subtype ==
+			      RELAX_LITERAL_POOL_CANDIDATE_BEGIN)
+			    xg_promote_candidate_litpool (lps, lp);
+
 			  if (! litfrag->tc_frag_data.literal_frag)
 			    {
 			      /* Take earliest use of this literal to avoid
@@ -11413,7 +11431,6 @@ xtensa_switch_to_literal_fragment (emit_state *result)
 static void
 xtensa_switch_to_non_abs_literal_fragment (emit_state *result)
 {
-  static bfd_boolean recursive = FALSE;
   fragS *pool_location = get_literal_pool_location (now_seg);
   segT lit_seg;
   bfd_boolean is_init =
@@ -11423,23 +11440,14 @@ xtensa_switch_to_non_abs_literal_fragment (emit_state *result)
 
   if (pool_location == NULL
       && !use_literal_section
-      && !recursive
       && !is_init && ! is_fini)
     {
       if (!auto_litpools)
 	{
 	  as_bad (_("literal pool location required for text-section-literals; specify with .literal_position"));
 	}
-
-      /* When we mark a literal pool location, we want to put a frag in
-	 the literal pool that points to it.  But to do that, we want to
-	 switch_to_literal_fragment.  But literal sections don't have
-	 literal pools, so their location is always null, so we would
-	 recurse forever.  This is kind of hacky, but it works.  */
-
-      recursive = TRUE;
-      xtensa_mark_literal_pool_location ();
-      recursive = FALSE;
+      xtensa_maybe_create_literal_pool_frag (TRUE, TRUE);
+      pool_location = get_literal_pool_location (now_seg);
     }
 
   lit_seg = cache_literal_section (FALSE);
diff --git a/gas/testsuite/gas/xtensa/all.exp b/gas/testsuite/gas/xtensa/all.exp
index 98041b5..1ab3827 100644
--- a/gas/testsuite/gas/xtensa/all.exp
+++ b/gas/testsuite/gas/xtensa/all.exp
@@ -101,6 +101,8 @@ if [istarget xtensa*-*-*] then {
     run_dump_test "trampoline"
     run_dump_test "first_frag_align"
     run_dump_test "auto-litpools"
+    run_dump_test "auto-litpools-first1"
+    run_dump_test "auto-litpools-first2"
     run_dump_test "loc"
     run_dump_test "init-fini-literals"
 }
diff --git a/gas/testsuite/gas/xtensa/auto-litpools-first1.d b/gas/testsuite/gas/xtensa/auto-litpools-first1.d
new file mode 100644
index 0000000..322cdc5
--- /dev/null
+++ b/gas/testsuite/gas/xtensa/auto-litpools-first1.d
@@ -0,0 +1,12 @@
+#as: --auto-litpools
+#objdump: -ds
+#name: auto-litpools-first1 (check that literal pool is created when source starts with literal)
+
+.*: +file format .*xtensa.*
+#...
+Contents of section .text:
+ 0000 ........ 20170331 .*
+#...
+00000000 <f>:
+.*0:.*j.8 .*
+#...
diff --git a/gas/testsuite/gas/xtensa/auto-litpools-first1.s b/gas/testsuite/gas/xtensa/auto-litpools-first1.s
new file mode 100644
index 0000000..7ac0bf8
--- /dev/null
+++ b/gas/testsuite/gas/xtensa/auto-litpools-first1.s
@@ -0,0 +1,3 @@
+f:
+	.literal .L0, 0x20170331
+	l32r	a2, .L0
diff --git a/gas/testsuite/gas/xtensa/auto-litpools-first2.d b/gas/testsuite/gas/xtensa/auto-litpools-first2.d
new file mode 100644
index 0000000..a6b798e
--- /dev/null
+++ b/gas/testsuite/gas/xtensa/auto-litpools-first2.d
@@ -0,0 +1,15 @@
+#as: --auto-litpools
+#objdump: -ds
+#name: auto-litpools-first2 (check that literal pool with jump around is created for generated literal)
+
+.*: +file format .*xtensa.*
+#...
+Contents of section .text:
+ 0000 ........ ........ 20170331 .*
+#...
+00000000 <f>:
+   0:.*addi.*a1.*
+   3:.*j.*c.*
+#...
+   c:.*l32r.*a2, 8.*
+#...
diff --git a/gas/testsuite/gas/xtensa/auto-litpools-first2.s b/gas/testsuite/gas/xtensa/auto-litpools-first2.s
new file mode 100644
index 0000000..c097dac
--- /dev/null
+++ b/gas/testsuite/gas/xtensa/auto-litpools-first2.s
@@ -0,0 +1,3 @@
+f:
+	addi	a1, a1, -16
+	movi	a2, 0x20170331
diff --git a/gas/testsuite/gas/xtensa/auto-litpools.d b/gas/testsuite/gas/xtensa/auto-litpools.d
index 4d1a690..fc6f5cb 100644
--- a/gas/testsuite/gas/xtensa/auto-litpools.d
+++ b/gas/testsuite/gas/xtensa/auto-litpools.d
@@ -4,9 +4,9 @@
 
 .*: +file format .*xtensa.*
 #...
-.*4:.*l32r.a2, 0 .*
+.*8:.*l32r.a2, 4 .*
 #...
-.*3e437:.*j.3e440 .*
+.*3e43b:.*j.3e444 .*
 #...
-.*40750:.*l32r.a2, 3e43c .*
+.*40754:.*l32r.a2, 3e440 .*
 #...
-- 
2.1.4

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

* Re: [PATCH] gas: xtensa: fix incorrect code generated with auto litpools
  2017-04-05 21:20 [PATCH] gas: xtensa: fix incorrect code generated with auto litpools Max Filippov
@ 2017-04-10 12:15 ` Nick Clifton
  0 siblings, 0 replies; 2+ messages in thread
From: Nick Clifton @ 2017-04-10 12:15 UTC (permalink / raw)
  To: Max Filippov, binutils
  Cc: Sterling Augustine, David Weatherford, Marc Gauthier, linux-xtensa

Hi Max,

> gas/
> 2017-03-31  Max Filippov  <jcmvbkbc@gmail.com>
> 
> 	* config/tc-xtensa.c (xtensa_maybe_create_literal_pool_frag):
> 	Initialize lps->frag_count with auto_litpool_limit.
> 	(xg_promote_candidate_litpool): New function.
> 	(xtensa_move_literals): Extract candidate litpool promotion code
> 	into separate function. Call it for all possible found
> 	candidates.
> 	(xtensa_switch_to_literal_fragment): Drop 'recursive' flag and
> 	call to xtensa_mark_literal_pool_location that it guards.
> 	Replace it with call to xtensa_maybe_create_literal_pool_frag.
> 	Initialize pool_location with created literal pool candidate.
> 	* testsuite/gas/xtensa/all.exp: Add new tests.
> 	* testsuite/gas/xtensa/auto-litpools-first1.d: New test results.
> 	* testsuite/gas/xtensa/auto-litpools-first1.s: New test.
> 	* testsuite/gas/xtensa/auto-litpools-first2.d: New test results.
> 	* testsuite/gas/xtensa/auto-litpools-first2.s: New test.
> 	* testsuite/gas/xtensa/auto-litpools.d: Fix offsets changed due
> 	to additional jump instruction.
 
Approved and applied.

Cheers
  Nick

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

end of thread, other threads:[~2017-04-10 12:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-05 21:20 [PATCH] gas: xtensa: fix incorrect code generated with auto litpools Max Filippov
2017-04-10 12:15 ` 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).