diff mbox

[FFmpeg-devel] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__

Message ID CAFP8O3L086oFgAz=2Ng-mL9n3TGq86ziONvU5QL0d-qpsd5K8w@mail.gmail.com
State Superseded
Headers show

Commit Message

Fangrui Song March 7, 2019, 3:53 a.m. UTC 
Sorry, wrong signed-off-by line..

(isn't using my usual mail client mutt and i get very sick of the
webmail...)

On Thu, Mar 7, 2019 at 11:36 AM Fāng-ruì Sòng <maskray@google.com> wrote:

> On Wed, Mar 6, 2019 at 4:14 PM Carl Eugen Hoyos <ceffmpeg@gmail.com>
> wrote:
>
>> 2019-02-21 2:37 GMT+01:00, Fāng-ruì Sòng <
>> maskray-at-google.com@ffmpeg.org>:
>> > Sorry if this doesn't attach to the correct thread as I didn't
>> > subscribe to this list and don't know the Message-ID of the thread.
>>
>> The thread works fine here with gmail but your patch was corrupted,
>> you have to attach it.
>>
>> Carl Eugen
>>
>>
> Sorry. Here is the attached patch (git format-patch -1 --stdout).
>

Comments

Fangrui Song March 14, 2019, 1:42 a.m. UTC | #1 
Ping :)

In short, when people build .so from ffmpeg .o/.a files, they no longer
need -Bsymbolic
-Bsymbolic is bad because it breaks C++ semantics, e.g. address uniqueness
of inline functions and their static local variables, uniqueness of
template specializations, address uniqueness of type_info objects (used for
C++ exceptions), etc

On Thu, Mar 7, 2019 at 11:53 AM Fāng-ruì Sòng <maskray@google.com> wrote:

> Sorry, wrong signed-off-by line..
>
> (isn't using my usual mail client mutt and i get very sick of the
> webmail...)
>
> On Thu, Mar 7, 2019 at 11:36 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>
>> On Wed, Mar 6, 2019 at 4:14 PM Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> wrote:
>>
>>> 2019-02-21 2:37 GMT+01:00, Fāng-ruì Sòng <
>>> maskray-at-google.com@ffmpeg.org>:
>>> > Sorry if this doesn't attach to the correct thread as I didn't
>>> > subscribe to this list and don't know the Message-ID of the thread.
>>>
>>> The thread works fine here with gmail but your patch was corrupted,
>>> you have to attach it.
>>>
>>> Carl Eugen
>>>
>>>
>> Sorry. Here is the attached patch (git format-patch -1 --stdout).
>>
>
>
> --
> 宋方睿
>
diff mbox

Patch

From 63d1c46c9796ca4a804cc89573c8f4613716b50d Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Wed, 20 Feb 2019 16:25:46 +0800
Subject: [PATCH] avutil/mem: Mark DECLARE_ASM_ALIGNED as visibility("hidden") for __GNUC__

Inline asm code assumes these DECLARE_ASM_ALIGNED declared global constants are non-preemptive, e.g.

libavcodec/x86/cabac.h
        "lea    "MANGLE(ff_h264_cabac_tables)", %0      \n\t"

On ELF platforms, when -Wl,-Bsymbolic
-Wl,--version-script,libavcodec/libavcodec.ver are removed from the
linker command line, the symbol will be considered preemptive and fail
to link to a DSO:

    ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol ff_h264_cabac_tables; recompile with -fPIC

It is better to express the intention explicitly and mark such global
constants hidden. It also improves portability as no linker magic is
required.

DECLARE_ASM_CONST uses the "static" specifier to indicate internal
linkage. The visibility annotation is unnecessary.

Signed-off-by: Fangrui Song <maskray@google.com>
---
 libavutil/mem.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavutil/mem.h b/libavutil/mem.h
index 5fb1a02..9afeed0 100644
--- a/libavutil/mem.h
+++ b/libavutil/mem.h
@@ -100,6 +100,12 @@ 
  * @param v Name of the variable
  */
 
+#if defined(__GNUC__) && !(defined(_WIN32) || defined(__CYGWIN__))
+    #define DECLARE_HIDDEN __attribute__ ((visibility ("hidden")))
+#else
+    #define DECLARE_HIDDEN
+#endif
+
 #if defined(__INTEL_COMPILER) && __INTEL_COMPILER < 1110 || defined(__SUNPRO_C)
     #define DECLARE_ALIGNED(n,t,v)      t __attribute__ ((aligned (n))) v
     #define DECLARE_ASM_ALIGNED(n,t,v)  t __attribute__ ((aligned (n))) v
@@ -110,7 +116,7 @@ 
     #define DECLARE_ASM_CONST(n,t,v)    static const t av_used __attribute__ ((aligned (FFMIN(n, 16)))) v
 #elif defined(__GNUC__) || defined(__clang__)
     #define DECLARE_ALIGNED(n,t,v)      t __attribute__ ((aligned (n))) v
-    #define DECLARE_ASM_ALIGNED(n,t,v)  t av_used __attribute__ ((aligned (n))) v
+    #define DECLARE_ASM_ALIGNED(n,t,v)  t av_used __attribute__ ((aligned (n))) DECLARE_HIDDEN v
     #define DECLARE_ASM_CONST(n,t,v)    static const t av_used __attribute__ ((aligned (n))) v
 #elif defined(_MSC_VER)
     #define DECLARE_ALIGNED(n,t,v)      __declspec(align(n)) t v
-- 
2.20.1