This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, GCC/LTO, ping] Fix PR69866: LTO with def for weak alias in regular object file
- From: Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>, Jeff Law <law at redhat dot com>
- Date: Mon, 12 Jun 2017 10:13:33 +0100
- Subject: Re: [PATCH, GCC/LTO, ping] Fix PR69866: LTO with def for weak alias in regular object file
- Authentication-results: sourceware.org; auth=none
- References: <654b4277-ced9-e778-bcda-3054e1983727@foss.arm.com> <1db1d20c-6f06-9bd3-2e7d-e862e31199cd@foss.arm.com> <b1de6120-d3a0-519d-79d9-6b5140381ccb@foss.arm.com> <2924a838-1869-9cd4-8b32-3c2c51a12bc5@redhat.com> <FE96403C-90ED-4638-B792-82EAC0BD59F6@suse.de> <2485e103-8b05-2019-10e2-b75d8cc0f374@foss.arm.com> <7a758c6f-31d5-7133-0df3-3258d08b6b56@foss.arm.com> <20170509223658.GB82149@kam.mff.cuni.cz> <530b495f-e3e8-8223-cda0-1fe0525ba46b@foss.arm.com>
Ping?
Best regards,
Thomas
On 06/06/17 11:12, Thomas Preudhomme wrote:
On 09/05/17 23:36, Jan Hubicka wrote:
Ping?
Sorry for late reply
My turn to apologize now.
Hi,
This patch fixes an assert failure when linking one LTOed object file
having a weak alias with a regular object file containing a strong
definition for that same symbol. The patch is twofold:
+ do not add an alias to a partition if it is external
+ do not declare (.globl) an alias if it is external
Adding external alises to partitions is important to keep the information
that two symbols are the same.
So how about simply relaxing the assert then? Right now it trips for any
external symbol, even external aliases.
How about the following:
ChangeLog entries are as follow:
*** gcc/lto/ChangeLog ***
2017-06-02 Thomas Preud'homme <thomas.preudhomme@arm.com>
* lto/lto-partition.c (add_symbol_to_partition_1): Change assert to
allow external aliases to be added.
*** gcc/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
* cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
declare external aliases.
*** gcc/testsuite/ChangeLog ***
2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
* gcc.dg/lto/pr69866_0.c: New test.
* gcc.dg/lto/pr69866_1.c: Likewise.
Bootstrapped with LTO on Aarch64 and ARM and testsuite on both of these
architectures do not show any regression.
Is this ok for trunk?
Best regards,
Thomas
The second part makes sense to me. What breaks when you drop the first
change?
Honza
ChangeLog entries are as follow:
*** gcc/lto/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* lto/lto-partition.c (add_symbol_to_partition_1): Do not add external
aliases to partition.
*** gcc/ChangeLog ***
2017-03-01 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* cgraphunit.c (cgraph_node::assemble_thunks_and_aliases): Do not
declare external aliases.
*** gcc/testsuite/ChangeLog ***
2017-02-28 Thomas Preud'homme <thomas.preudhomme@arm.com>
PR lto/69866
* gcc.dg/lto/pr69866_0.c: New test.
* gcc.dg/lto/pr69866_1.c: Likewise.
Testing: Testsuite shows no regression when targeting Cortex-M3 with an
arm-none-eabi GCC cross-compiler, neither does it show any regression with
native LTO-bootstrapped x86-64_linux-gnu and aarch64-linux-gnu compilers.
Is this ok for stage4?
Best regards,
Thomas
On 31/03/17 18:07, Richard Biener wrote:
On March 31, 2017 5:23:03 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
On 03/16/2017 08:05 AM, Thomas Preudhomme wrote:
Ping?
Is this ok for stage4?
Given the lack of response from Richi, I'd suggest deferring to stage1.
Honza needs to review this, i habe too little knowledge here.
Richard.
jeff
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index
c82a88a599ca61b068dd9783d2a6158163809b37..580500ff922b8546d33119261a2455235edbf16d
100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1972,7 +1972,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index
e27d0d1690c1fcfb39e2fac03ce0f4154031fc7c..f44fd435ed075a27e373bdfdf0464eb06e1731ef
100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -178,7 +178,8 @@ add_symbol_to_partition_1 (ltrans_partition part,
symtab_node *node)
/* Add all aliases associated with the symbol. */
FOR_EACH_ALIAS (node, ref)
- if (!ref->referring->transparent_alias)
+ if (!ref->referring->transparent_alias
+ && ref->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
add_symbol_to_partition_1 (part, ref->referring);
else
{
@@ -189,7 +190,8 @@ add_symbol_to_partition_1 (ltrans_partition part,
symtab_node *node)
{
/* Nested transparent aliases are not permitted. */
gcc_checking_assert (!ref2->referring->transparent_alias);
- add_symbol_to_partition_1 (part, ref2->referring);
+ if (ref2->referring->get_partitioning_class () != SYMBOL_EXTERNAL)
+ add_symbol_to_partition_1 (part, ref2->referring);
}
}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c
b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index
0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c
b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index
0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 7b4f47e6efb41e7c401e7347d86fffca6618c4e9..c2cc9df6419573bdc6223e1d2ee348f8af69ccca 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1984,7 +1984,7 @@ cgraph_node::assemble_thunks_and_aliases (void)
FOR_EACH_ALIAS (this, ref)
{
cgraph_node *alias = dyn_cast <cgraph_node *> (ref->referring);
- if (!alias->transparent_alias)
+ if (!alias->transparent_alias && !DECL_EXTERNAL (alias->decl))
{
bool saved_written = TREE_ASM_WRITTEN (decl);
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c
index 3600ab23bd9157aad76751912306abd498acae92..620deac090b7e718f05d3f37e6ef68e759de0008 100644
--- a/gcc/lto/lto-partition.c
+++ b/gcc/lto/lto-partition.c
@@ -132,7 +132,7 @@ add_symbol_to_partition_1 (ltrans_partition part, symtab_node *node)
/* Be sure that we never try to duplicate partitioned symbol
or add external symbol. */
- gcc_assert (c != SYMBOL_EXTERNAL
+ gcc_assert ((c != SYMBOL_EXTERNAL || node->alias)
&& (c == SYMBOL_DUPLICATE || !symbol_partitioned_p (node)));
part->symbols++;
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_0.c b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
new file mode 100644
index 0000000000000000000000000000000000000000..f49ef8d4c1da7a21d1bfb5409d647bd18141595b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_0.c
@@ -0,0 +1,13 @@
+/* { dg-lto-do link } */
+
+int _umh(int i)
+{
+ return i+1;
+}
+
+int weaks(int i) __attribute__((weak, alias("_umh")));
+
+int main()
+{
+ return weaks(10);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr69866_1.c b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..3a14f850eefaffbf659ce4642adef7900330f4ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr69866_1.c
@@ -0,0 +1,6 @@
+/* { dg-options { -fno-lto } } */
+
+int weaks(int i)
+{
+ return i+1;
+}