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

["Martin Storsjö" <martin@martin.st>]

On Wed, 20 Jul 2022, Jan Beulich wrote:

> On 19.07.2022 22:33, Martin Storsjo wrote:
>> @@ -946,6 +957,7 @@ diropts[] =
>>    { "-attr", SECTIONS },
>>    { "-export", EXPORTS },
>>    { "-aligncomm", ALIGNCOMM },
>> +  { "-exclude-symbols", EXCLUDE_SYMBOLS },

Thanks for having a look!

> I was a little concerned by the - in the middle, but the (custom)
> def-file lexer looks to be treating - the same as ISALPHA() chars.

Thanks for checking!

> Nevertheless it would be quite helpful if there were two testcases
> here - one with the new directive used in an object file's .drectve
> section and another with it used in a .def file.

A testcase certainly would be good here. I'm not familiar with binutils' 
tests from before - I presume I'd make a native build (not having binutils 
set up with a cross target) and run "make check", and I'd find examples of 
existing tests in ld/testsuite/ld-pe?

I guess it'd be good with one test for i386 and one for x86_64 (or both 
cases merged into one?), assembling an .s file and linking it, and then 
checking the export table. Are there any similar existing tests to serve 
as example?

This directive isn't supposed to be used from a def file though (just like 
the aligncomm directive), see the comment in deffile.h.

> The other thing is: According to def_exclude_symbols() each
> directive is followed by exactly one symbol afaict. Hence
> -exclude-symbol (singular) would seem more appropriate for the
> option.

Actually, when the option values later are added to the exclusion list by 
pe_dll_add_excludes, they're split/tokenized by commas (and colon, 
apparently). I guess this could be made clearer by renaming the struct 
field though, maybe to "symbol_names"?

I'd prefer to keep the plural form in the directive name in any case, for 
consistency with the command line option, even if it uses the 
-<name>:<value> option form (like all other embedded directives) instead 
of --<name>=<value> or --<name> <value> on the command line.

// Martin