contextify: use offset/length from Uint8Array

Do not blindly take data from underlying `ArrayBuffer`, use
`ByteOffset`/`ByteLength` of `Uint8Array` itself.

Additionally, fix tests that weren't actually properly running because
of V8's internal code cache. The code should be different, otherwise the
cached data won't be used at all.

Fix: #4939
PR-URL: #4947
Reviewed-By: Ben Noordhuis <[email protected]>

src/node_contextify.cc

@@ -507,10 +507,11 @@ class ContextifyScript : public BaseObject {
 
     ScriptCompiler::CachedData* cached_data = nullptr;
     if (!cached_data_buf.IsEmpty()) {
-      ArrayBuffer::Contents contents =
-          cached_data_buf.ToLocalChecked()->Buffer()->GetContents();
+      Local<Uint8Array> ui8 = cached_data_buf.ToLocalChecked();
+      ArrayBuffer::Contents contents = ui8->Buffer()->GetContents();
       cached_data = new ScriptCompiler::CachedData(
-          static_cast<uint8_t*>(contents.Data()), contents.ByteLength());
+          static_cast<uint8_t*>(contents.Data()) + ui8->ByteOffset(),
+          ui8->ByteLength());
     }
 
     ScriptOrigin origin(filename, lineOffset, columnOffset);

test/parallel/test-vm-cached-data.js

@@ -4,30 +4,58 @@ const assert = require('assert');
 const vm = require('vm');
 const Buffer = require('buffer').Buffer;
 
-const originalSource = '(function bcd() { return \'original\'; })';
+function getSource(tag) {
+  return `(function ${tag}() { return \'${tag}\'; })`;
+}
 
-// It should produce code cache
-const original = new vm.Script(originalSource, {
-  produceCachedData: true
-});
-assert(original.cachedData instanceof Buffer);
+function produce(source) {
+  const script = new vm.Script(source, {
+    produceCachedData: true
+  });
+  assert(script.cachedData instanceof Buffer);
 
-assert.equal(original.runInThisContext()(), 'original');
+  return script.cachedData;
+}
 
-// It should consume code cache
-const success = new vm.Script(originalSource, {
-  cachedData: original.cachedData
-});
-assert(!success.cachedDataRejected);
+function testProduceConsume() {
+  const source = getSource('original');
 
-assert.equal(success.runInThisContext()(), 'original');
+  const data = produce(source);
 
-// It should reject invalid code cache
-const reject = new vm.Script('(function abc() { return \'invalid\'; })', {
-  cachedData: original.cachedData
-});
-assert(reject.cachedDataRejected);
-assert.equal(reject.runInThisContext()(), 'invalid');
+  // It should consume code cache
+  const script = new vm.Script(source, {
+    cachedData: data
+  });
+  assert(!script.cachedDataRejected);
+  assert.equal(script.runInThisContext()(), 'original');
+}
+testProduceConsume();
+
+function testRejectInvalid() {
+  const source = getSource('invalid');
+
+  const data = produce(source);
+
+  // It should reject invalid code cache
+  const script = new vm.Script(getSource('invalid_1'), {
+    cachedData: data
+  });
+  assert(script.cachedDataRejected);
+  assert.equal(script.runInThisContext()(), 'invalid_1');
+}
+testRejectInvalid();
+
+function testRejectSlice() {
+  const source = getSource('slice');
+
+  const data = produce(source).slice(4);
+
+  const script = new vm.Script(source, {
+    cachedData: data
+  });
+  assert(script.cachedDataRejected);
+}
+testRejectSlice();
 
 // It should throw on non-Buffer cachedData
 assert.throws(() => {