* [PATCH][AArch64] Fix symbol offset limit
@ 2016-08-23 14:11 Wilco Dijkstra
2016-08-26 10:43 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2016-08-23 14:11 UTC (permalink / raw)
To: GCC Patches; +Cc: nd
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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2016-08-23 14:11 [PATCH][AArch64] Fix symbol offset limit Wilco Dijkstra
@ 2016-08-26 10:43 ` Richard Earnshaw (lists)
2016-08-26 19:07 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Richard Earnshaw (lists) @ 2016-08-26 10:43 UTC (permalink / raw)
To: Wilco Dijkstra, GCC Patches; +Cc: nd
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\\\+" } } */
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH][AArch64] Fix symbol offset limit
2016-08-26 10:43 ` Richard Earnshaw (lists)
@ 2016-08-26 19:07 ` Wilco Dijkstra
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2016-08-26 19:07 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
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\\\+" } } */
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-08-26 19:07 ` Wilco Dijkstra
@ 2016-09-12 15:30 ` Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
` (2 more replies)
0 siblings, 3 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2016-09-12 15:30 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
Wilco wrote:
> 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.
We could also check the bounds of each symbol if they exist, like the patch below.
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 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 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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..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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
@ 2016-09-21 14:48 ` Wilco Dijkstra
2016-10-17 12:42 ` Wilco Dijkstra
2016-12-06 15:07 ` Wilco Dijkstra
2 siblings, 0 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2016-09-21 14:48 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
Wilco wrote:
> 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.
We could also check the bounds of each symbol if they exist, like the patch below.
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 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 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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..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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
@ 2016-10-17 12:42 ` Wilco Dijkstra
2016-10-25 9:47 ` Wilco Dijkstra
2016-12-06 15:07 ` Wilco Dijkstra
2 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2016-10-17 12:42 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> 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.
We could also check the bounds of each symbol if they exist, like the patch below.
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 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 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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..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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-10-17 12:42 ` Wilco Dijkstra
@ 2016-10-25 9:47 ` Wilco Dijkstra
2016-11-02 16:48 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2016-10-25 9:47 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> 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.
We could also check the bounds of each symbol if they exist, like the patch below.
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 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 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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..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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-10-25 9:47 ` Wilco Dijkstra
@ 2016-11-02 16:48 ` Wilco Dijkstra
2016-11-14 13:07 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2016-11-02 16:48 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> 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.
We could also check the bounds of each symbol if they exist, like the patch below.
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 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 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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..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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-11-02 16:48 ` Wilco Dijkstra
@ 2016-11-14 13:07 ` Wilco Dijkstra
0 siblings, 0 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2016-11-14 13:07 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> 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.
We could also check the bounds of each symbol if they exist, like the patch below.
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 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 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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..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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2][AArch64] Fix symbol offset limit
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
2016-10-17 12:42 ` Wilco Dijkstra
@ 2016-12-06 15:07 ` Wilco Dijkstra
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
2 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2016-12-06 15:07 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 12 September 2016 15:50
To: Richard Earnshaw; GCC Patches
Cc: nd
Subject: Re: [PATCH v2][AArch64] Fix symbol offset limit
Wilco wrote:
> 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.
We could also check the bounds of each symbol if they exist, like the patch below.
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2016-09-12 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 385bd560fb12cd5d404e6ddb2f01edf1fe72d729..275a828ac9e6e9b8187380c1b602ffb1b2bcfb21 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9351,6 +9351,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9359,25 +9361,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (decl_size
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_shwi (decl_size)))
+ 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..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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2016-12-06 15:07 ` Wilco Dijkstra
@ 2017-01-17 15:14 ` Wilco Dijkstra
2017-02-02 14:44 ` Wilco Dijkstra
2017-04-20 16:03 ` Wilco Dijkstra
0 siblings, 2 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2017-01-17 15:14 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
@ 2017-02-02 14:44 ` Wilco Dijkstra
2017-02-23 16:58 ` Wilco Dijkstra
2017-04-20 16:03 ` Wilco Dijkstra
1 sibling, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-02-02 14:44 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-02-02 14:44 ` Wilco Dijkstra
@ 2017-02-23 16:58 ` Wilco Dijkstra
0 siblings, 0 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2017-02-23 16:58 UTC (permalink / raw)
To: Richard Earnshaw, GCC Patches, James Greenhalgh; +Cc: nd
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
2017-02-02 14:44 ` Wilco Dijkstra
@ 2017-04-20 16:03 ` Wilco Dijkstra
2017-06-13 14:00 ` Wilco Dijkstra
1 sibling, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-04-20 16:03 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-04-20 16:03 ` Wilco Dijkstra
@ 2017-06-13 14:00 ` Wilco Dijkstra
2017-06-14 14:07 ` James Greenhalgh
2017-06-27 15:36 ` Wilco Dijkstra
0 siblings, 2 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2017-06-13 14:00 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-13 14:00 ` Wilco Dijkstra
@ 2017-06-14 14:07 ` James Greenhalgh
2017-06-14 16:03 ` Wilco Dijkstra
2017-06-15 15:13 ` Richard Earnshaw (lists)
2017-06-27 15:36 ` Wilco Dijkstra
1 sibling, 2 replies; 32+ messages in thread
From: James Greenhalgh @ 2017-06-14 14:07 UTC (permalink / raw)
To: Wilco Dijkstra; +Cc: GCC Patches, nd, Richard Earnshaw, marcus.shawcroft
On Tue, Jun 13, 2017 at 03:00:28PM +0100, Wilco Dijkstra wrote:
>
> ping
I've been avoiding reviewing this patch as Richard was the last to comment
on it, and I wasn't sure that his comments had been resolved to his
satisfaction. The conversation was back in August 2016 on v1 of the patch:
> 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.
Richard, do you have anything further to say on this patch? Or can we start
progressing the review again.
Thanks,
James
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-14 14:07 ` James Greenhalgh
@ 2017-06-14 16:03 ` Wilco Dijkstra
2017-06-15 15:13 ` Richard Earnshaw (lists)
1 sibling, 0 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2017-06-14 16:03 UTC (permalink / raw)
To: James Greenhalgh; +Cc: GCC Patches, nd, Richard Earnshaw, Marcus Shawcroft
Hi,
Let's get back to the patch and the bug it fixes. The only outstanding question is what
constant offsets we should allow when generating a relocation:
> So the question is whether we should allow
> largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
> small offsets (small negative and positive offsets just outside a symbol are common).
> The only thing we can't allow is any offset like we currently do...
Wilco
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-14 14:07 ` James Greenhalgh
2017-06-14 16:03 ` Wilco Dijkstra
@ 2017-06-15 15:13 ` Richard Earnshaw (lists)
2017-06-15 16:55 ` Wilco Dijkstra
1 sibling, 1 reply; 32+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 15:13 UTC (permalink / raw)
To: James Greenhalgh, Wilco Dijkstra; +Cc: GCC Patches, nd, marcus.shawcroft
On 14/06/17 15:07, James Greenhalgh wrote:
> On Tue, Jun 13, 2017 at 03:00:28PM +0100, Wilco Dijkstra wrote:
>>
>> ping
>
> I've been avoiding reviewing this patch as Richard was the last to comment
> on it, and I wasn't sure that his comments had been resolved to his
> satisfaction. The conversation was back in August 2016 on v1 of the patch:
>
>> 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.
>
> Richard, do you have anything further to say on this patch? Or can we start
> progressing the review again.
>
> Thanks,
> James
>
Yes, I still believe that this is a bug in the way we've documented the
-mcmodel=tiny and -mcmodel=small options.
-mcmode=tiny should read:
@item -mcmodel=tiny
@opindex mcmodel=tiny
Generate code for the tiny code model. The program and its static data
must fit within 1MB. Programs can be statically or dynamically linked.
The limit is not enforced by the compiler, but if you exceed the limit
you may get errors during linking saying that relocations have been
truncated.
It's the same basic text for -mcmodel=small, except that the limit is 4GB.
R.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 15:13 ` Richard Earnshaw (lists)
@ 2017-06-15 16:55 ` Wilco Dijkstra
2017-06-15 17:39 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 16:55 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
> Yes, I still believe that this is a bug in the way we've documented the
> -mcmodel=tiny and -mcmodel=small options.
In what way could this possibly be a documentation bug? It's not at all related
to the size of a binary. There is no limit to the offset you can apply to a symbol,
I can write int a; int *p = &a + 0x80000000; and GCC is happy to create a
relocation with that offset.
Wilco
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 16:55 ` Wilco Dijkstra
@ 2017-06-15 17:39 ` Richard Earnshaw (lists)
2017-06-15 17:51 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 17:39 UTC (permalink / raw)
To: Wilco Dijkstra, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
On 15/06/17 17:55, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>> Yes, I still believe that this is a bug in the way we've documented the
>> -mcmodel=tiny and -mcmodel=small options.
>
> In what way could this possibly be a documentation bug? It's not at all
> related
> to the size of a binary. There is no limit to the offset you can apply
> to a symbol,
> I can write int a; int *p = &a + 0x80000000; and GCC is happy to create a
> relocation with that offset.
>
> Wilco
You can write it, but it's meaningless by the C standard. You can't
take the address beyond one after the size of the object, so anything
more than &a+1 has no meaning.
R.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 17:39 ` Richard Earnshaw (lists)
@ 2017-06-15 17:51 ` Wilco Dijkstra
2017-06-15 18:11 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 17:51 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
>
> You can write it, but it's meaningless by the C standard. You can't
> take the address beyond one after the size of the object, so anything
> more than &a+1 has no meaning.
No it's perfectly valid and such out-of-range cases occur thousands of
times when building any non-trivial code. For example a[i + C] transforms
into (&a + C)[i].
Wilco
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 17:51 ` Wilco Dijkstra
@ 2017-06-15 18:11 ` Richard Earnshaw (lists)
2017-06-15 18:18 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 18:11 UTC (permalink / raw)
To: Wilco Dijkstra, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
On 15/06/17 18:51, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>>
>> You can write it, but it's meaningless by the C standard. You can't
>> take the address beyond one after the size of the object, so anything
>> more than &a+1 has no meaning.
>
> No it's perfectly valid and such out-of-range cases occur thousands of
> times when building any non-trivial code. For example a[i + C] transforms
> into (&a + C)[i].
>
> Wilco
>
>
C11: Summary of undefined behaviours.
— Addition or subtraction of a pointer into, or just beyond, an array
object and an
integer type produces a result that does not point into, or just beyond,
the same array
object (6.5.6).
R.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:11 ` Richard Earnshaw (lists)
@ 2017-06-15 18:18 ` Wilco Dijkstra
2017-06-15 18:34 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 18:18 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
C11: Summary of undefined behaviours.
— Addition or subtraction of a pointer into, or just beyond, an array
object and an
integer type produces a result that does not point into, or just beyond,
the same array
object (6.5.6).
That's totally irrelevant given the addition is created by the optimizer.
Wilco
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:18 ` Wilco Dijkstra
@ 2017-06-15 18:34 ` Richard Earnshaw (lists)
2017-06-15 18:55 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Richard Earnshaw (lists) @ 2017-06-15 18:34 UTC (permalink / raw)
To: Wilco Dijkstra, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
On 15/06/17 19:18, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
>
> C11: Summary of undefined behaviours.
>
> — Addition or subtraction of a pointer into, or just beyond, an array
> object and an
> integer type produces a result that does not point into, or just beyond,
> the same array
> object (6.5.6).
>
> That's totally irrelevant given the addition is created by the optimizer.
>
> Wilco
>
No it's not. The optimizer doesn't create totally random bases. If the
code + data is less than 1M in size, then any offsets it does create
will fit within the size of the relocations selected by the compiler.
R.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:34 ` Richard Earnshaw (lists)
@ 2017-06-15 18:55 ` Wilco Dijkstra
2017-06-15 19:52 ` Joseph Myers
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-06-15 18:55 UTC (permalink / raw)
To: Richard Earnshaw, James Greenhalgh; +Cc: GCC Patches, nd, Marcus Shawcroft
Richard Earnshaw wrote:
> No it's not. The optimizer doesn't create totally random bases. If the
> code + data is less than 1M in size, then any offsets it does create
> will fit within the size of the relocations selected by the compiler.
No that's completely false. There is no way you can guarantee that without
my patch. My patch is precisely there to ensure we only allow offsets that
guarantee linking succeeds if all code and data fits in 1M or 4GB.
Even then there are many cases where we don't know the size of an object and
so have to constrain the offset to something conservative (which cannot be
the full range it allows today).
Wilco
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 18:55 ` Wilco Dijkstra
@ 2017-06-15 19:52 ` Joseph Myers
2017-06-16 15:14 ` Nathan Sidwell
0 siblings, 1 reply; 32+ messages in thread
From: Joseph Myers @ 2017-06-15 19:52 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: Richard Earnshaw, James Greenhalgh, GCC Patches, nd, Marcus Shawcroft
[-- Attachment #1: Type: text/plain, Size: 936 bytes --]
On Thu, 15 Jun 2017, Wilco Dijkstra wrote:
> Richard Earnshaw wrote:
> > No it's not. The optimizer doesn't create totally random bases. If the
> > code + data is less than 1M in size, then any offsets it does create
> > will fit within the size of the relocations selected by the compiler.
>
> No that's completely false. There is no way you can guarantee that without
> my patch. My patch is precisely there to ensure we only allow offsets that
> guarantee linking succeeds if all code and data fits in 1M or 4GB.
For example, given (array + (i - INT_MAX)), it's quite possible the
compiler could create a relocation for array - INT_MAX, and the original
expression is perfectly OK if i == INT_MAX even though array - INT_MAX is
far out of range. (And array - INT_MAX as a C expression is only
undefined at runtime, not at compile time if it's in code that is never
executed.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-15 19:52 ` Joseph Myers
@ 2017-06-16 15:14 ` Nathan Sidwell
0 siblings, 0 replies; 32+ messages in thread
From: Nathan Sidwell @ 2017-06-16 15:14 UTC (permalink / raw)
To: Joseph Myers, Wilco Dijkstra
Cc: Richard Earnshaw, James Greenhalgh, GCC Patches, nd, Marcus Shawcroft
On 06/15/2017 03:52 PM, Joseph Myers wrote:
> For example, given (array + (i - INT_MAX)), it's quite possible the
> compiler could create a relocation for array - INT_MAX, and the original
> expression is perfectly OK if i == INT_MAX even though array - INT_MAX is
> far out of range. (And array - INT_MAX as a C expression is only
> undefined at runtime, not at compile time if it's in code that is never
> executed.)
Some targets (typically uclinux-like things) cannot support this, as
they move the data segment relative to the text segment upon loading,
and need to know whether an address is text-relative or data-relative
(and symbol information may not be available).
There's a target hook for that, but I can't find it now.
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-13 14:00 ` Wilco Dijkstra
2017-06-14 14:07 ` James Greenhalgh
@ 2017-06-27 15:36 ` Wilco Dijkstra
2017-07-14 14:28 ` Wilco Dijkstra
1 sibling, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-06-27 15:36 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-06-27 15:36 ` Wilco Dijkstra
@ 2017-07-14 14:28 ` Wilco Dijkstra
2017-07-21 11:23 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-07-14 14:28 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-07-14 14:28 ` Wilco Dijkstra
@ 2017-07-21 11:23 ` Wilco Dijkstra
2017-08-01 10:19 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-07-21 11:23 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-07-21 11:23 ` Wilco Dijkstra
@ 2017-08-01 10:19 ` Wilco Dijkstra
2017-08-15 17:36 ` Wilco Dijkstra
0 siblings, 1 reply; 32+ messages in thread
From: Wilco Dijkstra @ 2017-08-01 10:19 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v3][AArch64] Fix symbol offset limit
2017-08-01 10:19 ` Wilco Dijkstra
@ 2017-08-15 17:36 ` Wilco Dijkstra
0 siblings, 0 replies; 32+ messages in thread
From: Wilco Dijkstra @ 2017-08-15 17:36 UTC (permalink / raw)
To: GCC Patches, James Greenhalgh; +Cc: nd, Richard Earnshaw
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this version), or
small offsets (small negative and positive offsets just outside a symbol are common).
The only thing we can't allow is any offset like we currently do...
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.
For symbols with a defined size, limit the offset to be within the size of the symbol.
ChangeLog:
2017-01-17 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 e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ 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\\\+" } } */
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2017-08-15 16:27 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-23 14:11 [PATCH][AArch64] Fix symbol offset limit Wilco Dijkstra
2016-08-26 10:43 ` Richard Earnshaw (lists)
2016-08-26 19:07 ` Wilco Dijkstra
2016-09-12 15:30 ` [PATCH v2][AArch64] " Wilco Dijkstra
2016-09-21 14:48 ` Wilco Dijkstra
2016-10-17 12:42 ` Wilco Dijkstra
2016-10-25 9:47 ` Wilco Dijkstra
2016-11-02 16:48 ` Wilco Dijkstra
2016-11-14 13:07 ` Wilco Dijkstra
2016-12-06 15:07 ` Wilco Dijkstra
2017-01-17 15:14 ` [PATCH v3][AArch64] " Wilco Dijkstra
2017-02-02 14:44 ` Wilco Dijkstra
2017-02-23 16:58 ` Wilco Dijkstra
2017-04-20 16:03 ` Wilco Dijkstra
2017-06-13 14:00 ` Wilco Dijkstra
2017-06-14 14:07 ` James Greenhalgh
2017-06-14 16:03 ` Wilco Dijkstra
2017-06-15 15:13 ` Richard Earnshaw (lists)
2017-06-15 16:55 ` Wilco Dijkstra
2017-06-15 17:39 ` Richard Earnshaw (lists)
2017-06-15 17:51 ` Wilco Dijkstra
2017-06-15 18:11 ` Richard Earnshaw (lists)
2017-06-15 18:18 ` Wilco Dijkstra
2017-06-15 18:34 ` Richard Earnshaw (lists)
2017-06-15 18:55 ` Wilco Dijkstra
2017-06-15 19:52 ` Joseph Myers
2017-06-16 15:14 ` Nathan Sidwell
2017-06-27 15:36 ` Wilco Dijkstra
2017-07-14 14:28 ` Wilco Dijkstra
2017-07-21 11:23 ` Wilco Dijkstra
2017-08-01 10:19 ` Wilco Dijkstra
2017-08-15 17:36 ` Wilco Dijkstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).