From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84528 invoked by alias); 9 Jan 2018 21:09:13 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 84363 invoked by uid 89); 9 Jan 2018 21:09:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=sizing X-HELO: mail-wm0-f42.google.com Received: from mail-wm0-f42.google.com (HELO mail-wm0-f42.google.com) (74.125.82.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 09 Jan 2018 21:09:10 +0000 Received: by mail-wm0-f42.google.com with SMTP id i186so6628047wmi.4 for ; Tue, 09 Jan 2018 13:09:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=J8Qi2FioobDrlIX6tQUs5/9Ag+jzr9s/OzgEf+E0VhY=; b=kW2Mlze8Fu9q5O5Y1Qp0bpVMJ8285WfMKCQDMCnZ116s6wthVazJKQ8db6vyVOdz/z InGLv8DO6kClPxRkcFuqY3zKtB3cxzAD4n3I6UnMn8svl/t8gxMVBLCRK4BdRT3uC7ED QHfVAYbvKfkvtGocV7Z1IhTj+P/eXsz5eifWetN0vQvvwd05tHz4aOLsW8qFpSffa+yh QPcZ2GunpOhx624w7kGnORLT+cKlXOI5YGDb1tX34/U5is5b5OzRHL7ZF7zKDA4JUaB6 GlRRtnjoD75FQI+K2GRXJ/5lFKxcuq/cc/xAe1SFvzr9IDkPG0ul//BLrwVJSycGVc92 ot1w== X-Gm-Message-State: AKGB3mIzwpeF7vHWVZxfGbYNgBpTJZBNWGUVHhX8Ave/Cqn2VGbjNkYn Sz/JovWRZo4LQcZl8XU2IIx5UCpE X-Google-Smtp-Source: ACJfBosLOXSrml729MvfosV02wc4J+WwbJ8829ctznNiJPk6jQzsz9Bg7hVpAgj2vDAYIs61jwRpRw== X-Received: by 10.28.51.70 with SMTP id z67mr13885805wmz.78.1515532147469; Tue, 09 Jan 2018 13:09:07 -0800 (PST) Received: from localhost ([81.141.199.69]) by smtp.gmail.com with ESMTPSA id g64sm14566988wmf.41.2018.01.09.13.09.06 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 09 Jan 2018 13:09:06 -0800 (PST) From: Andrew Burgess To: binutils@sourceware.org Cc: Andrew Burgess Subject: [PATCH 3/4] ld: Fix issue where PROVIDE overrides defined symbol Date: Tue, 09 Jan 2018 21:09:00 -0000 Message-Id: <44612a2bda38da51e325b9e8efb4a0550fc1f94c.1515531328.git.andrew.burgess@embecosm.com> In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg00131.txt.bz2 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. --- ld/ChangeLog | 13 ++++++ ld/ldexp.c | 89 ++++++++++++++++++++----------------- 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, 122 insertions(+), 42 deletions(-) create mode 100644 ld/testsuite/ld-scripts/provide-6.d create mode 100644 ld/testsuite/ld-scripts/provide-6.t create mode 100644 ld/testsuite/ld-scripts/provide-7.d create mode 100644 ld/testsuite/ld-scripts/provide-7.t create mode 100644 ld/testsuite/ld-scripts/provide-8.d create mode 100644 ld/testsuite/ld-scripts/provide-8.t diff --git a/ld/ldexp.c b/ld/ldexp.c index 800c7f80859..9508bad4a56 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,50 @@ 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.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); } - 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 294e6323012..1526d7b2ec1 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 00000000000..dd405151197 --- /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 00000000000..6b5d11c04bd --- /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 00000000000..c524fe428ab --- /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 00000000000..882883eda05 --- /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 00000000000..a4029370ca2 --- /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 00000000000..ffc3467911f --- /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) + } +} -- 2.14.3