[PATCH][AArch64] Fix symbol offset limit

[PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself.  This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.

To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.

Bootstrapped on AArch64, passes regress, OK for commit?

ChangeLog:
2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>

    gcc/
	* config/aarch64/aarch64.c (aarch64_classify_symbol):
	Apply reasonable limit to symbol offsets.

    testsuite/
	* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
	* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.

--

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
 	     the offset does not cause overflow of the final address.  But
 	     we have no way of knowing the address of symbol at compile time
 	     so we can't accurately say if the distance between the PC and
-	     symbol + offset is outside the addressible range of +/-1M in the
-	     TINY code model.  So we rely on images not being greater than
-	     1M and cap the offset at 1M and anything beyond 1M will have to
-	     be loaded using an alternative mechanism.  Furthermore if the
-	     symbol is a weak reference to something that isn't known to
-	     resolve to a symbol in this module, then force to memory.  */
+	     symbol + offset is outside the addressible range of +/-1MB in the
+	     TINY code model.  So we limit the maximum offset to +/-64KB and
+	     assume the offset to the symbol is not larger than +/-(1MB - 64KB).
+	     Furthermore force to memory if the symbol is a weak reference to
+	     something that doesn't resolve to a symbol in this module.  */
 	  if ((SYMBOL_REF_WEAK (x)
 	       && !aarch64_symbol_binds_local_p (x))
-	      || !IN_RANGE (offset, -1048575, 1048575))
+	      || !IN_RANGE (offset, -0x10000, 0x10000))
 	    return SYMBOL_FORCE_TO_MEM;
+
 	  return SYMBOL_TINY_ABSOLUTE;
 
 	case AARCH64_CMODEL_SMALL:
 	  /* Same reasoning as the tiny code model, but the offset cap here is
-	     4G.  */
+	     1MB, allowing +/-3.9GB for the offset to the symbol.  */
 	  if ((SYMBOL_REF_WEAK (x)
 	       && !aarch64_symbol_binds_local_p (x))
-	      || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
-			    HOST_WIDE_INT_C (4294967264)))
+	      || !IN_RANGE (offset, -0x100000, 0x100000))
 	    return SYMBOL_FORCE_TO_MEM;
+
 	  return SYMBOL_SMALL_ABSOLUTE;
 
 	case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
 
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x00080000];
+  return fixed_regs[0x000ff000];
 }
 
 /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=small" } */
 
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x100000000ULL];
+  return fixed_regs[0xfffff000ULL];
 }
 
 /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

ping
   
 
In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
 This means the offset can use all of the +/-4GB offset, leaving no offset available
 for the symbol itself.  This results in relocation overflow and link-time errors
 for simple expressions like &global_char + 0xffffff00.
 
 To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
 3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
 most of the 1MB range for code/data between the symbol and its references.
 
 Bootstrapped on AArch64, passes regress, OK for commit?
 
 ChangeLog:
 2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>
 
     gcc/
         * config/aarch64/aarch64.c (aarch64_classify_symbol):
         Apply reasonable limit to symbol offsets.
 
     testsuite/
         * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
         * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
 
 --
 
 diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
 index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
 --- a/gcc/config/aarch64/aarch64.c
 +++ b/gcc/config/aarch64/aarch64.c
 @@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
               the offset does not cause overflow of the final address.  But
               we have no way of knowing the address of symbol at compile time
               so we can't accurately say if the distance between the PC and
 -            symbol + offset is outside the addressible range of +/-1M in the
 -            TINY code model.  So we rely on images not being greater than
 -            1M and cap the offset at 1M and anything beyond 1M will have to
 -            be loaded using an alternative mechanism.  Furthermore if the
 -            symbol is a weak reference to something that isn't known to
 -            resolve to a symbol in this module, then force to memory.  */
 +            symbol + offset is outside the addressible range of +/-1MB in the
 +            TINY code model.  So we limit the maximum offset to +/-64KB and
 +            assume the offset to the symbol is not larger than +/-(1MB - 64KB).
 +            Furthermore force to memory if the symbol is a weak reference to
 +            something that doesn't resolve to a symbol in this module.  */
            if ((SYMBOL_REF_WEAK (x)
                 && !aarch64_symbol_binds_local_p (x))
 -             || !IN_RANGE (offset, -1048575, 1048575))
 +             || !IN_RANGE (offset, -0x10000, 0x10000))
              return SYMBOL_FORCE_TO_MEM;
 +
            return SYMBOL_TINY_ABSOLUTE;
  
          case AARCH64_CMODEL_SMALL:
            /* Same reasoning as the tiny code model, but the offset cap here is
 -            4G.  */
 +            1MB, allowing +/-3.9GB for the offset to the symbol.  */
            if ((SYMBOL_REF_WEAK (x)
                 && !aarch64_symbol_binds_local_p (x))
 -             || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
 -                           HOST_WIDE_INT_C (4294967264)))
 +             || !IN_RANGE (offset, -0x100000, 0x100000))
              return SYMBOL_FORCE_TO_MEM;
 +
            return SYMBOL_SMALL_ABSOLUTE;
  
          case AARCH64_CMODEL_TINY_PIC:
 diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
 index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
 --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
 +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
 @@ -1,12 +1,12 @@
 -/* { dg-do compile } */
 +/* { dg-do link } */
  /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
  
 -int fixed_regs[0x00200000];
 +char fixed_regs[0x00200000];
  
  int
 -foo()
 +main ()
  {
 -  return fixed_regs[0x00080000];
 +  return fixed_regs[0x000ff000];
  }
  
  /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
 diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
 index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
 --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
 +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
 @@ -1,12 +1,12 @@
 -/* { dg-do compile } */
 +/* { dg-do link } */
  /* { dg-options "-O3 -save-temps -mcmodel=small" } */
  
 -int fixed_regs[0x200000000ULL];
 +char fixed_regs[0x200000000ULL];
  
  int
 -foo()
 +main ()
  {
 -  return fixed_regs[0x100000000ULL];
 +  return fixed_regs[0xfffff000ULL];
  }
  
  /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
 
     

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

   
 
ping
    
  
 In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
  This means the offset can use all of the +/-4GB offset, leaving no offset available
  for the symbol itself.  This results in relocation overflow and link-time errors
  for simple expressions like &global_char + 0xffffff00.
  
  To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
  3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
  most of the 1MB range for code/data between the symbol and its references.
  
  Bootstrapped on AArch64, passes regress, OK for commit?
  
  ChangeLog:
  2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>
  
      gcc/
          * config/aarch64/aarch64.c (aarch64_classify_symbol):
          Apply reasonable limit to symbol offsets.
  
      testsuite/
          * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
          * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
  
  --
  
  diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
  index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
  --- a/gcc/config/aarch64/aarch64.c
  +++ b/gcc/config/aarch64/aarch64.c
  @@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
                the offset does not cause overflow of the final address.  But
                we have no way of knowing the address of symbol at compile time
                so we can't accurately say if the distance between the PC and
  -            symbol + offset is outside the addressible range of +/-1M in the
  -            TINY code model.  So we rely on images not being greater than
  -            1M and cap the offset at 1M and anything beyond 1M will have to
  -            be loaded using an alternative mechanism.  Furthermore if the
  -            symbol is a weak reference to something that isn't known to
  -            resolve to a symbol in this module, then force to memory.  */
  +            symbol + offset is outside the addressible range of +/-1MB in the
  +            TINY code model.  So we limit the maximum offset to +/-64KB and
  +            assume the offset to the symbol is not larger than +/-(1MB - 64KB).
  +            Furthermore force to memory if the symbol is a weak reference to
  +            something that doesn't resolve to a symbol in this module.  */
             if ((SYMBOL_REF_WEAK (x)
                  && !aarch64_symbol_binds_local_p (x))
  -             || !IN_RANGE (offset, -1048575, 1048575))
  +             || !IN_RANGE (offset, -0x10000, 0x10000))
               return SYMBOL_FORCE_TO_MEM;
  +
             return SYMBOL_TINY_ABSOLUTE;
   
           case AARCH64_CMODEL_SMALL:
             /* Same reasoning as the tiny code model, but the offset cap here is
  -            4G.  */
  +            1MB, allowing +/-3.9GB for the offset to the symbol.  */
             if ((SYMBOL_REF_WEAK (x)
                  && !aarch64_symbol_binds_local_p (x))
  -             || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
  -                           HOST_WIDE_INT_C (4294967264)))
  +             || !IN_RANGE (offset, -0x100000, 0x100000))
               return SYMBOL_FORCE_TO_MEM;
  +
             return SYMBOL_SMALL_ABSOLUTE;
   
           case AARCH64_CMODEL_TINY_PIC:
  diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
  index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
  --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
  +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
  @@ -1,12 +1,12 @@
  -/* { dg-do compile } */
  +/* { dg-do link } */
   /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
   
  -int fixed_regs[0x00200000];
  +char fixed_regs[0x00200000];
   
   int
  -foo()
  +main ()
   {
  -  return fixed_regs[0x00080000];
  +  return fixed_regs[0x000ff000];
   }
   
   /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
  diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
  index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
  --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
  +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
  @@ -1,12 +1,12 @@
  -/* { dg-do compile } */
  +/* { dg-do link } */
   /* { dg-options "-O3 -save-temps -mcmodel=small" } */
   
  -int fixed_regs[0x200000000ULL];
  +char fixed_regs[0x200000000ULL];
   
   int
  -foo()
  +main ()
   {
  -  return fixed_regs[0x100000000ULL];
  +  return fixed_regs[0xfffff000ULL];
   }
   
   /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
  
          

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

     
 ping
     
   
  In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
   This means the offset can use all of the +/-4GB offset, leaving no offset available
   for the symbol itself.  This results in relocation overflow and link-time errors
   for simple expressions like &global_char + 0xffffff00.
   
   To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
   3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
   most of the 1MB range for code/data between the symbol and its references.
   
   Bootstrapped on AArch64, passes regress, OK for commit?
   
   ChangeLog:
   2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>
   
       gcc/
           * config/aarch64/aarch64.c (aarch64_classify_symbol):
           Apply reasonable limit to symbol offsets.
   
       testsuite/
           * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
           * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
   
   --
   
   diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
   index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
   --- a/gcc/config/aarch64/aarch64.c
   +++ b/gcc/config/aarch64/aarch64.c
   @@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
                 the offset does not cause overflow of the final address.  But
                 we have no way of knowing the address of symbol at compile time
                 so we can't accurately say if the distance between the PC and
   -            symbol + offset is outside the addressible range of +/-1M in the
   -            TINY code model.  So we rely on images not being greater than
   -            1M and cap the offset at 1M and anything beyond 1M will have to
   -            be loaded using an alternative mechanism.  Furthermore if the
   -            symbol is a weak reference to something that isn't known to
   -            resolve to a symbol in this module, then force to memory.  */
   +            symbol + offset is outside the addressible range of +/-1MB in the
   +            TINY code model.  So we limit the maximum offset to +/-64KB and
   +            assume the offset to the symbol is not larger than +/-(1MB - 64KB).
   +            Furthermore force to memory if the symbol is a weak reference to
   +            something that doesn't resolve to a symbol in this module.  */
              if ((SYMBOL_REF_WEAK (x)
                   && !aarch64_symbol_binds_local_p (x))
   -             || !IN_RANGE (offset, -1048575, 1048575))
   +             || !IN_RANGE (offset, -0x10000, 0x10000))
                return SYMBOL_FORCE_TO_MEM;
   +
              return SYMBOL_TINY_ABSOLUTE;
    
            case AARCH64_CMODEL_SMALL:
              /* Same reasoning as the tiny code model, but the offset cap here is
   -            4G.  */
   +            1MB, allowing +/-3.9GB for the offset to the symbol.  */
              if ((SYMBOL_REF_WEAK (x)
                   && !aarch64_symbol_binds_local_p (x))
   -             || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
   -                           HOST_WIDE_INT_C (4294967264)))
   +             || !IN_RANGE (offset, -0x100000, 0x100000))
                return SYMBOL_FORCE_TO_MEM;
   +
              return SYMBOL_SMALL_ABSOLUTE;
    
            case AARCH64_CMODEL_TINY_PIC:
   diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
   index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
   --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
   +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
   @@ -1,12 +1,12 @@
   -/* { dg-do compile } */
   +/* { dg-do link } */
    /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
    
   -int fixed_regs[0x00200000];
   +char fixed_regs[0x00200000];
    
    int
   -foo()
   +main ()
    {
   -  return fixed_regs[0x00080000];
   +  return fixed_regs[0x000ff000];
    }
    
    /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
   diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
   index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
   --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
   +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
   @@ -1,12 +1,12 @@
   -/* { dg-do compile } */
   +/* { dg-do link } */
    /* { dg-options "-O3 -save-temps -mcmodel=small" } */
    
   -int fixed_regs[0x200000000ULL];
   +char fixed_regs[0x200000000ULL];
    
    int
   -foo()
   +main ()
    {
   -  return fixed_regs[0x100000000ULL];
   +  return fixed_regs[0xfffff000ULL];
    }
    
    /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
   
               

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

   
     
  ping
      
    
   In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
    This means the offset can use all of the +/-4GB offset, leaving no offset available
    for the symbol itself.  This results in relocation overflow and link-time errors
    for simple expressions like &global_char + 0xffffff00.
    
    To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
    3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
    most of the 1MB range for code/data between the symbol and its references.
    
    Bootstrapped on AArch64, passes regress, OK for commit?
    
    ChangeLog:
    2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>
    
        gcc/
            * config/aarch64/aarch64.c (aarch64_classify_symbol):
            Apply reasonable limit to symbol offsets.
    
        testsuite/
            * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
            * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
    
    --
    
    diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
    index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
    --- a/gcc/config/aarch64/aarch64.c
    +++ b/gcc/config/aarch64/aarch64.c
    @@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
                  the offset does not cause overflow of the final address.  But
                  we have no way of knowing the address of symbol at compile time
                  so we can't accurately say if the distance between the PC and
    -            symbol + offset is outside the addressible range of +/-1M in the
    -            TINY code model.  So we rely on images not being greater than
    -            1M and cap the offset at 1M and anything beyond 1M will have to
    -            be loaded using an alternative mechanism.  Furthermore if the
    -            symbol is a weak reference to something that isn't known to
    -            resolve to a symbol in this module, then force to memory.  */
    +            symbol + offset is outside the addressible range of +/-1MB in the
    +            TINY code model.  So we limit the maximum offset to +/-64KB and
    +            assume the offset to the symbol is not larger than +/-(1MB - 64KB).
    +            Furthermore force to memory if the symbol is a weak reference to
    +            something that doesn't resolve to a symbol in this module.  */
               if ((SYMBOL_REF_WEAK (x)
                    && !aarch64_symbol_binds_local_p (x))
    -             || !IN_RANGE (offset, -1048575, 1048575))
    +             || !IN_RANGE (offset, -0x10000, 0x10000))
                 return SYMBOL_FORCE_TO_MEM;
    +
               return SYMBOL_TINY_ABSOLUTE;
     
             case AARCH64_CMODEL_SMALL:
               /* Same reasoning as the tiny code model, but the offset cap here is
    -            4G.  */
    +            1MB, allowing +/-3.9GB for the offset to the symbol.  */
               if ((SYMBOL_REF_WEAK (x)
                    && !aarch64_symbol_binds_local_p (x))
    -             || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
    -                           HOST_WIDE_INT_C (4294967264)))
    +             || !IN_RANGE (offset, -0x100000, 0x100000))
                 return SYMBOL_FORCE_TO_MEM;
    +
               return SYMBOL_SMALL_ABSOLUTE;
     
             case AARCH64_CMODEL_TINY_PIC:
    diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
    index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
    --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
    +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
    @@ -1,12 +1,12 @@
    -/* { dg-do compile } */
    +/* { dg-do link } */
     /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
     
    -int fixed_regs[0x00200000];
    +char fixed_regs[0x00200000];
     
     int
    -foo()
    +main ()
     {
    -  return fixed_regs[0x00080000];
    +  return fixed_regs[0x000ff000];
     }
     
     /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
    diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
    index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
    --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
    +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
    @@ -1,12 +1,12 @@
    -/* { dg-do compile } */
    +/* { dg-do link } */
     /* { dg-options "-O3 -save-temps -mcmodel=small" } */
     
    -int fixed_regs[0x200000000ULL];
    +char fixed_regs[0x200000000ULL];
     
     int
    -foo()
    +main ()
     {
    -  return fixed_regs[0x100000000ULL];
    +  return fixed_regs[0xfffff000ULL];
     }
     
     /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
    
                    

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

ping

    In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
    This means the offset can use all of the +/-4GB offset, leaving no offset available
    for the symbol itself.  This results in relocation overflow and link-time errors
    for simple expressions like &global_char + 0xffffff00.
    
    To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
    3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
    most of the 1MB range for code/data between the symbol and its references.
    
    Bootstrapped on AArch64, passes regress, OK for commit?
    
    ChangeLog:
    2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>
    
        gcc/
            * config/aarch64/aarch64.c (aarch64_classify_symbol):
            Apply reasonable limit to symbol offsets.
    
        testsuite/
            * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
            * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
    
    --
    
    diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
    index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
    --- a/gcc/config/aarch64/aarch64.c
    +++ b/gcc/config/aarch64/aarch64.c
    @@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
                  the offset does not cause overflow of the final address.  But
                  we have no way of knowing the address of symbol at compile time
                  so we can't accurately say if the distance between the PC and
    -            symbol + offset is outside the addressible range of +/-1M in the
    -            TINY code model.  So we rely on images not being greater than
    -            1M and cap the offset at 1M and anything beyond 1M will have to
    -            be loaded using an alternative mechanism.  Furthermore if the
    -            symbol is a weak reference to something that isn't known to
    -            resolve to a symbol in this module, then force to memory.  */
    +            symbol + offset is outside the addressible range of +/-1MB in the
    +            TINY code model.  So we limit the maximum offset to +/-64KB and
    +            assume the offset to the symbol is not larger than +/-(1MB - 64KB).
    +            Furthermore force to memory if the symbol is a weak reference to
    +            something that doesn't resolve to a symbol in this module.  */
               if ((SYMBOL_REF_WEAK (x)
                    && !aarch64_symbol_binds_local_p (x))
    -             || !IN_RANGE (offset, -1048575, 1048575))
    +             || !IN_RANGE (offset, -0x10000, 0x10000))
                 return SYMBOL_FORCE_TO_MEM;
    +
               return SYMBOL_TINY_ABSOLUTE;
     
             case AARCH64_CMODEL_SMALL:
               /* Same reasoning as the tiny code model, but the offset cap here is
    -            4G.  */
    +            1MB, allowing +/-3.9GB for the offset to the symbol.  */
               if ((SYMBOL_REF_WEAK (x)
                    && !aarch64_symbol_binds_local_p (x))
    -             || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
    -                           HOST_WIDE_INT_C (4294967264)))
    +             || !IN_RANGE (offset, -0x100000, 0x100000))
                 return SYMBOL_FORCE_TO_MEM;
    +
               return SYMBOL_SMALL_ABSOLUTE;
     
             case AARCH64_CMODEL_TINY_PIC:
    diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
    index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
    --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
    +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
    @@ -1,12 +1,12 @@
    -/* { dg-do compile } */
    +/* { dg-do link } */
     /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
     
    -int fixed_regs[0x00200000];
    +char fixed_regs[0x00200000];
     
     int
    -foo()
    +main ()
     {
    -  return fixed_regs[0x00080000];
    +  return fixed_regs[0x000ff000];
     }
     
     /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
    diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
    index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
    --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
    +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
    @@ -1,12 +1,12 @@
    -/* { dg-do compile } */
    +/* { dg-do link } */
     /* { dg-options "-O3 -save-temps -mcmodel=small" } */
     
    -int fixed_regs[0x200000000ULL];
    +char fixed_regs[0x200000000ULL];
     
     int
    -foo()
    +main ()
     {
    -  return fixed_regs[0x100000000ULL];
    +  return fixed_regs[0xfffff000ULL];
     }
     
     /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */

Re: [PATCH][AArch64] Fix symbol offset limit

[Richard Sandiford]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> ping
>
>     In aarch64_classify_symbol symbols are allowed full-range offsets on relocations.
>     This means the offset can use all of the +/-4GB offset, leaving no offset available
>     for the symbol itself.  This results in relocation overflow and link-time errors
>     for simple expressions like &global_char + 0xffffff00.

If global_char really is a char then isn't that UB?  I guess we still
have to compile it without error though...

>     To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
>     3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
>     most of the 1MB range for code/data between the symbol and its references.

These new values seem a bit magical.  Would it work for the original
testcase to use FORCE_TO_MEM if !offset_within_block_p (x, offset)?

Richard

>     Bootstrapped on AArch64, passes regress, OK for commit?
>
>     ChangeLog:
>     2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>
>
>         gcc/
>             * config/aarch64/aarch64.c (aarch64_classify_symbol):
>             Apply reasonable limit to symbol offsets.
>
>         testsuite/
>             * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
>             * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
>
>     --
>
>     diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>     index 83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2 100644
>     --- a/gcc/config/aarch64/aarch64.c
>     +++ b/gcc/config/aarch64/aarch64.c
>     @@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
>                   the offset does not cause overflow of the final address.  But
>                   we have no way of knowing the address of symbol at compile time
>                   so we can't accurately say if the distance between the PC and
>     -            symbol + offset is outside the addressible range of +/-1M in the
>     -            TINY code model.  So we rely on images not being greater than
>     -            1M and cap the offset at 1M and anything beyond 1M will have to
>     -            be loaded using an alternative mechanism.  Furthermore if the
>     -            symbol is a weak reference to something that isn't known to
>     -            resolve to a symbol in this module, then force to memory.  */
>     +            symbol + offset is outside the addressible range of +/-1MB in the
>     +            TINY code model.  So we limit the maximum offset to +/-64KB and
>     +            assume the offset to the symbol is not larger than +/-(1MB - 64KB).
>     +            Furthermore force to memory if the symbol is a weak reference to
>     +            something that doesn't resolve to a symbol in this module.  */
>                if ((SYMBOL_REF_WEAK (x)
>                     && !aarch64_symbol_binds_local_p (x))
>     -             || !IN_RANGE (offset, -1048575, 1048575))
>     +             || !IN_RANGE (offset, -0x10000, 0x10000))
>                  return SYMBOL_FORCE_TO_MEM;
>     +
>                return SYMBOL_TINY_ABSOLUTE;
>
>              case AARCH64_CMODEL_SMALL:
>                /* Same reasoning as the tiny code model, but the offset cap here is
>     -            4G.  */
>     +            1MB, allowing +/-3.9GB for the offset to the symbol.  */
>                if ((SYMBOL_REF_WEAK (x)
>                     && !aarch64_symbol_binds_local_p (x))
>     -             || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
>     -                           HOST_WIDE_INT_C (4294967264)))
>     +             || !IN_RANGE (offset, -0x100000, 0x100000))
>                  return SYMBOL_FORCE_TO_MEM;
>     +
>                return SYMBOL_SMALL_ABSOLUTE;
>
>              case AARCH64_CMODEL_TINY_PIC:
>     diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
>     index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
>     --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
>     +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
>     @@ -1,12 +1,12 @@
>     -/* { dg-do compile } */
>     +/* { dg-do link } */
>      /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
>
>     -int fixed_regs[0x00200000];
>     +char fixed_regs[0x00200000];
>
>      int
>     -foo()
>     +main ()
>      {
>     -  return fixed_regs[0x00080000];
>     +  return fixed_regs[0x000ff000];
>      }
>
>      /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
>     diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
>     index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
>     --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
>     +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
>     @@ -1,12 +1,12 @@
>     -/* { dg-do compile } */
>     +/* { dg-do link } */
>      /* { dg-options "-O3 -save-temps -mcmodel=small" } */
>
>     -int fixed_regs[0x200000000ULL];
>     +char fixed_regs[0x200000000ULL];
>
>      int
>     -foo()
>     +main ()
>      {
>     -  return fixed_regs[0x100000000ULL];
>     +  return fixed_regs[0xfffff000ULL];
>      }
>
>      /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

Hi Richard,

> If global_char really is a char then isn't that UB?  

No why? We can do all kinds of arithmetic based on pointers, either using
pointer types or converted to uintptr_t. Note that the optimizer actually creates
these expressions, for example arr[N-x] can be evaluated as (&arr[0] + N) - x.
So this remains legal even if N is well outside the bounds of the array.

> I guess we still have to compile it without error though...

*THIS* a million times... Any non-trivial application relies on UB behaviour with
100% certainty. But we still have to compile it correctly!

>>     To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
>>     3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
>>     most of the 1MB range for code/data between the symbol and its references.
>
> These new values seem a bit magical.  Would it work for the original
> testcase to use FORCE_TO_MEM if !offset_within_block_p (x, offset)?

No - the testcases fail with that. It also reduces codequality by not allowing
commonly used offsets as part of the symbol relocation.

So how would you want to make the offsets less magical? There isn't a clear limitation
like for MOVW/MOVT relocations (which are limited to +-32KB), so it just is an arbitrary
decision which allows this optimization for all frequently used offsets but avoids relocation
failures caused by large offsets. The values I used were chosen so that codequality isn't
affected, but it is practically impossible to trigger relocation errors.

And they can always be changed again if required - this is not meant to be the final
AArch64 patch ever!

Cheers,
Wilco 

Re: [PATCH][AArch64] Fix symbol offset limit

[Richard Sandiford]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Hi Richard,
>
>> If global_char really is a char then isn't that UB?
>
> No why?

Well, "simple expressions like &global_char + 0xffffff00" made it sound
like there really was a:

   extern char global_char;

Only &global_char and &global_char + 1 are defined in that case.
I was probably taking the example too literally though.

> We can do all kinds of arithmetic based on pointers, either using
> pointer types or converted to uintptr_t. Note that the optimizer
> actually creates these expressions, for example arr[N-x] can be
> evaluated as (&arr[0] + N) - x.  So this remains legal even if N is
> well outside the bounds of the array.

Yeah, true, and I remember valid "x[n - big_offset]" -> invalid
"x + big_offset - n" being a problem for MIPS too.  It was one of
the things offset_within_block_p was helping to avoid.

>> I guess we still have to compile it without error though...
>
> *THIS* a million times... Any non-trivial application relies on UB behaviour with
> 100% certainty. But we still have to compile it correctly!
>
>>>     To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
>>>     3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
>>>     most of the 1MB range for code/data between the symbol and its references.
>>
>> These new values seem a bit magical.  Would it work for the original
>> testcase to use FORCE_TO_MEM if !offset_within_block_p (x, offset)?
>
> No - the testcases fail with that.

Hmm, OK.  Could you give more details?  What does the motivating case
actually look like?

One of the reasons this is a hard patch to review is that it doesn't
include a testcase for the kind of situation it's trying to fix.

> It also reduces codequality by not allowing commonly used offsets as
> part of the symbol relocation.

Which kinds of cases specifically?  Although I'm guessing the problem is
when indexing into external arrays of unknown size.

I think the starting point for this stuff is that the address of every
referenced object has to be within reach for the code model. In other
words, offset 0 must be within reach.  It follows that the end + 1
address of every referenced object except the last (in memory) will
also be within reach.

The question then is: do we want, as a special exception, to allow
the end + 1 address of the last object to be out of reach?  I think
it would be very difficult to support that reliably without more
information.  The compiler would be left guessing which object is
the final one and how much of it is out of reach.

So IMO we should be able to assume that the start and end + 1 addresses
of any referenced object are within reach.  For section anchors, we can
extend that to the containing anchor block, which is really just a
super-sized object.

So when offset_within_block_p, it seems like we should apply the
documented ranges for the code model, or even ignore them altogether.
Limiting the range to 64k for the tiny model would regress valid
cases like:

  char x[0x20000];
  char f (void) { return x[0x1ffff]; }

I'm not saying this case is important.  I'm just saying there's no
obvious reason why the offset shouldn't be valid given the above
assumptions.

The question then is what to do about symbols whose size isn't known.
And I agree that unconditionally applying the full code-model offset
range is too aggressive in that case.

> So how would you want to make the offsets less magical? There isn't a clear limitation
> like for MOVW/MOVT relocations (which are limited to +-32KB), so it just is an arbitrary
> decision which allows this optimization for all frequently used offsets but avoids relocation
> failures caused by large offsets. The values I used were chosen so that codequality isn't
> affected, but it is practically impossible to trigger relocation errors.
>
> And they can always be changed again if required - this is not meant to be the final
> AArch64 patch ever!

Well, for one thing, if code quality isn't affected by using +/-64k
for the tiny model, do we have any evidence that we need a larger offset
for the other code models?

But more importantly, we can't say definitively that code quality isn't
affected, only that it wasn't affected for the cases we've looked at.
People could patch the compiler if the new ranges turn out not to strike
the right balance for their use cases, but not everyone wants to do that.

Maybe we need a new command-line option.  It seems like the cheap
way out, but it might be justified.  The problem is that, without one,
the compiler would be unconditionally applying an offset range that has
no obvious relation (from a user's POV) to the documented -mcmodel range.

We can then set the option's default value to whatever we think is
reasonable.  And FWIW I've no specific issue with the values you've
chosen except for the question above.

Thanks,
Richard

Re: [PATCH][AArch64] Fix symbol offset limit

[Richard Sandiford]

Richard Sandiford <richard.sandiford@arm.com> writes:
> Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
>> We can do all kinds of arithmetic based on pointers, either using
>> pointer types or converted to uintptr_t. Note that the optimizer
>> actually creates these expressions, for example arr[N-x] can be
>> evaluated as (&arr[0] + N) - x.  So this remains legal even if N is
>> well outside the bounds of the array.
>
> Yeah, true, and I remember valid "x[n - big_offset]" -> invalid
> "x + big_offset - n" being a problem for MIPS too.  It was one of
> the things offset_within_block_p was helping to avoid.

Doh, I of course meant "x - big_offset + n" here ;-)

Re: [PATCH][AArch64] Fix symbol offset limit

[Jeff Law]

On 10/12/19 3:56 AM, Richard Sandiford wrote:
> Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
>> Hi Richard,
>>
>>> If global_char really is a char then isn't that UB?
>>
>> No why?
> 
> Well, "simple expressions like &global_char + 0xffffff00" made it sound
> like there really was a:
> 
>    extern char global_char;
> 
> Only &global_char and &global_char + 1 are defined in that case.
> I was probably taking the example too literally though.
> 
>> We can do all kinds of arithmetic based on pointers, either using
>> pointer types or converted to uintptr_t. Note that the optimizer
>> actually creates these expressions, for example arr[N-x] can be
>> evaluated as (&arr[0] + N) - x.  So this remains legal even if N is
>> well outside the bounds of the array.
> 
> Yeah, true, and I remember valid "x[n - big_offset]" -> invalid
> "x + big_offset - n" being a problem for MIPS too.  It was one of
> the things offset_within_block_p was helping to avoid.
Multiple targets struggle with this.  PA, mn103 also come to mind
immediately.

[ ... ]

> 
> But more importantly, we can't say definitively that code quality isn't
> affected, only that it wasn't affected for the cases we've looked at.
> People could patch the compiler if the new ranges turn out not to strike
> the right balance for their use cases, but not everyone wants to do that.
Absolutely.  IIRC These kinds of addresses are regularly created for
Ada.  Or at least they were.

One of the concepts that really helped the PA from a codegen standpoint
was to distinguish between pointers which we know point into the object
from those which might point outside the object.  Given that knowledge
we could generate more efficient code for the former (particularly WRT
indexed and scaled indexed address modes) and punt the latter.
REGNO_POINTER_FLAG carries this information.

Jeff

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

Hi Richard,

>> No - the testcases fail with that.
>
> Hmm, OK.  Could you give more details?  What does the motivating case
> actually look like?

Well it's now a very long time ago since I first posted this patch but the failure
was in SPEC. It did something like &array[0xffffff000 - x], presumably after some
optimization with some specific options I was using at the time. The exact details
don't matter since I've got minimal testcases.

> One of the reasons this is a hard patch to review is that it doesn't
> include a testcase for the kind of situation it's trying to fix.

There is a very simple testcase which fails before and passes with my patch.

>> It also reduces codequality by not allowing commonly used offsets as
>> part of the symbol relocation.
>
> Which kinds of cases specifically?  Although I'm guessing the problem is
> when indexing into external arrays of unknown size.

Well there are 41000 uses in SPEC2017 that fail the offset_within_block_p
test but pass my range test. There are 3 cases where the reverse is true
(a huge offset: 17694720).

Overall my range test allows 99.99% of the offsets, so we can safely conclude
my patch doesn't regress any existing code.

> So IMO we should be able to assume that the start and end + 1 addresses
> of any referenced object are within reach.  For section anchors, we can
> extend that to the containing anchor block, which is really just a
> super-sized object.

This isn't about section anchors - in many cases the array is an extern.

> The question then is what to do about symbols whose size isn't known.
> And I agree that unconditionally applying the full code-model offset
> range is too aggressive in that case.

That's very common indeed, so we need to apply some kind of reasonable
range check.

> Well, for one thing, if code quality isn't affected by using +/-64k
> for the tiny model, do we have any evidence that we need a larger offset
> for the other code models?

Certainly for SPEC +-64KB is too small, but SPEC won't build in the tiny code
model. For the tiny code model the emphasis should be on ensuring that code
that fits should build correctly rather than trying to optimize it to the max and
getting relocations that are out of range... So I believe it is reasonable to use a 
more conservative range in the tiny model.

> But more importantly, we can't say definitively that code quality isn't
> affected, only that it wasn't affected for the cases we've looked at.
> People could patch the compiler if the new ranges turn out not to strike
> the right balance for their use cases, but not everyone wants to do that.

Well if we're worried about codequality then the offset in block approach
affects it the most. Offsets larger than 1MB are extremely rare, so the chance
that there will ever be a request for a larger range is simply zero.

> Maybe we need a new command-line option.

That's way overkill... All this analysis is overcomplicating what is really a very
basic problem with a very simple solution.

Cheers,
Wilco

Re: [PATCH][AArch64] Fix symbol offset limit

[Richard Sandiford]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Hi Richard,
>
>>> No - the testcases fail with that.
>>
>> Hmm, OK.  Could you give more details?  What does the motivating case
>> actually look like?
>
> Well it's now a very long time ago since I first posted this patch but the failure
> was in SPEC. It did something like &array[0xffffff000 - x], presumably after some
> optimization with some specific options I was using at the time. The exact details
> don't matter since I've got minimal testcases.
>
>> One of the reasons this is a hard patch to review is that it doesn't
>> include a testcase for the kind of situation it's trying to fix.
>
> There is a very simple testcase which fails before and passes with my patch.
>
>>> It also reduces codequality by not allowing commonly used offsets as
>>> part of the symbol relocation.
>>
>> Which kinds of cases specifically?  Although I'm guessing the problem is
>> when indexing into external arrays of unknown size.
>
> Well there are 41000 uses in SPEC2017 that fail the offset_within_block_p
> test but pass my range test. There are 3 cases where the reverse is true
> (a huge offset: 17694720).
>
> Overall my range test allows 99.99% of the offsets, so we can safely conclude
> my patch doesn't regress any existing code.
>
>> So IMO we should be able to assume that the start and end + 1 addresses
>> of any referenced object are within reach.  For section anchors, we can
>> extend that to the containing anchor block, which is really just a
>> super-sized object.
>
> This isn't about section anchors - in many cases the array is an extern.

Sure, the "extern array of unknown size" case isn't about section anchors.
But this part of my message (snipped above) was about the other case
(objects of known size), and applied to individual objects as well as
section anchors.

What I was trying to say is: yes, we need better offsets for references
to extern objects of unknown size.  But your patch does that by reducing
the offset range for all objects, including ones with known sizes in
which the documented ranges should be safe.

I was trying to explain why I don't think we need to reduce the range
in that case too.  If offset_within_block_p then any offset should be
OK (the aggressive interpretation) or the original documented ranges
should be OK.  I think we only need the smaller range when
offset_within_block_p returns false.

>> The question then is what to do about symbols whose size isn't known.
>> And I agree that unconditionally applying the full code-model offset
>> range is too aggressive in that case.
>
> That's very common indeed, so we need to apply some kind of reasonable
> range check.
>
>> Well, for one thing, if code quality isn't affected by using +/-64k
>> for the tiny model, do we have any evidence that we need a larger offset
>> for the other code models?
>
> Certainly for SPEC +-64KB is too small, but SPEC won't build in the tiny code
> model. For the tiny code model the emphasis should be on ensuring that code
> that fits should build correctly rather than trying to optimize it to the max and
> getting relocations that are out of range... So I believe it is reasonable to use a
> more conservative range in the tiny model.
>
>> But more importantly, we can't say definitively that code quality isn't
>> affected, only that it wasn't affected for the cases we've looked at.
>> People could patch the compiler if the new ranges turn out not to strike
>> the right balance for their use cases, but not everyone wants to do that.
>
> Well if we're worried about codequality then the offset in block approach
> affects it the most. Offsets larger than 1MB are extremely rare, so the chance
> that there will ever be a request for a larger range is simply zero.
>
>> Maybe we need a new command-line option.
>
> That's way overkill... All this analysis is overcomplicating what is really a very
> basic problem with a very simple solution.

Well, I'm not going to object if another maintainer is happy to approve
the patch as-is.

Richard

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

Hi Richard,

> Sure, the "extern array of unknown size" case isn't about section anchors.
> But this part of my message (snipped above) was about the other case
> (objects of known size), and applied to individual objects as well as
> section anchors.
>
> What I was trying to say is: yes, we need better offsets for references
> to extern objects of unknown size.  But your patch does that by reducing
> the offset range for all objects, including ones with known sizes in
> which the documented ranges should be safe.
>
> I was trying to explain why I don't think we need to reduce the range
> in that case too.  If offset_within_block_p then any offset should be
> OK (the aggressive interpretation) or the original documented ranges
> should be OK.  I think we only need the smaller range when
> offset_within_block_p returns false.

Right I see now. Yes given offset_within_block_p is not sufficient, we could test
both conditions. Here is the updated patch:

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 7ee31a66b12d7354759f06449955e933421f5fe0..31c394a7e8567dd7d4f1698e5ba98aeb8807df38 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -14079,26 +14079,30 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
 	     the offset does not cause overflow of the final address.  But
 	     we have no way of knowing the address of symbol at compile time
 	     so we can't accurately say if the distance between the PC and
-	     symbol + offset is outside the addressible range of +/-1M in the
-	     TINY code model.  So we rely on images not being greater than
-	     1M and cap the offset at 1M and anything beyond 1M will have to
-	     be loaded using an alternative mechanism.  Furthermore if the
-	     symbol is a weak reference to something that isn't known to
-	     resolve to a symbol in this module, then force to memory.  */
-	  if ((SYMBOL_REF_WEAK (x)
-	       && !aarch64_symbol_binds_local_p (x))
-	      || !IN_RANGE (offset, -1048575, 1048575))
+	     symbol + offset is outside the addressible range of +/-1MB in the
+	     TINY code model.  So we limit the maximum offset to +/-64KB and
+	     assume the offset to the symbol is not larger than +/-(1MB - 64KB).
+	     Furthermore force to memory if the symbol is a weak reference to
+	     something that doesn't resolve to a symbol in this module.  */
+
+	  if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
+	    return SYMBOL_FORCE_TO_MEM;
+	  if (!(IN_RANGE (offset, -0x10000, 0x10000)
+		|| offset_within_block_p (x, offset)))
 	    return SYMBOL_FORCE_TO_MEM;
+
 	  return SYMBOL_TINY_ABSOLUTE;
 
 	case AARCH64_CMODEL_SMALL:
 	  /* Same reasoning as the tiny code model, but the offset cap here is
-	     4G.  */
-	  if ((SYMBOL_REF_WEAK (x)
-	       && !aarch64_symbol_binds_local_p (x))
-	      || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
-			    HOST_WIDE_INT_C (4294967264)))
+	     1MB, allowing +/-3.9GB for the offset to the symbol.  */
+
+	  if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
+	    return SYMBOL_FORCE_TO_MEM;
+	  if (!(IN_RANGE (offset, -0x100000, 0x100000)
+		|| offset_within_block_p (x, offset)))
 	    return SYMBOL_FORCE_TO_MEM;
+
 	  return SYMBOL_SMALL_ABSOLUTE;
 
 	case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..fc6a4f3ec780d9fa86de1c8e1a42a55992ee8b2d 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
 
-int fixed_regs[0x00200000];
+char fixed_regs[0x00080000];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x00080000];
+  return fixed_regs[0x000ff000];
 }
 
 /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..d8e82fa1b2829fd300b6ccf7f80241e5573e7e17 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=small" } */
 
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x80000000];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x100000000ULL];
+  return fixed_regs[0xfffff000];
 }
 
 /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */

Re: [PATCH][AArch64] Fix symbol offset limit

[Richard Sandiford]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> Hi Richard,
>> Sure, the "extern array of unknown size" case isn't about section anchors.
>> But this part of my message (snipped above) was about the other case
>> (objects of known size), and applied to individual objects as well as
>> section anchors.
>>
>> What I was trying to say is: yes, we need better offsets for references
>> to extern objects of unknown size.  But your patch does that by reducing
>> the offset range for all objects, including ones with known sizes in
>> which the documented ranges should be safe.
>>
>> I was trying to explain why I don't think we need to reduce the range
>> in that case too.  If offset_within_block_p then any offset should be
>> OK (the aggressive interpretation) or the original documented ranges
>> should be OK.  I think we only need the smaller range when
>> offset_within_block_p returns false.
>
> Right I see now. Yes given offset_within_block_p is not sufficient, we could test
> both conditions. Here is the updated patch:
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 7ee31a66b12d7354759f06449955e933421f5fe0..31c394a7e8567dd7d4f1698e5ba98aeb8807df38 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -14079,26 +14079,30 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
>       the offset does not cause overflow of the final address.  But
>       we have no way of knowing the address of symbol at compile time
>       so we can't accurately say if the distance between the PC and
> -     symbol + offset is outside the addressible range of +/-1M in the
> -     TINY code model.  So we rely on images not being greater than
> -     1M and cap the offset at 1M and anything beyond 1M will have to
> -     be loaded using an alternative mechanism.  Furthermore if the
> -     symbol is a weak reference to something that isn't known to
> -     resolve to a symbol in this module, then force to memory.  */
> -  if ((SYMBOL_REF_WEAK (x)
> -       && !aarch64_symbol_binds_local_p (x))
> -      || !IN_RANGE (offset, -1048575, 1048575))
> +     symbol + offset is outside the addressible range of +/-1MB in the
> +     TINY code model.  So we limit the maximum offset to +/-64KB and
> +     assume the offset to the symbol is not larger than +/-(1MB - 64KB).
> +     Furthermore force to memory if the symbol is a weak reference to
> +     something that doesn't resolve to a symbol in this module.  */
> +
> +  if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
> +    return SYMBOL_FORCE_TO_MEM;
> +  if (!(IN_RANGE (offset, -0x10000, 0x10000)
> +|| offset_within_block_p (x, offset)))
>      return SYMBOL_FORCE_TO_MEM;
> +
>    return SYMBOL_TINY_ABSOLUTE;
>
>  case AARCH64_CMODEL_SMALL:
>    /* Same reasoning as the tiny code model, but the offset cap here is
> -     4G.  */
> -  if ((SYMBOL_REF_WEAK (x)
> -       && !aarch64_symbol_binds_local_p (x))
> -      || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
> -    HOST_WIDE_INT_C (4294967264)))
> +     1MB, allowing +/-3.9GB for the offset to the symbol.  */
> +
> +  if (SYMBOL_REF_WEAK (x) && !aarch64_symbol_binds_local_p (x))
> +    return SYMBOL_FORCE_TO_MEM;
> +  if (!(IN_RANGE (offset, -0x100000, 0x100000)
> +|| offset_within_block_p (x, offset)))
>      return SYMBOL_FORCE_TO_MEM;

Think this is just the mailer eating tabs, but: || not properly indented.

OK otherwise, thanks.

Richard

[PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself.  This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.

To avoid this, limit the offset to +/-1MB so that the symbol needs to be within a
3.9GB offset from its references.  For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.

Bootstrapped on AArch64, passes regress, OK for commit?

ChangeLog:
2018-11-09  Wilco Dijkstra  <wdijkstr@arm.com>

    gcc/
	* config/aarch64/aarch64.c (aarch64_classify_symbol):
	Apply reasonable limit to symbol offsets.

    testsuite/
	* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
	* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.

---

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b6ea6d3f14b212b69f76d21b72527b6a8ea8cb0e..be03aeea8cd9bab07a01a161c4a91283bd25c99d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11794,26 +11794,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
 	     the offset does not cause overflow of the final address.  But
 	     we have no way of knowing the address of symbol at compile time
 	     so we can't accurately say if the distance between the PC and
-	     symbol + offset is outside the addressible range of +/-1M in the
-	     TINY code model.  So we rely on images not being greater than
-	     1M and cap the offset at 1M and anything beyond 1M will have to
-	     be loaded using an alternative mechanism.  Furthermore if the
-	     symbol is a weak reference to something that isn't known to
-	     resolve to a symbol in this module, then force to memory.  */
+	     symbol + offset is outside the addressible range of +/-1MB in the
+	     TINY code model.  So we limit the maximum offset to +/-64KB and
+	     assume the offset to the symbol is not larger than +/-(1MB - 64KB).
+	     Furthermore force to memory if the symbol is a weak reference to
+	     something that doesn't resolve to a symbol in this module.  */
 	  if ((SYMBOL_REF_WEAK (x)
 	       && !aarch64_symbol_binds_local_p (x))
-	      || !IN_RANGE (offset, -1048575, 1048575))
+	      || !IN_RANGE (offset, -0x10000, 0x10000))
 	    return SYMBOL_FORCE_TO_MEM;
+
 	  return SYMBOL_TINY_ABSOLUTE;
 
 	case AARCH64_CMODEL_SMALL:
 	  /* Same reasoning as the tiny code model, but the offset cap here is
-	     4G.  */
+	     1MB, allowing +/-3.9GB for the offset to the symbol.  */
 	  if ((SYMBOL_REF_WEAK (x)
 	       && !aarch64_symbol_binds_local_p (x))
-	      || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
-			    HOST_WIDE_INT_C (4294967264)))
+	      || !IN_RANGE (offset, -0x100000, 0x100000))
 	    return SYMBOL_FORCE_TO_MEM;
+
 	  return SYMBOL_SMALL_ABSOLUTE;
 
 	case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
 
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x00080000];
+  return fixed_regs[0x000ff000];
 }
 
 /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=small" } */
 
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x100000000ULL];
+  return fixed_regs[0xfffff000ULL];
 }
 
 /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */

[PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
This means the offset can use all of the +/-4GB offset, leaving no offset available
for the symbol itself.  This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xffffff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
3GB offset from its references.  For the tiny code model use a 64KB offset, allowing
most of the 1MB range for code/data between the symbol and its references.

Bootstrap OK, updated tests now pass rather than failing with symbol out of range.

OK for commit? As this is a latent bug, OK to backport to GCC6.x?

ChangeLog:
2016-08-23  Wilco Dijkstra  <wdijkstr@arm.com>

    gcc/
	* config/aarch64/aarch64.c (aarch64_classify_symbol):
	Apply reasonable limit to symbol offsets.

    testsuite/
	* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
	* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.

--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ab609223db7c54f467183db39c4f1ae8b789bfb5..60cc61a095bc144d602597a35b51b9a426c76c69 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9367,24 +9367,22 @@ aarch64_classify_symbol (rtx x, rtx offset)
 	     we have no way of knowing the address of symbol at compile time
 	     so we can't accurately say if the distance between the PC and
 	     symbol + offset is outside the addressible range of +/-1M in the
-	     TINY code model.  So we rely on images not being greater than
-	     1M and cap the offset at 1M and anything beyond 1M will have to
-	     be loaded using an alternative mechanism.  Furthermore if the
-	     symbol is a weak reference to something that isn't known to
-	     resolve to a symbol in this module, then force to memory.  */
+	     TINY code model.  So we limit the maximum offset to +/-64KB and
+	     assume the offset to the symbol is not larger than +/-(1M - 64KB).
+	     Furthermore force to memory if the symbol is a weak reference to
+	     something that doesn't resolve to a symbol in this module.  */
 	  if ((SYMBOL_REF_WEAK (x)
 	       && !aarch64_symbol_binds_local_p (x))
-	      || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+	      || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
 	    return SYMBOL_FORCE_TO_MEM;
 	  return SYMBOL_TINY_ABSOLUTE;
 
 	case AARCH64_CMODEL_SMALL:
 	  /* Same reasoning as the tiny code model, but the offset cap here is
-	     4G.  */
+	     1G, allowing +/-3G for the offset to the symbol.  */
 	  if ((SYMBOL_REF_WEAK (x)
 	       && !aarch64_symbol_binds_local_p (x))
-	      || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
-			    HOST_WIDE_INT_C (4294967264)))
+	      || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
 	    return SYMBOL_FORCE_TO_MEM;
 	  return SYMBOL_SMALL_ABSOLUTE;
 
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
 
-int fixed_regs[0x00200000];
+char fixed_regs[0x00200000];
 
 int
-foo()
+main()
 {
-  return fixed_regs[0x00080000];
+  return fixed_regs[0x000ff000];
 }
 
 /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=small" } */
 
-int fixed_regs[0x200000000ULL];
+char fixed_regs[0x200000000ULL];
 
 int
-foo()
+main()
 {
-  return fixed_regs[0x100000000ULL];
+  return fixed_regs[0xfffff000ULL];
 }
 
 /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */

Re: [PATCH][AArch64] Fix symbol offset limit

[Richard Earnshaw (lists)]

On 23/08/16 15:10, Wilco Dijkstra wrote:
> In aarch64_classify_symbol symbols are allowed full-range offsets on relocations. 
> This means the offset can use all of the +/-4GB offset, leaving no offset available
> for the symbol itself.  This results in relocation overflow and link-time errors
> for simple expressions like &global_char + 0xffffff00.
> To avoid this, limit the offset to +/-1GB so that the symbol needs to be within a
> 3GB offset from its references.  For the tiny code model use a 64KB offset, allowing
> most of the 1MB range for code/data between the symbol and its references.
> 
> Bootstrap OK, updated tests now pass rather than failing with symbol out of range.

So isn't the real bug that we've permitted the user to create an object
that is too large for the data model?

Consider, for example:

char fixed_regs[0x200000000ULL];
char fixed_regs2[100];

int
main()
{
  return fixed_regs[0] + fixed_regs2[0];
}

$ gcc -o test -mcmodel=small test2.c
/tmp/ccadJpSk.o: In function `main':
test2.c:(.text+0x10): relocation truncated to fit:
R_AARCH64_ADR_PREL_PG_HI21 against symbol `fixed_regs2' defined in
COMMON section in /tmp/ccadJpSk.o
collect2: error: ld returned 1 exit status


Neither offset is too large, but we still generate relocation errors
when trying to reference fixed_regs2.

R.


> 
> OK for commit? As this is a latent bug, OK to backport to GCC6.x?

Not yet.  And any back-port would need a PR first.

> 
> ChangeLog:
> 2016-08-23  Wilco Dijkstra  <wdijkstr@arm.com>
> 
>     gcc/
> 	* config/aarch64/aarch64.c (aarch64_classify_symbol):
> 	Apply reasonable limit to symbol offsets.
> 
>     testsuite/
> 	* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
> 	* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
> 
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index ab609223db7c54f467183db39c4f1ae8b789bfb5..60cc61a095bc144d602597a35b51b9a426c76c69 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -9367,24 +9367,22 @@ aarch64_classify_symbol (rtx x, rtx offset)
>  	     we have no way of knowing the address of symbol at compile time
>  	     so we can't accurately say if the distance between the PC and
>  	     symbol + offset is outside the addressible range of +/-1M in the
> -	     TINY code model.  So we rely on images not being greater than
> -	     1M and cap the offset at 1M and anything beyond 1M will have to
> -	     be loaded using an alternative mechanism.  Furthermore if the
> -	     symbol is a weak reference to something that isn't known to
> -	     resolve to a symbol in this module, then force to memory.  */
> +	     TINY code model.  So we limit the maximum offset to +/-64KB and
> +	     assume the offset to the symbol is not larger than +/-(1M - 64KB).
> +	     Furthermore force to memory if the symbol is a weak reference to
> +	     something that doesn't resolve to a symbol in this module.  */
>  	  if ((SYMBOL_REF_WEAK (x)
>  	       && !aarch64_symbol_binds_local_p (x))
> -	      || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
> +	      || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
>  	    return SYMBOL_FORCE_TO_MEM;
>  	  return SYMBOL_TINY_ABSOLUTE;
>  
>  	case AARCH64_CMODEL_SMALL:
>  	  /* Same reasoning as the tiny code model, but the offset cap here is
> -	     4G.  */
> +	     1G, allowing +/-3G for the offset to the symbol.  */
>  	  if ((SYMBOL_REF_WEAK (x)
>  	       && !aarch64_symbol_binds_local_p (x))
> -	      || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
> -			    HOST_WIDE_INT_C (4294967264)))
> +	      || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
>  	    return SYMBOL_FORCE_TO_MEM;
>  	  return SYMBOL_SMALL_ABSOLUTE;
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
>  /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
>  
> -int fixed_regs[0x00200000];
> +char fixed_regs[0x00200000];
>  
>  int
> -foo()
> +main()
>  {
> -  return fixed_regs[0x00080000];
> +  return fixed_regs[0x000ff000];
>  }
>  
>  /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
>  /* { dg-options "-O3 -save-temps -mcmodel=small" } */
>  
> -int fixed_regs[0x200000000ULL];
> +char fixed_regs[0x200000000ULL];
>  
>  int
> -foo()
> +main()
>  {
> -  return fixed_regs[0x100000000ULL];
> +  return fixed_regs[0xfffff000ULL];
>  }
>  
>  /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
> 

Re: [PATCH][AArch64] Fix symbol offset limit

[Wilco Dijkstra]

Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>
> So isn't the real bug that we've permitted the user to create an object
> that is too large for the data model?

No that's a different issue I'm not trying to address here. The key is that as long
as the start of the symbol is in range, we should be able to link. Due to optimization
the offset may be huge even when the object is tiny, so the offset must be limited.

> Consider, for example:

char fixed_regs[0x200000000ULL];
char fixed_regs2[100];

int
main()
{
  return fixed_regs[0] + fixed_regs2[0];
}

> Neither offset is too large, but we still generate relocation errors
> when trying to reference fixed_regs2.

But so would creating a million objects of size 1. The linker could warn about
large objects as well as giving better error messages for relocations that are
out of range. But that's mostly QoI, what we have here is a case where legal
code fails to link due to optimization. The original example is from GCC itself,
the fixed_regs array is small but due to optimization we can end up with
&fixed_regs + 0xffffffff.

Wilco

 
> ChangeLog:
> 2016-08-23  Wilco Dijkstra  <wdijkstr@arm.com>
> 
>     gcc/
>        * config/aarch64/aarch64.c (aarch64_classify_symbol):
>        Apply reasonable limit to symbol offsets.
> 
>     testsuite/
>        * gcc.target/aarch64/symbol-range.c (foo): Set new limit.
>        * gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
> 
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index ab609223db7c54f467183db39c4f1ae8b789bfb5..60cc61a095bc144d602597a35b51b9a426c76c69 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -9367,24 +9367,22 @@ aarch64_classify_symbol (rtx x, rtx offset)
>             we have no way of knowing the address of symbol at compile time
>             so we can't accurately say if the distance between the PC and
>             symbol + offset is outside the addressible range of +/-1M in the
> -          TINY code model.  So we rely on images not being greater than
> -          1M and cap the offset at 1M and anything beyond 1M will have to
> -          be loaded using an alternative mechanism.  Furthermore if the
> -          symbol is a weak reference to something that isn't known to
> -          resolve to a symbol in this module, then force to memory.  */
> +          TINY code model.  So we limit the maximum offset to +/-64KB and
> +          assume the offset to the symbol is not larger than +/-(1M - 64KB).
> +          Furthermore force to memory if the symbol is a weak reference to
> +          something that doesn't resolve to a symbol in this module.  */
>          if ((SYMBOL_REF_WEAK (x)
>               && !aarch64_symbol_binds_local_p (x))
> -           || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
> +           || !IN_RANGE (INTVAL (offset), -0x10000, 0x10000))
>            return SYMBOL_FORCE_TO_MEM;
>          return SYMBOL_TINY_ABSOLUTE;
>  
>        case AARCH64_CMODEL_SMALL:
>          /* Same reasoning as the tiny code model, but the offset cap here is
> -          4G.  */
> +          1G, allowing +/-3G for the offset to the symbol.  */
>          if ((SYMBOL_REF_WEAK (x)
>               && !aarch64_symbol_binds_local_p (x))
> -           || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
> -                         HOST_WIDE_INT_C (4294967264)))
> +           || !IN_RANGE (INTVAL (offset), -0x40000000, 0x40000000))
>            return SYMBOL_FORCE_TO_MEM;
>          return SYMBOL_SMALL_ABSOLUTE;
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> index d7e46b059e41f2672b3a1da5506fa8944e752e01..d399a3637ed834ddc4bb429594c4ec229b5c2ea8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
>  /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
>  
> -int fixed_regs[0x00200000];
> +char fixed_regs[0x00200000];
>  
>  int
> -foo()
> +main()
>  {
> -  return fixed_regs[0x00080000];
> +  return fixed_regs[0x000ff000];
>  }
>  
>  /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> index 6574cf4310430b847e77ea56bf8f20ef312d53e4..4b4ec8dab9321026d1fae96d336565a7883c8203 100644
> --- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> +++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
> @@ -1,12 +1,12 @@
> -/* { dg-do compile } */
> +/* { dg-do link } */
>  /* { dg-options "-O3 -save-temps -mcmodel=small" } */
>  
> -int fixed_regs[0x200000000ULL];
> +char fixed_regs[0x200000000ULL];
>  
>  int
> -foo()
> +main()
>  {
> -  return fixed_regs[0x100000000ULL];
> +  return fixed_regs[0xfffff000ULL];
>  }
>  
>  /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */
>