[PATCH] aix: ensure reference to __tls_get_addr is in text section.

[PATCH] aix: ensure reference to __tls_get_addr is in text section.

[CHIGOT, CLEMENT]

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

The garbage collector of AIX linker might remove the reference to
__tls_get_addr if it's added inside an unused csect.


Clément Chigot
ATOS Bull SAS
1 rue de Provence - 38432 Échirolles - France


[-- Attachment #2: 0001-aix-ensure-reference-to-__tls_get_addr-is-in-text-se.patch --]
[-- Type: application/octet-stream, Size: 934 bytes --]

From 17832ad26ffd061f5304acf03ca4f34646421e93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= <clement.chigot@atos.net>
Date: Thu, 14 Oct 2021 09:03:13 +0200
Subject: [PATCH] aix: ensure reference to __tls_get_addr is in text section.

The garbage collector of AIX linker might remove the reference to
__tls_get_addr if it's added inside an unused csect.
---
 gcc/config/rs6000/rs6000.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index acba4d9f26c..62a98e7255c 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21636,6 +21636,7 @@ rs6000_xcoff_file_end (void)
   if (xcoff_tls_exec_model_detected)
     {
       /* Add a .ref to __tls_get_addr to force libpthread dependency.  */
+      switch_to_section (text_section);
       fputs ("\t.extern __tls_get_addr\n\t.ref __tls_get_addr\n", asm_out_file);
     }
 }
-- 
2.33.0

Re: [PATCH] aix: ensure reference to __tls_get_addr is in text section.

[David Edelsohn]

The reference to __tls_get_addr is in the data section.  And the code
just above creates a symbol in the text section referenced from the
data section to ensure the text section is retained.  So this change
doesn't make sense.  You're essentially saying that the data section
is not used, which makes the other code useless to ensure that the
text section is referenced.

Thanks, David

On Thu, Oct 14, 2021 at 3:06 AM CHIGOT, CLEMENT <clement.chigot@atos.net> wrote:
>
> The garbage collector of AIX linker might remove the reference to
> __tls_get_addr if it's added inside an unused csect.
>
>
> Clément Chigot
> ATOS Bull SAS
> 1 rue de Provence - 38432 Échirolles - France
>

Re: [PATCH] aix: ensure reference to __tls_get_addr is in text section.

[CHIGOT, CLEMENT]

Hi David,

The fact that csect .data is referencing csect .text doesn't mean that
if .text is kept, .data is kept too. It means the opposite. if .data is kept
then .text must be kept.

That's actually what is being done by the linker with the TLS support test
in configure.
$ cat test.c
__thread int a; int b; int main() { return a = b; }

With ".ref __tls_get_addr" in .data:
$ gcc -maix64 test.c -S -o test.s
$ cat test.s
...
_section_.text:
        .csect .data[RW],4
        .llong _section_.text
        .extern __tls_get_addr
        .ref __tls_get_addr
$ gcc -maix64 test.s -o test
$ dump -X64 -tv test
...
[142]   m   0x00000097     debug     0    FILE        C:PPC64     test.c
[143]   m   0x100006c0     .text     1  unamex                    .text
[144]   a4  0x0000005c       0    0     SD       PR    -    -
[147]   m   0x20001298      .bss     1  extern                    b
[148]   a4  0x00000004       0    0     CM       BS    -    -
[149]   m   0xffffffffffff8800     .tbss     1  extern                    a
[150]   a4  0x00000004       0    0     CM       UL    -    -
...

Csect .data is garbage-collected by the linker. Thus the .ref doesn't matter.

With ".ref __tls_get_addr" in .text:
$ cat test.s
_section_.text:
        .csect .data[RW],4
        .llong _section_.text
        .csect .text[PR],5
        .extern __tls_get_addr
        .ref __tls_get_addr
$ gcc  -maix64 test.s -o test
ld: 0711-317 ERROR: Undefined symbol: __tls_get_addr
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
collect2: error: ld returned 8 exit status

As csect .text is kept (because of main function), the .ref is still there and the error
is raise correctly. As "-pthread" isn't passed, __tls_get_addr is not available.

However, writing this mail, I'm wondering if we don't want to always keep both
csects. If .data is kept, then .text is and if .text is kept, then .data is.
Or always keeping .data would have too much side effects ?

Thanks,
Clément

________________________________
From: David Edelsohn <dje.gcc@gmail.com>
Sent: Thursday, October 14, 2021 3:42 PM
To: CHIGOT, CLEMENT <clement.chigot@atos.net>
Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] aix: ensure reference to __tls_get_addr is in text section.

Caution! External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.

The reference to __tls_get_addr is in the data section.  And the code
just above creates a symbol in the text section referenced from the
data section to ensure the text section is retained.  So this change
doesn't make sense.  You're essentially saying that the data section
is not used, which makes the other code useless to ensure that the
text section is referenced.

Thanks, David

On Thu, Oct 14, 2021 at 3:06 AM CHIGOT, CLEMENT <clement.chigot@atos.net> wrote:
>
> The garbage collector of AIX linker might remove the reference to
> __tls_get_addr if it's added inside an unused csect.
>
>
> Clément Chigot
> ATOS Bull SAS
> 1 rue de Provence - 38432 Échirolles - France
>

Re: [PATCH] aix: ensure reference to __tls_get_addr is in text section.

[David Edelsohn]

On Thu, Oct 14, 2021 at 10:10 AM CHIGOT, CLEMENT
<clement.chigot@atos.net> wrote:
>
> Hi David,
>
> The fact that csect .data is referencing csect .text doesn't mean that
> if .text is kept, .data is kept too. It means the opposite. if .data is kept
> then .text must be kept.

Yes, we are in agreement about the purpose of the other reference
between text and data.

The __tls_get_addr reference is an effort to artificially elicit an
error if pthread is not linked in a program that uses TLS.  It is not
truly necessary for correct TLS code generation from GCC.

Your patch moves the __tls_get_addr referenced from the data section
to the text section.  As we agree, the logic of the other code implies
that the data section is used to pull in the text section, so moving
__tls_get_addr to the text section seems more fragile.  It's a game of
which section will be preserved to ensure that __tls_get_addr is
referenced to ensure that an artificial error is generated.  And it's
possible that neither text nor data sections will be referenced if
fine-grained CSECTs are used.  There is no way to make this logic
perfect.

Thanks, David


>
> That's actually what is being done by the linker with the TLS support test
> in configure.
> $ cat test.c
> __thread int a; int b; int main() { return a = b; }
>
> With ".ref __tls_get_addr" in .data:
> $ gcc -maix64 test.c -S -o test.s
> $ cat test.s
> ...
> _section_.text:
>         .csect .data[RW],4
>         .llong _section_.text
>         .extern __tls_get_addr
>         .ref __tls_get_addr
> $ gcc -maix64 test.s -o test
> $ dump -X64 -tv test
> ...
> [142]   m   0x00000097     debug     0    FILE        C:PPC64     test.c
> [143]   m   0x100006c0     .text     1  unamex                    .text
> [144]   a4  0x0000005c       0    0     SD       PR    -    -
> [147]   m   0x20001298      .bss     1  extern                    b
> [148]   a4  0x00000004       0    0     CM       BS    -    -
> [149]   m   0xffffffffffff8800     .tbss     1  extern                    a
> [150]   a4  0x00000004       0    0     CM       UL    -    -
> ...
>
> Csect .data is garbage-collected by the linker. Thus the .ref doesn't matter.
>
> With ".ref __tls_get_addr" in .text:
> $ cat test.s
> _section_.text:
>         .csect .data[RW],4
>         .llong _section_.text
>         .csect .text[PR],5
>         .extern __tls_get_addr
>         .ref __tls_get_addr
> $ gcc  -maix64 test.s -o test
> ld: 0711-317 ERROR: Undefined symbol: __tls_get_addr
> ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
> collect2: error: ld returned 8 exit status
>
> As csect .text is kept (because of main function), the .ref is still there and the error
> is raise correctly. As "-pthread" isn't passed, __tls_get_addr is not available.
>
> However, writing this mail, I'm wondering if we don't want to always keep both
> csects. If .data is kept, then .text is and if .text is kept, then .data is.
> Or always keeping .data would have too much side effects ?
>
> Thanks,
> Clément
>
> ________________________________
> From: David Edelsohn <dje.gcc@gmail.com>
> Sent: Thursday, October 14, 2021 3:42 PM
> To: CHIGOT, CLEMENT <clement.chigot@atos.net>
> Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH] aix: ensure reference to __tls_get_addr is in text section.
>
> Caution! External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
> The reference to __tls_get_addr is in the data section.  And the code
> just above creates a symbol in the text section referenced from the
> data section to ensure the text section is retained.  So this change
> doesn't make sense.  You're essentially saying that the data section
> is not used, which makes the other code useless to ensure that the
> text section is referenced.
>
> Thanks, David
>
> On Thu, Oct 14, 2021 at 3:06 AM CHIGOT, CLEMENT <clement.chigot@atos.net> wrote:
> >
> > The garbage collector of AIX linker might remove the reference to
> > __tls_get_addr if it's added inside an unused csect.
> >
> >
> > Clément Chigot
> > ATOS Bull SAS
> > 1 rue de Provence - 38432 Échirolles - France
> >

[PATCH] aix: ensure reference to __tls_get_addr is in text section.

[CHIGOT, CLEMENT]

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

The garbage collector of AIX linker might remove the reference to
__tls_get_addr if it's added inside an unused csect, which can be
the case of .data with very simple programs.

gcc/ChangeLog:
2021-10-19  Clément Chigot  <clement.chigot@atos.net>

        * config/rs6000/rs6000.c (rs6000_xcoff_file_end): Move
        __tls_get_addr reference to .text csect.

Approved offline by David Edelson.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-aix-ensure-reference-to-__tls_get_addr-is-in-text-se.patch --]
[-- Type: text/x-patch; name="0001-aix-ensure-reference-to-__tls_get_addr-is-in-text-se.patch", Size: 1720 bytes --]

From 52e9e4554d8dba9f9c9c56267789fc1d08b1de98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= <clement.chigot@atos.net>
Date: Thu, 14 Oct 2021 09:03:13 +0200
Subject: [PATCH] aix: ensure reference to __tls_get_addr is in text section.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The garbage collector of AIX linker might remove the reference to
__tls_get_addr if it's added inside an unused csect, which can be
the case of .data with very simple programs.

gcc/ChangeLog:
2021-10-19  Clément Chigot  <clement.chigot@atos.net>

	* config/rs6000/rs6000.c (rs6000_xcoff_file_end): Move
	__tls_get_addr reference to .text csect.
---
 gcc/config/rs6000/rs6000.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 68111c3fe6a..bac959f4ef4 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21626,17 +21626,17 @@ static void
 rs6000_xcoff_file_end (void)
 {
   switch_to_section (text_section);
+  if (xcoff_tls_exec_model_detected)
+    {
+      /* Add a .ref to __tls_get_addr to force libpthread dependency.  */
+      fputs ("\t.extern __tls_get_addr\n\t.ref __tls_get_addr\n", asm_out_file);
+    }
   fputs ("_section_.text:\n", asm_out_file);
   switch_to_section (data_section);
   fputs (TARGET_32BIT
 	 ? "\t.long _section_.text\n" : "\t.llong _section_.text\n",
 	 asm_out_file);
 
-  if (xcoff_tls_exec_model_detected)
-    {
-      /* Add a .ref to __tls_get_addr to force libpthread dependency.  */
-      fputs ("\t.extern __tls_get_addr\n\t.ref __tls_get_addr\n", asm_out_file);
-    }
 }
 
 struct declare_alias_data
-- 
2.25.1