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

[Jan Beulich]

On 20.07.2022 14:39, Martin Storsjö wrote:
> 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?

Cross builds ought to be fine - you don't mean to run any resulting
executable anyway.

> 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?

The one I spotted was for aligncomm, in ld-pe. In how far this would
lend itself to cloning I can't easily tell.

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

Isn't supposed to be, or is prevented from being used that way? I have
to admit that I'd find it quite helpful if this new option was also
usable from a .def file. This might commonly be the more central place
to record exclusions.

>> 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).

Well, if that's the case (I simply didn't spot this tokenization then),
then plural ifs of course fine.

Jan

> 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
>