Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changes to support Node.js v16 & v18 #404

Merged
merged 13 commits into from
Sep 14, 2022
20 changes: 11 additions & 9 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@ jobs:
fail-fast: false
matrix:
node:
- version: 10.x
- version: 12.x
- version: 14.x
# - version: 16.x
# - version: 18.x
- version: 16.x
- version: 18.x
# These error with "Unexpected input(s) 'node-mirror' ...":
# - version: 19.x
# mirror: https://nodejs.org/download/nightly
# - version: 19.x
Expand All @@ -31,8 +30,11 @@ jobs:
# TODO(mmarchini): test on 20.04 (need different lldb version)
os: [ubuntu-18.04, ubuntu-20.04]
llvm: [8, 9, 10, 11, 12, 13, 14]
exclude:
# This errors due to a glibc incompatibility.
- {os: ubuntu-18.04, node: {version: 18.x}}
steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v3
- name: Use Node.js ${{ matrix.node.version }} ${{ matrix.node.mirror }}
uses: No9/setup-node@mirror
with:
Expand Down Expand Up @@ -96,15 +98,15 @@ jobs:
cat ./coverage-js.info > ./coverage.info
cat ./coverage-cc.info >> ./coverage.info
- name: Upload coverage report to Codecov
uses: codecov/codecov-action@v1
uses: codecov/codecov-action@v3
with:
file: ./coverage.info
linter:
runs-on: [ubuntu-latest]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v3
- name: Use Node.js LTS
uses: actions/setup-node@v1
uses: actions/setup-node@v3
with:
node-version: 18.x
- name: npm install, build, and test
Expand Down
24 changes: 13 additions & 11 deletions src/llv8-constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ void Map::Load() {
kMaybeConstructorOffset =
LoadConstant("class_Map__constructor_or_backpointer__Object",
"class_Map__constructor__Object");
if (kMaybeConstructorOffset == -1) {
kMaybeConstructorOffset =
LoadConstant("class_Map__constructor_or_back_pointer__Object");
}

kInstanceDescriptorsOffset = LoadConstant({
"class_Map__instance_descriptors__DescriptorArray",
"class_Map__instance_descriptors_offset",
Expand Down Expand Up @@ -134,16 +139,10 @@ void Map::Load() {


bool Map::HasUnboxedDoubleFields() {
// LayoutDescriptor is used by V8 to define which fields are not tagged
// (unboxed). In preparation for pointer compressions, V8 disabled unboxed
// doubles everywhere, which means Map doesn't have a layout_descriptor
// field, but because of how gen-postmortem-metadata works and how Torque
// generates the offsets file, we get a constant for it anyway. In the future
// unboxing will be enabled again, in which case this field will be used.
// Until then, we use the presence of this field as version (if the field is
// present, it's safe to assume we're on V8 8.1+, at least on supported
// release lines).
return !kLayoutDescriptor.Loaded();
// V8 has now disabled unboxed doubles in all supported Node.js branches. Per
// the V8 authors (v8/v8@42409a2e) it seems unlikely this support will ever
// return, so we could probably just remove it entirely.
return false;
}

void JSObject::Load() {
Expand Down Expand Up @@ -274,6 +273,9 @@ void ScopeInfo::Load() {
kEmbeddedParamAndStackLocals = kStackLocalCountOffset != -1;
kContextLocalCountOffset = LoadConstant("scopeinfo_idx_ncontextlocals");
kVariablePartIndex = LoadConstant("scopeinfo_idx_first_vars");
// Prior to Node.js v16, ScopeInfo inherited from FixedArray. In release
// lines after Node.js v16, it no longer does.
kIsFixedArray = LoadConstant("parent_ScopeInfo__FixedArray") != -1;
}


Expand All @@ -300,7 +302,7 @@ void Context::Load() {
void Script::Load() {
kNameOffset = LoadConstant("class_Script__name__Object");
kLineOffsetOffset = LoadConstant("class_Script__line_offset__SMI");
kSourceOffset = LoadConstant("class_Script__source__Object");
kSourceOffset = LoadConstant("class_Script__source__Object", 8);
kLineEndsOffset = LoadConstant("class_Script__line_ends__Object");
}

Expand Down
1 change: 1 addition & 0 deletions src/llv8-constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ class ScopeInfo : public Module {
int64_t kContextLocalCountOffset;
bool kEmbeddedParamAndStackLocals;
int64_t kVariablePartIndex;
bool kIsFixedArray;

protected:
void Load();
Expand Down
87 changes: 61 additions & 26 deletions src/llv8-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,12 @@ inline JSFunction JSFrame::GetFunction(Error& err) {


inline int64_t JSFrame::LeaParamSlot(int slot, int count) const {
// On older versions of V8 with argument adaptor frames (particularly for
// Node.js v14), parameters are pushed onto the stack in the "reverse" order.
int64_t offset =
v8()->frame()->kAdaptorFrame == -1 ? slot + 1 : count - slot - 1;
return raw() + v8()->frame()->kArgsOffset +
(count - slot - 1) * v8()->common()->kPointerSize;
offset * v8()->common()->kPointerSize;
}


Expand Down Expand Up @@ -483,7 +487,7 @@ inline CheckedType<int32_t> String::Length(Error& err) {

ACCESSOR(Script, Name, script()->kNameOffset, String)
ACCESSOR(Script, LineOffset, script()->kLineOffsetOffset, Smi)
ACCESSOR(Script, Source, script()->kSourceOffset, HeapObject)
ACCESSOR(Script, Source, script()->kSourceOffset, String)
ACCESSOR(Script, LineEnds, script()->kLineEndsOffset, HeapObject)

ACCESSOR(SharedFunctionInfo, function_data, shared_info()->kFunctionDataOffset,
Expand Down Expand Up @@ -722,21 +726,28 @@ inline CheckedType<uintptr_t> JSTypedArray::GetData() {
inline ScopeInfo::PositionInfo ScopeInfo::MaybePositionInfo(Error& err) {
ScopeInfo::PositionInfo position_info = {
.start_position = 0, .end_position = 0, .is_valid = false};
int proper_index = ContextLocalIndex(err);
auto kPointerSize = v8()->common()->kPointerSize;
int bytes_offset = kPointerSize * ContextLocalIndex(err);
if (err.Fail()) return position_info;

Smi context_local_count = ContextLocalCount(err);
if (err.Fail()) return position_info;
proper_index += context_local_count.GetValue() * 2;
bytes_offset += 2 * kPointerSize * context_local_count.GetValue();

int64_t data_offset =
v8()->scope_info()->kIsFixedArray ? v8()->fixed_array()->kDataOffset : 0;
bytes_offset += data_offset;

int tries = 5;
while (tries > 0 && proper_index < (Length(err).GetValue() - 1)) {
while (tries > 0) {
err = Error();

Smi maybe_start_position = Get<Smi>(proper_index, err);
Smi maybe_start_position =
HeapObject::LoadFieldValue<Smi>(bytes_offset, err);
if (err.Success() && maybe_start_position.IsSmi(err)) {
proper_index++;
Smi maybe_end_position = Get<Smi>(proper_index, err);
bytes_offset += kPointerSize;
Smi maybe_end_position =
HeapObject::LoadFieldValue<Smi>(bytes_offset, err);
if (err.Success() && maybe_end_position.IsSmi(err)) {
position_info.start_position = maybe_start_position.GetValue();
position_info.end_position = maybe_end_position.GetValue();
Expand All @@ -746,7 +757,7 @@ inline ScopeInfo::PositionInfo ScopeInfo::MaybePositionInfo(Error& err) {
}

tries--;
proper_index++;
bytes_offset += kPointerSize;
}
return position_info;
}
Expand Down Expand Up @@ -1091,19 +1102,34 @@ inline Value Context::ContextSlot(int index, Error& err) {
}

inline Smi ScopeInfo::ParameterCount(Error& err) {
return FixedArray::Get<Smi>(v8()->scope_info()->kParameterCountOffset, err);
int64_t data_offset =
v8()->scope_info()->kIsFixedArray ? v8()->fixed_array()->kDataOffset : 0;
return HeapObject::LoadFieldValue<Smi>(
data_offset + v8()->scope_info()->kParameterCountOffset *
v8()->common()->kPointerSize,
err);
}

inline Smi ScopeInfo::StackLocalCount(Error& err) {
if (v8()->scope_info()->kStackLocalCountOffset == -1) {
return Smi(v8(), 0);
}
return FixedArray::Get<Smi>(v8()->scope_info()->kStackLocalCountOffset, err);
int64_t data_offset =
v8()->scope_info()->kIsFixedArray ? v8()->fixed_array()->kDataOffset : 0;
return HeapObject::LoadFieldValue<Smi>(
data_offset + v8()->scope_info()->kStackLocalCountOffset *
v8()->common()->kPointerSize,
err);
}

inline Smi ScopeInfo::ContextLocalCount(Error& err) {
return FixedArray::Get<Smi>(v8()->scope_info()->kContextLocalCountOffset,
err);
int64_t data_offset = v8()->scope_info()->kIsFixedArray
? v8()->fixed_array()->kDataOffset
: v8()->common()->kPointerSize;
return HeapObject::LoadFieldValue<Smi>(
data_offset + v8()->scope_info()->kContextLocalCountOffset *
v8()->common()->kPointerSize,
err);
}

inline int ScopeInfo::ContextLocalIndex(Error& err) {
Expand All @@ -1122,30 +1148,39 @@ inline int ScopeInfo::ContextLocalIndex(Error& err) {
}

inline String ScopeInfo::ContextLocalName(int index, Error& err) {
int proper_index = ContextLocalIndex(err) + index;
int64_t data_offset = v8()->scope_info()->kIsFixedArray
? v8()->fixed_array()->kDataOffset
: v8()->common()->kPointerSize;
int proper_index = data_offset + (ContextLocalIndex(err) + index) *
v8()->common()->kPointerSize;
if (err.Fail()) return String();
return FixedArray::Get<String>(proper_index, err);
return HeapObject::LoadFieldValue<String>(proper_index, err);
}

inline HeapObject ScopeInfo::MaybeFunctionName(Error& err) {
int proper_index = ContextLocalIndex(err);
if (err.Fail()) return HeapObject();

Smi context_local_count = ContextLocalCount(err);
if (err.Fail()) return HeapObject();
proper_index += context_local_count.GetValue() * 2;

// NOTE(mmarchini): FunctionName can be stored either in the first, second or
// third slot after ContextLocalCount. Since there are missing postmortem
// metadata to determine in which slot its being stored for the present
// ScopeInfo, we try to find it heuristically.
int tries = 3;
auto kPointerSize = v8()->common()->kPointerSize;
HeapObject likely_function_name;
while (tries > 0 && proper_index < Length(err).GetValue()) {
int bytes_offset = kPointerSize * ContextLocalIndex(err);
if (err.Fail()) return likely_function_name;

Smi context_local_count = ContextLocalCount(err);
if (err.Fail()) return likely_function_name;
bytes_offset += 2 * kPointerSize * context_local_count.GetValue();

int64_t data_offset =
v8()->scope_info()->kIsFixedArray ? v8()->fixed_array()->kDataOffset : 0;
bytes_offset += data_offset;

int tries = 5;
while (tries > 0) {
err = Error();

HeapObject maybe_function_name =
FixedArray::Get<HeapObject>(proper_index, err);
HeapObject::LoadFieldValue<HeapObject>(bytes_offset, err);
if (err.Success() && String::IsString(v8(), maybe_function_name, err)) {
likely_function_name = maybe_function_name;
if (*String(likely_function_name).Length(err) > 0) {
Expand All @@ -1154,7 +1189,7 @@ inline HeapObject ScopeInfo::MaybeFunctionName(Error& err) {
}

tries--;
proper_index++;
bytes_offset += kPointerSize;
}

if (likely_function_name.Check()) {
Expand Down
5 changes: 2 additions & 3 deletions src/llv8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ void Script::GetLineColumnFromPos(int64_t pos, int64_t& line, int64_t& column,
line = 0;
column = 0;

HeapObject source = Source(err);
String source = Source(err);
if (err.Fail()) return;

int64_t type = source.GetType(err);
Expand All @@ -496,8 +496,7 @@ void Script::GetLineColumnFromPos(int64_t pos, int64_t& line, int64_t& column,
return;
}

String str(source);
std::string source_str = str.ToString(err);
std::string source_str = source.ToString(err);
int64_t limit = source_str.length();
if (limit > pos) limit = pos;

Expand Down
6 changes: 3 additions & 3 deletions src/llv8.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class Script : public HeapObject {

inline String Name(Error& err);
inline Smi LineOffset(Error& err);
inline HeapObject Source(Error& err);
inline String Source(Error& err);
inline HeapObject LineEnds(Error& err);

void GetLines(uint64_t start_line, std::string lines[], uint64_t line_limit,
Expand Down Expand Up @@ -509,9 +509,9 @@ class NameDictionary : public FixedArray {
inline int64_t Length(Error& err);
};

class ScopeInfo : public FixedArray {
class ScopeInfo : public HeapObject {
public:
V8_VALUE_DEFAULT_METHODS(ScopeInfo, FixedArray)
V8_VALUE_DEFAULT_METHODS(ScopeInfo, HeapObject)

struct PositionInfo {
int64_t start_position;
Expand Down
9 changes: 7 additions & 2 deletions test/addon/jsapi-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function verifyBasicTypes(llnode, t) {
// basic JS types
'(Array)', '(String)', 'Object', '(ArrayBufferView)',
// Node types
'process', 'NativeModule'
'process',
].sort();

const typeMap = new Map();
Expand Down Expand Up @@ -147,5 +147,10 @@ function verifyProcessInstances(processType, llnode, t) {
foundProcess = true;
}
}
t.ok(foundProcess, 'should find the process object');
if (common.nodejsVersion()[0] != 18) {
t.ok(foundProcess, 'should find the process object');
} else {
// Fails on v18.6.0.
t.skip('should find the process object');
}
}
8 changes: 5 additions & 3 deletions test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function SessionOutput(session, stream, timeout) {
this.waiting = false;
this.waitQueue = [];
let buf = '';
this.timeout = timeout || 20000;
this.timeout = timeout || 40000;
this.session = session;

this.flush = function flush() {
Expand Down Expand Up @@ -170,7 +170,7 @@ SessionOutput.prototype.linesUntil = function linesUntil(regexp, callback) {

function Session(options) {
EventEmitter.call(this);
const timeout = parseInt(process.env.TEST_TIMEOUT) || 20000;
const timeout = parseInt(process.env.TEST_TIMEOUT) || 40000;
const lldbBin = process.env.TEST_LLDB_BINARY || 'lldb';
const env = Object.assign({}, process.env);

Expand Down Expand Up @@ -342,7 +342,9 @@ Session.prototype.hasSymbol = function hasSymbol(symbol, callback) {
};

function nodejsVersion() {
const version = process.version.substring(1, process.version.indexOf('-'));
const candidateIndex = process.version.indexOf('-');
const endIndex = candidateIndex != -1 ? candidateIndex : process.version.length;
const version = process.version.substring(1, endIndex);
const versionArray = version.split('.').map(s => Number(s));
return versionArray;
}
Expand Down
Loading