search

mozilla / spidernode

Node.js on top of SpiderMonkey
https://ehsanakhgari.org/blog/2016-04-20/project-spidernode
Other
561 stars 42 forks source link

merge upstream Node #351

Closed mykmelez closed 7 years ago

mykmelez commented 7 years ago

@tbsaunde This is the latest SpiderNode master with your two nodemerge branches merged into it, then another merge from upstream Node.

Currently, I see two issues:

mykmelez commented 7 years ago

The fatal error: 'unicode/uchar.h' file not found error is happening because node.gyp has a new conditional dependency on v8 when node_use_bundled_v8=="true". Adding and node_engine=="v8" to that condition resolves that problem.

mykmelez commented 7 years ago

After a variety of fixes/stubs (and another merge from upstream), this is now building and passing tests for me locally. Let's see what Travis has to say.

mykmelez commented 7 years ago

After a variety of fixes/stubs (and another merge from upstream), this is now building and passing tests for me locally. Let's see what Travis has to say.

Travis said the ASAN builds were failing. Back in 7c9eab3, when I updated spidershim::InitLibraries to deal with the new output of js2c.py, I naively cast id##_name from its new type (const uint8_t) to const char*, which is what it used to be, and what SpiderMonkey's CompileOptions::setFile expects to receive.

But the new implementation of id##_name isn't null-terminated, so the address sanitizer complained about a global buffer overflow when some SpiderMonkey code tried to determine the length of the array via strlen. Thus 27ed18c updates the implementation of InitLibraries to copy id##_name to a null-terminated uint8_t array, which it can then cast to const char* and pass to CompileOptions::setFile safely.

I could have fixed this upstream of InitLibraries, by changing the implementation of js2c.py, but that would have affected more Node code (like the V8 and ChakraShim implementations of InitLibraries). I could also have fixed this downstream, by changing the implementation of CompileOptions::setFile, but that again has broader ramifications, since it affects the SpiderMonkey public API. Keeping the change confined to InitLibraries seems like the best choice.

mykmelez commented 7 years ago

I just reviewed the changes one more time to confirm that they're reasonable, and I made a couple of additional minor fixups. I'll go ahead and merge this once the Travis build completes.