| Message ID | 20220713211438.3134725-1-martin@martin.st |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,v2] libavcodec: Set hidden visibility on global symbols accessed from AArch64 assembly | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | fail | Make failed |
On Thu, Jul 14, 2022 at 12:14:38AM +0300, Martin Storsjö wrote: > The AArch64 assembly accesses those symbols directly, without > indirection via e.g. the GOT on ELF. In order for this not to > require text relocations, those symbols need to be resolved fully > at link time, i.e. those symbols can't be interposable. > > Normally, so far, this is achieved when linking shared libraries > in two ways; we have a version script (libavcodec/libavcodec.v) which > marks all symbols that don't start with av* as local. Additionally, > we try to add -Wl,-Bsymbolic to the linker options if supported, > making sure that such symbol references are resolved fully at link > time, instead of making them interposable. > > When the libavcodec static library is linked into another shared > library, there's no guarantee that it uses similar options (even though > that would be favourable), which would end up requiring text relocations > in the AArch64 assembly. > > Explicitly mark the symbols that are accessed from AArch64 assembly > as hidden, so that they are resolved fully at link time even without > the version script and -Wl,-Bsymbolic. > > Signed-off-by: Martin Storsjö <martin@martin.st> > --- > Moved the attribute to libavutil/internal.h, renamed to a different > namespace (not av_ prefixed), moved the attribute on ff_vp9_subpel_filters > to the header, as suggested. > --- > libavcodec/aacsbrdata.h | 2 +- > libavcodec/fft.h | 2 +- > libavcodec/vp9dsp.h | 2 +- > libavutil/internal.h | 6 ++++++ > 4 files changed, 9 insertions(+), 3 deletions(-) This seems to break build on linux x86-64, i guess iam missing something MAN doc/ffprobe-all.1 In file included from libavcodec/x86/vp9dsp_init_16bpp.c:26:0: ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; ^~~~~~~~~~~~~~~~~~~~~ ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init_16bpp.o' failed make: *** [libavcodec/x86/vp9dsp_init_16bpp.o] Error 1 make: *** Waiting for unfinished jobs.... In file included from libavcodec/x86/vp9dsp_init_16bpp_template.c:26:0, from libavcodec/x86/vp9dsp_init_12bpp.c:25: ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; ^~~~~~~~~~~~~~~~~~~~~ In file included from libavcodec/x86/vp9dsp_init_16bpp_template.c:26:0, from libavcodec/x86/vp9dsp_init_10bpp.c:25: ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; ^~~~~~~~~~~~~~~~~~~~~ ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init_12bpp.o' failed make: *** [libavcodec/x86/vp9dsp_init_12bpp.o] Error 1 ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init_10bpp.o' failed make: *** [libavcodec/x86/vp9dsp_init_10bpp.o] Error 1 In file included from libavcodec/x86/vp9dsp_init.c:26:0: ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; ^~~~~~~~~~~~~~~~~~~~~ ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init.o' failed make: *** [libavcodec/x86/vp9dsp_init.o] Error 1 [...]
On Thu, 14 Jul 2022, Michael Niedermayer wrote: > On Thu, Jul 14, 2022 at 12:14:38AM +0300, Martin Storsjö wrote: >> The AArch64 assembly accesses those symbols directly, without >> indirection via e.g. the GOT on ELF. In order for this not to >> require text relocations, those symbols need to be resolved fully >> at link time, i.e. those symbols can't be interposable. >> >> Normally, so far, this is achieved when linking shared libraries >> in two ways; we have a version script (libavcodec/libavcodec.v) which >> marks all symbols that don't start with av* as local. Additionally, >> we try to add -Wl,-Bsymbolic to the linker options if supported, >> making sure that such symbol references are resolved fully at link >> time, instead of making them interposable. >> >> When the libavcodec static library is linked into another shared >> library, there's no guarantee that it uses similar options (even though >> that would be favourable), which would end up requiring text relocations >> in the AArch64 assembly. >> >> Explicitly mark the symbols that are accessed from AArch64 assembly >> as hidden, so that they are resolved fully at link time even without >> the version script and -Wl,-Bsymbolic. >> >> Signed-off-by: Martin Storsjö <martin@martin.st> >> --- >> Moved the attribute to libavutil/internal.h, renamed to a different >> namespace (not av_ prefixed), moved the attribute on ff_vp9_subpel_filters >> to the header, as suggested. >> --- >> libavcodec/aacsbrdata.h | 2 +- >> libavcodec/fft.h | 2 +- >> libavcodec/vp9dsp.h | 2 +- >> libavutil/internal.h | 6 ++++++ >> 4 files changed, 9 insertions(+), 3 deletions(-) > > This seems to break build on linux x86-64, i guess iam missing something > > MAN doc/ffprobe-all.1 > In file included from libavcodec/x86/vp9dsp_init_16bpp.c:26:0: > ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ > extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; > ^~~~~~~~~~~~~~~~~~~~~ > ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init_16bpp.o' failed > make: *** [libavcodec/x86/vp9dsp_init_16bpp.o] Error 1 > make: *** Waiting for unfinished jobs.... > In file included from libavcodec/x86/vp9dsp_init_16bpp_template.c:26:0, > from libavcodec/x86/vp9dsp_init_12bpp.c:25: > ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ > extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; > ^~~~~~~~~~~~~~~~~~~~~ > In file included from libavcodec/x86/vp9dsp_init_16bpp_template.c:26:0, > from libavcodec/x86/vp9dsp_init_10bpp.c:25: > ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ > extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; > ^~~~~~~~~~~~~~~~~~~~~ > ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init_12bpp.o' failed > make: *** [libavcodec/x86/vp9dsp_init_12bpp.o] Error 1 > ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init_10bpp.o' failed > make: *** [libavcodec/x86/vp9dsp_init_10bpp.o] Error 1 > In file included from libavcodec/x86/vp9dsp_init.c:26:0: > ./libavcodec/vp9dsp.h:123:50: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ff_vp9_subpel_filters’ > extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; > ^~~~~~~~~~~~~~~~~~~~~ > ffbuild/common.mak:81: recipe for target 'libavcodec/x86/vp9dsp_init.o' failed > make: *** [libavcodec/x86/vp9dsp_init.o] Error 1 Thanks, it seems I need to add an explicit include of libavutil/internal.h in vp9dsp.h. Fixing. // Martin
diff --git a/libavcodec/aacsbrdata.h b/libavcodec/aacsbrdata.h index 7a11594c9b..b3899b3e5f 100644 --- a/libavcodec/aacsbrdata.h +++ b/libavcodec/aacsbrdata.h @@ -268,7 +268,7 @@ static const int8_t sbr_offset[6][16] = { }; /* First eight entries repeated at end to simplify SIMD implementations. */ -const DECLARE_ALIGNED(16, INTFLOAT, AAC_RENAME(ff_sbr_noise_table))[][2] = { +const attribute_visibility_hidden DECLARE_ALIGNED(16, INTFLOAT, AAC_RENAME(ff_sbr_noise_table))[][2] = { {Q31(-0.99948153278296f), Q31(-0.59483417516607f)}, {Q31( 0.97113454393991f), Q31(-0.67528515225647f)}, {Q31( 0.14130051758487f), Q31(-0.95090983575689f)}, {Q31(-0.47005496701697f), Q31(-0.37340549728647f)}, {Q31( 0.80705063769351f), Q31( 0.29653668284408f)}, {Q31(-0.38981478896926f), Q31( 0.89572605717087f)}, diff --git a/libavcodec/fft.h b/libavcodec/fft.h index 706c9d07f5..b4066fe0d5 100644 --- a/libavcodec/fft.h +++ b/libavcodec/fft.h @@ -114,7 +114,7 @@ void ff_init_ff_cos_tabs(int index); #endif #define COSTABLE(size) \ - COSTABLE_CONST DECLARE_ALIGNED(32, FFTSample, FFT_NAME(ff_cos_##size))[size/2] + COSTABLE_CONST attribute_visibility_hidden DECLARE_ALIGNED(32, FFTSample, FFT_NAME(ff_cos_##size))[size/2] extern COSTABLE(16); extern COSTABLE(32); diff --git a/libavcodec/vp9dsp.h b/libavcodec/vp9dsp.h index 700dd72de8..13117fe9bc 100644 --- a/libavcodec/vp9dsp.h +++ b/libavcodec/vp9dsp.h @@ -120,7 +120,7 @@ typedef struct VP9DSPContext { vp9_scaled_mc_func smc[5][N_FILTERS][2]; } VP9DSPContext; -extern const int16_t ff_vp9_subpel_filters[3][16][8]; +extern const int16_t attribute_visibility_hidden ff_vp9_subpel_filters[3][16][8]; void ff_vp9dsp_init(VP9DSPContext *dsp, int bpp, int bitexact); diff --git a/libavutil/internal.h b/libavutil/internal.h index b44cbaaa7b..36afc4687f 100644 --- a/libavutil/internal.h +++ b/libavutil/internal.h @@ -61,6 +61,12 @@ #endif #endif +#if (AV_GCC_VERSION_AT_LEAST(4,0) || defined(__clang__)) && (defined(__ELF__) || defined(__MACH__)) +# define attribute_visibility_hidden __attribute__((visibility("hidden"))) +#else +# define attribute_visibility_hidden +#endif + #if defined(_WIN32) && CONFIG_SHARED && !defined(BUILDING_avutil) # define av_export_avutil __declspec(dllimport) #else
The AArch64 assembly accesses those symbols directly, without indirection via e.g. the GOT on ELF. In order for this not to require text relocations, those symbols need to be resolved fully at link time, i.e. those symbols can't be interposable. Normally, so far, this is achieved when linking shared libraries in two ways; we have a version script (libavcodec/libavcodec.v) which marks all symbols that don't start with av* as local. Additionally, we try to add -Wl,-Bsymbolic to the linker options if supported, making sure that such symbol references are resolved fully at link time, instead of making them interposable. When the libavcodec static library is linked into another shared library, there's no guarantee that it uses similar options (even though that would be favourable), which would end up requiring text relocations in the AArch64 assembly. Explicitly mark the symbols that are accessed from AArch64 assembly as hidden, so that they are resolved fully at link time even without the version script and -Wl,-Bsymbolic. Signed-off-by: Martin Storsjö <martin@martin.st> --- Moved the attribute to libavutil/internal.h, renamed to a different namespace (not av_ prefixed), moved the attribute on ff_vp9_subpel_filters to the header, as suggested. --- libavcodec/aacsbrdata.h | 2 +- libavcodec/fft.h | 2 +- libavcodec/vp9dsp.h | 2 +- libavutil/internal.h | 6 ++++++ 4 files changed, 9 insertions(+), 3 deletions(-)