Skip to content

Commit 2afd59e

Browse files
mmarchinimmarchini
committed
src: introduce Constant class
Same idea as CheckedType: instead of relying on the default value for constants to determine if they were loaded correctly or not (default value usually was -1), have a class which will have information about the loaded constant. This can help us: - Check if a constant was properly loaded before using it (otherwise we'd get into undefined behavior, and there are a lot of places where this happens. - Check if a constant value was loaded or if we're looking at the default value. - Explicitly define a constant as optional (constants which are not relevant except for specific V8 versions). This will help us improve reliability and debugability of llnode. PR-URL: #303 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent ec01604 commit 2afd59e

File tree

7 files changed

+145
-60
lines changed

7 files changed

+145
-60
lines changed

‎src/constants.cc‎

Lines changed: 64 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
#include <cinttypes>
2+
#include <initializer_list>
3+
#include <iterator>
4+
#include <sstream>
25

36
#include <lldb/API/SBExpressionOptions.h>
47

@@ -14,59 +17,51 @@ namespace llnode {
1417

1518
template <typename T>
1619
T ReadSymbolFromTarget(SBTarget& target, SBAddress& start, const char* name,
17-
Error& err) {
18-
SBError sberr;
20+
SBError& sberr) {
1921
T res = 0;
2022
target.ReadMemory(start, &res, sizeof(T), sberr);
21-
if (!sberr.Fail()) {
22-
err = Error::Ok();
23-
} else {
24-
err = Error::Failure("Failed to read symbol %s", name);
25-
}
2623
return res;
2724
}
2825

29-
int64_t Constants::LookupConstant(SBTarget target, const char* name,
30-
int64_t def, Error& err) {
31-
int64_t res = 0;
32-
res = def;
26+
Constant<int64_t> Constants::LookupConstant(SBTarget target, const char* name) {
27+
int64_t res;
3328

3429
SBSymbolContextList context_list = target.FindSymbols(name);
3530

3631
if (!context_list.IsValid() || context_list.GetSize() == 0) {
37-
err = Error::Failure("Failed to find symbol %s", name);
38-
return res;
32+
return Constant<int64_t>();
3933
}
4034

4135
SBSymbolContext context = context_list.GetContextAtIndex(0);
4236
SBSymbol symbol = context.GetSymbol();
4337
if (!symbol.IsValid()) {
44-
err = Error::Failure("Failed to fetch symbol %s from context", name);
45-
return res;
38+
return Constant<int64_t>();
4639
}
4740

4841
SBAddress start = symbol.GetStartAddress();
4942
SBAddress end = symbol.GetEndAddress();
5043
uint32_t size = end.GetOffset() - start.GetOffset();
5144

5245
// NOTE: size could be bigger for at the end symbols
46+
SBError sberr;
5347
if (size >= 8) {
54-
res = ReadSymbolFromTarget<int64_t>(target, start, name, err);
48+
res = ReadSymbolFromTarget<int64_t>(target, start, name, sberr);
5549
} else if (size == 4) {
56-
int32_t tmp = ReadSymbolFromTarget<int32_t>(target, start, name, err);
50+
int32_t tmp = ReadSymbolFromTarget<int32_t>(target, start, name, sberr);
5751
res = static_cast<int64_t>(tmp);
5852
} else if (size == 2) {
59-
int16_t tmp = ReadSymbolFromTarget<int16_t>(target, start, name, err);
53+
int16_t tmp = ReadSymbolFromTarget<int16_t>(target, start, name, sberr);
6054
res = static_cast<int64_t>(tmp);
6155
} else if (size == 1) {
62-
int8_t tmp = ReadSymbolFromTarget<int8_t>(target, start, name, err);
56+
int8_t tmp = ReadSymbolFromTarget<int8_t>(target, start, name, sberr);
6357
res = static_cast<int64_t>(tmp);
6458
} else {
65-
err = Error::Failure("Unexpected symbol size %" PRIu32 " of symbol %s",
66-
size, name);
59+
return Constant<int64_t>();
6760
}
6861

69-
return res;
62+
if (sberr.Fail()) return Constant<int64_t>();
63+
64+
return Constant<int64_t>(res, name);
7065
}
7166

7267
void Constants::Assign(SBTarget target) {
@@ -76,44 +71,71 @@ void Constants::Assign(SBTarget target) {
7671

7772

7873
int64_t Constants::LoadRawConstant(const char* name, int64_t def) {
79-
Error err;
80-
int64_t v = Constants::LookupConstant(target_, name, def, err);
81-
if (err.Fail()) {
74+
auto constant = Constants::LookupConstant(target_, name);
75+
if (!constant.Check()) {
8276
PRINT_DEBUG("Failed to load raw constant %s, default to %" PRId64, name,
8377
def);
78+
return def;
8479
}
8580

86-
return v;
87-
}
88-
89-
int64_t Constants::LoadConstant(const char* name, Error& err, int64_t def) {
90-
int64_t v = Constants::LookupConstant(
91-
target_, (constant_prefix() + name).c_str(), def, err);
92-
return v;
81+
return *constant;
9382
}
9483

9584
int64_t Constants::LoadConstant(const char* name, int64_t def) {
96-
Error err;
97-
int64_t v = LoadConstant(name, err, def);
98-
if (err.Fail()) {
99-
PRINT_DEBUG("Failed to load constant %s, default to %" PRId64, name, def);
85+
auto constant =
86+
Constants::LookupConstant(target_, (constant_prefix() + name).c_str());
87+
if (!constant.Check()) {
88+
PRINT_DEBUG("Failed to load constant %s, default to %" PRId64, name);
89+
return def;
10090
}
10191

102-
return v;
92+
return *constant;
10393
}
10494

10595
int64_t Constants::LoadConstant(const char* name, const char* fallback,
10696
int64_t def) {
107-
Error err;
108-
int64_t v = LoadConstant(name, err, def);
109-
if (err.Fail()) v = LoadConstant(fallback, err, def);
110-
if (err.Fail()) {
97+
auto constant =
98+
Constants::LookupConstant(target_, (constant_prefix() + name).c_str());
99+
if (!constant.Check())
100+
constant = Constants::LookupConstant(
101+
target_, (constant_prefix() + fallback).c_str());
102+
if (!constant.Check()) {
111103
PRINT_DEBUG("Failed to load constant %s, fallback %s, default to %" PRId64,
112104
name, fallback, def);
105+
return def;
113106
}
114107

115-
return v;
108+
return *constant;
116109
}
117110

111+
Constant<int64_t> Constants::LoadConstant(
112+
std::initializer_list<const char*> names) {
113+
for (std::string name : names) {
114+
auto constant =
115+
Constants::LookupConstant(target_, (constant_prefix() + name).c_str());
116+
if (constant.Check()) return constant;
117+
}
118+
119+
if (Error::IsDebugMode()) {
120+
std::string joined = "";
121+
for (std::string name : names) {
122+
joined += (joined.empty() ? "'" : ", '") + name + "'";
123+
}
124+
PRINT_DEBUG("Failed to load constants: %s", joined.c_str());
125+
}
126+
127+
return Constant<int64_t>();
128+
}
129+
130+
Constant<int64_t> Constants::LoadOptionalConstant(
131+
std::initializer_list<const char*> names, int def) {
132+
for (std::string name : names) {
133+
auto constant =
134+
Constants::LookupConstant(target_, (constant_prefix() + name).c_str());
135+
if (constant.Check()) return constant;
136+
}
137+
138+
return Constant<int64_t>(def);
139+
}
118140

119141
} // namespace llnode

‎src/constants.h‎

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,56 @@ using lldb::SBTarget;
1010

1111
namespace llnode {
1212

13+
enum ConstantStatus { kInvalid, kValid, kLoaded };
14+
15+
// Class representing a constant which is used to interpret memory data. Most
16+
// constants represent offset of fields on an object, bit-masks or "tags" which
17+
// are used to identify types, but there are some constants with different
18+
// meanings as well.
19+
//
20+
// By default, constants are loaded from the binary debug symbols, and usually
21+
// those debug symbols are generated by V8's gen-postmortem-metadata.py or by
22+
// Node.js' node-postmortem-metadata.cc. Some constants can also come from C++
23+
// generated debug symbols.
24+
//
25+
// When a constant is successfully loaded, Check() and Loaded() will return
26+
// true, which means we can safely use this constant and make assumptions based
27+
// on its existence. In some cases, it's safe to assume defaults for a given
28+
// constant. If that's the case, the constant will return false on Loaded() but
29+
// true on Check(). A constant returning false for both Check() and Loaded() is
30+
// not safe to use.
31+
//
32+
// Use the dereference operator (*constant) to access a constant value.
33+
template <typename T>
34+
class Constant {
35+
public:
36+
Constant() : value_(-1), status_(kInvalid) {}
37+
inline bool Check() {
38+
return (status_ == ConstantStatus::kValid ||
39+
status_ == ConstantStatus::kLoaded);
40+
}
41+
42+
inline bool Loaded() { return status_ == kLoaded; }
43+
44+
T operator*() {
45+
// TODO(mmarchini): Check()
46+
return value_;
47+
}
48+
49+
inline std::string name() { return name_; }
50+
51+
protected:
52+
friend class Constants;
53+
explicit Constant(T value) : value_(value), status_(kValid), name_("") {}
54+
Constant(T value, std::string name)
55+
: value_(value), status_(kLoaded), name_(name) {}
56+
57+
private:
58+
T value_;
59+
ConstantStatus status_;
60+
std::string name_;
61+
};
62+
1363
#define CONSTANTS_DEFAULT_METHODS(NAME) \
1464
inline NAME* operator()() { \
1565
if (loaded_) return this; \
@@ -28,15 +78,16 @@ class Constants {
2878

2979
inline virtual std::string constant_prefix() { return ""; };
3080

31-
static int64_t LookupConstant(SBTarget target, const char* name, int64_t def,
32-
Error& err);
81+
static Constant<int64_t> LookupConstant(SBTarget target, const char* name);
3382

3483
protected:
3584
int64_t LoadRawConstant(const char* name, int64_t def = -1);
36-
int64_t LoadConstant(const char* name, Error& err, int64_t def = -1);
3785
int64_t LoadConstant(const char* name, int64_t def = -1);
3886
int64_t LoadConstant(const char* name, const char* fallback,
3987
int64_t def = -1);
88+
Constant<int64_t> LoadConstant(std::initializer_list<const char*> names);
89+
Constant<int64_t> LoadOptionalConstant(
90+
std::initializer_list<const char*> names, int def);
4091

4192
lldb::SBTarget target_;
4293
bool loaded_;

‎src/error.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class Error {
3030
inline const char* GetMessage() { return msg_.c_str(); }
3131

3232
static void SetDebugMode(bool mode) { is_debug_mode = mode; }
33+
static bool IsDebugMode() { return is_debug_mode; }
3334

3435
private:
3536
bool failed_;

‎src/llv8-constants.cc‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,10 @@ void HeapObject::Load() {
8181

8282
void Map::Load() {
8383
Error err;
84-
kInstanceAttrsOffset =
85-
LoadConstant("class_Map__instance_attributes__int", err);
86-
if (err.Fail()) {
87-
kInstanceAttrsOffset = LoadConstant("class_Map__instance_type__uint16_t");
84+
kInstanceAttrsOffset = LoadConstant({"class_Map__instance_attributes__int",
85+
"class_Map__instance_type__uint16_t"});
86+
if (kInstanceAttrsOffset.name() ==
87+
"v8dbg_class_Map__instance_type__uint16_t") {
8888
kMapTypeMask = 0xffff;
8989
} else {
9090
kMapTypeMask = 0xff;
@@ -93,8 +93,9 @@ void Map::Load() {
9393
kMaybeConstructorOffset =
9494
LoadConstant("class_Map__constructor_or_backpointer__Object",
9595
"class_Map__constructor__Object");
96-
kInstanceDescriptorsOffset =
97-
LoadConstant("class_Map__instance_descriptors__DescriptorArray");
96+
kInstanceDescriptorsOffset = LoadConstant({
97+
"class_Map__instance_descriptors__DescriptorArray",
98+
});
9899
kBitField3Offset =
99100
LoadConstant("class_Map__bit_field3__int", "class_Map__bit_field3__SMI");
100101
kInObjectPropertiesOffset = LoadConstant(

‎src/llv8-constants.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ class Map : public Module {
6969
CONSTANTS_DEFAULT_METHODS(Map);
7070

7171
int64_t kMapTypeMask;
72-
int64_t kInstanceAttrsOffset;
72+
Constant<int64_t> kInstanceAttrsOffset;
7373
int64_t kMaybeConstructorOffset;
74-
int64_t kInstanceDescriptorsOffset;
74+
Constant<int64_t> kInstanceDescriptorsOffset;
7575
int64_t kBitField3Offset;
7676
int64_t kInObjectPropertiesOffset;
7777
int64_t kInObjectPropertiesStartOffset;

‎src/llv8-inl.h‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,11 @@ inline bool HeapObject::IsJSErrorType(Error& err) {
165165
}
166166

167167

168+
// TODO(mmarchini): return CheckedType
168169
inline int64_t Map::GetType(Error& err) {
169-
int64_t type =
170-
v8()->LoadUnsigned(LeaField(v8()->map()->kInstanceAttrsOffset), 2, err);
170+
RETURN_IF_INVALID(v8()->map()->kInstanceAttrsOffset, -1);
171+
int64_t type = v8()->LoadUnsigned(
172+
LeaField(*(v8()->map()->kInstanceAttrsOffset)), 2, err);
171173
if (err.Fail()) return -1;
172174

173175
return type & v8()->map()->kMapTypeMask;
@@ -230,12 +232,19 @@ inline int64_t Map::NumberOfOwnDescriptors(Error& err) {
230232
return LoadFieldValue<TYPE>(v8()->OFF, err); \
231233
}
232234

235+
#define SAFE_ACCESSOR(CLASS, METHOD, OFF, TYPE) \
236+
inline TYPE CLASS::METHOD(Error& err) { \
237+
if (!Check()) return TYPE(); \
238+
if (!v8()->OFF.Check()) return TYPE(); \
239+
return LoadFieldValue<TYPE>(*(v8()->OFF), err); \
240+
}
241+
233242

234243
ACCESSOR(HeapObject, GetMap, heap_obj()->kMapOffset, HeapObject)
235244

236245
ACCESSOR(Map, MaybeConstructor, map()->kMaybeConstructorOffset, HeapObject)
237-
ACCESSOR(Map, InstanceDescriptors, map()->kInstanceDescriptorsOffset,
238-
HeapObject)
246+
SAFE_ACCESSOR(Map, InstanceDescriptors, map()->kInstanceDescriptorsOffset,
247+
HeapObject)
239248

240249
ACCESSOR(Symbol, Name, symbol()->kNameOffset, HeapObject)
241250

‎src/llv8.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,10 @@ class CodeMap;
3737
NAME(Value* v) : PARENT(v->v8(), v->raw()) {} \
3838
static inline const char* ClassName() { return #NAME; }
3939

40-
#define RETURN_IF_INVALID(var, ret) \
41-
if (!var.Check()) { \
42-
return ret; \
40+
#define RETURN_IF_INVALID(var, ret) \
41+
if (!var.Check()) { \
42+
PRINT_DEBUG("Unable to load variable %s correctly", #var); \
43+
return ret; \
4344
}
4445

4546
#define RETURN_IF_THIS_INVALID(ret) \

0 commit comments

Comments
 (0)