Segfault 11 on 0.10.31 #8208
Comments
|
Bad news, this is that CVE patch. cc @tjfontaine |
|
Hm... it is really odd that Is there any way to reproduce this segfault? |
|
Or can I connect to that machine and take a look at the core file? |
|
Unfortunately its private code so I can't share. I've got another system which uses some of the same libraries but the issue isn't occurring there. Some of the packages used in our application are: If there's any additional information I can provide please let me know and I will do what I can. |
|
Hm... Could you please gist a disassembly of |
|
Hm... May I ask you to build node.js in debug mode and try reproducing this issue again?
|
|
And the binary will be in |
|
Thank you, this is much more informative :) |
|
One last thing, could you please gist a |
|
Hm... it seems that that segv module isn't properly unwinding stack. Could you please turn on core dumps with |
|
Sorry I don't understand how to get the core files for you? Or how to generate them. I'm not familiar with |
|
|
Steps to reproduce... git clone https://github.com/acuminous/yadda.git
cd yadda
npm install
npm testThe tests run until my Hospital scenario, then boom! FileSearch
✓ should return all files by default
✓ should return the matching files when a regex is specified
✓ should ignore missing search paths
Hospital
make: *** [test] Segmentation fault: 11
npm ERR! Test failed. See above for more details.
npm ERR! not ok code 0I (and travis) get segmentation fault 11 consistently. Downgrading to 0.10.30 and the tests pass You can see the segmentation fault here |
|
Thanks! Here is a better stack trace: |
|
WRT yadda, the problem manifests itself here between lines 63 and 70. Which is a nested for loop used to calculate the Levenshtein distance in order to compare the similarity of two strings |
|
Yeah the original issue came from an application which used |
|
I think I am getting some clearer understanding of it now, will try to figure it out ASAP. |
|
This is a half-way fix: diff --git a/deps/v8/src/hydrogen.cc b/deps/v8/src/hydrogen.cc
index 50d8e49..2621b30 100644
--- a/deps/v8/src/hydrogen.cc
+++ b/deps/v8/src/hydrogen.cc
@@ -3531,7 +3531,8 @@ class BoundsCheckBbData: public ZoneObject {
lower_check_(lower_check),
upper_check_(upper_check),
next_in_bb_(next_in_bb),
- father_in_dt_(father_in_dt) { }
+ father_in_dt_(father_in_dt) {
+ }
private:
BoundsCheckKey* key_;
@@ -3581,7 +3582,6 @@ class BoundsCheckBbData: public ZoneObject {
HBoundsCheck* tighter_check) {
ASSERT(original_check->length() == tighter_check->length());
MoveIndexIfNecessary(tighter_check->index(), original_check, tighter_check);
- original_check->ReplaceAllUsesWith(original_check->index());
original_check->SetOperandAt(0, tighter_check->index());
}
};
@@ -3624,7 +3624,9 @@ void HGraph::EliminateRedundantBoundsChecks(HBasicBlock* bb,
BoundsCheckTable* table) {
BoundsCheckBbData* bb_data_list = NULL;
- for (HInstruction* i = bb->first(); i != NULL; i = i->next()) {
+ HInstruction* next;
+ for (HInstruction* i = bb->first(); i != NULL; i = next) {
+ next = i->next();
if (!i->IsBoundsCheck()) continue;
HBoundsCheck* check = HBoundsCheck::cast(i);
|
|
And here is the fix that works: diff --git a/deps/v8/src/hydrogen.cc b/deps/v8/src/hydrogen.cc
index 50d8e49..7c8da73 100644
--- a/deps/v8/src/hydrogen.cc
+++ b/deps/v8/src/hydrogen.cc
@@ -3546,7 +3546,11 @@ class BoundsCheckBbData: public ZoneObject {
void MoveIndexIfNecessary(HValue* index_raw,
HBoundsCheck* insert_before,
HInstruction* end_of_scan_range) {
- ASSERT(index_raw->IsAdd() || index_raw->IsSub());
+ if (!index_raw->IsAdd() && !index_raw->IsSub()) {
+ // index_raw can be HAdd(index_base, offset), HSub(index_base, offset),
+ // or index_base directly. In the latter case, no need to move anything.
+ return;
+ }
HBinaryOperation* index =
HArithmeticBinaryOperation::cast(index_raw);
HValue* left_input = index->left();
@@ -3581,7 +3585,6 @@ class BoundsCheckBbData: public ZoneObject {
HBoundsCheck* tighter_check) {
ASSERT(original_check->length() == tighter_check->length());
MoveIndexIfNecessary(tighter_check->index(), original_check, tighter_check);
- original_check->ReplaceAllUsesWith(original_check->index());
original_check->SetOperandAt(0, tighter_check->index());
}
};
@@ -3624,7 +3627,9 @@ void HGraph::EliminateRedundantBoundsChecks(HBasicBlock* bb,
BoundsCheckTable* table) {
BoundsCheckBbData* bb_data_list = NULL;
- for (HInstruction* i = bb->first(); i != NULL; i = i->next()) {
+ HInstruction* next;
+ for (HInstruction* i = bb->first(); i != NULL; i = next) {
+ next = i->next();
if (!i->IsBoundsCheck()) continue;
HBoundsCheck* check = HBoundsCheck::cast(i);Could you please give it a try? |
|
I'm giving this a try -- it crashes for me too. |
|
This fixes it for me. |
|
cc @tjfontaine |
|
@indutny Did you mean to close this issue? |
|
Yeah, when I have committed: 3122e0e |
|
@indutny thanks for the quick reply. Have found the discrepancy. Upon retesting with Release mode node instead with the patch, it works. My previous tests of the patch were with Debug mode node, and that is still crashing. |
|
So when will this get released? Projects are breaking all over ... |
|
we are in the release process at the moment |
|
Nevermind, http://nodejs.org/dist/v0.10.32/ |
|
0.10.32 fixes the segfault problem 👍 |
NodeJS 0.10.31 has a nasty bug : nodejs/node-v0.x-archive#8208
|
Life saver! Thank you |
|
@tjfontaine what do you think about unpublishing this version? |
fd80a31 has introduced a segfault during redundant boundary check elimination (nodejs#8208). The problem consists of two parts: 1. Abscense of instruction iterator in `EliminateRedundantBoundsChecks`. It was present in recent v8, but wasn't considered important at the time of backport. However, since the function is changing instructions order in block, it is important to not rely at `i->next()` at the end of the loop. 2. Too strict ASSERT in `MoveIndexIfNecessary`. It is essentially a backport of a45c96ab from v8's upstream. See v8/v8@a45c96ab for details. fix nodejs#8208
We're seeing this issue on 0.10.31 (but not 0.10.30) on both travis and OSX. I've used
segfault-handlerto get a trace. Hopefully this is the correct information you need and the correct place to raise this issue (i.e. its related to node and not a module).The text was updated successfully, but these errors were encountered: