On Thu, Feb 21, 2019 at 10:06 AM Fāng-ruì Sòng <mask...@google.com> wrote:
>
> > Why is it a good idea to remove them from the linker command line?
>
> In short, it improves portability. I'm not suggesting removing
> -Bsymbolic or --version-script from the ffmpeg build system. I mean
> users will no longer have to specify the two options to link ffmpeg
> object files into their own shared objects (AFAIK this patch address
> all C side issues. There are some other problem in *.asm code, though;
> I also saw BROKEN_RELOCATIONS in the code, thinking that it was
> probably added when developers noticed it could bring problems. I
> didn't dig up the history to learn more)
>
> When using a different build system when the relevant SHFLAGS options
> (-Bsymbolic and --version-script) aren't specified, there wouldn't be
> mysterious linker errors "relocation R_X86_64_PC32 cannot be used
> against ...". If the linker options aren't mandatory, ffmpeg can be
> more easily integrated to other projects or build systems.
>
> Or when linking libavcodec/*.o with other object files from the main
> program to form another shared object (not
> libavcodec/libavcodec.so.58). The current limitation (these global
> constants having default visibility) means every shared object linking
> in libavcodec/cabac.o (with the default visibility
> ff_h264_cabac_tables) and libavcodec/x86/constants.o have to use
> either -Bsymbolic, or to construct their own version script.
>
> * -Bsymbolic this option applies to all exported symbols on the linker
> command line, not just ffmpeg object files. This makes symbols
> non-preemptive, and in particular, breaks C++ [dcl.inline], which says
> "A static local variable in an inline function with external linkage
> always refers to the same object." If this option is used, two
> function-local static object may have different addresses.
> * --version-script libavcodec/libavcodec.ver specifies `global: av_*;
> local: *;` Again, the problem is that it applies to all exported
> symbols (may affect program code). If the program code doesn't want
> all its symbols to be marked local, it'll have to define its own
> version script. This is a hindrance that can be avoided.
>
>
> On Thu, Feb 21, 2019 at 9:37 AM Fāng-ruì Sòng <mask...@google.com> wrote:
> >
> > 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 word "also" indicates here that this should be an independent patch.
> >
> > I added `#if defined(__GNUC__) && !(defined(_WIN32) ||
> > defined(__CYGWIN__))`, not `#if (defined(__GNUC__) ||
> > defined(__clang__)) && !(defined(_WIN32) || defined(__CYGWIN__))`. For
> > consistency I removed the defined(__clang__) below. If that change
> > should be an independent one, here is the amended version without the
> > removal of defined(__clang__)
> >
> >
> > 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, if -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 (non-preemptive). 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 <mask...@google.com>
> > ---
> > libavutil/mem.h | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/mem.h b/libavutil/mem.h
> > index 5fb1a02dd9..9afeed0b43 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
>
Friendly ping :)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
