Convert the trailing array of BindingNames at the end of the various kinds of scope data into raw unsigned chars into which those BindingNames are placement-new'd, rather than memcpy-ing non-trivial classes around and failing to comply with the C++ object model

This commit is contained in:
trav90 2018-09-02 18:45:56 -05:00 • committed by Roy Tam
commit 4cbf54089a
6 changed files with 108 additions and 64 deletions

View file

@ -319,7 +319,7 @@ ScopeKindIsInBody(ScopeKind kind)
static inline void static inline void
MarkAllBindingsClosedOver(LexicalScope::Data& data) MarkAllBindingsClosedOver(LexicalScope::Data& data)
{ {
BindingName* names = data.names; TrailingNamesArray& names = data.trailingNames;
for (uint32_t i = 0; i < data.length; i++) for (uint32_t i = 0; i < data.length; i++)
names[i] = BindingName(names[i].name(), true); names[i] = BindingName(names[i].name(), true);
} }
@ -8978,7 +8978,8 @@ BytecodeEmitter::isRestParameter(ParseNode* pn, bool* result)
if (bindings->nonPositionalFormalStart > 0) { if (bindings->nonPositionalFormalStart > 0) {
// |paramName| can be nullptr when the rest destructuring syntax is // |paramName| can be nullptr when the rest destructuring syntax is
// used: `function f(...[]) {}`. // used: `function f(...[]) {}`.
JSAtom* paramName = bindings->names[bindings->nonPositionalFormalStart - 1].name(); JSAtom* paramName =
bindings->trailingNames[bindings->nonPositionalFormalStart - 1].name();
*result = paramName && name == paramName; *result = paramName && name == paramName;
return true; return true;
} }

View file

@ -838,7 +838,7 @@ LexicalScopeNode::dump(int indent)
if (!isEmptyScope()) { if (!isEmptyScope()) {
LexicalScope::Data* bindings = scopeBindings(); LexicalScope::Data* bindings = scopeBindings();
for (uint32_t i = 0; i < bindings->length; i++) { for (uint32_t i = 0; i < bindings->length; i++) {
JSAtom* name = bindings->names[i].name(); JSAtom* name = bindings->trailingNames[i].name();
JS::AutoCheckCannotGC nogc; JS::AutoCheckCannotGC nogc;
if (name->hasLatin1Chars()) if (name->hasLatin1Chars())
DumpName(name->latin1Chars(nogc), name->length()); DumpName(name->latin1Chars(nogc), name->length());

View file

@ -19,6 +19,8 @@
#include "frontend/Parser.h" #include "frontend/Parser.h"
#include <new>
#include "jsapi.h" #include "jsapi.h"
#include "jsatom.h" #include "jsatom.h"
#include "jscntxt.h" #include "jscntxt.h"
@ -1452,7 +1454,7 @@ static typename Scope::Data*
NewEmptyBindingData(ExclusiveContext* cx, LifoAlloc& alloc, uint32_t numBindings) NewEmptyBindingData(ExclusiveContext* cx, LifoAlloc& alloc, uint32_t numBindings)
{ {
size_t allocSize = Scope::sizeOfData(numBindings); size_t allocSize = Scope::sizeOfData(numBindings);
typename Scope::Data* bindings = static_cast<typename Scope::Data*>(alloc.alloc(allocSize)); auto* bindings = static_cast<typename Scope::Data*>(alloc.alloc(allocSize));
if (!bindings) { if (!bindings) {
ReportOutOfMemory(cx); ReportOutOfMemory(cx);
return nullptr; return nullptr;
@ -1461,6 +1463,18 @@ NewEmptyBindingData(ExclusiveContext* cx, LifoAlloc& alloc, uint32_t numBindings
return bindings; return bindings;
} }
/**
* Copy-construct |BindingName|s from |bindings| into |cursor|, then return
* the location one past the newly-constructed |BindingName|s.
*/
static MOZ_MUST_USE BindingName*
FreshlyInitializeBindings(BindingName* cursor, const Vector<BindingName>& bindings)
{
for (const BindingName& binding : bindings)
new (cursor++) BindingName(binding);
return cursor;
}
template <> template <>
Maybe<GlobalScope::Data*> Maybe<GlobalScope::Data*>
Parser<FullParseHandler>::newGlobalScopeData(ParseContext::Scope& scope) Parser<FullParseHandler>::newGlobalScopeData(ParseContext::Scope& scope)
@ -1505,22 +1519,20 @@ Parser<FullParseHandler>::newGlobalScopeData(ParseContext::Scope& scope)
return Nothing(); return Nothing();
// The ordering here is important. See comments in GlobalScope. // The ordering here is important. See comments in GlobalScope.
BindingName* start = bindings->names; BindingName* start = bindings->trailingNames.start();
BindingName* cursor = start; BindingName* cursor = start;
PodCopy(cursor, funs.begin(), funs.length()); cursor = FreshlyInitializeBindings(cursor, funs);
cursor += funs.length();
bindings->varStart = cursor - start; bindings->varStart = cursor - start;
PodCopy(cursor, vars.begin(), vars.length()); cursor = FreshlyInitializeBindings(cursor, vars);
cursor += vars.length();
bindings->letStart = cursor - start; bindings->letStart = cursor - start;
PodCopy(cursor, lets.begin(), lets.length()); cursor = FreshlyInitializeBindings(cursor, lets);
cursor += lets.length();
bindings->constStart = cursor - start; bindings->constStart = cursor - start;
PodCopy(cursor, consts.begin(), consts.length()); cursor = FreshlyInitializeBindings(cursor, consts);
bindings->length = numBindings; bindings->length = numBindings;
} }
@ -1572,22 +1584,20 @@ Parser<FullParseHandler>::newModuleScopeData(ParseContext::Scope& scope)
return Nothing(); return Nothing();
// The ordering here is important. See comments in ModuleScope. // The ordering here is important. See comments in ModuleScope.
BindingName* start = bindings->names; BindingName* start = bindings->trailingNames.start();
BindingName* cursor = start; BindingName* cursor = start;
PodCopy(cursor, imports.begin(), imports.length()); cursor = FreshlyInitializeBindings(cursor, imports);
cursor += imports.length();
bindings->varStart = cursor - start; bindings->varStart = cursor - start;
PodCopy(cursor, vars.begin(), vars.length()); cursor = FreshlyInitializeBindings(cursor, vars);
cursor += vars.length();
bindings->letStart = cursor - start; bindings->letStart = cursor - start;
PodCopy(cursor, lets.begin(), lets.length()); cursor = FreshlyInitializeBindings(cursor, lets);
cursor += lets.length();
bindings->constStart = cursor - start; bindings->constStart = cursor - start;
PodCopy(cursor, consts.begin(), consts.length()); cursor = FreshlyInitializeBindings(cursor, consts);
bindings->length = numBindings; bindings->length = numBindings;
} }
@ -1623,16 +1633,16 @@ Parser<FullParseHandler>::newEvalScopeData(ParseContext::Scope& scope)
if (!bindings) if (!bindings)
return Nothing(); return Nothing();
BindingName* start = bindings->names; BindingName* start = bindings->trailingNames.start();
BindingName* cursor = start; BindingName* cursor = start;
// Keep track of what vars are functions. This is only used in BCE to omit // Keep track of what vars are functions. This is only used in BCE to omit
// superfluous DEFVARs. // superfluous DEFVARs.
PodCopy(cursor, funs.begin(), funs.length()); cursor = FreshlyInitializeBindings(cursor, funs);
cursor += funs.length();
bindings->varStart = cursor - start; bindings->varStart = cursor - start;
PodCopy(cursor, vars.begin(), vars.length()); cursor = FreshlyInitializeBindings(cursor, vars);
bindings->length = numBindings; bindings->length = numBindings;
} }
@ -1719,18 +1729,17 @@ Parser<FullParseHandler>::newFunctionScopeData(ParseContext::Scope& scope, bool
return Nothing(); return Nothing();
// The ordering here is important. See comments in FunctionScope. // The ordering here is important. See comments in FunctionScope.
BindingName* start = bindings->names; BindingName* start = bindings->trailingNames.start();
BindingName* cursor = start; BindingName* cursor = start;
PodCopy(cursor, positionalFormals.begin(), positionalFormals.length()); cursor = FreshlyInitializeBindings(cursor, positionalFormals);
cursor += positionalFormals.length();
bindings->nonPositionalFormalStart = cursor - start; bindings->nonPositionalFormalStart = cursor - start;
PodCopy(cursor, formals.begin(), formals.length()); cursor = FreshlyInitializeBindings(cursor, formals);
cursor += formals.length();
bindings->varStart = cursor - start; bindings->varStart = cursor - start;
PodCopy(cursor, vars.begin(), vars.length()); cursor = FreshlyInitializeBindings(cursor, vars);
bindings->length = numBindings; bindings->length = numBindings;
} }
@ -1760,10 +1769,11 @@ Parser<FullParseHandler>::newVarScopeData(ParseContext::Scope& scope)
return Nothing(); return Nothing();
// The ordering here is important. See comments in FunctionScope. // The ordering here is important. See comments in FunctionScope.
BindingName* start = bindings->names; BindingName* start = bindings->trailingNames.start();
BindingName* cursor = start; BindingName* cursor = start;
PodCopy(cursor, vars.begin(), vars.length()); cursor = FreshlyInitializeBindings(cursor, vars);
bindings->length = numBindings; bindings->length = numBindings;
} }
@ -1808,14 +1818,14 @@ Parser<FullParseHandler>::newLexicalScopeData(ParseContext::Scope& scope)
return Nothing(); return Nothing();
// The ordering here is important. See comments in LexicalScope. // The ordering here is important. See comments in LexicalScope.
BindingName* cursor = bindings->names; BindingName* cursor = bindings->trailingNames.start();
BindingName* start = cursor; BindingName* start = cursor;
PodCopy(cursor, lets.begin(), lets.length()); cursor = FreshlyInitializeBindings(cursor, lets);
cursor += lets.length();
bindings->constStart = cursor - start; bindings->constStart = cursor - start;
PodCopy(cursor, consts.begin(), consts.length()); cursor = FreshlyInitializeBindings(cursor, consts);
bindings->length = numBindings; bindings->length = numBindings;
} }

View file

@ -1231,34 +1231,34 @@ BindingIter::trace(JSTracer* trc)
void void
LexicalScope::Data::trace(JSTracer* trc) LexicalScope::Data::trace(JSTracer* trc)
{ {
TraceBindingNames(trc, names, length); TraceBindingNames(trc, trailingNames.start(), length);
} }
void void
FunctionScope::Data::trace(JSTracer* trc) FunctionScope::Data::trace(JSTracer* trc)
{ {
TraceNullableEdge(trc, &canonicalFunction, "scope canonical function"); TraceNullableEdge(trc, &canonicalFunction, "scope canonical function");
TraceNullableBindingNames(trc, names, length); TraceNullableBindingNames(trc, trailingNames.start(), length);
} }
void void
VarScope::Data::trace(JSTracer* trc) VarScope::Data::trace(JSTracer* trc)
{ {
TraceBindingNames(trc, names, length); TraceBindingNames(trc, trailingNames.start(), length);
} }
void void
GlobalScope::Data::trace(JSTracer* trc) GlobalScope::Data::trace(JSTracer* trc)
{ {
TraceBindingNames(trc, names, length); TraceBindingNames(trc, trailingNames.start(), length);
} }
void void
EvalScope::Data::trace(JSTracer* trc) EvalScope::Data::trace(JSTracer* trc)
{ {
TraceBindingNames(trc, names, length); TraceBindingNames(trc, trailingNames.start(), length);
} }
void void
ModuleScope::Data::trace(JSTracer* trc) ModuleScope::Data::trace(JSTracer* trc)
{ {
TraceNullableEdge(trc, &module, "scope module"); TraceNullableEdge(trc, &module, "scope module");
TraceBindingNames(trc, names, length); TraceBindingNames(trc, trailingNames.start(), length);
} }
void void
Scope::traceChildren(JSTracer* trc) Scope::traceChildren(JSTracer* trc)
@ -1302,13 +1302,13 @@ js::GCMarker::eagerlyMarkChildren(Scope* scope)
traverseEdge(scope, static_cast<Scope*>(scope->enclosing_)); traverseEdge(scope, static_cast<Scope*>(scope->enclosing_));
if (scope->environmentShape_) if (scope->environmentShape_)
traverseEdge(scope, static_cast<Shape*>(scope->environmentShape_)); traverseEdge(scope, static_cast<Shape*>(scope->environmentShape_));
BindingName* names = nullptr; TrailingNamesArray* names = nullptr;
uint32_t length = 0; uint32_t length = 0;
switch (scope->kind_) { switch (scope->kind_) {
case ScopeKind::Function: { case ScopeKind::Function: {
FunctionScope::Data* data = reinterpret_cast<FunctionScope::Data*>(scope->data_); FunctionScope::Data* data = reinterpret_cast<FunctionScope::Data*>(scope->data_);
traverseEdge(scope, static_cast<JSObject*>(data->canonicalFunction)); traverseEdge(scope, static_cast<JSObject*>(data->canonicalFunction));
names = data->names; names = &data->trailingNames;
length = data->length; length = data->length;
break; break;
} }
@ -1316,7 +1316,7 @@ js::GCMarker::eagerlyMarkChildren(Scope* scope)
case ScopeKind::FunctionBodyVar: case ScopeKind::FunctionBodyVar:
case ScopeKind::ParameterExpressionVar: { case ScopeKind::ParameterExpressionVar: {
VarScope::Data* data = reinterpret_cast<VarScope::Data*>(scope->data_); VarScope::Data* data = reinterpret_cast<VarScope::Data*>(scope->data_);
names = data->names; names = &data->trailingNames;
length = data->length; length = data->length;
break; break;
} }
@ -1327,7 +1327,7 @@ js::GCMarker::eagerlyMarkChildren(Scope* scope)
case ScopeKind::NamedLambda: case ScopeKind::NamedLambda:
case ScopeKind::StrictNamedLambda: { case ScopeKind::StrictNamedLambda: {
LexicalScope::Data* data = reinterpret_cast<LexicalScope::Data*>(scope->data_); LexicalScope::Data* data = reinterpret_cast<LexicalScope::Data*>(scope->data_);
names = data->names; names = &data->trailingNames;
length = data->length; length = data->length;
break; break;
} }
@ -1335,7 +1335,7 @@ js::GCMarker::eagerlyMarkChildren(Scope* scope)
case ScopeKind::Global: case ScopeKind::Global:
case ScopeKind::NonSyntactic: { case ScopeKind::NonSyntactic: {
GlobalScope::Data* data = reinterpret_cast<GlobalScope::Data*>(scope->data_); GlobalScope::Data* data = reinterpret_cast<GlobalScope::Data*>(scope->data_);
names = data->names; names = &data->trailingNames;
length = data->length; length = data->length;
break; break;
} }
@ -1343,7 +1343,7 @@ js::GCMarker::eagerlyMarkChildren(Scope* scope)
case ScopeKind::Eval: case ScopeKind::Eval:
case ScopeKind::StrictEval: { case ScopeKind::StrictEval: {
EvalScope::Data* data = reinterpret_cast<EvalScope::Data*>(scope->data_); EvalScope::Data* data = reinterpret_cast<EvalScope::Data*>(scope->data_);
names = data->names; names = &data->trailingNames;
length = data->length; length = data->length;
break; break;
} }
@ -1351,7 +1351,7 @@ js::GCMarker::eagerlyMarkChildren(Scope* scope)
case ScopeKind::Module: { case ScopeKind::Module: {
ModuleScope::Data* data = reinterpret_cast<ModuleScope::Data*>(scope->data_); ModuleScope::Data* data = reinterpret_cast<ModuleScope::Data*>(scope->data_);
traverseEdge(scope, static_cast<JSObject*>(data->module)); traverseEdge(scope, static_cast<JSObject*>(data->module));
names = data->names; names = &data->trailingNames;
length = data->length; length = data->length;
break; break;
} }
@ -1361,12 +1361,12 @@ js::GCMarker::eagerlyMarkChildren(Scope* scope)
} }
if (scope->kind_ == ScopeKind::Function) { if (scope->kind_ == ScopeKind::Function) {
for (uint32_t i = 0; i < length; i++) { for (uint32_t i = 0; i < length; i++) {
if (JSAtom* name = names[i].name()) if (JSAtom* name = names->operator[](i).name())
traverseEdge(scope, static_cast<JSString*>(name)); traverseEdge(scope, static_cast<JSString*>(name));
} }
} else { } else {
for (uint32_t i = 0; i < length; i++) for (uint32_t i = 0; i < length; i++)
traverseEdge(scope, static_cast<JSString*>(names[i].name())); traverseEdge(scope, static_cast<JSString*>(names->operator[](i).name()));
} }
} }

View file

@ -273,7 +273,7 @@ Scope::XDRSizedBindingNames(XDRState<mode>* xdr, Handle<ConcreteScope*> scope,
} }
for (uint32_t i = 0; i < length; i++) { for (uint32_t i = 0; i < length; i++) {
if (!XDRBindingName(xdr, &data->names[i])) { if (!XDRBindingName(xdr, &data->trailingNames[i])) {
if (mode == XDR_DECODE) { if (mode == XDR_DECODE) {
DeleteScopeData(data.get()); DeleteScopeData(data.get());
data.set(nullptr); data.set(nullptr);
@ -1250,7 +1250,7 @@ BindingIter::init(LexicalScope::Data& data, uint32_t firstFrameSlot, uint8_t fla
init(0, 0, 0, 0, 0, 0, init(0, 0, 0, 0, 0, 0,
CanHaveEnvironmentSlots | flags, CanHaveEnvironmentSlots | flags,
firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_), firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_),
data.names, data.length); data.trailingNames.start(), data.length);
} else { } else {
// imports - [0, 0) // imports - [0, 0)
// positional formals - [0, 0) // positional formals - [0, 0)
@ -1262,7 +1262,7 @@ BindingIter::init(LexicalScope::Data& data, uint32_t firstFrameSlot, uint8_t fla
init(0, 0, 0, 0, 0, data.constStart, init(0, 0, 0, 0, 0, data.constStart,
CanHaveFrameSlots | CanHaveEnvironmentSlots | flags, CanHaveFrameSlots | CanHaveEnvironmentSlots | flags,
firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_), firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_),
data.names, data.length); data.trailingNames.start(), data.length);
} }
} }
@ -1283,7 +1283,7 @@ BindingIter::init(FunctionScope::Data& data, uint8_t flags)
init(0, data.nonPositionalFormalStart, data.varStart, data.varStart, data.length, data.length, init(0, data.nonPositionalFormalStart, data.varStart, data.varStart, data.length, data.length,
flags, flags,
0, JSSLOT_FREE(&CallObject::class_), 0, JSSLOT_FREE(&CallObject::class_),
data.names, data.length); data.trailingNames.start(), data.length);
} }
void void
@ -1299,7 +1299,7 @@ BindingIter::init(VarScope::Data& data, uint32_t firstFrameSlot)
init(0, 0, 0, 0, data.length, data.length, init(0, 0, 0, 0, data.length, data.length,
CanHaveFrameSlots | CanHaveEnvironmentSlots, CanHaveFrameSlots | CanHaveEnvironmentSlots,
firstFrameSlot, JSSLOT_FREE(&VarEnvironmentObject::class_), firstFrameSlot, JSSLOT_FREE(&VarEnvironmentObject::class_),
data.names, data.length); data.trailingNames.start(), data.length);
} }
void void
@ -1343,7 +1343,7 @@ BindingIter::init(EvalScope::Data& data, bool strict)
// consts - [data.length, data.length) // consts - [data.length, data.length)
init(0, 0, 0, data.varStart, data.length, data.length, init(0, 0, 0, data.varStart, data.length, data.length,
flags, firstFrameSlot, firstEnvironmentSlot, flags, firstFrameSlot, firstEnvironmentSlot,
data.names, data.length); data.trailingNames.start(), data.length);
} }
void void
@ -1359,7 +1359,7 @@ BindingIter::init(ModuleScope::Data& data)
init(data.varStart, data.varStart, data.varStart, data.varStart, data.letStart, data.constStart, init(data.varStart, data.varStart, data.varStart, data.varStart, data.letStart, data.constStart,
CanHaveFrameSlots | CanHaveEnvironmentSlots, CanHaveFrameSlots | CanHaveEnvironmentSlots,
0, JSSLOT_FREE(&ModuleEnvironmentObject::class_), 0, JSSLOT_FREE(&ModuleEnvironmentObject::class_),
data.names, data.length); data.trailingNames.start(), data.length);
} }
PositionalFormalParameterIter::PositionalFormalParameterIter(JSScript* script) PositionalFormalParameterIter::PositionalFormalParameterIter(JSScript* script)

View file

@ -111,6 +111,39 @@ class BindingName
void trace(JSTracer* trc); void trace(JSTracer* trc);
}; };
/**
* The various {Global,Module,...}Scope::Data classes consist of always-present
* bits, then a trailing array of BindingNames. The various Data classes all
* end in a TrailingNamesArray that contains sized/aligned space for *one*
* BindingName. Data instances that contain N BindingNames, are then allocated
* in sizeof(Data) + (space for (N - 1) BindingNames). Because this class's
* |data_| field is properly sized/aligned, the N-BindingName array can start
* at |data_|.
*
* This is concededly a very low-level representation, but we want to only
* allocate once for data+bindings both, and this does so approximately as
* elegantly as C++ allows.
*/
class TrailingNamesArray
{
private:
alignas(BindingName) unsigned char data_[sizeof(BindingName)];
private:
// Some versions of GCC treat it as a -Wstrict-aliasing violation (ergo a
// -Werror compile error) to reinterpret_cast<> |data_| to |T*|, even
// through |void*|. Placing the latter cast in these separate functions
// breaks the chain such that affected GCC versions no longer warn/error.
void* ptr() {
return data_;
}
public:
BindingName* start() { return reinterpret_cast<BindingName*>(ptr()); }
BindingName& operator[](size_t i) { return start()[i]; }
};
class BindingLocation class BindingLocation
{ {
public: public:
@ -346,7 +379,7 @@ class LexicalScope : public Scope
// The array of tagged JSAtom* names, allocated beyond the end of the // The array of tagged JSAtom* names, allocated beyond the end of the
// struct. // struct.
BindingName names[1]; TrailingNamesArray trailingNames;
void trace(JSTracer* trc); void trace(JSTracer* trc);
}; };
@ -462,7 +495,7 @@ class FunctionScope : public Scope
// The array of tagged JSAtom* names, allocated beyond the end of the // The array of tagged JSAtom* names, allocated beyond the end of the
// struct. // struct.
BindingName names[1]; TrailingNamesArray trailingNames;
void trace(JSTracer* trc); void trace(JSTracer* trc);
}; };
@ -556,7 +589,7 @@ class VarScope : public Scope
// The array of tagged JSAtom* names, allocated beyond the end of the // The array of tagged JSAtom* names, allocated beyond the end of the
// struct. // struct.
BindingName names[1]; TrailingNamesArray trailingNames;
void trace(JSTracer* trc); void trace(JSTracer* trc);
}; };
@ -645,7 +678,7 @@ class GlobalScope : public Scope
// The array of tagged JSAtom* names, allocated beyond the end of the // The array of tagged JSAtom* names, allocated beyond the end of the
// struct. // struct.
BindingName names[1]; TrailingNamesArray trailingNames;
void trace(JSTracer* trc); void trace(JSTracer* trc);
}; };
@ -745,7 +778,7 @@ class EvalScope : public Scope
// The array of tagged JSAtom* names, allocated beyond the end of the // The array of tagged JSAtom* names, allocated beyond the end of the
// struct. // struct.
BindingName names[1]; TrailingNamesArray trailingNames;
void trace(JSTracer* trc); void trace(JSTracer* trc);
}; };
@ -846,7 +879,7 @@ class ModuleScope : public Scope
// The array of tagged JSAtom* names, allocated beyond the end of the // The array of tagged JSAtom* names, allocated beyond the end of the
// struct. // struct.
BindingName names[1]; TrailingNamesArray trailingNames;
void trace(JSTracer* trc); void trace(JSTracer* trc);
}; };