[patch bfd]: Fix generation of .tls directory entry

[patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 455 bytes --]

Hello,

by recent changes I noticed that the TLS directory entry for PE-COFF
targets isn't set anymore. This patch fixes this issue

2010-12-14  Kai Tietz

        * peXXigen.c (_bfd_XXi_swap_aouthdr_out): Handle TLS directory
        entry.

Tested for x86_64-w64-mingw32, and i686-pc-cygwin. Ok for apply?

Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[-- Attachment #2: bfd_tls.txt --]
[-- Type: text/plain, Size: 814 bytes --]

Index: peXXigen.c
===================================================================
RCS file: /cvs/src/src/bfd/peXXigen.c,v
retrieving revision 1.67
diff -u -3 -r1.67 peXXigen.c
--- peXXigen.c	22 Sep 2010 08:01:56 -0000	1.67
+++ peXXigen.c	14 Dec 2010 16:17:39 -0000
@@ -616,6 +616,11 @@
        .idata is needed for backwards compatibility.  FIXME.  */
     add_data_entry (abfd, extra, 1, ".idata", ib);
 
+  if (extra->DataDirectory[PE_TLS_TABLE].VirtualAddress == 0)
+    /* Until other .tls fixes are made, the entry for .tls
+       is needed.  */
+    add_data_entry (abfd, extra, 9, ".tls", ib);
+
   /* For some reason, the virtual size (which is what's set by
      add_data_entry) for .reloc is not the same as the size recorded
      in this slot by MSVC; it doesn't seem to cause problems (so far),

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

PING

2010/12/14 Kai Tietz <ktietz70@googlemail.com>:
> Hello,
>
> by recent changes I noticed that the TLS directory entry for PE-COFF
> targets isn't set anymore. This patch fixes this issue
>
> 2010-12-14  Kai Tietz
>
>        * peXXigen.c (_bfd_XXi_swap_aouthdr_out): Handle TLS directory
>        entry.
>
> Tested for x86_64-w64-mingw32, and i686-pc-cygwin. Ok for apply?
>
> Kai
>
> --
> |  (\_/) This is Bunny. Copy and paste
> | (='.'=) Bunny into your signature to help
> | (")_(") him gain world domination
>



-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[RFA] tls directory entry size for windows 64-bit

[Pierre Muller]

  The DataDirectory entry for TLS table is supposed to give
the size of the IMAGE_TLS_DIRECTORY structure for
the current executable.
  This size is different for 32-bit and 64-bit windows executables.

  I tried to find out the correct conditionals to
to insure the system uses 64-bit addresses,
the same condition is used several times in the same
source code already.

  Is this patch OK?


Pierre Muller


2010-12-17  Pierre Muller  <muller@ics.u-strasbg.fr>

        * peXXigen.c (_bfd_XXi_final_link_postscript): Use correct size
        for windows 64-bit TLS table size.

Index: peXXigen.c
===================================================================
RCS file: /cvs/src/src/bfd/peXXigen.c,v
retrieving revision 1.67
diff -u -p -r1.67 peXXigen.c
--- peXXigen.c  22 Sep 2010 08:01:56 -0000      1.67
+++ peXXigen.c  17 Dec 2010 18:24:47 -0000
@@ -2437,8 +2437,15 @@ _bfd_XXi_final_link_postscript (bfd * ab
             abfd);
          result = FALSE;
        }
-
+     /* According to PECOFF specifications by Microsoft version 8.2
+       the TLS data directory consists of 4 pointers, followed
+       by two 4-byte integer. This implies that the total size
+       is different for 32-bit and 64-bit executables.  */
+#if !defined(COFF_WITH_pep) && !defined(COFF_WITH_pex64)
       pe_data (abfd)->pe_opthdr.DataDirectory[PE_TLS_TABLE].Size = 0x18;
+#else
+      pe_data (abfd)->pe_opthdr.DataDirectory[PE_TLS_TABLE].Size = 0x28;
+#endif
     }

 /* If there is a .pdata section and we have linked pdata finally, we
~

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

PING? What's the issue here?

binutils-owner@sourceware.org wrote on 17.12.2010 19:05:21:

> PING
> 
> 2010/12/14 Kai Tietz <ktietz70@googlemail.com>:
> > Hello,
> >
> > by recent changes I noticed that the TLS directory entry for PE-COFF
> > targets isn't set anymore. This patch fixes this issue
> >
> > 2010-12-14  Kai Tietz
> >
> >        * peXXigen.c (_bfd_XXi_swap_aouthdr_out): Handle TLS directory
> >        entry.
> >
> > Tested for x86_64-w64-mingw32, and i686-pc-cygwin. Ok for apply?
> >
> > Kai
> >
> > --
> > |  (\_/) This is Bunny. Copy and paste
> > | (='.'=) Bunny into your signature to help
> > | (")_(") him gain world domination
> >
> 
> 
> 
> -- 
> |  (\_/) This is Bunny. Copy and paste
> | (='.'=) Bunny into your signature to help
> | (")_(") him gain world domination
> 


Regards,
Kai

Re: [RFA] tls directory entry size for windows 64-bit

[Dave Korn]

On 17/12/2010 20:56, Pierre Muller wrote:
>   The DataDirectory entry for TLS table is supposed to give
> the size of the IMAGE_TLS_DIRECTORY structure for
> the current executable.
>   This size is different for 32-bit and 64-bit windows executables.
> 
>   I tried to find out the correct conditionals to
> to insure the system uses 64-bit addresses,
> the same condition is used several times in the same
> source code already.
> 
>   Is this patch OK?

  Yes, thank you.

    cheers,
      DaveK

Re: [patch bfd]: Fix generation of .tls directory entry

[Dave Korn]

On 20/12/2010 15:25, Kai Tietz wrote:
> PING? What's the issue here?

  Well there is all this "christmas" stuff going on at the moment ....

>>> by recent changes I noticed that the TLS directory entry for PE-COFF
>>> targets isn't set anymore. This patch fixes this issue
>>>
>>> 2010-12-14  Kai Tietz
>>>
>>>        * peXXigen.c (_bfd_XXi_swap_aouthdr_out): Handle TLS directory
>>>        entry.
>>>
>>> Tested for x86_64-w64-mingw32, and i686-pc-cygwin. Ok for apply?

  I'm not so sure about this one.  What exactly has caused the TLS directory
entry not to be set any more, and what are the "other .tls fixes" you refer to?

  If I understand the patch right, it's going to set the TLS directory entry
to point to any ".tls" section output during the link, regardless of whether
it actually contains the "__tls_used" symbol (and related structure) or not,
isn't it?  Unless it is always the case that .tls sections always contain this
structure, it is always at offset zero, and it is always the full size of the
section, that seems potentially incorrect.

    cheers,
      DaveK

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

2010/12/20 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 20/12/2010 15:25, Kai Tietz wrote:
>> PING? What's the issue here?
>
>  Well there is all this "christmas" stuff going on at the moment ....

Well, was just curious to see not at least a line "Sorry, will come to
it with next <x>-days". I know that you are busy about some gcc
patches you are working on.

>>>> by recent changes I noticed that the TLS directory entry for PE-COFF
>>>> targets isn't set anymore. This patch fixes this issue
>>>>
>>>> 2010-12-14  Kai Tietz
>>>>
>>>>        * peXXigen.c (_bfd_XXi_swap_aouthdr_out): Handle TLS directory
>>>>        entry.
>>>>
>>>> Tested for x86_64-w64-mingw32, and i686-pc-cygwin. Ok for apply?
>
>  I'm not so sure about this one.  What exactly has caused the TLS directory
> entry not to be set any more, and what are the "other .tls fixes" you refer to?

Well, the issue here is that, if a .tls section is present, it needs
to be put into directory entry. Why this isn't happening anymore, I am
a bit puzzled too.

The more robust thing to do here (as it can be a matter of size on
wince AFAIR) is to map the TLS directory entry in the same way as IAT
directory entry is done and the size of the TLS directory entry should
have the size of 24 bytes for PE and for PE+ 40 bytes. Nevertheless it
is better for now to have a TLS directory entry generated, if a .tls
section is present.

>  If I understand the patch right, it's going to set the TLS directory entry
> to point to any ".tls" section output during the link, regardless of whether
> it actually contains the "__tls_used" symbol (and related structure) or not,
> isn't it?  Unless it is always the case that .tls sections always contain this
> structure, it is always at offset zero, and it is always the full size of the
> section, that seems potentially incorrect.
>
>    cheers,
>      DaveK
>

Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [patch bfd]: Fix generation of .tls directory entry

[Dave Korn]

On 20/12/2010 19:46, Kai Tietz wrote:

> Well, the issue here is that, if a .tls section is present, it needs
> to be put into directory entry. 

  As I read the spec, the directory entry should point only and exactly to the
actual MS-defined TLS struct pointed to by the __tls_used symbol.  That was
why I asked about whether it's always the case that we can assume a .tls
section is exactly that and nothing else.

> Why this isn't happening anymore, I am a bit puzzled too.

  I think we should investigate this first.  Got a testcase?

    cheers,
      DaveK

RE: [RFA] tls directory entry size for windows 64-bit

[Pierre Muller]

Thanks for the approval,

patch just committed.


Pierre Muller
GDB pascal language maintainer




> -----Message d'origine-----
> De : binutils-owner@sourceware.org [mailto:binutils-
> owner@sourceware.org] De la part de Dave Korn
> Envoyé : lundi 20 décembre 2010 20:45
> À : Pierre Muller
> Cc : 'Kai Tietz'; 'Binutils'
> Objet : Re: [RFA] tls directory entry size for windows 64-bit
> 
> On 17/12/2010 20:56, Pierre Muller wrote:
> >   The DataDirectory entry for TLS table is supposed to give
> > the size of the IMAGE_TLS_DIRECTORY structure for
> > the current executable.
> >   This size is different for 32-bit and 64-bit windows executables.
> >
> >   I tried to find out the correct conditionals to
> > to insure the system uses 64-bit addresses,
> > the same condition is used several times in the same
> > source code already.
> >
> >   Is this patch OK?
> 
>   Yes, thank you.
> 
>     cheers,
>       DaveK

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

2010/12/20 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 20/12/2010 19:46, Kai Tietz wrote:
>
>> Well, the issue here is that, if a .tls section is present, it needs
>> to be put into directory entry.
>
>  As I read the spec, the directory entry should point only and exactly to the
> actual MS-defined TLS struct pointed to by the __tls_used symbol.  That was
> why I asked about whether it's always the case that we can assume a .tls
> section is exactly that and nothing else.
>
>> Why this isn't happening anymore, I am a bit puzzled too.
>
>  I think we should investigate this first.  Got a testcase?
>
>    cheers,
>      DaveK
>

I found the issue. The cause for this is that bfd searches for
'__tls_used', which is the underscored version of '_tls_used'
variable. Caused by the fact that x64 windows doesn't prefix symbols
by underscore, the symbol wasn't found, as it remains '_tls_used'.

So we have two chances here to solve this: a) Change for windows x64
the symbol to '__tls_used', or b) search in bfd for x64 windows for
'_tls_used' instead of '_tls_used'.

Kai
-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

2010/12/21 Kai Tietz <ktietz70@googlemail.com>:
> 2010/12/20 Dave Korn <dave.korn.cygwin@gmail.com>:
>> On 20/12/2010 19:46, Kai Tietz wrote:
>>
>>> Well, the issue here is that, if a .tls section is present, it needs
>>> to be put into directory entry.
>>
>>  As I read the spec, the directory entry should point only and exactly to the
>> actual MS-defined TLS struct pointed to by the __tls_used symbol.  That was
>> why I asked about whether it's always the case that we can assume a .tls
>> section is exactly that and nothing else.
>>
>>> Why this isn't happening anymore, I am a bit puzzled too.
>>
>>  I think we should investigate this first.  Got a testcase?
>>
>>    cheers,
>>      DaveK
>>
>
> I found the issue. The cause for this is that bfd searches for
> '__tls_used', which is the underscored version of '_tls_used'
> variable. Caused by the fact that x64 windows doesn't prefix symbols
> by underscore, the symbol wasn't found, as it remains '_tls_used'.
>
> So we have two chances here to solve this: a) Change for windows x64
> the symbol to '__tls_used', or b) search in bfd for x64 windows for
> '_tls_used' instead of '_tls_used'.
>
> Kai
> --
> |  (\_/) This is Bunny. Copy and paste
> | (='.'=) Bunny into your signature to help
> | (")_(") him gain world domination
>

This patch should fix this underscoring issue (should affect wince arm too).

ChangeLog

2010-12-21  Kai Tietz

        * peXXigen.c (_bfd_XXi_final_link_postscript): Use
        TARGET_UNDERSCORE to determine "_tls_used" name.
        (TARGET_UNDERSCORE): Define to default zero, if not present.

        * ld-pe/pe.exp: Add TLS directory test.
        * ld-pe/tlssec.s: New.
        * ld-pe/tlssec64.d: New.
        * ld-pe/tlssec32.d: New.

Tested for x86_64-w64-mingw32, i686-pc-mingw32, and i686-pc-cygwin. Ok
for apply?

Kai
-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[-- Attachment #2: fixtls_noprefix.txt --]
[-- Type: text/plain, Size: 3053 bytes --]

Index: src/bfd/peXXigen.c
===================================================================
--- src.orig/bfd/peXXigen.c	2010-12-21 10:08:50.000000000 +0100
+++ src/bfd/peXXigen.c	2010-12-21 10:51:59.155103300 +0100
@@ -2417,8 +2417,14 @@ _bfd_XXi_final_link_postscript (bfd * ab
         }
     }
 
+#ifndef TARGET_UNDERSCORE
+#define TARGET_UNDERSCORE 0
+#endif
+
   h1 = coff_link_hash_lookup (coff_hash_table (info),
-			      "__tls_used", FALSE, FALSE, TRUE);
+			      (TARGET_UNDERSCORE != 0 ? "__tls_used"
+						      : "_tls_used"),
+			      FALSE, FALSE, TRUE);
   if (h1 != NULL)
     {
       if ((h1->root.type == bfd_link_hash_defined
Index: src/ld/testsuite/ld-pe/pe.exp
===================================================================
--- src.orig/ld/testsuite/ld-pe/pe.exp	2010-04-07 09:31:26.000000000 +0200
+++ src/ld/testsuite/ld-pe/pe.exp	2010-12-21 11:37:15.851325100 +0100
@@ -38,6 +38,8 @@ if {[istarget i*86-*-cygwin*]
 	 {{objdump -s secrel_64.d}} "secrel.x"}
 	{"Empty export table" "" "" "exports.s"
 	 {{objdump -p exports64.d}} "exports.dll"}
+	{"TLS directory entry" "" "" "tlssec.s"
+	 {{objdump -p tlssec64.d}} "tlssec.dll"}
       }
     } elseif {[istarget i*86-*-cygwin*] } {
       set pe_tests {
@@ -45,6 +47,8 @@ if {[istarget i*86-*-cygwin*]
 	 {{objdump -s secrel.d}} "secrel.x"}
 	{"Empty export table" "" "" "exports.s"
 	 {{objdump -p exports.d}} "exports.dll"}
+	{"TLS directory entry" "" "" "tlssec.s"
+	 {{objdump -p tlssec32.d}} "tlssec.dll"}
       }
     } else {
       set pe_tests {
@@ -52,6 +56,8 @@ if {[istarget i*86-*-cygwin*]
 	 {{objdump -s secrel.d}} "secrel.x"}
 	{"Empty export table" "" "" "exports.s"
 	 {{objdump -p exports.d}} "exports.dll"}
+	{"TLS directory entry" "" "" "tlssec.s"
+	 {{objdump -p tlssec32.d}} "tlssec.dll"}
       }
     }
 
Index: src/ld/testsuite/ld-pe/tlssec.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/ld/testsuite/ld-pe/tlssec.s	2010-12-21 11:29:09.770089200 +0100
@@ -0,0 +1,20 @@
+.global _tls_used
+.global __tls_used
+.global _start
+.global start
+.global _mainCRTStartup
+.global mainCRTStartup
+
+.text
+_start:
+mainCRTStartup:
+_mainCRTStartup:
+        .byte 1
+
+.section .tls
+_tls_used:
+__tls_used:
+.long 1,2,3,4,5,6,7,8,9,10
+.long 11,12,13,14,15,16,17,18,19,20
+.long 21,22,23,24,25,26,27,28,29,30
+
Index: src/ld/testsuite/ld-pe/tlssec32.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/ld/testsuite/ld-pe/tlssec32.d	2010-12-21 11:53:41.935250500 +0100
@@ -0,0 +1,3 @@
+#...
+Entry 9 00003000 00000018 Thread Storage Directory \[\.tls\]
+#...
Index: src/ld/testsuite/ld-pe/tlssec64.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/ld/testsuite/ld-pe/tlssec64.d	2010-12-21 11:34:21.131456900 +0100
@@ -0,0 +1,3 @@
+#...
+Entry 9 0000000000003000 00000028 Thread Storage Directory \[\.tls\]
+#...

Re: [patch bfd]: Fix generation of .tls directory entry

[Dave Korn]

On 21/12/2010 11:03, Kai Tietz wrote:
> 2010/12/21 Kai Tietz <ktietz70@googlemail.com>:

>> I found the issue.

  Thanks for persisting.

>> The cause for this is that bfd searches for
>> '__tls_used', which is the underscored version of '_tls_used'
>> variable. Caused by the fact that x64 windows doesn't prefix symbols
>> by underscore, the symbol wasn't found, as it remains '_tls_used'.
>>
>> So we have two chances here to solve this: a) Change for windows x64
>> the symbol to '__tls_used', or b) search in bfd for x64 windows for
>> '_tls_used' instead of '_tls_used'.

  We should do whatever MSVC does when targeting 64-bit windows, shouldn't we?

> This patch should fix this underscoring issue (should affect wince arm too).

  This is clearly a much better solution! :)

>         * peXXigen.c (_bfd_XXi_final_link_postscript): Use
>         TARGET_UNDERSCORE to determine "_tls_used" name.
>         (TARGET_UNDERSCORE): Define to default zero, if not present.
> 
>         * ld-pe/pe.exp: Add TLS directory test.
>         * ld-pe/tlssec.s: New.
>         * ld-pe/tlssec64.d: New.
>         * ld-pe/tlssec32.d: New.
> 
> Tested for x86_64-w64-mingw32, i686-pc-mingw32, and i686-pc-cygwin. Ok
> for apply?

  OK with one change: please move the TARGET_UNDERSCORE default definition up
to the top of the file, underneath the header includes, where there are
already some other macro definitions getting adjusted.  (I think it's cleaner
not to have supposedly global symbols suddenly entering scope half way through
a file...)

  Also, please do verify that the MS 64-bit compiler does the same thing.  (It
seems very highly likely that it does, which is why I'm OK for the patch to go
in first and you to adjust it afterwards if necessary.)

    cheers,
      DaveK

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

2010/12/21 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 21/12/2010 11:03, Kai Tietz wrote:
>> 2010/12/21 Kai Tietz <ktietz70@googlemail.com>:
>
>>> I found the issue.
>
>  Thanks for persisting.
>
>>> The cause for this is that bfd searches for
>>> '__tls_used', which is the underscored version of '_tls_used'
>>> variable. Caused by the fact that x64 windows doesn't prefix symbols
>>> by underscore, the symbol wasn't found, as it remains '_tls_used'.
>>>
>>> So we have two chances here to solve this: a) Change for windows x64
>>> the symbol to '__tls_used', or b) search in bfd for x64 windows for
>>> '_tls_used' instead of '_tls_used'.
>
>  We should do whatever MSVC does when targeting 64-bit windows, shouldn't we?
>
>> This patch should fix this underscoring issue (should affect wince arm too).
>
>  This is clearly a much better solution! :)
>
>>         * peXXigen.c (_bfd_XXi_final_link_postscript): Use
>>         TARGET_UNDERSCORE to determine "_tls_used" name.
>>         (TARGET_UNDERSCORE): Define to default zero, if not present.
>>
>>         * ld-pe/pe.exp: Add TLS directory test.
>>         * ld-pe/tlssec.s: New.
>>         * ld-pe/tlssec64.d: New.
>>         * ld-pe/tlssec32.d: New.
>>
>> Tested for x86_64-w64-mingw32, i686-pc-mingw32, and i686-pc-cygwin. Ok
>> for apply?
>
>  OK with one change: please move the TARGET_UNDERSCORE default definition up
> to the top of the file, underneath the header includes, where there are
> already some other macro definitions getting adjusted.  (I think it's cleaner
> not to have supposedly global symbols suddenly entering scope half way through
> a file...)
>
>  Also, please do verify that the MS 64-bit compiler does the same thing.  (It
> seems very highly likely that it does, which is why I'm OK for the patch to go
> in first and you to adjust it afterwards if necessary.)
>
>    cheers,
>      DaveK
>
>

Ok, I remove this TARGET_UNDERSCORE use at all. It would be always the
default definition, which is for 32-bit wrong. Instead I replace in
patch its use by bfd_get_symbol_leading_char(abfd). Updated patch
comes soon, after testing.

Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 2702 bytes --]

2010/12/21 Kai Tietz <ktietz70@googlemail.com>:
> 2010/12/21 Dave Korn <dave.korn.cygwin@gmail.com>:
>> On 21/12/2010 11:03, Kai Tietz wrote:
>>> 2010/12/21 Kai Tietz <ktietz70@googlemail.com>:
>>
>>>> I found the issue.
>>
>>  Thanks for persisting.
>>
>>>> The cause for this is that bfd searches for
>>>> '__tls_used', which is the underscored version of '_tls_used'
>>>> variable. Caused by the fact that x64 windows doesn't prefix symbols
>>>> by underscore, the symbol wasn't found, as it remains '_tls_used'.
>>>>
>>>> So we have two chances here to solve this: a) Change for windows x64
>>>> the symbol to '__tls_used', or b) search in bfd for x64 windows for
>>>> '_tls_used' instead of '_tls_used'.
>>
>>  We should do whatever MSVC does when targeting 64-bit windows, shouldn't we?
>>
>>> This patch should fix this underscoring issue (should affect wince arm too).
>>
>>  This is clearly a much better solution! :)
>>
>>>         * peXXigen.c (_bfd_XXi_final_link_postscript): Use
>>>         TARGET_UNDERSCORE to determine "_tls_used" name.
>>>         (TARGET_UNDERSCORE): Define to default zero, if not present.
>>>
>>>         * ld-pe/pe.exp: Add TLS directory test.
>>>         * ld-pe/tlssec.s: New.
>>>         * ld-pe/tlssec64.d: New.
>>>         * ld-pe/tlssec32.d: New.
>>>
>>> Tested for x86_64-w64-mingw32, i686-pc-mingw32, and i686-pc-cygwin. Ok
>>> for apply?
>>
>>  OK with one change: please move the TARGET_UNDERSCORE default definition up
>> to the top of the file, underneath the header includes, where there are
>> already some other macro definitions getting adjusted.  (I think it's cleaner
>> not to have supposedly global symbols suddenly entering scope half way through
>> a file...)
>>
>>  Also, please do verify that the MS 64-bit compiler does the same thing.  (It
>> seems very highly likely that it does, which is why I'm OK for the patch to go
>> in first and you to adjust it afterwards if necessary.)
>>
>>    cheers,
>>      DaveK
>>
>>
>
> Ok, I remove this TARGET_UNDERSCORE use at all. It would be always the
> default definition, which is for 32-bit wrong. Instead I replace in
> patch its use by bfd_get_symbol_leading_char(abfd). Updated patch
> comes soon, after testing.

So here is the new patch (tested for the same targets without
regression). I re-read documentation on msdn about _tls_used, and
there is no special-casing for x64, which means that they are doing in
their linker here the same magic - the _tls_used is a C-symbol.

Ok for apply?

Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[-- Attachment #2: fixtls_noprefix.txt --]
[-- Type: text/plain, Size: 3263 bytes --]

Index: src/bfd/peXXigen.c
===================================================================
--- src.orig/bfd/peXXigen.c	2010-12-21 10:08:50.000000000 +0100
+++ src/bfd/peXXigen.c	2010-12-21 15:07:46.752622600 +0100
@@ -82,6 +82,11 @@
 #include "libcoff.h"
 #include "libpei.h"
 
+/* Make sure we have default definition.  */
+#ifndef TARGET_UNDERSCORE
+#define TARGET_UNDERSCORE 0
+#endif
+
 #if defined COFF_WITH_pep || defined COFF_WITH_pex64
 # undef AOUTSZ
 # define AOUTSZ		PEPAOUTSZ
@@ -2418,7 +2423,9 @@ _bfd_XXi_final_link_postscript (bfd * ab
     }
 
   h1 = coff_link_hash_lookup (coff_hash_table (info),
-			      "__tls_used", FALSE, FALSE, TRUE);
+			      (bfd_get_symbol_leading_char(abfd) != 0
+			       ? "__tls_used" : "_tls_used"),
+			      FALSE, FALSE, TRUE);
   if (h1 != NULL)
     {
       if ((h1->root.type == bfd_link_hash_defined
Index: src/ld/testsuite/ld-pe/pe.exp
===================================================================
--- src.orig/ld/testsuite/ld-pe/pe.exp	2010-04-07 09:31:26.000000000 +0200
+++ src/ld/testsuite/ld-pe/pe.exp	2010-12-21 11:37:15.851325100 +0100
@@ -38,6 +38,8 @@ if {[istarget i*86-*-cygwin*]
 	 {{objdump -s secrel_64.d}} "secrel.x"}
 	{"Empty export table" "" "" "exports.s"
 	 {{objdump -p exports64.d}} "exports.dll"}
+	{"TLS directory entry" "" "" "tlssec.s"
+	 {{objdump -p tlssec64.d}} "tlssec.dll"}
       }
     } elseif {[istarget i*86-*-cygwin*] } {
       set pe_tests {
@@ -45,6 +47,8 @@ if {[istarget i*86-*-cygwin*]
 	 {{objdump -s secrel.d}} "secrel.x"}
 	{"Empty export table" "" "" "exports.s"
 	 {{objdump -p exports.d}} "exports.dll"}
+	{"TLS directory entry" "" "" "tlssec.s"
+	 {{objdump -p tlssec32.d}} "tlssec.dll"}
       }
     } else {
       set pe_tests {
@@ -52,6 +56,8 @@ if {[istarget i*86-*-cygwin*]
 	 {{objdump -s secrel.d}} "secrel.x"}
 	{"Empty export table" "" "" "exports.s"
 	 {{objdump -p exports.d}} "exports.dll"}
+	{"TLS directory entry" "" "" "tlssec.s"
+	 {{objdump -p tlssec32.d}} "tlssec.dll"}
       }
     }
 
Index: src/ld/testsuite/ld-pe/tlssec.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/ld/testsuite/ld-pe/tlssec.s	2010-12-21 11:29:09.770089200 +0100
@@ -0,0 +1,20 @@
+.global _tls_used
+.global __tls_used
+.global _start
+.global start
+.global _mainCRTStartup
+.global mainCRTStartup
+
+.text
+_start:
+mainCRTStartup:
+_mainCRTStartup:
+        .byte 1
+
+.section .tls
+_tls_used:
+__tls_used:
+.long 1,2,3,4,5,6,7,8,9,10
+.long 11,12,13,14,15,16,17,18,19,20
+.long 21,22,23,24,25,26,27,28,29,30
+
Index: src/ld/testsuite/ld-pe/tlssec32.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/ld/testsuite/ld-pe/tlssec32.d	2010-12-21 11:53:41.935250500 +0100
@@ -0,0 +1,3 @@
+#...
+Entry 9 00003000 00000018 Thread Storage Directory \[\.tls\]
+#...
Index: src/ld/testsuite/ld-pe/tlssec64.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/ld/testsuite/ld-pe/tlssec64.d	2010-12-21 11:34:21.131456900 +0100
@@ -0,0 +1,3 @@
+#...
+Entry 9 0000000000003000 00000028 Thread Storage Directory \[\.tls\]
+#...

Re: [patch bfd]: Fix generation of .tls directory entry

[Dave Korn]

On 21/12/2010 14:15, Kai Tietz wrote:
> 2010/12/21 Kai Tietz <ktietz70@googlemail.com>:

>> Ok, I remove this TARGET_UNDERSCORE use at all. It would be always the
>> default definition, which is for 32-bit wrong. Instead I replace in
>> patch its use by bfd_get_symbol_leading_char(abfd). 

  Hah, I forgot about that function too.

> So here is the new patch (tested for the same targets without
> regression). I re-read documentation on msdn about _tls_used, and
> there is no special-casing for x64, which means that they are doing in
> their linker here the same magic - the _tls_used is a C-symbol.
> 
> Ok for apply?

  The default definition of TARGET_UNDERSCORE is superfluous now, isn't it?
OK once you delete that.

    cheers,
      DaveK

Re: [patch bfd]: Fix generation of .tls directory entry

[Kai Tietz]

2010/12/21 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 21/12/2010 14:15, Kai Tietz wrote:
>> 2010/12/21 Kai Tietz <ktietz70@googlemail.com>:
>
>>> Ok, I remove this TARGET_UNDERSCORE use at all. It would be always the
>>> default definition, which is for 32-bit wrong. Instead I replace in
>>> patch its use by bfd_get_symbol_leading_char(abfd).
>
>  Hah, I forgot about that function too.
>
>> So here is the new patch (tested for the same targets without
>> regression). I re-read documentation on msdn about _tls_used, and
>> there is no special-casing for x64, which means that they are doing in
>> their linker here the same magic - the _tls_used is a C-symbol.
>>
>> Ok for apply?
>
>  The default definition of TARGET_UNDERSCORE is superfluous now, isn't it?
> OK once you delete that.
>
>    cheers,
>      DaveK
>
>

Thanks for the catch. Removed this unnecessary define and committed.

Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination