This is the mail archive of the
binutils-cvs@sourceware.org
mailing list for the binutils project.
[binutils-gdb] ld: Fix issue where PROVIDE overrides defined symbol
- From: Andrew Burgess <aburgess at sourceware dot org>
- To: bfd-cvs at sourceware dot org
- Date: 11 Jan 2018 17:37:08 -0000
- Subject: [binutils-gdb] ld: Fix issue where PROVIDE overrides defined symbol
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=eab62f2f018417121e2520acb0623985b1708b02
commit eab62f2f018417121e2520acb0623985b1708b02
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Thu Apr 27 18:05:08 2017 +0100
ld: Fix issue where PROVIDE overrides defined symbol
In a linker script, a sequence like this:
foo = ADDR (.some_section);
bar = foo;
PROVIDE (foo = 0);
will result in 'bar = ADDR (.some_section)' and 'foo = 0', which seems
like incorrect behaviour, foo is clearly defined elsewhere, and so the
PROVIDE should not trigger.
The problem is that an expression like this:
foo = ADDR (.some_section);
can't be evaluated until a late phase of the linker, due to the need
for the section '.some_section' to have been placed, then the PROVIDE
was being marked as being used during an earlier phase. At the end of
the link, both lines:
foo = ADDR (.some_section);
PROVIDE (foo = 0);
are active, and this causes the final value of 'foo' to be 0.
The solution proposed in this commit is that, during earlier phases of
the linker, when we see the expression 'foo = ADDR (.some_section);',
instead of ignoring the expression, we create a "fake" definition of
'foo'. The existence of this "fake" definition prevents the PROVIDE
from being marked used, and during the final phase the real definition
of 'foo' will replace the "fake" definition.
The new test provide-6 covers the exact case described above. The
provide-7 test is similar to the above, but using constant
expressions, this was never broken, but is added here to increase
coverage.
The provide-8 case also didn't fail before this commit, but I did
manage to break this case during development of this patch. This case
was only covered by a mmix test before, so I've added this here to
increase coverage.
ld/ChangeLog:
* ldexp.c (exp_fold_tree_1): Rework condition underwhich provide
nodes are ignored in the tree walk, and move the location at which
we change provide nodes into provided nodes.
(exp_init_os): Add etree_provided.
* testsuite/ld-scripts/provide-6.d: New file.
* testsuite/ld-scripts/provide-6.t: New file.
* testsuite/ld-scripts/provide-7.d: New file.
* testsuite/ld-scripts/provide-7.t: New file.
* testsuite/ld-scripts/provide-8.d: New file.
* testsuite/ld-scripts/provide-8.t: New file.
Diff:
---
ld/ChangeLog | 13 ++++++
ld/ldexp.c | 88 +++++++++++++++++++------------------
ld/ldlang.c | 1 +
ld/testsuite/ld-scripts/provide-6.d | 9 ++++
ld/testsuite/ld-scripts/provide-6.t | 11 +++++
ld/testsuite/ld-scripts/provide-7.d | 8 ++++
ld/testsuite/ld-scripts/provide-7.t | 11 +++++
ld/testsuite/ld-scripts/provide-8.d | 8 ++++
ld/testsuite/ld-scripts/provide-8.t | 14 ++++++
9 files changed, 121 insertions(+), 42 deletions(-)
diff --git a/ld/ChangeLog b/ld/ChangeLog
index f96517e..956a856 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,5 +1,18 @@
2018-01-11 Andrew Burgess <andrew.burgess@embecosm.com>
+ * ldexp.c (exp_fold_tree_1): Rework condition underwhich provide
+ nodes are ignored in the tree walk, and move the location at which
+ we change provide nodes into provided nodes.
+ (exp_init_os): Add etree_provided.
+ * testsuite/ld-scripts/provide-6.d: New file.
+ * testsuite/ld-scripts/provide-6.t: New file.
+ * testsuite/ld-scripts/provide-7.d: New file.
+ * testsuite/ld-scripts/provide-7.t: New file.
+ * testsuite/ld-scripts/provide-8.d: New file.
+ * testsuite/ld-scripts/provide-8.t: New file.
+
+2018-01-11 Andrew Burgess <andrew.burgess@embecosm.com>
+
* testsuite/ld-scripts/provide-3.d: Add xfail directive.
* testsuite/ld-scripts/provide-4.d: Use new map file name.
* testsuite/ld-scripts/provide-5.d: Use new map file name.
diff --git a/ld/ldexp.c b/ld/ldexp.c
index 800c7f8..6398a2d 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -1153,13 +1153,12 @@ exp_fold_tree_1 (etree_type *tree)
2) Section relative symbol values cannot be correctly
converted to absolute values, as is required by many
expressions, until final section sizing is complete. */
- if ((expld.result.valid_p
- && (expld.phase == lang_final_phase_enum
- || expld.assign_name != NULL))
- || (expld.phase <= lang_mark_phase_enum
- && tree->type.node_class == etree_assign
- && tree->assign.defsym))
+ if (expld.phase == lang_final_phase_enum
+ || expld.assign_name != NULL)
{
+ if (tree->type.node_class == etree_provide)
+ tree->type.node_class = etree_provided;
+
if (h == NULL)
{
h = bfd_link_hash_lookup (link_info.hash, tree->assign.dst,
@@ -1169,44 +1168,49 @@ exp_fold_tree_1 (etree_type *tree)
tree->assign.dst);
}
- if (expld.result.section == NULL)
- expld.result.section = expld.section;
- if (!update_definedness (tree->assign.dst, h) && 0)
+ /* If the expression is not valid then fake a zero value. In
+ the final phase any errors will already have been raised,
+ in earlier phases we want to create this definition so
+ that it can be seen by other expressions. */
+ if (!expld.result.valid_p
+ && h->type == bfd_link_hash_new)
+ {
+ expld.result.value = 0;
+ expld.result.section = NULL;
+ expld.result.valid_p = TRUE;
+ }
+
+ if (expld.result.valid_p)
{
- /* Symbol was already defined. For now this error
- is disabled because it causes failures in the ld
- testsuite: ld-elf/var1, ld-scripts/defined5, and
- ld-scripts/pr14962. Some of these no doubt
- reflect scripts used in the wild. */
- (*link_info.callbacks->multiple_definition)
- (&link_info, h, link_info.output_bfd,
- expld.result.section, expld.result.value);
+ if (expld.result.section == NULL)
+ expld.result.section = expld.section;
+ if (!update_definedness (tree->assign.dst, h) && 0)
+ {
+ /* Symbol was already defined. For now this error
+ is disabled because it causes failures in the ld
+ testsuite: ld-elf/var1, ld-scripts/defined5, and
+ ld-scripts/pr14962. Some of these no doubt
+ reflect scripts used in the wild. */
+ (*link_info.callbacks->multiple_definition)
+ (&link_info, h, link_info.output_bfd,
+ expld.result.section, expld.result.value);
+ }
+ h->type = bfd_link_hash_defined;
+ h->u.def.value = expld.result.value;
+ h->u.def.section = expld.result.section;
+ h->linker_def = ! tree->assign.type.lineno;
+ h->ldscript_def = 1;
+
+ /* Copy the symbol type if this is an expression only
+ referencing a single symbol. (If the expression
+ contains ternary conditions, ignoring symbols on
+ false branches.) */
+ if (expld.assign_src != NULL
+ && (expld.assign_src
+ != (struct bfd_link_hash_entry *) 0 - 1))
+ bfd_copy_link_hash_symbol_type (link_info.output_bfd, h,
+ expld.assign_src);
}
- h->type = bfd_link_hash_defined;
- h->u.def.value = expld.result.value;
- h->u.def.section = expld.result.section;
- h->linker_def = ! tree->assign.type.lineno;
- h->ldscript_def = 1;
- if (tree->type.node_class == etree_provide)
- tree->type.node_class = etree_provided;
-
- /* Copy the symbol type if this is an expression only
- referencing a single symbol. (If the expression
- contains ternary conditions, ignoring symbols on
- false branches.) */
- if (expld.result.valid_p
- && expld.assign_src != NULL
- && expld.assign_src != (struct bfd_link_hash_entry *) 0 - 1)
- bfd_copy_link_hash_symbol_type (link_info.output_bfd, h,
- expld.assign_src);
- }
- else if (expld.phase == lang_final_phase_enum)
- {
- h = bfd_link_hash_lookup (link_info.hash, tree->assign.dst,
- FALSE, FALSE, TRUE);
- if (h != NULL
- && h->type == bfd_link_hash_new)
- h->type = bfd_link_hash_undefined;
}
expld.assign_name = NULL;
}
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 294e632..1526d7b 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -2207,6 +2207,7 @@ exp_init_os (etree_type *exp)
{
case etree_assign:
case etree_provide:
+ case etree_provided:
exp_init_os (exp->assign.src);
break;
diff --git a/ld/testsuite/ld-scripts/provide-6.d b/ld/testsuite/ld-scripts/provide-6.d
new file mode 100644
index 0000000..dd40515
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-6.d
@@ -0,0 +1,9 @@
+#source: provide-5.s
+#ld: -T provide-6.t
+#PROG: nm
+#xfail: x86_64-*-cygwin
+
+#...
+0+1000 D foo
+0+1000 D foo_2
+#...
diff --git a/ld/testsuite/ld-scripts/provide-6.t b/ld/testsuite/ld-scripts/provide-6.t
new file mode 100644
index 0000000..6b5d11c
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-6.t
@@ -0,0 +1,11 @@
+SECTIONS
+{
+ .data 0x1000 :
+ {
+ *(.data)
+ }
+
+ foo = ADDR (.data);
+ foo_2 = foo;
+ PROVIDE (foo = 0x20);
+}
diff --git a/ld/testsuite/ld-scripts/provide-7.d b/ld/testsuite/ld-scripts/provide-7.d
new file mode 100644
index 0000000..c524fe4
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-7.d
@@ -0,0 +1,8 @@
+#source: provide-5.s
+#ld: -T provide-7.t
+#PROG: nm
+
+#...
+0+10 A foo
+0+10 A foo_2
+#...
diff --git a/ld/testsuite/ld-scripts/provide-7.t b/ld/testsuite/ld-scripts/provide-7.t
new file mode 100644
index 0000000..882883e
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-7.t
@@ -0,0 +1,11 @@
+SECTIONS
+{
+ .data 0x1000 :
+ {
+ *(.data)
+ }
+
+ foo = 0x10;
+ foo_2 = foo;
+ PROVIDE (foo = 0x20);
+}
diff --git a/ld/testsuite/ld-scripts/provide-8.d b/ld/testsuite/ld-scripts/provide-8.d
new file mode 100644
index 0000000..a402937
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-8.d
@@ -0,0 +1,8 @@
+#source: provide-5.s
+#ld: -T provide-8.t
+#PROG: nm
+#xfail: x86_64-*-cygwin mmix-*-* sh-*-pe spu-*-*
+
+#...
+0+4000 D __FOO
+#...
diff --git a/ld/testsuite/ld-scripts/provide-8.t b/ld/testsuite/ld-scripts/provide-8.t
new file mode 100644
index 0000000..ffc3467
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-8.t
@@ -0,0 +1,14 @@
+SECTIONS
+{
+ .data 0x1000 :
+ {
+ *(.data)
+ QUAD (__FOO);
+ }
+
+ .foo 0x4000 :
+ {
+ PROVIDE (__FOO = .);
+ *(.foo)
+ }
+}