Christophe Leroy [off-list ref] writes:

Hi Michael,

Le 18/09/2024 à 04:33, Michael Ellerman a écrit :

quoted

Christophe Leroy [off-list ref] writes:

quoted

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

quoted

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

quoted

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