[PR87793] reject non-toplevel unspecs in debug loc exprs on x86

Message ID or1s7x49lc.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • [PR87793] reject non-toplevel unspecs in debug loc exprs on x86
Related show

Commit Message

Alexandre Oliva Nov. 7, 2018, 7:42 a.m.
Before revision 254025, we'd reject UNSPECs in debug loc exprs.
TARGET_CONST_NOT_OK_FOR_DEBUG_P still rejects that by default, on all
ports that override it, except for x86, that accepts @gotoff unspecs.
We can indeed accept them in top-level expressions, but not as
subexpressions: the assembler rejects the difference between two
@gotoff symbols, for example.

We could simplify such a difference and drop the @gotoffs, provided
that the symbols are in the same section; we could also accept
@gotoffs plus literal constants.  However, accepting those but
rejecting such combinations as subexpressions would be ugly, and most
likely not worth the trouble: sym@gotoff+litconst hardly makes sense
as a standalone expression, and the difference between @gotoffs should
be avoided to begin with, as follows.

Ideally, the debug loc exprs would use the symbolic data in
REG_EQUIV/REG_EQUAL notes, or delegitimized addresses, instead of
simplifying the difference between two legitimized addresses so that
the occurrences of the GOT register cancel each other.  That would
require some more elaborate surgery in var-tracking and cselib than
would be appropriate at this stage.

Regstrapped on x86_64- and i686-linux-gnu.  Ok to install?


for  gcc/ChangeLog

	PR target/87793
	* config/i386/i386.c (ix86_const_not_ok_for_debug_p): Reject
	non-toplevel UNSPEC.

for  gcc/testsuite/ChangeLog

	PR target/87793
	* gcc.dg/pr87793.c: New.
---
 gcc/config/i386/i386.c         |   12 +++++++++++
 gcc/testsuite/gcc.dg/pr87793.c |   42 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr87793.c


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

Comments

Jeff Law Nov. 7, 2018, 10:46 p.m. | #1
On 11/7/18 12:42 AM, Alexandre Oliva wrote:
> Before revision 254025, we'd reject UNSPECs in debug loc exprs.

> TARGET_CONST_NOT_OK_FOR_DEBUG_P still rejects that by default, on all

> ports that override it, except for x86, that accepts @gotoff unspecs.

> We can indeed accept them in top-level expressions, but not as

> subexpressions: the assembler rejects the difference between two

> @gotoff symbols, for example.

> 

> We could simplify such a difference and drop the @gotoffs, provided

> that the symbols are in the same section; we could also accept

> @gotoffs plus literal constants.  However, accepting those but

> rejecting such combinations as subexpressions would be ugly, and most

> likely not worth the trouble: sym@gotoff+litconst hardly makes sense

> as a standalone expression, and the difference between @gotoffs should

> be avoided to begin with, as follows.

> 

> Ideally, the debug loc exprs would use the symbolic data in

> REG_EQUIV/REG_EQUAL notes, or delegitimized addresses, instead of

> simplifying the difference between two legitimized addresses so that

> the occurrences of the GOT register cancel each other.  That would

> require some more elaborate surgery in var-tracking and cselib than

> would be appropriate at this stage.

> 

> Regstrapped on x86_64- and i686-linux-gnu.  Ok to install?

> 

> 

> for  gcc/ChangeLog

> 

> 	PR target/87793

> 	* config/i386/i386.c (ix86_const_not_ok_for_debug_p): Reject

> 	non-toplevel UNSPEC.

> 

> for  gcc/testsuite/ChangeLog

> 

> 	PR target/87793

> 	* gcc.dg/pr87793.c: New.

OK.
jeff

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ae8971c82b0a..424a4b20631c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -17172,6 +17172,18 @@  ix86_const_not_ok_for_debug_p (rtx x)
   if (SYMBOL_REF_P (x) && strcmp (XSTR (x, 0), GOT_SYMBOL_NAME) == 0)
     return true;
 
+  /* Reject UNSPECs within expressions.  We could accept symbol@gotoff
+     + literal_constant, but that would hardly come up in practice,
+     and it's not worth the trouble of having to reject that as an
+     operand to pretty much anything else.  */
+  if (UNARY_P (x)
+      && GET_CODE (XEXP (x, 0)) == UNSPEC)
+    return true;
+  if (BINARY_P (x)
+      && (GET_CODE (XEXP (x, 0)) == UNSPEC
+	  || GET_CODE (XEXP (x, 1)) == UNSPEC))
+    return true;
+
   return false;
 }
 
diff --git a/gcc/testsuite/gcc.dg/pr87793.c b/gcc/testsuite/gcc.dg/pr87793.c
new file mode 100644
index 000000000000..3194313a265d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr87793.c
@@ -0,0 +1,42 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fpic -Os -g" } */
+
+struct fit_loadable_tbl {
+	int type;
+	void (*handler)(int data, int size);
+};
+
+#define ll_entry_start(_type, _list)					\
+({									\
+	static char start[0] __attribute__((aligned(4)))		\
+		__attribute__((unused, section(".u_boot_list_2_"#_list"_1")));	\
+	(_type *)&start;						\
+})
+
+#define ll_entry_end(_type, _list)					\
+({									\
+	static char end[0] __attribute__((aligned(4)))			\
+		__attribute__((unused, section(".u_boot_list_2_"#_list"_3")));	\
+	(_type *)&end;							\
+})
+
+#define ll_entry_count(_type, _list)					\
+	({								\
+		_type *start = ll_entry_start(_type, _list);		\
+		_type *end = ll_entry_end(_type, _list);		\
+		unsigned int _ll_result = end - start;			\
+		_ll_result;						\
+	})
+
+void test(int img_type, int img_data, int img_len)
+{
+	int i;
+	const unsigned int count =
+		ll_entry_count(struct fit_loadable_tbl, fit_loadable);
+	struct fit_loadable_tbl *fit_loadable_handler =
+		ll_entry_start(struct fit_loadable_tbl, fit_loadable);
+
+	for (i = 0; i < count; i++, fit_loadable_handler++)
+		if (fit_loadable_handler->type == img_type)
+			fit_loadable_handler->handler(img_data, img_len);
+}