From 8b7fdd1fc409029dce248bf543cd7060965539a9 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 11 Mar 2019 22:14:58 -0700 Subject: [PATCH] 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 Reviewed-By: Refael Ackermann --- addon.gypi | 4 ++ .../node_modules/duplicate_symbols/binding.cc | 10 +++++ .../duplicate_symbols/binding.gyp | 19 ++++++++++ test/node_modules/duplicate_symbols/common.h | 37 +++++++++++++++++++ test/node_modules/duplicate_symbols/extra.cc | 6 +++ test/node_modules/duplicate_symbols/index.js | 5 +++ .../duplicate_symbols/package.json | 14 +++++++ test/test-addon.js | 25 +++++++++++++ 8 files changed, 120 insertions(+) create mode 100644 test/node_modules/duplicate_symbols/binding.cc create mode 100644 test/node_modules/duplicate_symbols/binding.gyp create mode 100644 test/node_modules/duplicate_symbols/common.h create mode 100644 test/node_modules/duplicate_symbols/extra.cc create mode 100644 test/node_modules/duplicate_symbols/index.js create mode 100644 test/node_modules/duplicate_symbols/package.json diff --git a/addon.gypi b/addon.gypi index e8b95c5501..e1c4b24af2 100644 --- a/addon.gypi +++ b/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' }, }], diff --git a/test/node_modules/duplicate_symbols/binding.cc b/test/node_modules/duplicate_symbols/binding.cc new file mode 100644 index 0000000000..bc5f6f5134 --- /dev/null +++ b/test/node_modules/duplicate_symbols/binding.cc @@ -0,0 +1,10 @@ +#include +#include "common.h" + +void Init(v8::Local exports) { + exports->Set(Nan::New("pointerCheck").ToLocalChecked(), + Nan::New(Something::PointerCheck) + ->GetFunction()); +} + +NODE_MODULE(NODE_GYP_MODULE_NAME, Init) diff --git a/test/node_modules/duplicate_symbols/binding.gyp b/test/node_modules/duplicate_symbols/binding.gyp new file mode 100644 index 0000000000..46f1c8cba4 --- /dev/null +++ b/test/node_modules/duplicate_symbols/binding.gyp @@ -0,0 +1,19 @@ +{ + "target_defaults": { + "include_dirs": [ + " + +class Something { + public: + static void PointerCheck(const Nan::FunctionCallbackInfo& 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& info) { + v8::Local 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(Something::PointerCheck) == + info[0].As()->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( + reinterpret_cast(Something::PointerCheck)); + } + info.GetReturnValue().Set(v8result); +} + +#endif // DUPLICATE_SYMBOLS_COMMON_H_ diff --git a/test/node_modules/duplicate_symbols/extra.cc b/test/node_modules/duplicate_symbols/extra.cc new file mode 100644 index 0000000000..0c9d5dbebd --- /dev/null +++ b/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. diff --git a/test/node_modules/duplicate_symbols/index.js b/test/node_modules/duplicate_symbols/index.js new file mode 100644 index 0000000000..09dbed81b7 --- /dev/null +++ b/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 +}; diff --git a/test/node_modules/duplicate_symbols/package.json b/test/node_modules/duplicate_symbols/package.json new file mode 100644 index 0000000000..ee21161cf1 --- /dev/null +++ b/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" + } +} diff --git a/test/test-addon.js b/test/test-addon.js index 900b0b049d..12687de2ac 100644 --- a/test/test-addon.js +++ b/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)