[RESOLVED] Node addon which uses std::unordered_set will crash on Node.js 9.3.0

[fs-eire]

• Version: 9.3.0
• Platform: Linux ubuntu 44.0-31-generic path.resolve across drives in Windows #50~1404.1-Ubuntu SMP Wed Jul 13 UTC 2016
• Subsystem:

A simple node addon which uses std::unordered_set will crash on Node.js 9.3.0

• This crash starts in commit cf7e15c . ( identified by git bisect )
• Happens in Ubuntu 14.04. Does NOT happen on Windows/mac OS/Ubuntu 16.04
• Happens in G++5.x and G++6.x. Does NOT happen on G++4.x.
• Here is the link to travis Cl build which contains the stacktrace info from core dump.
• Here is the corresponding code. ( A modified hello-world addon )

EDIT (2018-05-01):
Solution: add linker flag 'ldflags': ['-Wl,-Bsymbolic', '-Wl,-Bsymbolic-functions'] to resolve this issue.

[bnoordhuis]

Thanks for the detailed bug report. I don't have an easy way to reproduce but my first hunch would be a libstdc++ ABI issue. Does it work when you add a 'cflags': ['-fabi-version=7'] stanza to your binding.gyp?

Relevant links:

https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html

[fs-eire]

I've tried to add the flag but it doesn't fix the crash.

{
  "targets": [
    {
      "target_name": "addon",
      "sources": [ "hello.cpp" ],
      'cflags': [ '-fabi-version=7' ],
      'cflags_cc': [ '-fabi-version=7' ]
    }
  ]
}

[seishun]

Does this happen regardless if you build Node.js yourself or use the official binary?

[fs-eire]

@seishun Yes.

For official build, it is good on 9.2.1 and crashes on 9.3.0.

For my build, I tested by git bisect and found it failed on commit cf7e15c.

[TimothyGu]

Sounds possibly like some kind of uninitialized memory usage/corruption… @fs-eire have you tried using Valgrind? I’ll try to reproduce later today.

[fs-eire]

@TimothyGu No I didn't. Actually I know it just now. Will learn how to use it later.

BTW, I am not sure whether the crash is introduced in the specific commit or not... It may be a good idea to reproduce with nvm install 9.3.0 (this was the first place where I found the crash).

[fs-eire]

Happy new year guys :) Any updates on this issue?

[fs-eire]

The crash is still happening in Node 9.4.0

Is anyone investigating this issue?

[bnoordhuis]

I don't think so. I wasn't able to reproduce with ubuntu 14.04 + g++ 4.8 (like you said) and I wasn't quite ready to upgrade to g++ >= 5.

[fs-eire]

@bnoordhuis Yes the crash only happens with the binary that built by G++ >=5.

There is no worry about upgrading G++ to 5. You can have both G++4.x and 5.x installed and use update-alternatives to switch the one you want. Please let me know if you still have problem reproducing the issue.

[fs-eire]

Recently I found this issue is also happening in Node.js 8.10.0 (8.9.4 is no problem)

Is there a way to get rid of this issue?

[fs-eire]

Update the broken commit identified by git bisect: 331b175.

Although it is the different commit, it's the same change landed on different branch cf7e15c ; 331b175

[AdamMajer]

I can't reproduce this problem with Node 8.10.0 or Node 9.10.1 compiled with g++ (SUSE Linux) 7.3.1 20180307 [gcc-7-branch revision 258314]

> a = require('./addon')
before emplace
emplace console{ hello: [Function: hello] }
> a.hello()
'world'
>

compiler problem?

[fs-eire]

@AdamMajer me too. The problem only happen on gcc5.x/6.x+Ubuntu14.04. Other compiler/platform combination does not have this issue.

May be a compiler problem, or compiler-OS compatibility problem.

[fs-eire]

Node executable exposes some symbols that defined in stdlibc++.so

When addon links to the same symbol, the wrong address may be used and causes crash.

Specifically, the commit mentioned above instantiate a std::unordered_set<std::string> object and causes node executable to expose the symbol _ZNSt10_HashtableISsSsSaISsENSt8__detail9_IdentityESt8equal_toISsESt4hashISsENS1_18_Mod_range_hashingENS1_20_Default_ranged_hashENS1_20_Prime_rehash_policyENS1_17_Hashtable_traitsILb1ELb1ELb1EEEE21_M_insert_unique_nodeEmmPNS1_10_Hash_nodeISsLb1EEE. When the addon create a std::unordered_set<std::string> object it will call into the address of node instead of stdlibc++.so.

This issue actually exists on Windows, too. By specifying /MT to VC linker will resolve this issue. However, I failed on linux, seems -static-libgcc/-static-libstdc++ does not help.

I'm not that familiar with linux platform, I'd be appreciated if you can help. @AdamMajer @bnoordhuis

Also, I think the best solution is to limit node executable to expose any symbols that from standard library.

[bnoordhuis]

Okay, that sounds plausible. This however:

When the addon create a std::unordered_set<std::string> object it will call into the address of node instead of stdlibc++.so.

...isn't quite correct. std::unordered_set is a template class, libstdc++ doesn't contain its definition. Your add-on is supposed to use its own copy that's generated by the compiler, but instead it picks the one that node exports. You can fix that by changing the link order of your add-on.

That said, being more discriminate with what we export is a good idea. (But not entirely trivial to implement - you can find discussions going back at least five years.)

[fs-eire]

@bnoordhuis Thanks for the information! Regarding this-

Your add-on is supposed to use its own copy that's generated by the compiler, but instead it picks the one that node exports. You can fix that by changing the link order of your add-on.

I am not sure if this can be done. From the build log outputted by node-gyp build --verbose:

make: Entering directory '/home/eire/git/try-node-crash/build'
  g++ '-DNODE_GYP_MODULE_NAME=addon' '-DUSING_UV_SHARED=1' '-DUSING_V8_SHARED=1' '-DV8_DEPRECATION_WARNINGS=1' '-D_LARGEFILE_SOURCE' '-D_FILE_OFFSET_BITS=64' '-DBUILDING_NODE_EXTENSION' -I/home/eire/.node-gyp/10.0.0/include/node -I/home/eire/.node-gyp/10.0.0/src -I/home/eire/.node-gyp/10.0.0/deps/uv/include -I/home/eire/.node-gyp/10.0.0/deps/v8/include  -fPIC -pthread -Wall -Wextra -Wno-unused-parameter -m64 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++1y -MMD -MF ./Release/.deps/Release/obj.target/addon/hello.o.d.raw   -c -o Release/obj.target/addon/hello.o ../hello.cpp
  g++ -shared -pthread -rdynamic -m64  -Wl,-soname=addon.node -o Release/obj.target/addon.node -Wl,--start-group Release/obj.target/addon/hello.o -Wl,--end-group
  rm -rf "Release/addon.node" && cp -af "Release/obj.target/addon.node" "Release/addon.node"
make: Leaving directory '/home/eire/git/try-node-crash/build'

It looks no link order concern since the only one object linked is hello.o. I've tried several ways but cannot solve it.. Node seems always prefer the one inside when the same symbol exists in both node and addon.node 😞

[bnoordhuis]

Does it work when you add -fvisibility=hidden to your binding.gyp's cflags?

[fs-eire]

Seems not working with -fvisibility=hidden.

Using nm addon.node still output the symbol (type 'W')

[bnoordhuis]

Huh, so it's weak? That explains why the symbol from the node binary is used but it's weird that it's weak. What happens when you add a 'ldflags': ['-Wl,-z,now'] stanza to binding.gyp?

[fs-eire]

No change happens to addon.node.

BTW, the symbol is also W in node. And a bunch other STL types are also W in addon.node and node.

[bnoordhuis]

Ah wait, I mean 'ldflags': ['-Wl,-Bsymbolic', '-Wl,-Bsymbolic-functions']. '-Wl,-z,now' is still a good idea but for different reasons.

[fs-eire]

'ldflags': ['-Wl,-Bsymbolic', '-Wl,-Bsymbolic-functions'] - this flag works. :)

-Bsymbolic is -

When creating a shared library, bind references to global symbols to the definition within the shared library, if any. Normally, it is possible for a program linked against a shared library to override the definition within the shared library. This option is only meaningful on ELF platforms which support shared libraries.

So node is the program who overrides the definition in addon.node. For addon, it looks a good idea to always specify those flags unless there is a reason not to.

It's still a mystery to me why the crash happens on some of the OS/compiler combinations and not on the others...

[jasnell]

does this need to remain open?

[TimothyGu]

Seems like it's been resolved.