Re: [PATCH] Better supported way to set symbol versions for legacy syscalls

[Alexander Miller]

On Mon, 12 Dec 2022 18:37:14 +0900, Mike Frysinger <vapier@gentoo.org> wrote:

> i'm not anti this patch, but some of the commit message claims seem ...
> inaccurate and/or misleading.

I tried to describe the current situation and the reasoning as accurately
as possible, but there's always room for improvements.  I'm open to reword
those claims if you can suggest a clearer wording, but...

>> Using a linker script to set symbol versions is an undocumented
>> hack and doesn't work reliably in many cases.
>
> which part is undocumented ?
>
> https://sourceware.org/binutils/docs-2.39/ld/Implicit-Linker-Scripts.html
[...]
> https://sourceware.org/binutils/docs-2.39/ld/Assignments.html
[...]
> https://sourceware.org/binutils/docs-2.39/ld/Simple-Assignments.html
[...]
> https://sourceware.org/binutils/docs-2.39/ld/VERSION.html
> this talks indirectly about defined symbols and how they're bound to versions.
> there isn't explicit mention of where/how symbols must be defined, but i think
> it's reasonable to extrapolate that a symbol defined using the @ syntax will 
> be
> processed as such.

That's exactly what I mean. It's mentioned *nowhere* in the documentation
that you can set symbol versions with an assignment in a linker script.
It may seem a reasonable extrapolation, but it's not necessarily what
the developers intended and implemented.
On the other hand, it *is* documented that you can use a version script
or the .symver directive in assembler.

>> It works (to some degree) with the bfd linker, but fails with gold or lld.
>
> older lld versions had bugs in how it processed linker scripts & symbol
> assignments, including with quotes, but they fixed those.
>
> gold's processing has been reported as a bug.
[...]
>> And even with bfd it can break when using --gc-sections or LTO.
>
> if these functions are culling symbols referenced via linker scripts,
> you should report them as bugs to the respective projects.

So you say the processing of these assignments to versioned symbols is
buggy in *every* available linker implementation?
Then I think it's fair to say that such assignments are unsupported.

Honestly, I don't care whether these are linker bugs or an abuse of
linker scripts.  I'm just stating that it doesn't work (at least it
didn't when I last checked a few months ago; some bugs may have been
fixed in the meantime, but that doesn't matter here).


...So I don't see why any of the statements you cited would be misleading
or inaccurate.  But, as already mentioned above, if you feel that
"undocumented hack" is too harsh or can otherwise improve the wording,
I'm open for suggestions.

>> +#  define SYMVER(cn, vn) __typeof(cn) cn __attribute__((noreorder)); \
>> +                     __asm__(".symver " #cn "," vn)
>
> attribute names should use the __ scoped values like you've already done with
> the attribute & asm directives.  this avoids namespace clashes.  while not a
> likely issue currently, it's standard good hygiene.

While it shouldn't be necessary, using the __ variants can't hurt.

More importantly, I just spotted a typo in that snippet, a missing
underscore in the middle.  That should be __attribute__((no_reorder))
(or __no_reorder__); the __has_attribute() a few lines above is correct.
Didn't notice while testing because it only affects old gcc versions,
and sometimes everything works even without the attribute.

Alex