On 06/24/2018 11:56 PM, Jan Hubicka wrote:
>> 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.
>
Well, I suppose when encountering .byte/.value/.long/.quad, gas has no
way of knowing whether it's dealing with data or instructions, even in
the text section. An even if it would know it's dealing with
instructions, it wouldn't know where an instruction begins or ends. So
to me the behaviour seems reasonable.
If we'd implemented something like this in gas:
...
.insn
.byte 0x66
.endinsn
...
we could fix this more generically.
Or maybe we'd want this, which allows us to express that the .byte is a
prefix to an existing insn:
...
.insn
.byte 0x66
leaq x@tlsgd(%rip),%rdi
.endinsn
...
> 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)
>
Committed (after moving the testcase to gcc.target/i386).
Thanks,
- Tom
> 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 <tdevr...@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" } } */
