Document adding -fvisibility=hidden flag for macOS users

[NickNaso]

Added section to remember the macOS user to add the -fvisibility=hidden flag.

[gabrielschulhof]

LGTM with some nits.

[gabrielschulhof]

LGTM with one minor nit: Please start the commit message with a verb in the imperative, like
"Document adding -fvisibility=hidden flag for macOS users"

[mhdawson]

LGTM

[vweevers]

@NickNaso @gabrielschulhof Could you summarize why you recommend -fvisibility=hidden? Are there any risks / downsides? It seems to stem from #456, is that right?

I'm asking because leveldown has a similar issue (Level/leveldown#687) and -fvisibility=hidden might solve it but I'm not sure what the implications are for a native addon.

Thanks!

[NickNaso]

Hi @vweevers,
what I remember is that on OSX symbols are exported by default, and they overlap if two different copies of the same symbol appear in a process. This means that if you build two different native add-ons and export the same symbol on OSX there is a risk to call a function on the wrong native add-on. In our case #456 we built two different native add-ons one that support the exception (binding.node) and another not (binding_noexcept.node) and we discovered that on OSX loading the native add-ons on the same process it could happens that a function within the wrong native add-on will be called.
@gabrielschulhof reported this to node-gyp maintainers you can follow the status of the duplicate symbol check here on this PR nodejs/node-gyp#1891
The right solution for now is to follow the suggestion proposed here: https://github.com/nodejs/node-addon-api/blob/master/doc/setup.md#installation-and-usage at point 4.
I hope that this answer help to you.

[mvduin]

Note that -fvisibility=hidden should be enabled not just on macOS but also on Linux and every other platform that supports it, which I'm guessing is probably every platform except Windows where this behaviour is default anyway (more or less), not just for node addons but for shared libraries in general.

Even if it happens to not crash, you obviously don't want symbols from one module to unintentionally resolve against the same symbol defined in a different module, that's a recipe for horrifyingly weird problems that only occur when certain combinations of modules are loaded. This is especially true for libraries like node-addon-api which is partially parameterized through preprocessor macros.

Note that the disturbing default behaviour for shared libraries on ELF-based targets (such as Linux) is that every symbol with external linkage is resolved at runtime by the dynamic linker even if that symbol is defined in the same file, i.e. if you write:

void foo() {}
void bar() { foo(); }

then bar may in fact call foo imported from elsewhere rather than the one defined right above it. This also applies to functions declared as inline if the compiler chose to not actually inline it, and it prevents inlining of functions that aren't explicitly declared as inline (such as inlining foo into bar above).

As a result, -fvisibility=hidden also tends to significiantly reduce the size of and improve the performance of shared libraries.