Cleanup .gnu.version_r after removing symbol version

[sulix]

If --clear-symbol-version is used to remove all occurrances of a symbol version, it nevertheless remains in the .gnu.version_r section as being required by a particular dependency.

Instead, after removing symbol versions, loop over the .gnu.version and gnu.version_r tables to find any unused version, and unlink them from the linked-lists in the .gnu.version_r section.

While an ideal implementation would probably rebuild the entire .gnu.version_r section, this works well enough to stop objdump noting the no-longer-valid version dependency.

Note that the test is very glibc-specific — it works on my machine, but I wouldn't trust it too much further. I assume that's one of the reasons symbol versions aren't tested much yet, so if you'd rather I remove the test, that's fine.

[martinetd]

Thank you! This is awesome! You're doing pretty much exactly what I was suggesting, so I'm obviously happy with it :) I've tested it on the couple of binaries I need to edit and it works as advertised for me.

For the test, I agree it's not great as is, and checking we don't remove too much in .gnu.version_r by adding two symbols and removing them one at a time? I've just tried that, but I managed to produce a binary with versions without gnu.version_r and patchelf fails on these:

$ cat foo.map 
VERS1 {
  global:
    vers1_1; vers1_2;
  local:
    *;
};

VERS2 {
  global:
    vers2;
  local:
    *;
} VERS1;
$ cat foo.c
void vers1_1(void) {}
void vers1_2(void) {}
void vers2(void) {}
$ gcc -Wl,--version-script=foo.map -fpic -shared -o libfoo.so foo.c
$ ~/code/patchelf/src/patchelf --clear-symbol-version vers1_1 libfoo.so
patchelf: cannot find section '.gnu.version_r'
$ echo $?
1
$ readelf -V libfoo.so 

Version symbols section '.gnu.version' contains 10 entries:
 Addr: 0x000000000000040a  Offset: 0x00040a  Link: 3 (.dynsym)
  000:   0 (*local*)       1 (*global*)      1 (*global*)      1 (*global*)   
  004:   1 (*global*)      2 (VERS1)         2 (VERS1)         3 (VERS2)      
  008:   3 (VERS2)         2 (VERS1)      

Version definition section '.gnu.version_d' contains 3 entries:
 Addr: 0x0000000000000420  Offset: 0x000420  Link: 4 (.dynstr)
  000000: Rev: 1  Flags: BASE  Index: 1  Cnt: 1  Name: libfoo.so
  0x001c: Rev: 1  Flags: none  Index: 2  Cnt: 1  Name: VERS1
  0x0038: Rev: 1  Flags: none  Index: 3  Cnt: 2  Name: VERS2
  0x0054: Parent 1: VERS1

I'm running out of time right now but I'll have a look tomorrow to see if I can get it to use version_r instead for our tests.

[Mic92]

I'm running out of time right now but I'll have a look tomorrow to see if I can get it to use version_r instead for our tests.

Do you still plan to do this?

[martinetd]

I'm running out of time right now but I'll have a look tomorrow to see if I can get it to use version_r instead for our tests.

Do you still plan to do this?

Sorry didn't manage to do it quickly enough back then and forgot about it... Turns out I was silly and too focused on producing a lib with multiple versions (.gnu.version_d), but version_r is about using anything we care about so it's much simpler:

$ cat user.c
#include <sys/stat.h>

int foo() {
	struct stat buf;

	fstat(0, &buf);
	stat(".", &buf);
	return 0;
}
$ gcc -fpic -shared -o libuser.so user.c
$ nm -D libuser.so | grep @
                 w __cxa_finalize@GLIBC_2.2.5
                 U fstat@GLIBC_2.33
                 U stat@GLIBC_2.33
$ objdump -x libuser.so | grep -A 3 Version
Version References:
  required from libc.so.6:
    0x09691a75 0x00 03 GLIBC_2.2.5
    0x069691b3 0x00 02 GLIBC_2.33
$ ../src/patchelf --clear-symbol-version stat libuser.so
$ nm -D libuser.so | grep @
                 w __cxa_finalize@GLIBC_2.2.5
                 U fstat@GLIBC_2.33
$ objdump -x libuser.so | grep -A 3 Version
Version References:
  required from libc.so.6:
    0x09691a75 0x00 03 GLIBC_2.2.5
    0x069691b3 0x00 02 GLIBC_2.33
$ ../src/patchelf --clear-symbol-version fstat libuser.so
$ nm -D libuser.so | grep @
                 w __cxa_finalize@GLIBC_2.2.5
$ objdump -x libuser.so | grep -A 3 Version
Version References:
  required from libc.so.6:
    0x09691a75 0x00 03 GLIBC_2.2.5

( so 2.33 is gone after removing both symbols and only after removing both symbols from it - which is what I think I wanted to test?)

ideally could go as far as actually running it to make sure linker is happy, but that'd mean providing the symbols and that feels too much like work to do on a busy weekend; please tell me and I'll try not to forget this time :)

[sulix]

I needed this again today, so I rebased the change, and tidied up the test slightly.

It's still not the ideal way of testing this: we probably should build our own separate library with symbol versions, and a program which links against it. But that's significantly more work, and glibc's own symbol versions are pretty much everyone's use-case.

[Mic92]

Hi. I opened up a new PR to add some more pointer checks on top. I know this is not consequently done in our codebase yet, but I want to converge over time to it: https://github.com/NixOS/patchelf/pull/394/commits/e1aa6d671c6e81172f7924b812ff8c8669e1a9e7

[bobbobbio]

I found myself wanting this today. Is there any chance this will eventually get merged in the future? What are the outstanding concerns with this approach? I couldn't understand from the existing comments.