diff mbox series

[FFmpeg-devel,2/2] libavcodec: Set hidden visibility on global symbols accessed from AArch64 assembly

Message ID 20220711091850.4158449-2-martin@martin.st
State Superseded
Headers show
Series [FFmpeg-devel,1/2] libavutil: Add av_visibility_hidden for setting hidden symbol visibility | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Martin Storsjö July 11, 2022, 9:18 a.m. UTC
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>
---
 libavcodec/aacsbrdata.h | 2 +-
 libavcodec/fft.h        | 2 +-
 libavcodec/vp9dsp.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Andreas Rheinhardt July 11, 2022, 10:58 a.m. UTC | #1
Martin Storsjö:
> 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>
> ---
>  libavcodec/aacsbrdata.h | 2 +-
>  libavcodec/fft.h        | 2 +-
>  libavcodec/vp9dsp.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/aacsbrdata.h b/libavcodec/aacsbrdata.h
> index 7a11594c9b..aa382c4b52 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 av_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..c2241fbc7c 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 av_visibility_hidden DECLARE_ALIGNED(32, FFTSample, FFT_NAME(ff_cos_##size))[size/2]
>  
>  extern COSTABLE(16);
>  extern COSTABLE(32);
> diff --git a/libavcodec/vp9dsp.c b/libavcodec/vp9dsp.c
> index d8ddf74d4f..1be942d78b 100644
> --- a/libavcodec/vp9dsp.c
> +++ b/libavcodec/vp9dsp.c
> @@ -29,7 +29,7 @@
>  
>  #include "vp9dsp.h"
>  
> -const DECLARE_ALIGNED(16, int16_t, ff_vp9_subpel_filters)[3][16][8] = {
> +const av_visibility_hidden DECLARE_ALIGNED(16, int16_t, ff_vp9_subpel_filters)[3][16][8] = {

Shouldn't you set this in vp9dsp.h, so that all users of it can benefit
from knowing that this symbol is always in the same DSO?

>      [FILTER_8TAP_REGULAR] = {
>          {  0,  0,   0, 128,   0,   0,  0,  0 },
>          {  0,  1,  -5, 126,   8,  -3,  1,  0 },
diff mbox series

Patch

diff --git a/libavcodec/aacsbrdata.h b/libavcodec/aacsbrdata.h
index 7a11594c9b..aa382c4b52 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 av_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..c2241fbc7c 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 av_visibility_hidden DECLARE_ALIGNED(32, FFTSample, FFT_NAME(ff_cos_##size))[size/2]
 
 extern COSTABLE(16);
 extern COSTABLE(32);
diff --git a/libavcodec/vp9dsp.c b/libavcodec/vp9dsp.c
index d8ddf74d4f..1be942d78b 100644
--- a/libavcodec/vp9dsp.c
+++ b/libavcodec/vp9dsp.c
@@ -29,7 +29,7 @@ 
 
 #include "vp9dsp.h"
 
-const DECLARE_ALIGNED(16, int16_t, ff_vp9_subpel_filters)[3][16][8] = {
+const av_visibility_hidden DECLARE_ALIGNED(16, int16_t, ff_vp9_subpel_filters)[3][16][8] = {
     [FILTER_8TAP_REGULAR] = {
         {  0,  0,   0, 128,   0,   0,  0,  0 },
         {  0,  1,  -5, 126,   8,  -3,  1,  0 },