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

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

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