From: Fangrui Song <maskray@google.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: Binutils <binutils@sourceware.org>
Subject: Re: V3 [PATCH] ld: Add --export-dynamic-symbol and --export-dynamic-symbol-list
Date: Sat, 23 May 2020 14:56:32 -0700 [thread overview]
Message-ID: <20200523215632.atyzuawjevr6g5do@google.com> (raw)
In-Reply-To: <CAMe9rOrs+shaYZPEHGm1CsoBvkGMymmw0kFctL91nzxKj1SReA@mail.gmail.com>
On 2020-05-23, H.J. Lu wrote:
>On Sat, May 23, 2020 at 11:29 AM Fangrui Song <maskray@google.com> wrote:
>>
>> On 2020-05-23, H.J. Lu wrote:
>> >From: Fangrui Song <maskray@google.com>
>> >
>> >--export-dynamic-symbol-list is like a dynamic list, but without
>> >the symbolic property for unspecified symbols.
>> >
>> >When creating an executable, --export-dynamic-symbol-list is treated
>> >like --dynamic-list.
>> >
>> >When creating a shared library, it is treated like --dynamic-list if
>> >-Bsymbolic or --dynamic-list are used, otherwise, it is ignored, so
>> >that references to matched symbols will not be bound to the definitions
>> >within the shared library.
>> >
>> >2020-05-XX Fangrui Song <maskray@google.com>
>> > H.J. Lu <hongjiu.lu@intel.com>
>> >
>> > PR ld/25910
>> > * NEWS: Mention --export-dynamic-symbol[-list].
>> > * ld.texi: Document --export-dynamic-symbol[-list].
>> > * ldgram.y: Pass current_dynamic_list_p to
>> > lang_append_dynamic_list.
>> > * ldlang.c (current_dynamic_list_p): New.
>> > (ang_append_dynamic_list): Updated to take a pointer to
>> > struct bfd_elf_dynamic_list * argument instead of using
>> > link_info.dynamic_list.
>> > (lang_append_dynamic_list_cpp_typeinfo): Pass
>> > &link_info.dynamic_list to ang_append_dynamic_list.
>> > (lang_append_dynamic_list_cpp_new): Likewise.
>> > * ldlang.h (current_dynamic_list_p): New.
>> > (lang_append_dynamic_list): Add a pointer to
>> > struct bfd_elf_dynamic_list * argument.
>> > * ldlex.h (option_values): Add OPTION_EXPORT_DYNAMIC_SYMBOL and
>> > OPTION_EXPORT_DYNAMIC_SYMBOL_LIST.
>> > * lexsup.c (ld_options): Add entries for
>> > OPTION_EXPORT_DYNAMIC_SYMBOL and
>> > OPTION_EXPORT_DYNAMIC_SYMBOL_LIST.
>> > (parse_args): Handle --export-dynamic-symbol and
>> > --export-dynamic-symbol-list.
>> > * testsuite/ld-dynamic/export-dynamic-symbol-1.d: New.
>> > * testsuite/ld-dynamic/export-dynamic-symbol-2.d: New.
>> > * testsuite/ld-dynamic/export-dynamic-symbol-glob.d: New.
>> > * testsuite/ld-dynamic/export-dynamic-symbol-list-1.d: New.
>> > * testsuite/ld-dynamic/export-dynamic-symbol-list-2.d: New.
>> > * testsuite/ld-dynamic/export-dynamic-symbol-list-glob.d: New.
>> > * testsuite/ld-dynamic/export-dynamic-symbol.exp: New.
>> > * testsuite/ld-dynamic/export-dynamic-symbol.s: New.
>> > * testsuite/ld-dynamic/foo-bar.list: New.
>> > * testsuite/ld-dynamic/foo.list: New.
>> > * testsuite/ld-dynamic/foo.s: New.
>> > * testsuite/ld-dynamic/fstar.list: New.
>> > * testsuite/ld-elf/dlempty.list: New.
>> > * testsuite/ld-elf/shared.exp: Add tests for
>> > --export-dynamic-symbol and --export-dynamic-symbol-list.
>> >---
>> > ld/NEWS | 3 +
>> > ld/ld.texi | 18 +++++
>> > ld/ldgram.y | 2 +-
>> > ld/ldlang.c | 16 ++--
>> > ld/ldlang.h | 5 +-
>> > ld/ldlex.h | 2 +
>> > ld/lexsup.c | 75 +++++++++++++++++++
>> > .../ld-dynamic/export-dynamic-symbol-1.d | 9 +++
>> > .../ld-dynamic/export-dynamic-symbol-2.d | 9 +++
>> > .../ld-dynamic/export-dynamic-symbol-glob.d | 8 ++
>> > .../ld-dynamic/export-dynamic-symbol-list-1.d | 9 +++
>> > .../ld-dynamic/export-dynamic-symbol-list-2.d | 9 +++
>> > .../export-dynamic-symbol-list-glob.d | 8 ++
>> > .../ld-dynamic/export-dynamic-symbol.exp | 39 ++++++++++
>> > .../ld-dynamic/export-dynamic-symbol.s | 9 +++
>> > ld/testsuite/ld-dynamic/foo-bar.list | 1 +
>> > ld/testsuite/ld-dynamic/foo.list | 1 +
>> > ld/testsuite/ld-dynamic/foo.s | 4 +
>> > ld/testsuite/ld-dynamic/fstar.list | 1 +
>> > ld/testsuite/ld-elf/dlempty.list | 3 +
>> > ld/testsuite/ld-elf/shared.exp | 36 +++++++++
>> > 21 files changed, 258 insertions(+), 9 deletions(-)
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol-1.d
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol-2.d
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol-glob.d
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol-list-1.d
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol-list-2.d
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol-list-glob.d
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol.exp
>> > create mode 100644 ld/testsuite/ld-dynamic/export-dynamic-symbol.s
>> > create mode 100644 ld/testsuite/ld-dynamic/foo-bar.list
>> > create mode 100644 ld/testsuite/ld-dynamic/foo.list
>> > create mode 100644 ld/testsuite/ld-dynamic/foo.s
>> > create mode 100644 ld/testsuite/ld-dynamic/fstar.list
>> > create mode 100644 ld/testsuite/ld-elf/dlempty.list
>> >
>> >diff --git a/ld/NEWS b/ld/NEWS
>> >index 9f5bbe51cf..870c480bde 100644
>> >--- a/ld/NEWS
>> >+++ b/ld/NEWS
>> >@@ -1,5 +1,8 @@
>> > -*- text -*-
>> >
>> >+* Add ELF linker command-line options, --export-dynamic-symbol and
>> >+ --export-dynamic-symbol-list, to make symbols dynamic.
>> >+
>> > * Add command-line options --enable-non-contiguous-regions and
>> > --enable-non-contiguous-regions-warnings.
>> >
>> >diff --git a/ld/ld.texi b/ld/ld.texi
>> >index 4dc78e65fa..92e47c6324 100644
>> >--- a/ld/ld.texi
>> >+++ b/ld/ld.texi
>> >@@ -569,6 +569,24 @@ Note that this option is specific to ELF targeted ports. PE targets
>> > support a similar function to export all symbols from a DLL or EXE; see
>> > the description of @samp{--export-all-symbols} below.
>> >
>> >+@kindex --export-dynamic-symbol=@var{glob}
>> >+@cindex export dynamic symbol
>> >+@item --export-dynamic-symbol=@var{glob}
>> >+When creating a dynamically linked executable, symbols matching
>> >+@var{glob} will be added to the dynamic symbol table. When creating a
>> >+shared library, references to symbols matching @var{glob} will not be
>> >+bound to the definitions within the shared library. This option is a
>> >+no-op when creating a shared library and @samp{-Bsymbolic} or
>> >+@samp{--dynamic-list} are not specified. This option is only meaningful
>> >+on ELF platforms which support shared libraries.
>>
>> Is --export-dynamic-symbol still effective for -shared?
>> I think either way is ok.
>
>Yes.
>
>> >+@kindex --export-dynamic-symbol-list=@var{file}
>> >+@cindex export dynamic symbol list
>> >+@item --export-dynamic-symbol-list=@var{file}
>> >+Specify a @samp{--export-dynamic-symbol} for each pattern in the file.
>> >+The format of the file is the same as the version node without
>> >+scope and node name. See @ref{VERSION} for more information.
>> >+
>> > @ifclear SingleFormat
>> > @cindex big-endian objects
>> > @cindex endianness
>> >diff --git a/ld/ldgram.y b/ld/ldgram.y
>> >index df5c035c03..36845c4c30 100644
>> >--- a/ld/ldgram.y
>> >+++ b/ld/ldgram.y
>> >@@ -1313,7 +1313,7 @@ dynamic_list_node:
>> > dynamic_list_tag:
>> > vers_defns ';'
>> > {
>> >- lang_append_dynamic_list ($1);
>> >+ lang_append_dynamic_list (current_dynamic_list_p, $1);
>> > }
>> > ;
>> >
>> >diff --git a/ld/ldlang.c b/ld/ldlang.c
>> >index 3d653d460d..14a6a577d2 100644
>> >--- a/ld/ldlang.c
>> >+++ b/ld/ldlang.c
>> >@@ -118,6 +118,7 @@ lang_statement_list_type file_chain = { NULL, NULL };
>> > lang_statement_union). */
>> > lang_statement_list_type input_file_chain;
>> > static const char *current_input_file;
>> >+struct bfd_elf_dynamic_list **current_dynamic_list_p;
>> > struct bfd_sym_chain entry_symbol = { NULL, NULL };
>> > const char *entry_section = ".text";
>> > struct lang_input_statement_flags input_flags;
>> >@@ -9324,15 +9325,16 @@ lang_add_unique (const char *name)
>> > /* Append the list of dynamic symbols to the existing one. */
>> >
>> > void
>> >-lang_append_dynamic_list (struct bfd_elf_version_expr *dynamic)
>> >+lang_append_dynamic_list (struct bfd_elf_dynamic_list **list_p,
>> >+ struct bfd_elf_version_expr *dynamic)
>> > {
>> >- if (link_info.dynamic_list)
>> >+ if (*list_p)
>> > {
>> > struct bfd_elf_version_expr *tail;
>> > for (tail = dynamic; tail->next != NULL; tail = tail->next)
>> > ;
>> >- tail->next = link_info.dynamic_list->head.list;
>> >- link_info.dynamic_list->head.list = dynamic;
>> >+ tail->next = (*list_p)->head.list;
>> >+ (*list_p)->head.list = dynamic;
>> > }
>> > else
>> > {
>> >@@ -9341,7 +9343,7 @@ lang_append_dynamic_list (struct bfd_elf_version_expr *dynamic)
>> > d = (struct bfd_elf_dynamic_list *) xcalloc (1, sizeof *d);
>> > d->head.list = dynamic;
>> > d->match = lang_vers_match;
>> >- link_info.dynamic_list = d;
>> >+ *list_p = d;
>> > }
>> > }
>> >
>> >@@ -9363,7 +9365,7 @@ lang_append_dynamic_list_cpp_typeinfo (void)
>> > dynamic = lang_new_vers_pattern (dynamic, symbols [i], "C++",
>> > FALSE);
>> >
>> >- lang_append_dynamic_list (dynamic);
>> >+ lang_append_dynamic_list (&link_info.dynamic_list, dynamic);
>> > }
>> >
>> > /* Append the list of C++ operator new and delete dynamic symbols to the
>> >@@ -9384,7 +9386,7 @@ lang_append_dynamic_list_cpp_new (void)
>> > dynamic = lang_new_vers_pattern (dynamic, symbols [i], "C++",
>> > FALSE);
>> >
>> >- lang_append_dynamic_list (dynamic);
>> >+ lang_append_dynamic_list (&link_info.dynamic_list, dynamic);
>> > }
>> >
>> > /* Scan a space and/or comma separated string of features. */
>> >diff --git a/ld/ldlang.h b/ld/ldlang.h
>> >index 3018c3e2ba..529ccd1585 100644
>> >--- a/ld/ldlang.h
>> >+++ b/ld/ldlang.h
>> >@@ -513,6 +513,8 @@ extern bfd_boolean entry_from_cmdline;
>> > extern lang_statement_list_type file_chain;
>> > extern lang_statement_list_type input_file_chain;
>> >
>> >+extern struct bfd_elf_dynamic_list **current_dynamic_list_p;
>> >+
>> > extern int lang_statement_iteration;
>> > extern struct asneeded_minfo **asneeded_list_tail;
>> >
>> >@@ -673,7 +675,8 @@ extern struct bfd_elf_version_deps *lang_add_vers_depend
>> > (struct bfd_elf_version_deps *, const char *);
>> > extern void lang_register_vers_node
>> > (const char *, struct bfd_elf_version_tree *, struct bfd_elf_version_deps *);
>> >-extern void lang_append_dynamic_list (struct bfd_elf_version_expr *);
>> >+extern void lang_append_dynamic_list (struct bfd_elf_dynamic_list **,
>> >+ struct bfd_elf_version_expr *);
>> > extern void lang_append_dynamic_list_cpp_typeinfo (void);
>> > extern void lang_append_dynamic_list_cpp_new (void);
>> > extern void lang_add_unique
>> >diff --git a/ld/ldlex.h b/ld/ldlex.h
>> >index 22b928d2d9..2c8d043a09 100644
>> >--- a/ld/ldlex.h
>> >+++ b/ld/ldlex.h
>> >@@ -81,6 +81,8 @@ enum option_values
>> > OPTION_DYNAMIC_LIST_CPP_NEW,
>> > OPTION_DYNAMIC_LIST_CPP_TYPEINFO,
>> > OPTION_DYNAMIC_LIST_DATA,
>> >+ OPTION_EXPORT_DYNAMIC_SYMBOL,
>> >+ OPTION_EXPORT_DYNAMIC_SYMBOL_LIST,
>> > OPTION_WARN_COMMON,
>> > OPTION_WARN_CONSTRUCTORS,
>> > OPTION_WARN_FATAL,
>> >diff --git a/ld/lexsup.c b/ld/lexsup.c
>> >index fe9526b527..2bac1631d6 100644
>> >--- a/ld/lexsup.c
>> >+++ b/ld/lexsup.c
>> >@@ -504,6 +504,10 @@ static const struct ld_option ld_options[] =
>> > '\0', NULL, N_("Use C++ typeinfo dynamic list"), TWO_DASHES },
>> > { {"dynamic-list", required_argument, NULL, OPTION_DYNAMIC_LIST},
>> > '\0', N_("FILE"), N_("Read dynamic list"), TWO_DASHES },
>> >+ { {"export-dynamic-symbol", required_argument, NULL, OPTION_EXPORT_DYNAMIC_SYMBOL},
>> >+ '\0', N_("SYMBOL"), N_("Export the specified symbol"), EXACTLY_TWO_DASHES },
>> >+ { {"export-dynamic-symbol-list", required_argument, NULL, OPTION_EXPORT_DYNAMIC_SYMBOL_LIST},
>> >+ '\0', N_("FILE"), N_("Read export dynamic symbol list"), EXACTLY_TWO_DASHES },
>> > { {"warn-common", no_argument, NULL, OPTION_WARN_COMMON},
>> > '\0', NULL, N_("Warn about duplicate common symbols"), TWO_DASHES },
>> > { {"warn-constructors", no_argument, NULL, OPTION_WARN_CONSTRUCTORS},
>> >@@ -581,6 +585,7 @@ parse_args (unsigned argc, char **argv)
>> > dynamic_list_data,
>> > dynamic_list
>> > } opt_dynamic_list = dynamic_list_unset;
>> >+ struct bfd_elf_dynamic_list *export_list = NULL;
>> >
>> > shortopts = (char *) xmalloc (OPTION_COUNT * 3 + 2);
>> > longopts = (struct option *)
>> >@@ -1418,6 +1423,7 @@ parse_args (unsigned argc, char **argv)
>> > ldfile_open_command_file (optarg);
>> > saved_script_handle = hold_script_handle;
>> > parser_input = input_dynamic_list;
>> >+ current_dynamic_list_p = &link_info.dynamic_list;
>> > yyparse ();
>> > }
>> > if (opt_dynamic_list != dynamic_list_data)
>> >@@ -1425,6 +1431,29 @@ parse_args (unsigned argc, char **argv)
>> > if (opt_symbolic == symbolic)
>> > opt_symbolic = symbolic_unset;
>> > break;
>> >+ case OPTION_EXPORT_DYNAMIC_SYMBOL:
>> >+ {
>> >+ struct bfd_elf_version_expr *expr
>> >+ = lang_new_vers_pattern (NULL, xstrdup (optarg), NULL,
>> >+ FALSE);
>> >+ lang_append_dynamic_list (&export_list, expr);
>> >+ }
>> >+ break;
>> >+ case OPTION_EXPORT_DYNAMIC_SYMBOL_LIST:
>> >+ /* This option indicates a small script that only specifies
>> >+ an export list. Read it, but don't assume that we've
>> >+ seen a linker script. */
>> >+ {
>> >+ FILE *hold_script_handle;
>> >+
>> >+ hold_script_handle = saved_script_handle;
>> >+ ldfile_open_command_file (optarg);
>> >+ saved_script_handle = hold_script_handle;
>> >+ parser_input = input_dynamic_list;
>> >+ current_dynamic_list_p = &export_list;
>> >+ yyparse ();
>> >+ }
>> >+ break;
>> > case OPTION_WARN_COMMON:
>> > config.warn_common = TRUE;
>> > break;
>> >@@ -1632,6 +1661,52 @@ parse_args (unsigned argc, char **argv)
>> > && command_line.check_section_addresses < 0)
>> > command_line.check_section_addresses = 0;
>> >
>> >+ if (export_list && !bfd_link_relocatable (&link_info))
>> >+ {
>> >+ struct bfd_elf_version_expr *head = export_list->head.list;
>> >+ struct bfd_elf_version_expr *next;
>> >+
>> >+ /* For --export-dynamic-symbol[-list]:
>> >+ 1. When building executable, treat like --dynamic-list.
>> >+ 2. When building shared object:
>> >+ a. If -Bsymbolic or --dynamic-list are used, treat like
>> >+ --dynamic-list.
>> >+ b. Otherwise, ignored.
>> >+ */
>> >+ bfd_boolean kept = (bfd_link_executable (&link_info)
>> >+ || opt_symbolic != symbolic_unset
>> >+ || opt_dynamic_list != dynamic_list_unset);
>> >+
>> >+ if (kept)
>> >+ {
>> >+ /* Append the export list to link_info.dynamic_list. */
>> >+ if (link_info.dynamic_list)
>> >+ {
>> >+ for (next = head; next->next != NULL; next = next->next)
>> >+ ;
>> >+ next->next = link_info.dynamic_list->head.list;
>> >+ link_info.dynamic_list->head.list = head;
>> >+ }
>> >+ else
>> >+ link_info.dynamic_list = export_list;
>> >+
>> >+ if (opt_dynamic_list != dynamic_list_data)
>> >+ opt_dynamic_list = dynamic_list;
>> >+ if (opt_symbolic == symbolic)
>> >+ opt_symbolic = symbolic_unset;
>>
>> The interaction with -Bsymbolic appears to be more complex now.
>> I think users will pretty much avoid the combination of -Bsymbolic and
>> (--dynamic-list or --export-dynamic-symbol or --export-dynamic-symbol-list).
>>
>> So I beg for a re-consideration of
>> https://sourceware.org/bugzilla/show_bug.cgi?id=26018
>> https://sourceware.org/pipermail/binutils/2020-May/111223.html
>>
>
>Here is the updated patch on top of
>
>https://sourceware.org/pipermail/binutils/2020-May/111228.html
Thanks for PATCH v3. LGTM.
I have verified the semantics of new tests dl2d ~ dl2k.
next prev parent reply other threads:[~2020-05-23 21:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-23 14:47 V2 " H.J. Lu
2020-05-23 18:29 ` Fangrui Song
2020-05-23 19:50 ` V3 " H.J. Lu
2020-05-23 21:56 ` Fangrui Song [this message]
2020-05-27 14:29 ` V4 " H.J. Lu
2020-05-27 16:19 ` Fangrui Song
2020-05-27 16:24 ` H.J. Lu
2020-05-27 20:24 ` V5 " H.J. Lu
2020-05-28 12:44 ` V6 " H.J. Lu
2020-06-03 10:51 ` Nick Clifton
2020-06-03 12:12 ` Alan Modra
2020-06-03 13:35 ` [PATCH] x86: Silence -fsanitize=undefined H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200523215632.atyzuawjevr6g5do@google.com \
--to=maskray@google.com \
--cc=binutils@sourceware.org \
--cc=hjl.tools@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).