Richard Earnshaw (lists) <richard.earns...@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 <wdijk...@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\\\+" } } */
>
