Cherry-pick patch from V8 upstream that fixes instanceof problem

[fhinkel]

Checklist

• make -j4 test (UNIX)
• test is included
• commit message follows commit guidelines

Affected core subsystem(s)

deps V8

Description of change

Float the following patch:
InstanceOfStub incorrectly interprets the hole as a prototype.
https://codereview.chromium.org/1810953002/

Fixes: #7592

/cc @bnoordhuis

[addaleax]

I think you may want to switch the order of the commits here, so that there is no point in the history in which make test fails?

[fhinkel]

Oh, you're totally right. Done.

[bnoordhuis]

Can you use const here and below?

[bnoordhuis]

@fhinkel Can you reword the commit log to be more like (e.g.) b88dee2? We use 'cherry-pick' for a clean cherry-pick and 'backport' if the patch needed nudging.

(At least, I do. Possibly some unwritten rule that we need to write down somewhere.)

[fhinkel]

I changed the commit message and the test. @bnoordhuis can you have another look, please?

[addaleax]

LGTM

[bnoordhuis]

Thanks, LGTM.

CI: https://ci.nodejs.org/job/node-test-pull-request/3236/
CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/191/
EDIT: https://ci.nodejs.org/job/node-test-commit-v8-linux/192/

[MylesBorins]

@bnoordhuis I started playing with making a backport / cherry-pick guide. Is there any other "unwritten rules" you can think of?

[bnoordhuis]

@thealphanerd "Run the V8 and node.js test suite on the CI". That's about it.

EDIT: "Don't forget to bump the patchlevel..."

[Fishrock123]

FreeBSD has an 'interesting' error:

ln -fs out/Release/node node ./node: not found Makefile:158: recipe for target 'test/addons/.buildstamp' failed gmake[1]: *** [test/addons/.buildstamp] Error 1 gmake[1]: *** Waiting for unfinished jobs.... gmake[1]: Leaving directory '/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64' Makefile:321: recipe for target 'run-ci' failed gmake: *** [run-ci] Error 2 Build step 'Execute shell' marked build as failure 

Not sure what's up with that.

Looks like ppc little-endian ubuntu is failing though, and the v8 CI also seems to have some failures.

not ok 579 parallel/test-instanceof # # assert.js:89 # throw new assert.AssertionError({# ^ # AssertionError: false == true # at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-plinux/nodes/ppcbe-ubuntu1404/test/parallel/test-instanceof.js:7:1) # at Module._compile (module.js:541:32) # at Object.Module._extensions..js (module.js:550:10) # at Module.load (module.js:458:32) # at tryModuleLoad (module.js:417:12) # at Function.Module._load (module.js:409:3) # at Module.runMain (module.js:575:10) # at run (bootstrap_node.js:352:7) # at startup (bootstrap_node.js:144:9) # at bootstrap_node.js:467:3 

[bnoordhuis]

I think it's because it's missing the PPC changes from v8/v8@3a903c4.

[mhdawson]

Quite possible that there are similar changes for s390 as well. Will look at adding s390 to the v8 test job as well.

[bnoordhuis]

It doesn't matter for this particular patch (no s390 in v6) but it would be good in general.

[fhinkel]

Thanks for running the CI. I'll add the PPC and X87 patches.

[fhinkel]

I cherry-picked the patches for the remaining architectures (not for s390 because it's not in v6). Can you have another look, please?

[addaleax]

CI: https://ci.nodejs.org/job/node-test-commit/4055/
CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/194/

[fhinkel]

Do I need to bump the patch level or are we only doing that for 5.1?

[bnoordhuis]

Yes, please bump the patchlevel.

[fhinkel]

Bumped and rebased, so test is added after cherry-picks.

[ofrobots]

LGTM.

[evanlucas]

landed in v6.x in 71f84b5...164981a. Thanks!

[bnoordhuis]

I would have preferred it if this PR had landed as a single commit. We now have commits in our history where the build is known broken on some architectures, making things like git-bisect more difficult.

[fhinkel]

@bnoordhuis I'm not sure I understand your concern. The test is added after the three cherry-picks, I don't think the build is broken in any of the revisions.

Should bumping the patch level be a separate commit or squashed? @thealphanerd where's your cherry-pick guide?

[bnoordhuis]

Sorry, I must have been thinking of another PR. You're right, here it was just the test that was failing without the PPC changes.

I have a preference for bumping the patch level in the same commit. It makes it an atomic unit, easy to roll back.

[rvagg]

@fhinkel do you think this should also land on master? it's dying out with the v6.x series but perhaps it's generic enough to warrant keeping around.

[fhinkel]

@rvagg You mean if the test should be ported to master? Yes, let me do that. Then we can let this die with v6.x.

[rvagg]

@fhinkel yeah, thanks, it came up as one of the commits on v6.x that are not on v7.x and I just wanted clarification regarding whether this should live on beyond v6.x and getting it on master is obviously the way to do that.