| Message ID | 20210313163901.610984-1-ibuclaw@gdcproject.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix ICE: in function_and_variable_visibility, at ipa-visibility.c:795 (PR99466) | expand |
Hi Iain, Iain Buclaw via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > This patch fixes an ICE caused by emutls routines generating a weak, > non-public symbol for storing the initializer of a weak TLS variable. > > In get_emutls_init_templ_addr, only declarations that were DECL_ONE_ONLY > would get a public initializer symbol, ignoring variables that were > declared with __attribute__((weak)). > > Because DECL_VISIBILITY is also copied to the emutls initializer, a > second test is included which checks that the expected visibility is > emitted too. > > Tested on x86_64-apple-darwin10, OK for mainline? > > The oldest version of gcc I've checked is 7.5.0, and the ICE is present > there too. Is this OK for backporting, and if so which versions should > it be backported to? > > Regards, > Iain. > > --- > gcc/ChangeLog: > > PR ipa/99466 > * tree-emutls.c (get_emutls_init_templ_addr): Mark initializer of weak > TLS declarations as public. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tls/pr98607-1.c: New test. > * gcc.dg/tls/pr98607-2.c: New test. ^^^ s/98607/99466/ ? > --- > gcc/testsuite/gcc.dg/tls/pr98607-1.c | 8 ++++++++ > gcc/testsuite/gcc.dg/tls/pr98607-2.c | 10 ++++++++++ > gcc/tree-emutls.c | 6 ++++-- > 3 files changed, 22 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tls/pr98607-1.c > create mode 100644 gcc/testsuite/gcc.dg/tls/pr98607-2.c > > diff --git a/gcc/testsuite/gcc.dg/tls/pr98607-1.c > b/gcc/testsuite/gcc.dg/tls/pr98607-1.c > new file mode 100644 > index 00000000000..446850e148b > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tls/pr98607-1.c > @@ -0,0 +1,8 @@ > +/* { dg-do compile } */ > +/* { dg-require-weak "" } */ > +/* { dg-require-effective-target tls_emulated } */ > +/* { dg-add-options tls } */ > +__attribute__((weak)) > +__thread int tlsvar = 3; > +/* { dg-final { scan-assembler ".weak_definition ___emutls_t.tlsvar" { > target *-*-darwin* } } } */ > +/* { dg-final { scan-assembler-not ".private_extern ___emutls_t.tlsvar" > { target *-*-darwin* } } } */ > diff --git a/gcc/testsuite/gcc.dg/tls/pr98607-2.c > b/gcc/testsuite/gcc.dg/tls/pr98607-2.c > new file mode 100644 > index 00000000000..86ffaad7f48 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tls/pr98607-2.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > +/* { dg-require-weak "" } */ > +/* { dg-require-visibility "" } */ > +/* { dg-require-effective-target tls_emulated } */ > +/* { dg-add-options tls } */ > +__attribute__((weak)) > +__attribute__((visibility ("hidden"))) > +__thread int tlsvar = 3; > +/* { dg-final { scan-assembler ".weak_definition ___emutls_t.tlsvar" { > target *-*-darwin* } } } */ > +/* { dg-final { scan-assembler ".private_extern ___emutls_t.tlsvar" { > target *-*-darwin* } } } */ > diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c > index f1053385944..1c9c5d5aee1 100644 > --- a/gcc/tree-emutls.c > +++ b/gcc/tree-emutls.c > @@ -242,16 +242,18 @@ get_emutls_init_templ_addr (tree decl) > DECL_PRESERVE_P (to) = DECL_PRESERVE_P (decl); > > DECL_WEAK (to) = DECL_WEAK (decl); > - if (DECL_ONE_ONLY (decl)) > + if (DECL_ONE_ONLY (decl) || DECL_WEAK (decl)) > { > TREE_STATIC (to) = TREE_STATIC (decl); > TREE_PUBLIC (to) = TREE_PUBLIC (decl); > DECL_VISIBILITY (to) = DECL_VISIBILITY (decl); > - make_decl_one_only (to, DECL_ASSEMBLER_NAME (to)); > } > else > TREE_STATIC (to) = 1; > > + if (DECL_ONE_ONLY (decl)) > + make_decl_one_only (to, DECL_ASSEMBLER_NAME (to)); > + > DECL_VISIBILITY_SPECIFIED (to) = DECL_VISIBILITY_SPECIFIED (decl); > DECL_INITIAL (to) = DECL_INITIAL (decl); > DECL_INITIAL (decl) = NULL; > -- > 2.27.0
Excerpts from Iain Sandoe's message of March 13, 2021 6:09 pm: > Hi Iain, > > Iain Buclaw via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > >> This patch fixes an ICE caused by emutls routines generating a weak, >> non-public symbol for storing the initializer of a weak TLS variable. >> >> In get_emutls_init_templ_addr, only declarations that were DECL_ONE_ONLY >> would get a public initializer symbol, ignoring variables that were >> declared with __attribute__((weak)). >> >> Because DECL_VISIBILITY is also copied to the emutls initializer, a >> second test is included which checks that the expected visibility is >> emitted too. >> >> Tested on x86_64-apple-darwin10, OK for mainline? >> >> The oldest version of gcc I've checked is 7.5.0, and the ICE is present >> there too. Is this OK for backporting, and if so which versions should >> it be backported to? >> >> Regards, >> Iain. >> >> --- >> gcc/ChangeLog: >> >> PR ipa/99466 >> * tree-emutls.c (get_emutls_init_templ_addr): Mark initializer of weak >> TLS declarations as public. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.dg/tls/pr98607-1.c: New test. >> * gcc.dg/tls/pr98607-2.c: New test. > > ^^^ s/98607/99466/ ? > Oops, good catch. I must have copied the number from the wrong tab. Mechanically updated the PR numbers and trying again... --- gcc/ChangeLog: PR ipa/99466 * tree-emutls.c (get_emutls_init_templ_addr): Mark initializer of weak TLS declarations as public. gcc/testsuite/ChangeLog: PR ipa/99466 * gcc.dg/tls/pr99466-1.c: New test. * gcc.dg/tls/pr99466-2.c: New test. --- gcc/testsuite/gcc.dg/tls/pr99466-1.c | 8 ++++++++ gcc/testsuite/gcc.dg/tls/pr99466-2.c | 10 ++++++++++ gcc/tree-emutls.c | 6 ++++-- 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tls/pr99466-1.c create mode 100644 gcc/testsuite/gcc.dg/tls/pr99466-2.c diff --git a/gcc/testsuite/gcc.dg/tls/pr99466-1.c b/gcc/testsuite/gcc.dg/tls/pr99466-1.c new file mode 100644 index 00000000000..446850e148b --- /dev/null +++ b/gcc/testsuite/gcc.dg/tls/pr99466-1.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-require-weak "" } */ +/* { dg-require-effective-target tls_emulated } */ +/* { dg-add-options tls } */ +__attribute__((weak)) +__thread int tlsvar = 3; +/* { dg-final { scan-assembler ".weak_definition ___emutls_t.tlsvar" { target *-*-darwin* } } } */ +/* { dg-final { scan-assembler-not ".private_extern ___emutls_t.tlsvar" { target *-*-darwin* } } } */ diff --git a/gcc/testsuite/gcc.dg/tls/pr99466-2.c b/gcc/testsuite/gcc.dg/tls/pr99466-2.c new file mode 100644 index 00000000000..86ffaad7f48 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tls/pr99466-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-require-weak "" } */ +/* { dg-require-visibility "" } */ +/* { dg-require-effective-target tls_emulated } */ +/* { dg-add-options tls } */ +__attribute__((weak)) +__attribute__((visibility ("hidden"))) +__thread int tlsvar = 3; +/* { dg-final { scan-assembler ".weak_definition ___emutls_t.tlsvar" { target *-*-darwin* } } } */ +/* { dg-final { scan-assembler ".private_extern ___emutls_t.tlsvar" { target *-*-darwin* } } } */ diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c index f1053385944..1c9c5d5aee1 100644 --- a/gcc/tree-emutls.c +++ b/gcc/tree-emutls.c @@ -242,16 +242,18 @@ get_emutls_init_templ_addr (tree decl) DECL_PRESERVE_P (to) = DECL_PRESERVE_P (decl); DECL_WEAK (to) = DECL_WEAK (decl); - if (DECL_ONE_ONLY (decl)) + if (DECL_ONE_ONLY (decl) || DECL_WEAK (decl)) { TREE_STATIC (to) = TREE_STATIC (decl); TREE_PUBLIC (to) = TREE_PUBLIC (decl); DECL_VISIBILITY (to) = DECL_VISIBILITY (decl); - make_decl_one_only (to, DECL_ASSEMBLER_NAME (to)); } else TREE_STATIC (to) = 1; + if (DECL_ONE_ONLY (decl)) + make_decl_one_only (to, DECL_ASSEMBLER_NAME (to)); + DECL_VISIBILITY_SPECIFIED (to) = DECL_VISIBILITY_SPECIFIED (decl); DECL_INITIAL (to) = DECL_INITIAL (decl); DECL_INITIAL (decl) = NULL;
On 3/14/2021 8:03 AM, Iain Buclaw via Gcc-patches wrote: > Excerpts from Iain Sandoe's message of March 13, 2021 6:09 pm: >> Hi Iain, >> >> Iain Buclaw via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: >> >>> This patch fixes an ICE caused by emutls routines generating a weak, >>> non-public symbol for storing the initializer of a weak TLS variable. >>> >>> In get_emutls_init_templ_addr, only declarations that were DECL_ONE_ONLY >>> would get a public initializer symbol, ignoring variables that were >>> declared with __attribute__((weak)). >>> >>> Because DECL_VISIBILITY is also copied to the emutls initializer, a >>> second test is included which checks that the expected visibility is >>> emitted too. >>> >>> Tested on x86_64-apple-darwin10, OK for mainline? >>> >>> The oldest version of gcc I've checked is 7.5.0, and the ICE is present >>> there too. Is this OK for backporting, and if so which versions should >>> it be backported to? >>> >>> Regards, >>> Iain. >>> >>> --- >>> gcc/ChangeLog: >>> >>> PR ipa/99466 >>> * tree-emutls.c (get_emutls_init_templ_addr): Mark initializer of weak >>> TLS declarations as public. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * gcc.dg/tls/pr98607-1.c: New test. >>> * gcc.dg/tls/pr98607-2.c: New test. >> ^^^ s/98607/99466/ ? >> > Oops, good catch. I must have copied the number from the wrong tab. > Mechanically updated the PR numbers and trying again... > > --- > gcc/ChangeLog: > > PR ipa/99466 > * tree-emutls.c (get_emutls_init_templ_addr): Mark initializer of weak > TLS declarations as public. > > gcc/testsuite/ChangeLog: > > PR ipa/99466 > * gcc.dg/tls/pr99466-1.c: New test. > * gcc.dg/tls/pr99466-2.c: New test. OK for the trunk. I'd probably go back to gcc-9 and gcc-10. Jeff
diff --git a/gcc/testsuite/gcc.dg/tls/pr98607-1.c b/gcc/testsuite/gcc.dg/tls/pr98607-1.c new file mode 100644 index 00000000000..446850e148b --- /dev/null +++ b/gcc/testsuite/gcc.dg/tls/pr98607-1.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-require-weak "" } */ +/* { dg-require-effective-target tls_emulated } */ +/* { dg-add-options tls } */ +__attribute__((weak)) +__thread int tlsvar = 3; +/* { dg-final { scan-assembler ".weak_definition ___emutls_t.tlsvar" { target *-*-darwin* } } } */ +/* { dg-final { scan-assembler-not ".private_extern ___emutls_t.tlsvar" { target *-*-darwin* } } } */ diff --git a/gcc/testsuite/gcc.dg/tls/pr98607-2.c b/gcc/testsuite/gcc.dg/tls/pr98607-2.c new file mode 100644 index 00000000000..86ffaad7f48 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tls/pr98607-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-require-weak "" } */ +/* { dg-require-visibility "" } */ +/* { dg-require-effective-target tls_emulated } */ +/* { dg-add-options tls } */ +__attribute__((weak)) +__attribute__((visibility ("hidden"))) +__thread int tlsvar = 3; +/* { dg-final { scan-assembler ".weak_definition ___emutls_t.tlsvar" { target *-*-darwin* } } } */ +/* { dg-final { scan-assembler ".private_extern ___emutls_t.tlsvar" { target *-*-darwin* } } } */ diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c index f1053385944..1c9c5d5aee1 100644 --- a/gcc/tree-emutls.c +++ b/gcc/tree-emutls.c @@ -242,16 +242,18 @@ get_emutls_init_templ_addr (tree decl) DECL_PRESERVE_P (to) = DECL_PRESERVE_P (decl); DECL_WEAK (to) = DECL_WEAK (decl); - if (DECL_ONE_ONLY (decl)) + if (DECL_ONE_ONLY (decl) || DECL_WEAK (decl)) { TREE_STATIC (to) = TREE_STATIC (decl); TREE_PUBLIC (to) = TREE_PUBLIC (decl); DECL_VISIBILITY (to) = DECL_VISIBILITY (decl); - make_decl_one_only (to, DECL_ASSEMBLER_NAME (to)); } else TREE_STATIC (to) = 1; + if (DECL_ONE_ONLY (decl)) + make_decl_one_only (to, DECL_ASSEMBLER_NAME (to)); + DECL_VISIBILITY_SPECIFIED (to) = DECL_VISIBILITY_SPECIFIED (decl); DECL_INITIAL (to) = DECL_INITIAL (decl); DECL_INITIAL (decl) = NULL;