diff mbox series

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

Message ID 20220827193340.737970-1-martin@martin.st
State Accepted
Commit e4ac156b7c47725327dff78bb83f5eecbaee3add
Headers show
Series [FFmpeg-devel,v4] libavcodec: Set hidden visibility on global symbols accessed from AArch64 assembly | expand

Commit Message

Martin Storsjö Aug. 27, 2022, 7:33 p.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>
---
v4: Moved the attribute definition to a new, standalone header (which
only depends on libavutil/attributes.h).
---
 libavcodec/aacsbrdata.h         |  3 ++-
 libavcodec/fft.h                |  3 ++-
 libavcodec/vp9dsp.h             |  3 ++-
 libavutil/attributes_internal.h | 30 ++++++++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 libavutil/attributes_internal.h

Comments

Martin Storsjö Sept. 2, 2022, 1:25 p.m. UTC | #1 
On Sat, 27 Aug 2022, 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>
> ---
> v4: Moved the attribute definition to a new, standalone header (which
> only depends on libavutil/attributes.h).
> ---
> libavcodec/aacsbrdata.h         |  3 ++-
> libavcodec/fft.h                |  3 ++-
> libavcodec/vp9dsp.h             |  3 ++-
> libavutil/attributes_internal.h | 30 ++++++++++++++++++++++++++++++
> 4 files changed, 36 insertions(+), 3 deletions(-)
> create mode 100644 libavutil/attributes_internal.h

OK'd by Andreas on irc, will push later today.

// Martin
diff mbox series

Patch

diff --git a/libavcodec/aacsbrdata.h b/libavcodec/aacsbrdata.h
index 7a11594c9b..7bb45b229e 100644
--- a/libavcodec/aacsbrdata.h
+++ b/libavcodec/aacsbrdata.h
@@ -29,6 +29,7 @@ 
 #define AVCODEC_AACSBRDATA_H
 
 #include <stdint.h>
+#include "libavutil/attributes_internal.h"
 #include "libavutil/mem_internal.h"
 #include "aac_defines.h"
 
@@ -268,7 +269,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..d46e5a3f0b 100644
--- a/libavcodec/fft.h
+++ b/libavcodec/fft.h
@@ -29,6 +29,7 @@ 
 #include <stdint.h>
 #include "config.h"
 
+#include "libavutil/attributes_internal.h"
 #include "libavutil/mem_internal.h"
 
 #if FFT_FLOAT
@@ -114,7 +115,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..be0ac0b181 100644
--- a/libavcodec/vp9dsp.h
+++ b/libavcodec/vp9dsp.h
@@ -28,6 +28,7 @@ 
 #include <stdint.h>
 
 #include "libavcodec/vp9.h"
+#include "libavutil/attributes_internal.h"
 
 typedef void (*vp9_mc_func)(uint8_t *dst, ptrdiff_t dst_stride,
                             const uint8_t *ref, ptrdiff_t ref_stride,
@@ -120,7 +121,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/attributes_internal.h b/libavutil/attributes_internal.h
new file mode 100644
index 0000000000..9d3d10b63e
--- /dev/null
+++ b/libavutil/attributes_internal.h
@@ -0,0 +1,30 @@ 
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVUTIL_ATTRIBUTES_INTERNAL_H
+#define AVUTIL_ATTRIBUTES_INTERNAL_H
+
+#include "attributes.h"
+
+#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
+
+#endif /* AVUTIL_ATTRIBUTES_INTERNAL_H */