Christophe Leroy <christophe.le...@csgroup.eu> writes:
> Hi Michael,
>
> Le 18/09/2024 à 04:33, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.le...@csgroup.eu> writes:
>>> On powerpc64 as shown below by readelf, vDSO functions symbols have
>>> type NOTYPE.
>>>
>>> $ powerpc64-linux-gnu-readelf -a arch/powerpc/kernel/vdso/vdso64.so.dbg
>>> ELF Header:
>>> Magic: 7f 45 4c 46 02 02 01 00 00 00 00 00 00 00 00 00
>>> Class: ELF64
>>> Data: 2's complement, big endian
>>> Version: 1 (current)
>>> OS/ABI: UNIX - System V
>>> ABI Version: 0
>>> Type: DYN (Shared object file)
>>> Machine: PowerPC64
>>> Version: 0x1
>>> ...
>>>
>>> Symbol table '.dynsym' contains 12 entries:
>>> Num: Value Size Type Bind Vis Ndx Name
>>> ...
>>> 1: 0000000000000524 84 NOTYPE GLOBAL DEFAULT 8
>>> __[...]@@LINUX_2.6.15
>>> ...
>>> 4: 0000000000000000 0 OBJECT GLOBAL DEFAULT ABS LINUX_2.6.15
>>> 5: 00000000000006c0 48 NOTYPE GLOBAL DEFAULT 8
>>> __[...]@@LINUX_2.6.15
>>>
>>> Symbol table '.symtab' contains 56 entries:
>>> Num: Value Size Type Bind Vis Ndx Name
>>> ...
>>> 45: 0000000000000000 0 OBJECT GLOBAL DEFAULT ABS LINUX_2.6.15
>>> 46: 00000000000006c0 48 NOTYPE GLOBAL DEFAULT 8 __kernel_getcpu
>>> 47: 0000000000000524 84 NOTYPE GLOBAL DEFAULT 8
>>> __kernel_clock_getres
>>>
>>> To overcome that, commit ba83b3239e65 ("selftests: vDSO: fix vDSO
>>> symbols lookup for powerpc64") was proposed to make selftests also
>>> look for NOTYPE symbols, but is it the correct fix ?
>>>
>>> VDSO32 functions are flagged as functions, why not VDSO64 functions ?
>>> Is it because VDSO functions are not traditional C functions using
>>> the standard API ?
>>
>> Yes. There's some explanation in the original commit:
>>
>> Note that the symbols exposed by the vDSO aren't "normal" function
>> symbols, apps
>> can't be expected to link against them directly, the vDSO's are both
>> seen
>> as if they were linked at 0 and the symbols just contain offsets to the
>> various functions. This is done on purpose to avoid a relocation step
>> (ppc64 functions normally have descriptors with abs addresses in them).
>> When glibc uses those functions, it's expected to use it's own
>> trampolines
>> that know how to reach them.
>>
>> From
>> https://github.com/mpe/linux-fullhistory/commit/5f2dd691b62da9d9cc54b938f8b29c22c93cb805
>>
>> The descriptors it's talking about are the OPD function descriptors used
>> on ABI v1 (big endian).
>>
>>> But it is exactly the same for VDSO32 functions, allthough they are
>>> flagged as functions.
>>
>> It's not quite the same because of the function descriptors.
>>
>> On ppc64/ABIv1 a function pointer for "F" points to an opd, which then
>> points to ".F" which has the actual text. It's the ".F" symbol that has
>> type "function".
>>
>>> So lets flag them as functions and revert the selftest change.
>>>
>>> What's your opinion on that ?
>>
>> I think it's fine on ppc64le, I worry slightly that it risks breaking
>> glibc or something else on big endian.
>>
>> It is more correct for the text symbol to have type function, even if
>> there's no function descriptor for it.
>>
>> glibc has a special case already for handling the VDSO symbols which
>> creates a fake opd pointing at the kernel symbol. So changing the VDSO
>> symbol type to function shouldn't affect that AFAICS.
>>
>> I think the only cause of breakage would be if something is explicitly
>> looking for NOTYPE symbols, which seems unlikely, but you never know.
>>
>> So I think we could attempt to take this change for v6.13, giving it
>> lots of time to get some test coverage in next before going to mainline.
>
> Will you take the RFC as is for 6.13 or would you like me to include the
> above explainations and repost as non-RFC ?
If you can come up with a consolidated changelog and post a non-RFC
version that would help, thanks.
cheers
