build,test: add duplicate symbol test

On OSX symbols are exported by default, and they overlap if two
different copies of the same symbol appear in a process. This change
ensures that, on OSX, symbols are hidden by default.

Re: https://github.com/nodejs/node-addon-api/pull/456
Fixes: https://github.com/nodejs/node/issues/26765
PR-URL: https://github.com/nodejs/node-gyp/pull/1689
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>

addon.gypi

@@ -90,10 +90,14 @@
 
     'conditions': [
       [ 'OS=="mac"', {
+        'cflags': [
+          '-fvisibility=hidden'
+        ],
         'defines': [
           '_DARWIN_USE_64_BIT_INODE=1'
         ],
         'xcode_settings': {
+          'GCC_SYMBOLS_PRIVATE_EXTERN': 'YES', # -fvisibility=hidden
           'DYLIB_INSTALL_NAME_BASE': '@rpath'
         },
       }],

test/node_modules/duplicate_symbols/binding.cc

@@ -0,0 +1,10 @@
+#include <nan.h>
+#include "common.h"
+
+void Init(v8::Local<v8::Object> exports) {
+  exports->Set(Nan::New("pointerCheck").ToLocalChecked(),
+               Nan::New<v8::FunctionTemplate>(Something::PointerCheck)
+      ->GetFunction());
+}
+
+NODE_MODULE(NODE_GYP_MODULE_NAME, Init)

test/node_modules/duplicate_symbols/binding.gyp

@@ -0,0 +1,19 @@
+{
+  "target_defaults": {
+    "include_dirs": [
+      "<!(node -e \"require('nan')\")"
+    ],
+    "sources": [
+      "binding.cc",
+      "extra.cc"
+    ]
+  },
+  "targets": [
+    {
+      "target_name": "binding1",
+    },
+    {
+      "target_name": "binding2",
+    }
+  ]
+}

test/node_modules/duplicate_symbols/common.h

@@ -0,0 +1,37 @@
+#ifndef DUPLICATE_SYMBOLS_COMMON_H_
+#define DUPLICATE_SYMBOLS_COMMON_H_
+
+#include <nan.h>
+
+class Something {
+ public:
+  static void PointerCheck(const Nan::FunctionCallbackInfo<v8::Value>& info);
+};
+
+// Removing the inline keyword below will result in the addon failing to link
+// on OSX because of a duplicate symbol.
+inline void
+Something::PointerCheck(const Nan::FunctionCallbackInfo<v8::Value>& info) {
+  v8::Local<v8::Value> v8result;
+
+  if (info.Length() > 0) {
+    // If an argument was passed in, it is a pointer to the `PointerCheck`
+    // method from the other addon. So, we compare it to the value of the
+    // pointer to the `PointerCheck` method in this addon, and return
+    // `"equal"` if they are equal, and `"not equal"` otherwise".
+
+    const char* result =
+      (reinterpret_cast<void*>(Something::PointerCheck) ==
+          info[0].As<v8::External>()->Value()) ?
+              "equal" : "not equal";
+    v8result = Nan::New(result).ToLocalChecked();
+  } else {
+    // If no argument was passed in, we wrap the pointer to the `PointerCheck`
+    // method in this addon into a `v8::External` and pass it into JavaScript.
+    v8result = Nan::New<v8::External>(
+        reinterpret_cast<void*>(Something::PointerCheck));
+  }
+  info.GetReturnValue().Set(v8result);
+}
+
+#endif  // DUPLICATE_SYMBOLS_COMMON_H_

test/node_modules/duplicate_symbols/extra.cc

@@ -0,0 +1,6 @@
+#include "common.h"
+
+// It is important that common.h be included from two different translation
+// units, because doing so can create duplicate symbols in some instances. If
+// it does so and fails to build because of it, that is considered a test
+// failure.

test/node_modules/duplicate_symbols/index.js

@@ -0,0 +1,5 @@
+'use strict'
+module.exports = {
+  pointerCheck1: require('bindings')('binding1').pointerCheck,
+  pointerCheck2: require('bindings')('binding2').pointerCheck
+};

test/node_modules/duplicate_symbols/package.json

@@ -0,0 +1,14 @@
+{
+  "name": "duplicate_symbols",
+  "version": "0.0.0",
+  "description": "Duplicate Symbols Test",
+  "main": "index.js",
+  "private": true,
+  "dependencies": {
+    "bindings": "~1.2.1",
+    "nan": "^2.0.0"
+  },
+  "scripts": {
+    "test": "node index.js"
+  }
+}

test/test-addon.js

@@ -19,6 +19,16 @@ function runHello(hostProcess) {
   return execFileSync(hostProcess, ['-e', testCode], { cwd: __dirname }).toString()
 }
 
+function runDuplicateBindings() {
+  const hostProcess = process.execPath;
+  var testCode =
+    "console.log((function(bindings) {" +
+    "return bindings.pointerCheck1(bindings.pointerCheck2());" +
+    "})(require('duplicate_symbols')))"
+  console.log('running ', hostProcess);
+  return execFileSync(hostProcess, ['-e', testCode], { cwd: __dirname }).toString()
+}
+
 function getEncoding() {
   var code = 'import locale;print locale.getdefaultlocale()[1]'
   return execFileSync('python', ['-c', code]).toString().trim()
@@ -52,6 +62,21 @@ test('build simple addon', function (t) {
   proc.stderr.setEncoding('utf-8')
 })
 
+test('make sure addon symbols do not overlap', function (t) {
+  t.plan(3)
+
+  var addonPath = path.resolve(__dirname, 'node_modules', 'duplicate_symbols')
+  // Set the loglevel otherwise the output disappears when run via 'npm test'
+  var cmd = [nodeGyp, 'rebuild', '-C', addonPath, '--loglevel=verbose']
+  execFile(process.execPath, cmd, function (err, stdout, stderr) {
+    var logLines = stderr.trim().split(/\r?\n/)
+    var lastLine = logLines[logLines.length-1]
+    t.strictEqual(err, null)
+    t.strictEqual(lastLine, 'gyp info ok', 'should end in ok')
+    t.strictEqual(runDuplicateBindings().trim(), 'not equal')
+  })
+})
+
 test('build simple addon in path with non-ascii characters', function (t) {
   t.plan(1)