[PATCH v2 1/2] ld: Add support for a new option, -exclude-symbols, in COFF object file directives

[PATCH v2 1/2] ld: Add support for a new option, -exclude-symbols, in COFF object file directives

[Martin Storsjo]

This maps to the same as ld's --exclude-symbols command line option,
but allowing specifying the option via directives embedded in the
object files instead of passed manually on the command line.
---
Fixed parsing multiple comma separated symbols. While pe_dll_add_excludes
does tokenize the string, the symbol tokens in deffilep.y don't
include commas, so we need to handle that at that level.
---
 ld/deffile.h                                  |  6 +++
 ld/deffilep.y                                 | 50 ++++++++++++++++++-
 ld/pe-dll.c                                   | 10 ++++
 ld/testsuite/ld-pe/exclude-symbols-embedded.d | 10 ++++
 ld/testsuite/ld-pe/exclude-symbols-embedded.s | 15 ++++++
 ld/testsuite/ld-pe/pe.exp                     |  2 +
 6 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-pe/exclude-symbols-embedded.d
 create mode 100644 ld/testsuite/ld-pe/exclude-symbols-embedded.s

diff --git a/ld/deffile.h b/ld/deffile.h
index d68fa8c6ba1..306ae3a6f75 100644
--- a/ld/deffile.h
+++ b/ld/deffile.h
@@ -61,6 +61,11 @@ typedef struct def_file_aligncomm {
   unsigned int alignment;	/* log-2 alignment.        */
 } def_file_aligncomm;
 
+typedef struct def_file_exclude_symbol {
+  struct def_file_exclude_symbol *next;	/* Chain pointer.  */
+  char *symbol_name;		/* Name of excluded symbol.  */
+} def_file_exclude_symbol;
+
 typedef struct def_file {
   /* From the NAME or LIBRARY command.  */
   char *name;
@@ -94,6 +99,7 @@ typedef struct def_file {
 
   /* Only expected from .drectve sections, not .DEF files.  */
   def_file_aligncomm *aligncomms;
+  def_file_exclude_symbol *exclude_symbols;
 
 } def_file;
 
diff --git a/ld/deffilep.y b/ld/deffilep.y
index 3f610ddbe14..126f33a1352 100644
--- a/ld/deffilep.y
+++ b/ld/deffilep.y
@@ -101,6 +101,7 @@ static void def_stacksize (int, int);
 static void def_version (int, int);
 static void def_directive (char *);
 static void def_aligncomm (char *str, int align);
+static void def_exclude_symbols (char *str);
 static int def_parse (void);
 static void def_error (const char *);
 static int def_lex (void);
@@ -121,7 +122,7 @@ static const char *lex_parse_string_end = 0;
 
 %token NAME LIBRARY DESCRIPTION STACKSIZE_K HEAPSIZE CODE DATAU DATAL
 %token SECTIONS EXPORTS IMPORTS VERSIONK BASE CONSTANTU CONSTANTL
-%token PRIVATEU PRIVATEL ALIGNCOMM
+%token PRIVATEU PRIVATEL ALIGNCOMM EXCLUDE_SYMBOLS
 %token READ WRITE EXECUTE SHARED_K NONAMEU NONAMEL DIRECTIVE EQUAL
 %token <id> ID
 %token <digits> DIGITS
@@ -131,7 +132,7 @@ static const char *lex_parse_string_end = 0;
 %type  <number> opt_ordinal
 %type  <number> attr attr_list opt_number exp_opt_list exp_opt
 %type  <id> opt_name opt_name2 opt_equal_name anylang_id opt_id
-%type  <id> opt_equalequal_name
+%type  <id> opt_equalequal_name symbol_list
 %type  <id_const> keyword_as_name
 
 %%
@@ -155,6 +156,7 @@ command:
 	|	VERSIONK NUMBER '.' NUMBER { def_version ($2, $4);}
 	|	DIRECTIVE ID { def_directive ($2);}
 	|	ALIGNCOMM anylang_id ',' NUMBER { def_aligncomm ($2, $4);}
+	|	EXCLUDE_SYMBOLS symbol_list
 	;
 
 
@@ -333,6 +335,11 @@ anylang_id: ID		{ $$ = $1; }
 	  }
 	;
 
+symbol_list:
+	anylang_id { def_exclude_symbols ($1); }
+	|	symbol_list ',' anylang_id { def_exclude_symbols ($3); }
+	;
+
 opt_digits: DIGITS	{ $$ = $1; }
 	|		{ $$ = ""; }
 	;
@@ -488,6 +495,15 @@ def_file_free (def_file *fdef)
       free (c);
     }
 
+  while (fdef->exclude_symbols)
+    {
+      def_file_exclude_symbol *e = fdef->exclude_symbols;
+
+      fdef->exclude_symbols = fdef->exclude_symbols->next;
+      free (e->symbol_name);
+      free (e);
+    }
+
   free (fdef);
 }
 
@@ -946,6 +962,7 @@ diropts[] =
   { "-attr", SECTIONS },
   { "-export", EXPORTS },
   { "-aligncomm", ALIGNCOMM },
+  { "-exclude-symbols", EXCLUDE_SYMBOLS },
   { 0, 0 }
 };
 
@@ -1261,6 +1278,35 @@ def_aligncomm (char *str, int align)
     }
 }
 
+static void
+def_exclude_symbols (char *str)
+{
+  def_file_exclude_symbol *c, *p;
+
+  p = NULL;
+  c = def->exclude_symbols;
+  while (c != NULL)
+    {
+      int e = strcmp (c->symbol_name, str);
+      if (!e)
+        return;
+      c = (p = c)->next;
+    }
+
+  c = xmalloc (sizeof (def_file_exclude_symbol));
+  c->symbol_name = xstrdup (str);
+  if (!p)
+    {
+      c->next = def->exclude_symbols;
+      def->exclude_symbols = c;
+    }
+  else
+    {
+      c->next = p->next;
+      p->next = c;
+    }
+}
+
 static void
 def_error (const char *err)
 {
diff --git a/ld/pe-dll.c b/ld/pe-dll.c
index db2c75da9b5..33be53d6164 100644
--- a/ld/pe-dll.c
+++ b/ld/pe-dll.c
@@ -718,6 +718,16 @@ process_def_file_and_drectve (bfd *abfd ATTRIBUTE_UNUSED, struct bfd_link_info *
 	}
     }
 
+  if (pe_def_file->exclude_symbols)
+    {
+      def_file_exclude_symbol *ac = pe_def_file->exclude_symbols;
+      while (ac)
+	{
+	  pe_dll_add_excludes (ac->symbol_name, EXCLUDESYMS);
+	  ac = ac->next;
+	}
+    }
+
   /* If we are building an executable and there is nothing
      to export, we do not build an export table at all.  */
   if (bfd_link_executable (info) && pe_def_file->num_exports == 0
diff --git a/ld/testsuite/ld-pe/exclude-symbols-embedded.d b/ld/testsuite/ld-pe/exclude-symbols-embedded.d
new file mode 100644
index 00000000000..cf00cc619c6
--- /dev/null
+++ b/ld/testsuite/ld-pe/exclude-symbols-embedded.d
@@ -0,0 +1,10 @@
+#source: exclude-symbols-embedded.s
+#target: i*86-*-cygwin* i*86-*-pe i*86-*-mingw*
+#ld: -shared
+#objdump: -p
+
+#...
+.*\[[ 	]*0\] sym1
+.*\[[ 	]*1\] sym3
+.*\[[ 	]*2\] sym5
+#pass
diff --git a/ld/testsuite/ld-pe/exclude-symbols-embedded.s b/ld/testsuite/ld-pe/exclude-symbols-embedded.s
new file mode 100644
index 00000000000..20564b57017
--- /dev/null
+++ b/ld/testsuite/ld-pe/exclude-symbols-embedded.s
@@ -0,0 +1,15 @@
+.global _sym1
+.global _sym2
+.global _sym3
+.global _sym4
+.global _sym5
+_sym1:
+_sym2:
+_sym3:
+_sym4:
+_sym5:
+  ret
+
+.section .drectve,"yn"
+.ascii " -exclude-symbols:sym2,unknownsym"
+.ascii " -exclude-symbols:sym4"
diff --git a/ld/testsuite/ld-pe/pe.exp b/ld/testsuite/ld-pe/pe.exp
index 413b316f152..8dfdd0cc16e 100644
--- a/ld/testsuite/ld-pe/pe.exp
+++ b/ld/testsuite/ld-pe/pe.exp
@@ -124,3 +124,5 @@ set foreign_sym_test {
 # MCore rearranges symbol order.
 setup_xfail mcore-*-pe
 run_ld_link_tests $foreign_sym_test
+
+run_dump_test "exclude-symbols-embedded"
-- 
2.25.1

[PATCH v2 2/2] ld: Support the -exclude-symbols option via COFF def files, with the EXCLUDE_SYMBOLS keyword

[Martin Storsjo]

This was requested in review.
---
WIP: The testcase doesn't run successfully. How do I pass the
full path to exclude-symbols-def.def as parameter to the ld command?
---
 ld/deffilep.y                              |  3 +++
 ld/testsuite/ld-pe/exclude-symbols-def.d   | 10 ++++++++++
 ld/testsuite/ld-pe/exclude-symbols-def.def |  4 ++++
 ld/testsuite/ld-pe/exclude-symbols-def.s   | 11 +++++++++++
 ld/testsuite/ld-pe/pe.exp                  |  1 +
 5 files changed, 29 insertions(+)
 create mode 100644 ld/testsuite/ld-pe/exclude-symbols-def.d
 create mode 100644 ld/testsuite/ld-pe/exclude-symbols-def.def
 create mode 100644 ld/testsuite/ld-pe/exclude-symbols-def.s

diff --git a/ld/deffilep.y b/ld/deffilep.y
index 126f33a1352..e8d31c6dae7 100644
--- a/ld/deffilep.y
+++ b/ld/deffilep.y
@@ -249,6 +249,7 @@ keyword_as_name: BASE { $$ = "BASE"; }
 	 | DATAL { $$ = "data"; }
 	 | DESCRIPTION { $$ = "DESCRIPTION"; }
 	 | DIRECTIVE { $$ = "DIRECTIVE"; }
+	 | EXCLUDE_SYMBOLS { $$ = "EXCLUDE_SYMBOLS"; }
 	 | EXECUTE { $$ = "EXECUTE"; }
 	 | EXPORTS { $$ = "EXPORTS"; }
 	 | HEAPSIZE { $$ = "HEAPSIZE"; }
@@ -337,6 +338,7 @@ anylang_id: ID		{ $$ = $1; }
 
 symbol_list:
 	anylang_id { def_exclude_symbols ($1); }
+	|	symbol_list anylang_id { def_exclude_symbols ($2); }
 	|	symbol_list ',' anylang_id { def_exclude_symbols ($3); }
 	;
 
@@ -1355,6 +1357,7 @@ tokens[] =
   { "data", DATAL },
   { "DESCRIPTION", DESCRIPTION },
   { "DIRECTIVE", DIRECTIVE },
+  { "EXCLUDE_SYMBOLS", EXCLUDE_SYMBOLS },
   { "EXECUTE", EXECUTE },
   { "EXPORTS", EXPORTS },
   { "HEAPSIZE", HEAPSIZE },
diff --git a/ld/testsuite/ld-pe/exclude-symbols-def.d b/ld/testsuite/ld-pe/exclude-symbols-def.d
new file mode 100644
index 00000000000..4582d03c670
--- /dev/null
+++ b/ld/testsuite/ld-pe/exclude-symbols-def.d
@@ -0,0 +1,10 @@
+#source: exclude-symbols-def.s
+#target: i*86-*-cygwin* i*86-*-pe i*86-*-mingw*
+#ld: -shared exclude-symbols-def.def
+#objdump: -p
+
+#...
+.*\[[ 	]*0\] sym1
+.*\[[ 	]*1\] sym3
+.*\[[ 	]*2\] sym5
+#pass
diff --git a/ld/testsuite/ld-pe/exclude-symbols-def.def b/ld/testsuite/ld-pe/exclude-symbols-def.def
new file mode 100644
index 00000000000..c0cee7b1e70
--- /dev/null
+++ b/ld/testsuite/ld-pe/exclude-symbols-def.def
@@ -0,0 +1,4 @@
+LIBRARY exclude-symbols-def.dll
+EXCLUDE_SYMBOLS
+sym2
+sym4
diff --git a/ld/testsuite/ld-pe/exclude-symbols-def.s b/ld/testsuite/ld-pe/exclude-symbols-def.s
new file mode 100644
index 00000000000..fd533bb44c9
--- /dev/null
+++ b/ld/testsuite/ld-pe/exclude-symbols-def.s
@@ -0,0 +1,11 @@
+.global _sym1
+.global _sym2
+.global _sym3
+.global _sym4
+.global _sym5
+_sym1:
+_sym2:
+_sym3:
+_sym4:
+_sym5:
+  ret
diff --git a/ld/testsuite/ld-pe/pe.exp b/ld/testsuite/ld-pe/pe.exp
index 8dfdd0cc16e..45b16f843ac 100644
--- a/ld/testsuite/ld-pe/pe.exp
+++ b/ld/testsuite/ld-pe/pe.exp
@@ -126,3 +126,4 @@ setup_xfail mcore-*-pe
 run_ld_link_tests $foreign_sym_test
 
 run_dump_test "exclude-symbols-embedded"
+run_dump_test "exclude-symbols-def"
-- 
2.25.1

Re: [PATCH v2 1/2] ld: Add support for a new option, -exclude-symbols, in COFF object file directives

[Jan Beulich]

On 23.07.2022 00:02, Martin Storsjo wrote:
> This maps to the same as ld's --exclude-symbols command line option,
> but allowing specifying the option via directives embedded in the
> object files instead of passed manually on the command line.
> ---
> Fixed parsing multiple comma separated symbols. While pe_dll_add_excludes
> does tokenize the string, the symbol tokens in deffilep.y don't
> include commas, so we need to handle that at that level.

This patch is about object files only, so doesn't all of this belong in the
next patch?

> --- /dev/null
> +++ b/ld/testsuite/ld-pe/exclude-symbols-embedded.d
> @@ -0,0 +1,10 @@
> +#source: exclude-symbols-embedded.s
> +#target: i*86-*-cygwin* i*86-*-pe i*86-*-mingw*

What about x86-64 at the very least? That's the most common target nowadays,
i.e. also the one with the pest chances of catching regressions early. Also
while I realize that there are several uses of i*86-* there already, I think
that's a little too broad. Can I talk you into using i?86-* instead?

Jan

Re: [PATCH v2 2/2] ld: Support the -exclude-symbols option via COFF def files, with the EXCLUDE_SYMBOLS keyword

[Jan Beulich]

On 23.07.2022 00:02, Martin Storsjo wrote:
> This was requested in review.
> ---
> WIP: The testcase doesn't run successfully. How do I pass the
> full path to exclude-symbols-def.def as parameter to the ld command?

Without having tried but merely deriving from gas tests perhaps
${srcdir}/${subdir}/exclude-symbols-def.def. A few other ld test
cases look to have such or similar uses as well.

Jan

> ---
>  ld/deffilep.y                              |  3 +++
>  ld/testsuite/ld-pe/exclude-symbols-def.d   | 10 ++++++++++
>  ld/testsuite/ld-pe/exclude-symbols-def.def |  4 ++++
>  ld/testsuite/ld-pe/exclude-symbols-def.s   | 11 +++++++++++
>  ld/testsuite/ld-pe/pe.exp                  |  1 +
>  5 files changed, 29 insertions(+)
>  create mode 100644 ld/testsuite/ld-pe/exclude-symbols-def.d
>  create mode 100644 ld/testsuite/ld-pe/exclude-symbols-def.def
>  create mode 100644 ld/testsuite/ld-pe/exclude-symbols-def.s
> 
> diff --git a/ld/deffilep.y b/ld/deffilep.y
> index 126f33a1352..e8d31c6dae7 100644
> --- a/ld/deffilep.y
> +++ b/ld/deffilep.y
> @@ -249,6 +249,7 @@ keyword_as_name: BASE { $$ = "BASE"; }
>  	 | DATAL { $$ = "data"; }
>  	 | DESCRIPTION { $$ = "DESCRIPTION"; }
>  	 | DIRECTIVE { $$ = "DIRECTIVE"; }
> +	 | EXCLUDE_SYMBOLS { $$ = "EXCLUDE_SYMBOLS"; }
>  	 | EXECUTE { $$ = "EXECUTE"; }
>  	 | EXPORTS { $$ = "EXPORTS"; }
>  	 | HEAPSIZE { $$ = "HEAPSIZE"; }
> @@ -337,6 +338,7 @@ anylang_id: ID		{ $$ = $1; }
>  
>  symbol_list:
>  	anylang_id { def_exclude_symbols ($1); }
> +	|	symbol_list anylang_id { def_exclude_symbols ($2); }
>  	|	symbol_list ',' anylang_id { def_exclude_symbols ($3); }
>  	;
>  
> @@ -1355,6 +1357,7 @@ tokens[] =
>    { "data", DATAL },
>    { "DESCRIPTION", DESCRIPTION },
>    { "DIRECTIVE", DIRECTIVE },
> +  { "EXCLUDE_SYMBOLS", EXCLUDE_SYMBOLS },
>    { "EXECUTE", EXECUTE },
>    { "EXPORTS", EXPORTS },
>    { "HEAPSIZE", HEAPSIZE },
> diff --git a/ld/testsuite/ld-pe/exclude-symbols-def.d b/ld/testsuite/ld-pe/exclude-symbols-def.d
> new file mode 100644
> index 00000000000..4582d03c670
> --- /dev/null
> +++ b/ld/testsuite/ld-pe/exclude-symbols-def.d
> @@ -0,0 +1,10 @@
> +#source: exclude-symbols-def.s
> +#target: i*86-*-cygwin* i*86-*-pe i*86-*-mingw*
> +#ld: -shared exclude-symbols-def.def
> +#objdump: -p
> +
> +#...
> +.*\[[ 	]*0\] sym1
> +.*\[[ 	]*1\] sym3
> +.*\[[ 	]*2\] sym5
> +#pass
> diff --git a/ld/testsuite/ld-pe/exclude-symbols-def.def b/ld/testsuite/ld-pe/exclude-symbols-def.def
> new file mode 100644
> index 00000000000..c0cee7b1e70
> --- /dev/null
> +++ b/ld/testsuite/ld-pe/exclude-symbols-def.def
> @@ -0,0 +1,4 @@
> +LIBRARY exclude-symbols-def.dll
> +EXCLUDE_SYMBOLS
> +sym2
> +sym4
> diff --git a/ld/testsuite/ld-pe/exclude-symbols-def.s b/ld/testsuite/ld-pe/exclude-symbols-def.s
> new file mode 100644
> index 00000000000..fd533bb44c9
> --- /dev/null
> +++ b/ld/testsuite/ld-pe/exclude-symbols-def.s
> @@ -0,0 +1,11 @@
> +.global _sym1
> +.global _sym2
> +.global _sym3
> +.global _sym4
> +.global _sym5
> +_sym1:
> +_sym2:
> +_sym3:
> +_sym4:
> +_sym5:
> +  ret
> diff --git a/ld/testsuite/ld-pe/pe.exp b/ld/testsuite/ld-pe/pe.exp
> index 8dfdd0cc16e..45b16f843ac 100644
> --- a/ld/testsuite/ld-pe/pe.exp
> +++ b/ld/testsuite/ld-pe/pe.exp
> @@ -126,3 +126,4 @@ setup_xfail mcore-*-pe
>  run_ld_link_tests $foreign_sym_test
>  
>  run_dump_test "exclude-symbols-embedded"
> +run_dump_test "exclude-symbols-def"

Re: [PATCH v2 1/2] ld: Add support for a new option, -exclude-symbols, in COFF object file directives

[Martin Storsjö]

On Mon, 25 Jul 2022, Jan Beulich wrote:

> On 23.07.2022 00:02, Martin Storsjo wrote:
>> This maps to the same as ld's --exclude-symbols command line option,
>> but allowing specifying the option via directives embedded in the
>> object files instead of passed manually on the command line.
>> ---
>> Fixed parsing multiple comma separated symbols. While pe_dll_add_excludes
>> does tokenize the string, the symbol tokens in deffilep.y don't
>> include commas, so we need to handle that at that level.
>
> This patch is about object files only, so doesn't all of this belong in the
> next patch?

Surprisingly, the def parser is used to tokenize the argument to the 
option in the embedded directive - in the previous patch, it only included 
the first symbol up to the first comma. So due to that, we'd either need a 
def parser token for "identifier string including commas", or do the 
comma splitting at the parser level.

>> --- /dev/null
>> +++ b/ld/testsuite/ld-pe/exclude-symbols-embedded.d
>> @@ -0,0 +1,10 @@
>> +#source: exclude-symbols-embedded.s
>> +#target: i*86-*-cygwin* i*86-*-pe i*86-*-mingw*
>
> What about x86-64 at the very least? That's the most common target nowadays,
> i.e. also the one with the pest chances of catching regressions early.

Sure, I can add that too, sorry for missing it. Here, i386 is the more 
interesting one though, as it has symbol prefixes, and the testcase 
defines how that's handled. But I'll add an x86_64 one too.

> Also while I realize that there are several uses of i*86-* there already, I think
> that's a little too broad. Can I talk you into using i?86-* instead?

Sure, I can absolutely change that. That was just a copypaste from some 
other test.

// Martin

Re: [PATCH v2 2/2] ld: Support the -exclude-symbols option via COFF def files, with the EXCLUDE_SYMBOLS keyword

[Martin Storsjö]

On Mon, 25 Jul 2022, Jan Beulich wrote:

> On 23.07.2022 00:02, Martin Storsjo wrote:
>> This was requested in review.
>> ---
>> WIP: The testcase doesn't run successfully. How do I pass the
>> full path to exclude-symbols-def.def as parameter to the ld command?
>
> Without having tried but merely deriving from gas tests perhaps
> ${srcdir}/${subdir}/exclude-symbols-def.def. A few other ld test
> cases look to have such or similar uses as well.

Thanks, will try that!

// Martin