[vlc-devel] [PATCH 1/2] configure: allow toggling of -Wl, -z, defs and -no-undefined

Rémi Denis-Courmont remi at remlab.net
Wed Mar 15 15:06:13 CET 2017


On mercredi 15 mars 2017 14:10:31 EET Thomas Guillem wrote:
> On Wed, Mar 15, 2017, at 13:51, Rémi Denis-Courmont wrote:
> > On March 15, 2017 11:10:27 AM GMT+02:00, "Filip Roséen" <filip at atch.se>
> > 
> > wrote:
> > >There are cases where it is annoying to build VLC with this
> > >restriction as you would like to use symbols that are simply not
> > >present (in terms of definition) when things are being built.
> > >
> > >SUMMARY:
> > > - "-Wl,-z,defs" is now opt-out + automatic detection
> > > - "-no-undefined" is now opt-out
> > >
> > >Both options are governed by a new configure flag,
> > >"--{enable,disable}-no-undefined-symbols" as they are tightly coupled.
> > >
> > >--
> > >
> > >RATIONALE:
> > > - simplify usage of -fsanitize together with llvm/clang
> > > - out-of-tree definitions of VLC related functions (I sometimes do
> > > 
> > >   this for extensive testing by building object files that I later
> > >   circle between when trying out samples).
> > >
> > >Using llvm/clang with -fsanitize=address, the symbols put in the
> > >resulting object-file related to the address sanitizer are not
> > >available, and we will get an error diagnostic due to the relevant
> > >flags.
> > >
> > >The below linked articles elaborates the issue further:
> > > - https://clang.llvm.org/docs/AddressSanitizer.html#usage
> > > - https://github.com/google/sanitizers/wiki/AddressSanitizer#faq
> > >
> > >One can circumvent the issue by manually specifying flags to
> > >effectivelly have the symbols resolved during linking, and preventing
> > >
> > >use of the relevant shared library. The drawbacks are, of course:
> > > - negative impact on build time
> > > - negative impact on execution speed
> > > - unnecessary complex for those who are not familiar with the
> > > 
> > >   intrinsics of libtools and configure.ac
> > >
> > >---
> > >
> > > configure.ac | 24 +++++++++++++++---------
> > > 1 file changed, 15 insertions(+), 9 deletions(-)
> > >
> > >diff --git a/configure.ac b/configure.ac
> > >index 71f19c43f9..1081e14ec8 100644
> > >--- a/configure.ac
> > >+++ b/configure.ac
> > >@@ -1086,16 +1086,22 @@ VLC_RESTORE_FLAGS
> > >
> > > SYMBOLIC_LDFLAGS="${ac_cv_ld_bsymbolic}"
> > > AC_SUBST(SYMBOLIC_LDFLAGS)
> > >
> > >-VLC_SAVE_FLAGS
> > >-LDFLAGS="${LDFLAGS} -Wl,-z,defs"
> > >-AC_CACHE_CHECK([if linker supports -z,defs], [ac_cv_ld_z_defs], [
> > >-  AC_TRY_LINK([],, [
> > >-    ac_cv_ld_z_defs="yes"
> > >-  ], [
> > >-    ac_cv_ld_z_defs="no"
> > >-  ])
> > >+dnl
> > >+dnl No undefined symbols for libvlc* objects
> > >+dnl
> > >+AC_ARG_ENABLE(no-undefined-symbols,
> > >+  [AS_HELP_STRING([--disable-no-undefined-symbols],
> > >+    [disable error on undefined symbols when linking \
> > >+     core VLC objects (default enabled)] )] )
> > >+
> > >+AS_IF([test "${enable_no_undefined_symbols}" != "no" ], [
> > >+  VLC_SAVE_FLAGS
> > >+  LDFLAGS="${LDFLAGS} -Wl,-z,defs"
> > >+  AC_CACHE_CHECK([if linker supports -z,defs], [ac_cv_ld_z_defs], [
> > >+    AC_TRY_LINK([],, [ac_cv_ld_z_defs="yes"], [ac_cv_ld_z_defs="no"])
> > >])
> > >+  AS_IF([test "${ac_cv_ld_z_defs}" = "no"], [VLC_RESTORE_FLAGS])
> > >+  AC_SUBST([LDFLAGS_no_undefined], [-no-undefined])
> > >
> > > ])
> > >
> > >-AS_IF([test "${ac_cv_ld_z_defs}" = "no"], [VLC_RESTORE_FLAGS])
> > >
> > > dnl Check for __attribute__((packed))
> > > AC_CACHE_CHECK([for __attribute__((packed))],
> > 
> > I added this because I am fed up with devs not checking their
> > dependencies correctly, breaking my and Debian builds frequently.
> > 
> > So obviously, I am very much against this change. This removes a very
> > useful build-time diagnostic and WILL cause more build breaks.
> 
> This this won't be enabled by default. I guess devs won't enable it

"Famous last words".

> unless they have to use a sanitizer. As sanitizer builds are very slow,
> devs will keep a normal build with this diagnostic enabled (by default).

I use sanitizers basically all the time. My main build has sanitizers on GCC. 
I have a separate build with sanitizers on LLVM. (I do have an unsanitized 
build, essentially only to run valgrind or whatever else is incompatible with 
sanitizers.)  In either cases, I have had no problems without this, so what 
gives?

You might need to add some -l flags depending how well integrated sanitizers 
are. That should not be an issue since you have to modify build flags to use 
sanitizers anyway.

This is adding complexity for... I don't really understand for what really. 
And gives developpers another way to shoot themselves in the foot.

Undefined symbols are forbidden on Windows, MacOS, Solaris (AFAIU). They are 
mandatory on FreeBSD, due to what looks like a libc bug. It's only optional on 
Linux, but forbidden by all main distros.

AFAICT, the only reason to use this is if you link plugins back to the main 
executable. We don't do this because it wouldn't work on other platforms.

> Personally, I develop mainly on a build with --enable-debug and without
> any other options. When I want to use a sanitizer, a use an other dir
> with different configure rules.

If you really want to make sanitizers easy to use, then add an option to 
enable them, like we do for coverage. Not this patch, please.



More information about the vlc-devel mailing list