[committed] Fix PR33256: LO_SUM ICE for -mabi=64 -msym32

Richard Sandiford richard@codesourcery.com
Thu Sep 6 18:33:00 GMT 2007


PR 33256 is an ICE on an invalid LO_SUM.  We start out with a valid
HIGH/LO_SUM DImode reference to "a", but because we only need the lower
half, we try to convert it to an SImode reference.  adjust_address
notices that the original DImode reference is 8-byte aligned, and that
adding 4 to the LO_SUM will therefore not induce a carry.  It therefore
(legitimately) adds 4 to the LO_SUM while leaving the high part alone.

However, we can't allow general "sym+offset" constants for -mabi=64
-msym32 because symbolic constants are sign-extended.  E.g., we can't
create relocs against "sym + 0x10000" when "sym" is 0x000000007fff8000,
because the relocation field will resolve to 0xffffffff80008000
instead of 0x0000000080008000.  (And this can happen for legitimate
C code in which the user isn't trying to calculate "sym+offset", let
alone reference it.)  So we only allow "sym+offset" if we can prove that
it's in the same section as "sym".

The problem is that the MIPS address code only accepts a LO_SUM if
operand 1 is a legitimate constant.  "sym+4" isn't.  This is related to:

    http://thread.gmane.org/gmane.comp.gcc.patches/139565/focus=140004

I still think the final fix in that thread:

    http://gcc.gnu.org/viewcvs/trunk/gcc/config/mips/mips.c?r1=124833&r2=124832&pathrev=124833

was the right one for that bug because we always have access to a
TLS's decl, so we can apply suitably small offsets to HIGHs as well
as LO_SUMs.  However, although we do have access to a decl in the
testcase for this PR, we won't for all HIGHs and LO_SUMs, so a similar
fix won't work here.

I think we have to trust the creator of the LO_SUM to do something
vaguely sane.  Target-independent code that creates a LO_SUM should also
create and verify the associated HIGH, which must use the same symbol as
the LO_SUM.  Target-independent code that adds an offset to a LO_SUM
should prove that the offset will not induce a carry.  Failure to do
either of these things would be a bug.  And the MIPS backend should
only create LO_SUMs for valid symbolic constants, with the high part
being either a HIGH or a copy of _gp.

This patch therefore relaxes the LO_SUM code so that it assumes the
offset is valid.  Note that the *low<mode> mips.md patterns are already OK,
because they use immediate_operand.

Regression-tested on mipsisa32r2-sde-elf.  David also bootstrapped it on
mipsel-linux-gnu (thanks).  Since this is a regression, I backported it
to 4.1 and 4.2, testing on mipsisa64-elf for both.  Applied to mainline,
4.1 and 4.2.

Richard


gcc/
	PR target/33256
	* config/mips/mips.c (mips_classify_symbolic_expression): New function.
	(mips_classify_address): Use it instead of mips_symbolic_constant_p.
	(print_operand_reloc): Likewise.

gcc/testsuite/
2007-09-07  David Daney  <ddaney@avtrex.com>
	    Richard Sandiford  <richard@codesourcery.com>

	PR target/33256
	* gcc.target/mips/mips.exp (setup_mips_tests): Set mips_forced_le.
	(dg-mips-options): Skip -EB and -meb tests when $mips_forced_le.
	* gcc.target/mips/pr33256.c: New test.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 128185)
+++ gcc/config/mips/mips.c	(working copy)
@@ -1553,6 +1553,21 @@ mips_classify_symbol (const_rtx x, enum 
   return SYMBOL_ABSOLUTE;
 }
 
+/* Classify symbolic expression X, given that it appears in context
+   CONTEXT.  */
+
+static enum mips_symbol_type
+mips_classify_symbolic_expression (rtx x, enum mips_symbol_context context)
+{
+  rtx offset;
+
+  split_const (x, &x, &offset);
+  if (UNSPEC_ADDRESS_P (x))
+    return UNSPEC_ADDRESS_TYPE (x);
+
+  return mips_classify_symbol (x, context);
+}
+
 /* Return true if OFFSET is within the range [0, ALIGN), where ALIGN
    is the alignment (in bytes) of SYMBOL_REF X.  */
 
@@ -1747,9 +1762,18 @@ mips_classify_address (struct mips_addre
       info->type = ADDRESS_LO_SUM;
       info->reg = XEXP (x, 0);
       info->offset = XEXP (x, 1);
+      /* We have to trust the creator of the LO_SUM to do something vaguely
+	 sane.  Target-independent code that creates a LO_SUM should also
+	 create and verify the matching HIGH.  Target-independent code that
+	 adds an offset to a LO_SUM must prove that the offset will not
+	 induce a carry.  Failure to do either of these things would be
+	 a bug, and we are not required to check for it here.  The MIPS
+	 backend itself should only create LO_SUMs for valid symbolic
+	 constants, with the high part being either a HIGH or a copy
+	 of _gp. */
+      info->symbol_type
+	= mips_classify_symbolic_expression (info->offset, SYMBOL_CONTEXT_MEM);
       return (mips_valid_base_register_p (info->reg, mode, strict)
-	      && mips_symbolic_constant_p (info->offset, SYMBOL_CONTEXT_MEM,
-					   &info->symbol_type)
 	      && mips_symbol_insns (info->symbol_type, mode) > 0
 	      && mips_lo_relocs[info->symbol_type] != 0);
 
@@ -6290,8 +6314,8 @@ print_operand_reloc (FILE *file, rtx op,
   enum mips_symbol_type symbol_type;
   const char *p;
 
-  if (!mips_symbolic_constant_p (op, context, &symbol_type)
-      || relocs[symbol_type] == 0)
+  symbol_type = mips_classify_symbolic_expression (op, context);
+  if (relocs[symbol_type] == 0)
     fatal_insn ("PRINT_OPERAND, invalid operand for relocation", op);
 
   fputs (relocs[symbol_type], file);
Index: gcc/testsuite/gcc.target/mips/mips.exp
===================================================================
--- gcc/testsuite/gcc.target/mips/mips.exp	(revision 128185)
+++ gcc/testsuite/gcc.target/mips/mips.exp	(working copy)
@@ -37,6 +37,7 @@ load_lib gcc-dg.exp
 #    $mips_forced_isa:	 true if the command line uses -march=* or -mips*
 #    $mips_forced_abi:	 true if the command line uses -mabi=* or -mgp*
 #    $mips_forced_float: true if the command line uses -mhard/soft-float
+#    $mips_forced_le	 true if the command line uses -EL or -mel
 proc setup_mips_tests {} {
     global mips_isa
     global mips_arch
@@ -47,6 +48,7 @@ proc setup_mips_tests {} {
     global mips_forced_isa
     global mips_forced_abi
     global mips_forced_float
+    global mips_forced_le
 
     global compiler_flags
     global tool
@@ -81,6 +83,7 @@ proc setup_mips_tests {} {
     set mips_forced_isa [regexp -- {(-mips|-march)} $compiler_flags]
     set mips_forced_abi [regexp -- {(-mgp|-mfp|-mabi)} $compiler_flags]
     set mips_forced_float [regexp -- {-m(hard|soft)-float} $compiler_flags]
+    set mips_forced_le [regexp -- {-(EL|mel)[[:>:]]} $compiler_flags]
 }
 
 # Return true if command-line option FLAG forces 32-bit code.
@@ -117,6 +120,10 @@ proc is_gp32_flag {flag} {
 #    -mhard-float
 #	Select the given floating-point mode.  Skip the test if the
 #	multilib flags force a different selection.
+#
+#    -EB
+#	Select big-endian code.  Skip the test if the multilib flags
+#	force a little-endian target.
 proc dg-mips-options {args} {
     upvar dg-extra-tool-flags extra_tool_flags
     upvar dg-do-what do_what
@@ -130,6 +137,7 @@ proc dg-mips-options {args} {
     global mips_forced_isa
     global mips_forced_abi
     global mips_forced_float
+    global mips_forced_le
 
     set flags [lindex $args 1]
     set matches 1
@@ -175,6 +183,10 @@ proc dg-mips-options {args} {
 	    if {$mips_float != $float && $mips_forced_float} {
 		set matches 0
 	    }
+	} elseif {[regexp -- {^-(EB|meb)$} $flag]} {
+	    if {$mips_forced_le} {
+		set matches 0
+	    }
 	}
     }
     if {$matches} {
Index: gcc/testsuite/gcc.target/mips/pr33256.c
===================================================================
--- gcc/testsuite/gcc.target/mips/pr33256.c	(revision 0)
+++ gcc/testsuite/gcc.target/mips/pr33256.c	(revision 0)
@@ -0,0 +1,11 @@
+/* GCC used to report an ICE for this test because we generated a LO_SUM
+   for an illegitimate constant.  */
+/* { dg-mips-options "-mabi=64 -mips3 -msym32 -O2 -EB -mno-abicalls" } */
+extern unsigned long a[];
+int b (int);
+
+int
+c (void)
+{
+  return b (a[0]);
+}



More information about the Gcc-patches mailing list