Bug 2087904 - LTO vs symbol versioning problem
Summary: LTO vs symbol versioning problem
Keywords:
Status: CLOSED EOL
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: 36
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-05-18 14:15 UTC by Hans de Goede
Modified: 2023-05-25 16:52 UTC (History)
11 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-05-25 16:52:15 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Hans de Goede 2022-05-18 14:15:30 UTC
This bug is the result of analyzing a timidity crash reported in bug 1993671, with a related alsa-lib (non) bug 2087786.

I believe that the problem is that the timidity code has 2 c files using alsa-lib APIs:

interface/alsaseq_c.c: does:

#include <alsa/asoundlib.h>

With I believe the relevant lines from that .h being:

int snd_pcm_hw_params_set_rate_near(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val, int *dir);

### vs ###

timidity/alsa_a.c:

#define ALSA_PCM_OLD_HW_PARAMS_API
#define ALSA_PCM_OLD_SW_PARAMS_API
#include <alsa/asoundlib.h>

With I believe the relevant lines from that .h *this time* being:

asm(".symver snd_pcm_hw_params_set_rate_near,snd_pcm_hw_params_set_rate_near");
unsigned int snd_pcm_hw_params_set_rate_near(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val, int *dir);

##########

Note the different prototypes and in the timidity/alsa_a.c #include case before the prototype there is an asm .symver line!

Also note that timidity/alsa_a.c is the only caller of snd_pcm_hw_params_set_rate_near() and it wants the old version which gets declared when setting the ALSA_PCM_OLD_HW_PARAMS_API .

But when building timidity++ for Fedora with LTO enabled alsa_a.c ends up calling the new (unversioned) version of the function which expects and "unsigned int *val" as 3th argument rather then an "unsigned int val" which timidity/alsa_a.c is passing, leading to a NULL pointer reference.

Disabling LTO makes things work as expected, with alsa_a.c correctly getting the old snd_pcm_hw_params_set_rate_near version of the function as shown by a printf in the compat-wrapper which I added to my local build of alsa-lib.

Comment 1 Hans de Goede 2022-05-18 14:24:45 UTC
I have tried adding:

#define ALSA_PCM_OLD_HW_PARAMS_API
#define ALSA_PCM_OLD_SW_PARAMS_API

To:

interface/alsaseq_c.c

So that the 2 files in timidity which include alsa/asoundlib.h now both these and this both end up defining the function like this:

asm(".symver snd_pcm_hw_params_set_rate_near,snd_pcm_hw_params_set_rate_near");
unsigned int snd_pcm_hw_params_set_rate_near(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val, int *dir);

I expected this to fix things, but unfortunately that does not fix things.

So I'll just disable LTO for the pkg for now (meaning there is no big hurry to address this).

Note that timidity is a pretty small C program which can easily be build locally with "fedpkg local" uncommenting the '%global _lto_cflags %nil' which I'm about to add to timidity++.spec and then doing a "fedpkg local" build, followed by:

TiMidity++-2.15.0/timidity/timidity -Os <somefile>.mid

is enough to easily and quickly reproduce this locally.

Comment 2 Jakub Jelinek 2022-05-18 14:26:27 UTC
Unless the symbol mentioned in the .symver line is __attribute__((used)), the compiler can't know it needs to preserve such a symbol, inline asm is a black box for the compiler (intentionally).
In any case, you shouldn't be using .symver in inline asm with LTO, but the symver attribute which is there since GCC 10.
See https://gcc.gnu.org/onlinedocs/gcc-12.1.0/gcc/Common-Function-Attributes.html#index-symver-function-attribute
and https://gcc.gnu.org/PR48200 for more details.

Comment 3 Hans de Goede 2022-05-18 15:01:48 UTC
(In reply to Jakub Jelinek from comment #2)
> Unless the symbol mentioned in the .symver line is __attribute__((used)),
> the compiler can't know it needs to preserve such a symbol, inline asm is a
> black box for the compiler (intentionally).
> In any case, you shouldn't be using .symver in inline asm with LTO, but the
> symver attribute which is there since GCC 10.
> See
> https://gcc.gnu.org/onlinedocs/gcc-12.1.0/gcc/Common-Function-Attributes.
> html#index-symver-function-attribute
> and https://gcc.gnu.org/PR48200 for more details.

Thanks, but this is in a header for a consumer of the function, where we want to specify which version the consumer must use (rather then the default symbol version).

I've tried using the __attribute__ ((__symver__ ... syntax in the header like this:

__attribute__ ((__symver__ ("snd_pcm_hw_params_set_rate_near,snd_pcm_hw_params_set_rate_near")))
unsigned int snd_pcm_hw_params_set_rate_near(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val, int *dir);

But that leads to compiler errors like these:

/usr/include/alsa/pcm_old.h:70:14: error: symbol needs to be defined to have a version
   70 | unsigned int snd_pcm_hw_params_set_rate_near(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val, int *dir);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

And the https://gcc.gnu.org/onlinedocs/gcc-12.1.0/gcc/Common-Function-Attributes.html#index-symver-function-attribute docs don't contain any info about how to use this in (public /usr/include) headers.

Comment 4 Ben Cotton 2023-04-25 17:11:46 UTC
This message is a reminder that Fedora Linux 36 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora Linux 36 on 2023-05-16.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
'version' of '36'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, change the 'version' 
to a later Fedora Linux version. Note that the version field may be hidden.
Click the "Show advanced fields" button if you do not see it.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora Linux 36 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora Linux, you are encouraged to change the 'version' to a later version
prior to this bug being closed.

Comment 5 Ludek Smid 2023-05-25 16:52:15 UTC
Fedora Linux 36 entered end-of-life (EOL) status on 2023-05-16.

Fedora Linux 36 is no longer maintained, which means that it
will not receive any further security or bug fix updates. As a result we
are closing this bug.

If you can reproduce this bug against a currently maintained version of Fedora Linux
please feel free to reopen this bug against that version. Note that the version
field may be hidden. Click the "Show advanced fields" button if you do not see
the version field.

If you are unable to reopen this bug, please file a new report against an
active release.

Thank you for reporting this bug and we are sorry it could not be fixed.


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