1166158 - Add -fvisibility-inlines-hidden to VISIBILITY_FLAGS to hide inline member functions

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: fvisibility-inlines-hidden.patch

gcc -fvisibility=hidden does not include -fvisibility-inlines-hidden:

https://gcc.gnu.org/wiki/Visibility

> Lastly, there's one other new command line switch: -fvisibility-inlines-hidden.
> This causes all inlined class member functions to have hidden visibility,
> causing significant export symbol table size & binary size reductions
> though not as much as using -fvisibility=hidden. However,
> -fvisibility-inlines-hidden can be used with no source alterations, unless
> you need to override it for inlines where address identity is important
> either for the function itself or any function local static data.

> In your build system (Makefile etc), you will probably wish to add the
> -fvisibility=hidden and -fvisibility-inlines-hidden options to the command
> line arguments of every GCC invocation. Remember to test your library
> thoroughly afterwards, including that all exceptions correctly traverse
> shared object boundaries. 

Simon Strandman caught this oversight almost 10 years ago in bug 273336 comment 62 ("A small question before this bug is forgotten."), but that bug had already been resolved and forgotten.

Some Try tests are failing, but they look like unrelated or intermittent issues:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=494e154719ce

Comment 1

[Mike Hommey [:glandium]]

Comment on attachment 8607346 [details] [diff] [review]
fvisibility-inlines-hidden.patch

Review of attachment 8607346 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +2622,5 @@
>    AC_DEFINE(HAVE_VISIBILITY_HIDDEN_ATTRIBUTE)
>    AC_DEFINE(HAVE_VISIBILITY_ATTRIBUTE)
>    case "$OS_TARGET" in
>    Darwin)
> +    VISIBILITY_FLAGS='-fvisibility=hidden -fvisibility-inlines-hidden'

AFAICT, this doesn't seem to have a negative impact. On the other hand, it doesn't seem to have much of a positive impact either. There are only 10 less exported function symbols, and one less exported data symbol, if I compare your try build with the corresponding m-i build.

@@ +2627,5 @@
>      ;;
>    *)
>      case $GCC_VERSION in
>      4.6*)
>        VISIBILITY_FLAGS='-I$(DIST)/system_wrappers -include $(MOZILLA_DIR)/config/gcc_hidden_dso_handle.h'

Thanks for having me notice this (filed bug 1167005)

Comment 2

[Chris Peterson [:cpeterson]]

Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/918a7267cfbc

Comment 3

[Ekanan Ketunuti]

wrong bug id in commit msg.

https://hg.mozilla.org/mozilla-central/rev/918a7267cfbc