Bug 210875 - libtracefs: SONAME/symlink is bogus
Summary: libtracefs: SONAME/symlink is bogus
Status: RESOLVED CODE_FIX
Alias: None
Product: Tools
Classification: Unclassified
Component: Trace-cmd/Kernelshark (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Default virtual assignee for Trace-cmd and kernelshark
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-23 11:12 UTC by Jan Engelhardt
Modified: 2022-02-21 22:40 UTC (History)
2 users (show)

See Also:
Kernel Version: libtracefs 1.0.0
Subsystem:
Regression: No
Bisected commit-id:


Attachments
Only show the version number in the SONAME (998 bytes, patch)
2020-12-23 15:31 UTC, Steven Rostedt
Details | Diff
Better version of making the link and SONAME match (1.55 KB, patch)
2020-12-23 15:38 UTC, Steven Rostedt
Details | Diff

Description Jan Engelhardt 2020-12-23 11:12:13 UTC
After building libtracefs 1.0.0,

» l -og lib/tracefs/
total 184
drwxr-xr-x 2   246 Dec 23 12:09 .
drwxr-xr-x 3    21 Dec 23 12:08 ..
-rw-r--r-- 1  4125 Dec 23 12:09 .tracefs-events.d
-rw-r--r-- 1  3848 Dec 23 12:09 .tracefs-instance.d
-rw-r--r-- 1  3779 Dec 23 12:09 .tracefs-utils.d
-rw-r--r-- 1 53678 Dec 23 12:09 libtracefs.a
lrwxrwxrwx 1    15 Dec 23 12:09 libtracefs.so -> libtracefs.so.1
lrwxrwxrwx 1    19 Dec 23 12:09 libtracefs.so.1 -> libtracefs.so.1.0.0
-rwxr-xr-x 1 54528 Dec 23 12:09 libtracefs.so.1.0.0
-rw-r--r-- 1 25440 Dec 23 12:09 tracefs-events.o
-rw-r--r-- 1 15456 Dec 23 12:09 tracefs-instance.o
-rw-r--r-- 1 11672 Dec 23 12:09 tracefs-utils.o
» readelf -aW lib/tracefs/libtracefs.so.1 | grep SONAME
 0x000000000000000e (SONAME)             Library soname: [libtracefs.so.1.0.0]

That is not how SONAMEs are supposed to work. Either it should be -Wl,-soname,libtracefs.so.1 , or, if keeping -Wl,-soname,libtracefs.so.1.0.0, the libtracefs.so.1 file is redundant and the symlinks should just directly point to libtracefs.so.1.0.0.
Comment 1 Steven Rostedt 2020-12-23 14:32:57 UTC
(In reply to Jan Engelhardt from comment #0)
 
> That is not how SONAMEs are supposed to work. Either it should be
> -Wl,-soname,libtracefs.so.1 , or, if keeping
> -Wl,-soname,libtracefs.so.1.0.0, the libtracefs.so.1 file is redundant and
> the symlinks should just directly point to libtracefs.so.1.0.0.

Thanks for the report. Is there any documentation on how SONAMEs are suppose to work?

Anyway, I'll make the update to have the SONAME point to the .1 instead of the .1.0.0.
Comment 2 Steven Rostedt 2020-12-23 15:31:07 UTC
Created attachment 294317 [details]
Only show the version number in the SONAME

This will only show libtracefs.so.1 in the SONAME fo the library.
Comment 3 Steven Rostedt 2020-12-23 15:38:14 UTC
Created attachment 294319 [details]
Better version of making the link and SONAME match

Second version (and commit) of the patch. Instead of using a chop macro to truncate the extra version numbers, use the actual file that is going to be linked to the .so file.
Comment 4 Jan Engelhardt 2020-12-23 15:55:58 UTC
>Is there any documentation on how SONAMEs are suppose to work?

The minimum requirement(s) can be briefly described as

1. Every set of {all exported functions and their ABI}, a unique SONAME string should be chosen. Besides the usual consideration of not using ".", "..", '/', '\0', no naming restrictions apply whatsoever. <=>
Whenever /usr/bin/abidiff outputs anything between any two versions of your lib (except “SONAME changed” obviously), choose a new SONAME.

2. A file (regular, symlink, etc.) by the name of <SONAME string> must exist and should resolve to the intended library.


There are more grants possible at the cost of additional rules to follow, but it wouldn't be terse anymore.
Comment 5 Steven Rostedt 2020-12-23 16:17:37 UTC
(In reply to Jan Engelhardt from comment #4)
> 1. Every set of {all exported functions and their ABI}, a unique SONAME
> string should be chosen. Besides the usual consideration of not using ".",
> "..", '/', '\0', no naming restrictions apply whatsoever. <=>
> Whenever /usr/bin/abidiff outputs anything between any two versions of your
> lib (except “SONAME changed” obviously), choose a new SONAME.

So question about this. I was under the assumption of:

 .so.X.Y.Z

Where Z is incremented on fixes.

Y is incremented on new functionality (a new function is added to the library)

X is incremented when the ABI breakes (Applications need to be updated).

I'm unaware of "abidiff" but will it show changes if you add new functionality? That is, a new function is added, but all the old applications will still work because the old functionality remains the same.

I wanted the SONAME to point to something that if an application works with it (you have minimum X.Y version) then you can upgrade to new X.Y versions without issue.

Am I understanding this correctly?
Comment 6 Jan Engelhardt 2020-12-23 16:39:55 UTC
>I'm unaware of "abidiff" but will it show changes if you add new
>functionality?

Very much so (extras when fed dwarfdebug-enabled objects). Give it a try.

>I wanted the SONAME to point to something that if an application works with it
>(you have minimum X.Y version) then you can upgrade to new X.Y versions
>without issue.

This is where the aforementioned "additional rules" come in, which involves extending CFLAGS, LDFLAGS, marking up some of the "extern" declarations for export, and assigning each function a version tag for the upgrade magic to work, and settings soname to the lowest compatible portion (often just the X part).

- CFLAGS = blah
+ CFLAGS = blah -fvisibility=hidden
- LDFLAGS = blah
+ LDFLAGS = blah -Wl,-soname,libtracefs.so.1 -Wl,--version-script=x.sym
- extern void old_func(void);
- extern void new_func(void);
+ extern __attribute__((visibility(default))) old_func(void);
+ extern __attribute__((visibility(default))) new_func(void);

x.sym:
 TRACEFS_1.0.0 {
    global:
    old_func;
    local:
    *;
 };
 TRACEFS_1.0.1 {
    global:
    new_func;
 } TRACEFS_1.0.0;
Comment 7 Steven Rostedt 2020-12-23 16:49:24 UTC
On Wed, 23 Dec 2020 16:39:55 +0000
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=210875
> 
> --- Comment #6 from Jan Engelhardt (ej+bko@inai.de) ---
> This is where the aforementioned "additional rules" come in, which involves
> extending CFLAGS, LDFLAGS, marking up some of the "extern" declarations for
> export, and assigning each function a version tag for the upgrade magic to
> work, and settings soname to the lowest compatible portion (often just the X
> part).
> 
> - CFLAGS = blah
> + CFLAGS = blah -fvisibility=hidden
> - LDFLAGS = blah
> + LDFLAGS = blah -Wl,-soname,libtracefs.so.1 -Wl,--version-script=x.sym
> - extern void old_func(void);
> - extern void new_func(void);
> + extern __attribute__((visibility(default))) old_func(void);
> + extern __attribute__((visibility(default))) new_func(void);

The above looks useful. We did it the other way around. That is, we marked
every global function with "__hidden" where:

#define __hidden __attribute__((visibility("hidden")))

Perhaps it would be more prudent to swap it, and that way we wont
accidentally export something we don't want.

CFLAGS = blah -fvisibility=hidden

#define __show __attribute__((visibility(("default")))


> 
> x.sym:
>  TRACEFS_1.0.0 {
>     global:
>     old_func;
>     local:
>     *;
>  };
>  TRACEFS_1.0.1 {
>     global:
>     new_func;
>  } TRACEFS_1.0.0;
> 

I'm not sure this answers my question though. The point is if we add a new
function, not touch an old one.
Comment 8 Jan Engelhardt 2020-12-23 18:18:40 UTC
>[x.sym] I'm not sure this answers my question though.

x.sym has to build over time of course, and it serves so that you don't run a {program that was built with 1.0.1} using 1.0.0.
Comment 9 Steven Rostedt 2020-12-23 21:33:25 UTC
On Wed, 23 Dec 2020 18:18:40 +0000
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=210875
> 
> --- Comment #8 from Jan Engelhardt (ej+bko@inai.de) ---
> >[x.sym] I'm not sure this answers my question though.  
> 
> x.sym has to build over time of course, and it serves so that you don't run a
> {program that was built with 1.0.1} using 1.0.0.
> 

Well, since the third number is just for fixes, a program built for 1.0.1
would work fine (maybe just buggy as if it were built for 1.0.0 ;-).

But a program built for 1.1.0 may not work with a program built for 1.0.1.
Should the SONAME have the second number?

again: X.Y.Z, where Z is fixes, Y is for new functionality, and is for
breakage of APIs (which I plan to never do, but you never know).

-- Steve
Comment 10 Adam Majer 2020-12-24 11:05:50 UTC
(In reply to Steven Rostedt from comment #9)
> Well, since the third number is just for fixes, a program built for 1.0.1
> would work fine (maybe just buggy as if it were built for 1.0.0 ;-).
> 
> But a program built for 1.1.0 may not work with a program built for 1.0.1.
> Should the SONAME have the second number?
> 
> again: X.Y.Z, where Z is fixes, Y is for new functionality, and is for
> breakage of APIs (which I plan to never do, but you never know).


This is just the convention (for versions). The SONAME just means that all programs build with library of SONAME=ABC will work where the version is at least as high as the one that has all the symbols. So using your example (ie. the GNU convention),


1.0.X vs. 1.0.0 == no symbol changes. So forward and backward compatibility, by this convention.

1.X.0 vs. 1.0.0 == new symbols added only. Backward compatible. Not forward compatible. You can't use 1.0.0 library with binary linked to new 1.X.0 symbols, for example. But you can use binary linked with 1.0.0 symbols and the new 1.X.0

Y.0.0 vs. 1.0.0 == symbols removed. ABI not compatible.


The dependency you end up with is libtracefs.1 > 1.X.0, which is just fine. Latest version of libtracefs.1 will satisfy all old versions too, so no need to change any soname.

Also, keep in mind that SONAME needs to have nothing to do with actual version. You can have libtracefs.1 > 23.3 and it's just fine. or something like,

   libtracefs.23.3.so     <== unique name
   libtracefs.uuid.so -> libtracefs.23.3.so   <== soname provided by the unique name
   libtracefs.so -> libtracefs.uuid.so   <== for developers

The point is that every version of libtracefs.{normal_version}.so also provides the soname symlink, like libtrace.uuid.so. The developers then need a symlink from the libtracefs.so to one of the other ones. The linker then finds the library, resolves symbols and stores the reference to the SONAME. The version dependency is then up to the users to figure out and not you ;)

For example, if you look at Debian, they track exported symbols for libraries by version. So if you link with libtracefs.uuid.so version 10 and the application uses only symbols that are provided since libtracefs.uuid.so version 3, then the dependency generated will be libtracefs.uuid.so >= 3, even when linked with later version library during build ;)

So the question is, "why have libtracefs.23.3.so *and* libtracefs.soname.so and not just the soname version??" The reason is you can then have someone install

libtracefs.23.3.so
libtracefs.23.2.so

and if something is broken with 23.3.so, then they can just switch the symlink to point to the other one and see if there is a regression without re-installing anything. It allows for different versions of the library to be concurrently installed, for whatever reason (like testing). It's also easy to see what version of the software you have installed on your system without resorting to checksums or something.

so, I hope this clears things up a bit :D

TL;DR: SONAME is just a label telling linker that it will work with all later versions with same label.
Comment 11 Jan Engelhardt 2020-12-24 12:40:47 UTC
>The dependency you end up with is libtracefs.1 > 1.X.0, which is just fine.
>Latest version of libtracefs.1 will satisfy all old versions too, so no need
>to change any soname.

It is not satisfactory for a package manager, though, because it does not know the semantics of "1.X.0" -- hence the symbol versions, which it can extract from the ELF headers and do something useful with.
Comment 12 Adam Majer 2020-12-24 12:48:03 UTC
(In reply to Jan Engelhardt from comment #11)
> >The dependency you end up with is libtracefs.1 > 1.X.0, which is just fine.
> >Latest version of libtracefs.1 will satisfy all old versions too, so no need
> >to change any soname.
> 
> It is not satisfactory for a package manager, though, because it does not
> know the semantics of "1.X.0" -- hence the symbol versions, which it can
> extract from the ELF headers and do something useful with.

Correct. With lack of version to symbol tracking, you would have to rely on version numbers of the library at link time. So something like,

  Depends: libtracefs.1 > %version_libtracefs_from_BR

But in case of SUSE, that is not happening and only SONAME/symbol versioning, as per,

  https://akkadia.org/drepper/symbol-versioning

which tends to require upstream involvement.

In case of Debian, they use,

  https://manpages.debian.org/testing/dpkg-dev/dpkg-gensymbols.1.en.html

to generate version tracking information. And this does not require any library symbol version information ;)

Anyway, this is all ancillary (extra) to the SONAME.
Comment 13 Steven Rostedt 2022-02-21 22:40:06 UTC
As of commit e1f6b50e194d ("libtracefs: Do not show the full version in the library SONAME") we now have:

$ readelf -aW /usr/local/lib64/libtracefs.so.1 | grep SONAME
 0x000000000000000e (SONAME)             Library soname: [libtracefs.so.1]

Note You need to log in before you can comment on or make changes to this bug.