AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert "[torque] Get rid of @noVerifier annotation"

Browse Source 
This reverts commit 9495817296.

Reason for revert: Breaks arm/arm64 ports, e.g. https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20arm64%20-%20sim/30120/blamelist

Original change's description:
> [torque] Get rid of @noVerifier annotation
>
> As one small step toward reducing annotations, I propose that all
> classes get generated verifiers unless they've opted out of C++ class
> generation via @doNotGenerateCppClass, and that generated verifiers
> always verify every Torque-defined field. If a generated verifier is
> incorrect, such as for JSFunction or DataHandler, we can just avoid
> calling it and hand-code the verification.
>
> Bug: v8:7793
> Change-Id: I7c0edb660574d0c688a59c7e90c41ee7ad464b42
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3171758
> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#77145}

Bug: v8:7793
Change-Id: I56da8a9726d23470e927be1be5e7bcede1399861
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3194262
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Owners-Override: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#77146}
This commit is contained in:
Maya Lekova 2021-09-29 17:04:28 +00:00 committed by V8 LUCI CQ
parent 9495817296
commit 8679a4e1a2
13 changed files with 66 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/builtins/base.tq
View File

@ -554,6 +554,7 @@ extern class Filler extends HeapObject generates 'TNode<HeapObject>';
// Like JSObject, but created from API function.
@apiExposedInstanceTypeValue(0x422)
@doNotGenerateCast
@noVerifier
extern class JSApiObject extends JSObject generates 'TNode<JSObject>';
// TODO(gsathya): This only exists to make JSApiObject instance type into a
@ -561,6 +562,7 @@ extern class JSApiObject extends JSObject generates 'TNode<JSObject>';
@apiExposedInstanceTypeValue(0x80A)
@doNotGenerateCast
@highestInstanceTypeWithinParentClassRange
@noVerifier
extern class JSLastDummyApiObject extends JSApiObject
generates 'TNode<JSObject>';

44
src/diagnostics/objects-debug.cc
View File

@ -166,9 +166,7 @@ void TaggedIndex::TaggedIndexVerify(Isolate* isolate) {
}
void HeapObject::HeapObjectVerify(Isolate* isolate) {
CHECK(IsHeapObject());
VerifyPointer(isolate, map(isolate));
CHECK(map(isolate).IsMap());
TorqueGeneratedClassVerifiers::HeapObjectVerify(*this, isolate);
switch (map().instance_type()) {
#define STRING_TYPE_CASE(TYPE, size, name, CamelName) case TYPE:
@ -829,24 +827,7 @@ void JSBoundFunction::JSBoundFunctionVerify(Isolate* isolate) {
}
void JSFunction::JSFunctionVerify(Isolate* isolate) {
// Don't call TorqueGeneratedClassVerifiers::JSFunctionVerify here because the
// Torque class definition contains the field `prototype_or_initial_map` which
// may not be allocated.
// This assertion exists to encourage updating this verification function if
// new fields are added in the Torque class layout definition.
STATIC_ASSERT(JSFunction::TorqueGeneratedClass::kHeaderSize ==
8 * kTaggedSize);
JSFunctionOrBoundFunctionVerify(isolate);
CHECK(IsJSFunction());
VerifyPointer(isolate, shared(isolate));
CHECK(shared(isolate).IsSharedFunctionInfo());
VerifyPointer(isolate, context(isolate, kRelaxedLoad));
CHECK(context(isolate, kRelaxedLoad).IsContext());
VerifyPointer(isolate, raw_feedback_cell(isolate));
CHECK(raw_feedback_cell(isolate).IsFeedbackCell());
VerifyPointer(isolate, raw_code(isolate));
TorqueGeneratedClassVerifiers::JSFunctionVerify(*this, isolate);
CHECK(raw_code(isolate).IsCodeT());
CHECK(map(isolate).is_callable());
Handle<JSFunction> function(*this, isolate);
@ -1249,9 +1230,8 @@ void SmallOrderedHashTable<Derived>::SmallOrderedHashTableVerify(
}
}
}
void SmallOrderedHashMap::SmallOrderedHashMapVerify(Isolate* isolate) {
CHECK(IsSmallOrderedHashMap());
TorqueGeneratedClassVerifiers::SmallOrderedHashMapVerify(*this, isolate);
SmallOrderedHashTable<SmallOrderedHashMap>::SmallOrderedHashTableVerify(
isolate);
for (int entry = NumberOfElements(); entry < NumberOfDeletedElements();
@ -1264,7 +1244,7 @@ void SmallOrderedHashMap::SmallOrderedHashMapVerify(Isolate* isolate) {
}
void SmallOrderedHashSet::SmallOrderedHashSetVerify(Isolate* isolate) {
CHECK(IsSmallOrderedHashSet());
TorqueGeneratedClassVerifiers::SmallOrderedHashSetVerify(*this, isolate);
SmallOrderedHashTable<SmallOrderedHashSet>::SmallOrderedHashTableVerify(
isolate);
for (int entry = NumberOfElements(); entry < NumberOfDeletedElements();
@ -1278,7 +1258,8 @@ void SmallOrderedHashSet::SmallOrderedHashSetVerify(Isolate* isolate) {
void SmallOrderedNameDictionary::SmallOrderedNameDictionaryVerify(
Isolate* isolate) {
CHECK(IsSmallOrderedNameDictionary());
TorqueGeneratedClassVerifiers::SmallOrderedNameDictionaryVerify(*this,
isolate);
SmallOrderedHashTable<
SmallOrderedNameDictionary>::SmallOrderedHashTableVerify(isolate);
for (int entry = NumberOfElements(); entry < NumberOfDeletedElements();
@ -1674,20 +1655,9 @@ void WasmExportedFunctionData::WasmExportedFunctionDataVerify(
#endif // V8_ENABLE_WEBASSEMBLY
void DataHandler::DataHandlerVerify(Isolate* isolate) {
// Don't call TorqueGeneratedClassVerifiers::DataHandlerVerify because the
// Torque definition of this class includes all of the optional fields.
// This assertion exists to encourage updating this verification function if
// new fields are added in the Torque class layout definition.
STATIC_ASSERT(DataHandler::kHeaderSize == 6 * kTaggedSize);
StructVerify(isolate);
CHECK(IsDataHandler());
VerifyPointer(isolate, smi_handler(isolate));
TorqueGeneratedClassVerifiers::DataHandlerVerify(*this, isolate);
CHECK_IMPLIES(!smi_handler().IsSmi(),
IsStoreHandler() && smi_handler().IsCodeT());
VerifyPointer(isolate, validity_cell(isolate));
CHECK(validity_cell().IsSmi() || validity_cell().IsCell());
int data_count = data_field_count();
if (data_count >= 1) {
VerifyMaybeObjectField(isolate, kData1Offset);

1
src/objects/bigint.tq
View File

@ -9,6 +9,7 @@ extern class BigIntBase extends PrimitiveHeapObject
type BigInt extends BigIntBase;
@noVerifier
@hasSameInstanceTypeAsParent
@doNotGenerateCast
extern class MutableBigInt extends BigIntBase generates 'TNode<BigInt>';

8
src/objects/data-handler.tq
View File

@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// This class does not use the generated verifier, so if you change anything
// here, please also update DataHandlerVerify in objects-debug.cc.
@abstract
extern class DataHandler extends Struct {
// [smi_handler]: A Smi which encodes a handler or Code object (we still
@ -17,7 +15,7 @@ extern class DataHandler extends Struct {
validity_cell: Smi|Cell;
// Space for the following fields may or may not be allocated.
data1: MaybeObject;
data2: MaybeObject;
data3: MaybeObject;
@noVerifier data1: MaybeObject;
@noVerifier data2: MaybeObject;
@noVerifier data3: MaybeObject;
}

4
src/objects/js-function.tq
View File

@ -17,8 +17,6 @@ extern class JSBoundFunction extends JSFunctionOrBoundFunction {
bound_arguments: FixedArray;
}
// This class does not use the generated verifier, so if you change anything
// here, please also update JSFunctionVerify in objects-debug.cc.
@highestInstanceTypeWithinParentClassRange
extern class JSFunction extends JSFunctionOrBoundFunction {
shared_function_info: SharedFunctionInfo;
@ -27,7 +25,7 @@ extern class JSFunction extends JSFunctionOrBoundFunction {
@if(V8_EXTERNAL_CODE_SPACE) code: CodeDataContainer;
@ifnot(V8_EXTERNAL_CODE_SPACE) code: Code;
// Space for the following field may or may not be allocated.
prototype_or_initial_map: JSReceiver|Map;
@noVerifier prototype_or_initial_map: JSReceiver|Map;
}
// Class constructors are special, because they are callable, but [[Call]] will

1
src/objects/ordered-hash-table.tq
View File

@ -14,6 +14,7 @@ const kSmallOrderedHashTableNotFound: constexpr int31
const kSmallOrderedHashTableLoadFactor: constexpr int31
generates 'SmallOrderedHashTable<int>::kLoadFactor';
@noVerifier
@abstract
@doNotGenerateCppClass
extern class SmallOrderedHashTable extends HeapObject

1
src/objects/swiss-name-dictionary.tq
View File

@ -4,6 +4,7 @@
#include 'src/objects/swiss-name-dictionary.h'
@noVerifier
@doNotGenerateCppClass
extern class SwissNameDictionary extends HeapObject {
hash: uint32;

1
src/torque/ast.h
View File

@ -939,6 +939,7 @@ struct ClassFieldExpression {
std::vector<ConditionalAnnotation> conditions;
bool weak;
bool const_qualified;
bool generate_verify;
FieldSynchronization read_synchronization;
FieldSynchronization write_synchronization;
};

28
src/torque/constants.h
View File

@ -84,6 +84,7 @@ static const char* const WEAK_HEAP_OBJECT = "WeakHeapObject";
static const char* const STATIC_ASSERT_MACRO_STRING = "StaticAssert";
static const char* const ANNOTATION_GENERATE_PRINT = "@generatePrint";
static const char* const ANNOTATION_NO_VERIFIER = "@noVerifier";
static const char* const ANNOTATION_ABSTRACT = "@abstract";
static const char* const ANNOTATION_HAS_SAME_INSTANCE_TYPE_AS_PARENT =
"@hasSameInstanceTypeAsParent";
@ -144,19 +145,20 @@ enum class ClassFlag {
kNone = 0,
kExtern = 1 << 0,
kGeneratePrint = 1 << 1,
kTransient = 1 << 2,
kAbstract = 1 << 3,
kIsShape = 1 << 4,
kHasSameInstanceTypeAsParent = 1 << 5,
kGenerateCppClassDefinitions = 1 << 6,
kCustomCppClass = 1 << 7,
kHighestInstanceTypeWithinParent = 1 << 8,
kLowestInstanceTypeWithinParent = 1 << 9,
kUndefinedLayout = 1 << 10,
kGenerateBodyDescriptor = 1 << 11,
kExport = 1 << 12,
kDoNotGenerateCast = 1 << 13,
kCustomMap = 1 << 14,
kGenerateVerify = 1 << 2,
kTransient = 1 << 3,
kAbstract = 1 << 4,
kIsShape = 1 << 5,
kHasSameInstanceTypeAsParent = 1 << 6,
kGenerateCppClassDefinitions = 1 << 7,
kCustomCppClass = 1 << 8,
kHighestInstanceTypeWithinParent = 1 << 9,
kLowestInstanceTypeWithinParent = 1 << 10,
kUndefinedLayout = 1 << 11,
kGenerateBodyDescriptor = 1 << 12,
kExport = 1 << 13,
kDoNotGenerateCast = 1 << 14,
kCustomMap = 1 << 15,
};
using ClassFlags = base::Flags<ClassFlag>;

25
src/torque/implementation-visitor.cc
View File

@ -77,7 +77,7 @@ const Type* ImplementationVisitor::Visit(Statement* stmt) {
void ImplementationVisitor::BeginGeneratedFiles() {
std::set<SourceId> contains_class_definitions;
for (const ClassType* type : TypeOracle::GetClasses()) {
if (type->ShouldGenerateCppClassDefinitions()) {
if (type->GenerateCppClassDefinitions()) {
contains_class_definitions.insert(type->AttributedToFile());
}
}
@ -3818,8 +3818,7 @@ void ImplementationVisitor::GenerateClassFieldOffsets(
for (const ClassType* type : TypeOracle::GetClasses()) {
// TODO(danno): Remove this once all classes use ClassFieldOffsetGenerator
// to generate field offsets without the use of macros.
if (!type->ShouldGenerateCppClassDefinitions() &&
!type->HasUndefinedLayout()) {
if (!type->GenerateCppClassDefinitions() && !type->HasUndefinedLayout()) {
MacroFieldOffsetsGenerator g(header, type);
for (auto f : type->fields()) {
CurrentSourcePosition::Scope scope(f.pos);
@ -4209,7 +4208,7 @@ void CppClassGenerator::GenerateClass() {
// hand-written definition.
base::Optional<const ClassType*> parent = type_->parent()->ClassSupertype();
while (parent) {
if ((*parent)->ShouldGenerateCppClassDefinitions() &&
if ((*parent)->GenerateCppClassDefinitions() &&
!(*parent)->ShouldGenerateFullClassDefinition() &&
(*parent)->AttributedToFile() == type_->AttributedToFile()) {
Error("Exported ", *type_,
@ -4682,7 +4681,7 @@ void ImplementationVisitor::GenerateClassDefinitions(
CurrentSourcePosition::Scope position_activator(type->GetPosition());
auto& streams = GlobalContext::GeneratedPerFile(type->AttributedToFile());
std::ostream& header = streams.class_definition_headerfile;
std::string name = type->ShouldGenerateCppClassDefinitions()
std::string name = type->GenerateCppClassDefinitions()
? type->name()
: type->GetGeneratedTNodeTypeName();
header << "class " << name << ";\n";
@ -4696,7 +4695,7 @@ void ImplementationVisitor::GenerateClassDefinitions(
std::ostream& inline_header = streams.class_definition_inline_headerfile;
std::ostream& implementation = streams.class_definition_ccfile;
if (type->ShouldGenerateCppClassDefinitions()) {
if (type->GenerateCppClassDefinitions()) {
CppClassGenerator g(type, header, inline_header, implementation);
g.GenerateClass();
}
@ -4882,7 +4881,7 @@ void ImplementationVisitor::GeneratePrintDefinitions(
for (const ClassType* type : TypeOracle::GetClasses()) {
if (!type->ShouldGeneratePrint()) continue;
if (type->ShouldGenerateCppClassDefinitions()) {
if (type->GenerateCppClassDefinitions()) {
const ClassType* super = type->GetSuperClass();
std::string gen_name = "TorqueGenerated" + type->name();
std::string gen_name_T =
@ -5120,6 +5119,7 @@ void GenerateClassFieldVerifier(const std::string& class_name,
const ClassType& class_type, const Field& f,
std::ostream& h_contents,
std::ostream& cc_contents) {
if (!f.generate_verify) return;
const Type* field_type = f.name_and_type.type;
// We only verify tagged types, not raw numbers or pointers. Structs
@ -5236,7 +5236,16 @@ void ImplementationVisitor::GenerateClassVerifiers(
}
if (super_type) {
std::string super_name = super_type->name();
cc_contents << " o." << super_name << "Verify(isolate);\n";
if (super_name == "HeapObject") {
// Special case: HeapObjectVerify checks the Map type and dispatches
// to more specific types, so calling it here would cause infinite
// recursion. We could consider moving that behavior into a
// different method to make the contract of *Verify methods more
// consistent, but for now we'll just avoid the bad case.
cc_contents << " " << super_name << "Verify(o, isolate);\n";
} else {
cc_contents << " o." << super_name << "Verify(isolate);\n";
}
}
// Second, verify that this object is what it claims to be.

11
src/torque/torque-parser.cc
View File

@ -888,7 +888,7 @@ base::Optional<ParseResult> MakeClassDeclaration(
ParseResultIterator* child_results) {
AnnotationSet annotations(
child_results,
{ANNOTATION_GENERATE_PRINT, ANNOTATION_ABSTRACT,
{ANNOTATION_GENERATE_PRINT, ANNOTATION_NO_VERIFIER, ANNOTATION_ABSTRACT,
ANNOTATION_HAS_SAME_INSTANCE_TYPE_AS_PARENT,
ANNOTATION_DO_NOT_GENERATE_CPP_CLASS, ANNOTATION_CUSTOM_CPP_CLASS,
ANNOTATION_CUSTOM_MAP, ANNOTATION_GENERATE_BODY_DESCRIPTOR,
@ -900,6 +900,8 @@ base::Optional<ParseResult> MakeClassDeclaration(
ClassFlags flags = ClassFlag::kNone;
bool generate_print = annotations.Contains(ANNOTATION_GENERATE_PRINT);
if (generate_print) flags |= ClassFlag::kGeneratePrint;
bool generate_verify = !annotations.Contains(ANNOTATION_NO_VERIFIER);
if (generate_verify) flags |= ClassFlag::kGenerateVerify;
if (annotations.Contains(ANNOTATION_ABSTRACT)) {
flags |= ClassFlag::kAbstract;
}
@ -1969,9 +1971,11 @@ base::Optional<ParseResult> MakeAnnotation(ParseResultIterator* child_results) {
base::Optional<ParseResult> MakeClassField(ParseResultIterator* child_results) {
AnnotationSet annotations(
child_results,
{ANNOTATION_CPP_RELAXED_STORE, ANNOTATION_CPP_RELAXED_LOAD,
ANNOTATION_CPP_RELEASE_STORE, ANNOTATION_CPP_ACQUIRE_LOAD},
{ANNOTATION_NO_VERIFIER, ANNOTATION_CPP_RELAXED_STORE,
ANNOTATION_CPP_RELAXED_LOAD, ANNOTATION_CPP_RELEASE_STORE,
ANNOTATION_CPP_ACQUIRE_LOAD},
{ANNOTATION_IF, ANNOTATION_IFNOT});
bool generate_verify = !annotations.Contains(ANNOTATION_NO_VERIFIER);
FieldSynchronization write_synchronization = FieldSynchronization::kNone;
if (annotations.Contains(ANNOTATION_CPP_RELEASE_STORE)) {
write_synchronization = FieldSynchronization::kAcquireRelease;
@ -2023,6 +2027,7 @@ base::Optional<ParseResult> MakeClassField(ParseResultIterator* child_results) {
std::move(conditions),
weak,
const_qualified,
generate_verify,
read_synchronization,
write_synchronization}};
}

4
src/torque/type-visitor.cc
View File

@ -211,6 +211,7 @@ const StructType* TypeVisitor::ComputeType(
offset.SingleValue(),
false,
field.const_qualified,
false,
FieldSynchronization::kNone,
FieldSynchronization::kNone};
auto optional_size = SizeOf(f.name_and_type.type);
@ -318,7 +319,7 @@ const ClassType* TypeVisitor::ComputeType(
Error("non-external classes must have defined layouts");
}
}
flags = flags | ClassFlag::kGeneratePrint;
flags = flags | ClassFlag::kGeneratePrint | ClassFlag::kGenerateVerify;
}
if (!(flags & ClassFlag::kExtern) &&
(flags & ClassFlag::kHasSameInstanceTypeAsParent)) {
@ -441,6 +442,7 @@ void TypeVisitor::VisitClassFieldsAndMethods(
class_offset.SingleValue(),
field_expression.weak,
field_expression.const_qualified,
field_expression.generate_verify,
field_expression.read_synchronization,
field_expression.write_synchronization});
ResidueClass field_size = std::get<0>(field.GetFieldSizeInformation());

10
src/torque/types.h
View File

@ -228,6 +228,7 @@ struct Field {
bool is_weak;
bool const_qualified;
bool generate_verify;
FieldSynchronization read_synchronization;
FieldSynchronization write_synchronization;
};
@ -673,8 +674,8 @@ class ClassType final : public AggregateType {
((flags_ & ClassFlag::kGeneratePrint) && !HasUndefinedLayout());
}
bool ShouldGenerateVerify() const {
return !IsExtern() || (ShouldGenerateCppClassDefinitions() &&
!HasUndefinedLayout() && !IsShape());
return !IsExtern() || ((flags_ & ClassFlag::kGenerateVerify) &&
(!HasUndefinedLayout() && !IsShape()));
}
bool ShouldGenerateBodyDescriptor() const {
return flags_ & ClassFlag::kGenerateBodyDescriptor ||
@ -688,8 +689,9 @@ class ClassType final : public AggregateType {
bool HasSameInstanceTypeAsParent() const {
return flags_ & ClassFlag::kHasSameInstanceTypeAsParent;
}
bool ShouldGenerateCppClassDefinitions() const {
return (flags_ & ClassFlag::kGenerateCppClassDefinitions) || !IsExtern();
bool GenerateCppClassDefinitions() const {
return flags_ & ClassFlag::kGenerateCppClassDefinitions || !IsExtern() ||
ShouldGenerateBodyDescriptor();
}
bool ShouldGenerateFullClassDefinition() const {
return !IsExtern() && !(flags_ & ClassFlag::kCustomCppClass);