* ld's sole user of update_definedness() vs documentation
@ 2021-02-19 11:16 Jan Beulich
2021-02-21 4:05 ` Alan Modra
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2021-02-19 11:16 UTC (permalink / raw)
To: Binutils
After having read through PROVIDE()'s documentation I've come to
the conclusion that a linker script defining a symbol defined by
an object would be an error. After having made a change where I
would have expected such an error to surface (which I then had
planned to take care of), no such error actually appeared.
Considering commit 422f1c65c9e9 ("Report an error for script
multiply defined symbols"), the description of which amends the
title by "or maybe not just yet", wouldn't it be helpful for
documentation to match reality? It could still state that such
duplicates may become an error in the future, but I don't think
it should lead people to believe that such an error will in
fact get diagnosed as long as this doesn't actually happen.
Perhaps, if there are scripts in the wild which do this, this
could at least be a warning for the time being?
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ld's sole user of update_definedness() vs documentation
2021-02-19 11:16 ld's sole user of update_definedness() vs documentation Jan Beulich
@ 2021-02-21 4:05 ` Alan Modra
2021-02-22 7:35 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Alan Modra @ 2021-02-21 4:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
> Perhaps, if there are scripts in the wild which do this, this
> could at least be a warning for the time being?
Yes, I didn't mean to leave this go so long without doing something.
I just committed the following.
Note that we don't even warn if scripts adjust a symbol as in
ld-elf/var1 and ld-scripts/pr14962.
include/
* bfdlink.h (struct bfd_link_info): Add warn_multiple_definition.
ld/
* ldexp.c (exp_fold_tree_1): Warn on script defining a symbol
defined in an object file.
* ldmain.c (multiple_definition): Heed info->warn_multiple_definition.
* testsuite/ld-scripts/defined5.d: Expect a warning.
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 079e31227e..95728b6f03 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -465,12 +465,16 @@ struct bfd_link_info
statics. */
unsigned int task_link: 1;
- /* TRUE if ok to have multiple definition. */
+ /* TRUE if ok to have multiple definitions, without warning. */
unsigned int allow_multiple_definition: 1;
- /* TRUE if ok to have prohibit multiple definition of absolute symbols. */
+ /* TRUE if multiple definition of absolute symbols (eg. from -R) should
+ be reported. */
unsigned int prohibit_multiple_definition_absolute: 1;
+ /* TRUE if multiple definitions should only warn. */
+ unsigned int warn_multiple_definition: 1;
+
/* TRUE if ok to have version with no definition. */
unsigned int allow_undefined_version: 1;
diff --git a/ld/ldexp.c b/ld/ldexp.c
index 084bb17c4b..016784505b 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -1186,16 +1186,19 @@ exp_fold_tree_1 (etree_type *tree)
{
if (expld.result.section == NULL)
expld.result.section = expld.section;
- if (!update_definedness (tree->assign.dst, h) && 0)
+ if (!update_definedness (tree->assign.dst, h)
+ && expld.assign_name != NULL)
{
- /* 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. */
+ /* Symbol was already defined, and the script isn't
+ modifying the symbol value for some reason as in
+ ld-elf/var1 and ld-scripts/pr14962.
+ For now this is only a warning. */
+ unsigned int warn = link_info.warn_multiple_definition;
+ link_info.warn_multiple_definition = 1;
(*link_info.callbacks->multiple_definition)
(&link_info, h, link_info.output_bfd,
expld.result.section, expld.result.value);
+ link_info.warn_multiple_definition = warn;
}
if (expld.phase == lang_fixed_phase_enum)
{
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 863df0293e..5c88ee744f 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -1074,7 +1074,9 @@ multiple_definition (struct bfd_link_info *info,
nval = oval;
obfd = NULL;
}
- einfo (_("%X%P: %C: multiple definition of `%pT'"),
+ if (!info->warn_multiple_definition)
+ einfo ("%X");
+ einfo (_("%P: %C: multiple definition of `%pT'"),
nbfd, nsec, nval, name);
if (obfd != NULL)
einfo (_("; %D: first defined here"), obfd, osec, oval);
diff --git a/ld/testsuite/ld-scripts/defined5.d b/ld/testsuite/ld-scripts/defined5.d
index 2530c0e09e..7aa680b85a 100644
--- a/ld/testsuite/ld-scripts/defined5.d
+++ b/ld/testsuite/ld-scripts/defined5.d
@@ -1,10 +1,11 @@
#ld: -Tdefined5.t
+#warning: .*multiple definition of `defined'.*
#nm: -B
-#source: defined5.s
#xfail: [is_xcoff_format]
# xcoff outputs value of "defined" from the object file
-# Check that arithmetic on DEFINED works.
+# Check that a script can override an object file symbol, if multiple
+# definitions are allowed. See pr12356.
#...
0+1000 D defined
#pass
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ld's sole user of update_definedness() vs documentation
2021-02-21 4:05 ` Alan Modra
@ 2021-02-22 7:35 ` Jan Beulich
2021-02-24 2:34 ` Alan Modra
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2021-02-22 7:35 UTC (permalink / raw)
To: Alan Modra; +Cc: Binutils
On 21.02.2021 05:05, Alan Modra wrote:
>> Perhaps, if there are scripts in the wild which do this, this
>> could at least be a warning for the time being?
>
> Yes, I didn't mean to leave this go so long without doing something.
> I just committed the following.
Thanks for the quick fix. Wouldn't it be better to also adjust the
PROVIDE() doc then, replacing "error" by "warning"? I can certainly
do this, if you agree.
Jan
> Note that we don't even warn if scripts adjust a symbol as in
> ld-elf/var1 and ld-scripts/pr14962.
>
> include/
> * bfdlink.h (struct bfd_link_info): Add warn_multiple_definition.
> ld/
> * ldexp.c (exp_fold_tree_1): Warn on script defining a symbol
> defined in an object file.
> * ldmain.c (multiple_definition): Heed info->warn_multiple_definition.
> * testsuite/ld-scripts/defined5.d: Expect a warning.
>
> diff --git a/include/bfdlink.h b/include/bfdlink.h
> index 079e31227e..95728b6f03 100644
> --- a/include/bfdlink.h
> +++ b/include/bfdlink.h
> @@ -465,12 +465,16 @@ struct bfd_link_info
> statics. */
> unsigned int task_link: 1;
>
> - /* TRUE if ok to have multiple definition. */
> + /* TRUE if ok to have multiple definitions, without warning. */
> unsigned int allow_multiple_definition: 1;
>
> - /* TRUE if ok to have prohibit multiple definition of absolute symbols. */
> + /* TRUE if multiple definition of absolute symbols (eg. from -R) should
> + be reported. */
> unsigned int prohibit_multiple_definition_absolute: 1;
>
> + /* TRUE if multiple definitions should only warn. */
> + unsigned int warn_multiple_definition: 1;
> +
> /* TRUE if ok to have version with no definition. */
> unsigned int allow_undefined_version: 1;
>
> diff --git a/ld/ldexp.c b/ld/ldexp.c
> index 084bb17c4b..016784505b 100644
> --- a/ld/ldexp.c
> +++ b/ld/ldexp.c
> @@ -1186,16 +1186,19 @@ exp_fold_tree_1 (etree_type *tree)
> {
> if (expld.result.section == NULL)
> expld.result.section = expld.section;
> - if (!update_definedness (tree->assign.dst, h) && 0)
> + if (!update_definedness (tree->assign.dst, h)
> + && expld.assign_name != NULL)
> {
> - /* 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. */
> + /* Symbol was already defined, and the script isn't
> + modifying the symbol value for some reason as in
> + ld-elf/var1 and ld-scripts/pr14962.
> + For now this is only a warning. */
> + unsigned int warn = link_info.warn_multiple_definition;
> + link_info.warn_multiple_definition = 1;
> (*link_info.callbacks->multiple_definition)
> (&link_info, h, link_info.output_bfd,
> expld.result.section, expld.result.value);
> + link_info.warn_multiple_definition = warn;
> }
> if (expld.phase == lang_fixed_phase_enum)
> {
> diff --git a/ld/ldmain.c b/ld/ldmain.c
> index 863df0293e..5c88ee744f 100644
> --- a/ld/ldmain.c
> +++ b/ld/ldmain.c
> @@ -1074,7 +1074,9 @@ multiple_definition (struct bfd_link_info *info,
> nval = oval;
> obfd = NULL;
> }
> - einfo (_("%X%P: %C: multiple definition of `%pT'"),
> + if (!info->warn_multiple_definition)
> + einfo ("%X");
> + einfo (_("%P: %C: multiple definition of `%pT'"),
> nbfd, nsec, nval, name);
> if (obfd != NULL)
> einfo (_("; %D: first defined here"), obfd, osec, oval);
> diff --git a/ld/testsuite/ld-scripts/defined5.d b/ld/testsuite/ld-scripts/defined5.d
> index 2530c0e09e..7aa680b85a 100644
> --- a/ld/testsuite/ld-scripts/defined5.d
> +++ b/ld/testsuite/ld-scripts/defined5.d
> @@ -1,10 +1,11 @@
> #ld: -Tdefined5.t
> +#warning: .*multiple definition of `defined'.*
> #nm: -B
> -#source: defined5.s
> #xfail: [is_xcoff_format]
> # xcoff outputs value of "defined" from the object file
>
> -# Check that arithmetic on DEFINED works.
> +# Check that a script can override an object file symbol, if multiple
> +# definitions are allowed. See pr12356.
> #...
> 0+1000 D defined
> #pass
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ld's sole user of update_definedness() vs documentation
2021-02-22 7:35 ` Jan Beulich
@ 2021-02-24 2:34 ` Alan Modra
0 siblings, 0 replies; 4+ messages in thread
From: Alan Modra @ 2021-02-24 2:34 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Mon, Feb 22, 2021 at 08:35:52AM +0100, Jan Beulich wrote:
> On 21.02.2021 05:05, Alan Modra wrote:
> >> Perhaps, if there are scripts in the wild which do this, this
> >> could at least be a warning for the time being?
> >
> > Yes, I didn't mean to leave this go so long without doing something.
> > I just committed the following.
>
> Thanks for the quick fix. Wouldn't it be better to also adjust the
> PROVIDE() doc then, replacing "error" by "warning"? I can certainly
> do this, if you agree.
Yes please.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-24 2:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 11:16 ld's sole user of update_definedness() vs documentation Jan Beulich
2021-02-21 4:05 ` Alan Modra
2021-02-22 7:35 ` Jan Beulich
2021-02-24 2:34 ` Alan Modra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).