Re: [PATCH, PR86257, i386/debug] Fix insn prefix in tls_global_dynamic_64_<mode>

[Jan Hubicka <hubicka at ucw dot cz>]

> Hi,
> 
> [ The analysis of this PR was done at https://stackoverflow.com/a/33557963 ,
> November 2015. ]
> 
> This tls sequence:
> ...
> 0x00 .byte 0x66
> 0x01 leaq  x@tlsgd(%rip),%rdi
> 0x08 .word 0x6666
> 0x0a rex64
> 0x0b call __tls_get_addr@plt
> ...
> starts with an insn prefix, produced using a .byte.
> 
> When using a .loc before the sequence, it's associated with the next assembly
> instruction, which is the leaq, so the .loc will end up pointing inside the
> sequence rather than to the start of the sequence.  And when the linker
> relaxes the sequence, the .loc may end up pointing inside an insn.  This will
> cause an executable containing such a misplaced .loc to crash when gdb
> continues from the associated breakpoint.

Hmm, I am not sure why .byte should be non-instruction and .data should be instruction,
but I assume gas simply behaves this way.

Don't we have also other cases wehre .byte is used to output instructions?

Patch is OK (and probably should be backported after some soaking in mainline)

Honza
> 
> This patch fixes the problem by using data16 to generate the prefix.
> 
> Bootstrapped and reg-tested on x86_64.
> 
> OK for trunk?
> 
> Thanks,
> - Tom
> 
> [i386/debug] Fix insn prefix in tls_global_dynamic_64_<mode>
> 
> 2018-06-22  Tom de Vries  <tdevries@suse.de>
> 
> 	PR debug/86257
> 	* config/i386/i386.md (define_insn "*tls_global_dynamic_64_<mode>"):
> 	Use data16 instead of .byte for insn prefix.
> 
> 	* gcc.dg/pr86257.c: New test.
> 
> ---
>  gcc/config/i386/i386.md        | 13 ++++++++++++-
>  gcc/testsuite/gcc.dg/pr86257.c | 14 ++++++++++++++
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> index eb77ef3c08f..6f2300057aa 100644
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -14733,7 +14733,18 @@
>    "TARGET_64BIT"
>  {
>    if (!TARGET_X32)
> -    fputs (ASM_BYTE "0x66\n", asm_out_file);
> +    /* The .loc directive has effect for 'the immediately following assembly
> +       instruction'.  So for a sequence:
> +         .loc f l
> +         .byte x
> +         insn1
> +       the 'immediately following assembly instruction' is insn1.
> +       We want to emit an insn prefix here, but if we use .byte (as shown in
> +       'ELF Handling For Thread-Local Storage'), a preceding .loc will point
> +       inside the insn sequence, rather than to the start.  After relaxation
> +       of the sequence by the linker, the .loc might point inside an insn.
> +       Use data16 prefix instead, which doesn't have this problem.  */
> +    fputs ("\tdata16", asm_out_file);
>    output_asm_insn
>      ("lea{q}\t{%E1@tlsgd(%%rip), %%rdi|rdi, %E1@tlsgd[rip]}", operands);
>    if (TARGET_SUN_TLS || flag_plt || !HAVE_AS_IX86_TLS_GET_ADDR_GOT)
> diff --git a/gcc/testsuite/gcc.dg/pr86257.c b/gcc/testsuite/gcc.dg/pr86257.c
> new file mode 100644
> index 00000000000..3287c190d36
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr86257.c
> @@ -0,0 +1,14 @@
> +/* { dg-require-effective-target fpic } */
> +/* { dg-require-effective-target tls } */
> +/* { dg-options "-g -fPIC" } */
> +
> +__thread int i;
> +
> +void
> +foo(void)
> +{
> +  i = 0;
> +}
> +
> +/* { dg-final { scan-assembler "data16\[ \t\]*leaq" } } */
> +/* { dg-final { scan-assembler-not "\.byte\[ \t\]*0x66\n\[ \t\]*leaq" } } */