This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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\\\+" } } */
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]