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

[torque] Strict verification of abstract types

Browse Source 
Originally, the Torque-generated verifier for a field with type
Undefined|Zero|NonNullForeign would check `f.IsUndefined() || f.IsZero()
|| f.IsNonNullForeign()`. At some point, we changed Torque so that it
now generates the much weaker `f.IsOddball() || f.IsSmi() ||
f.IsForeign()`. This change returns the verifiers to their initial
precision. Mostly we can use the names of abstract types to build up the
correct type check expression, but a few abstract types like
PodArrayOfWasmValueType have no way that we can tell them apart from
their parent type at runtime. It would be confusing to have a function
Object::IsPodArrayOfWasmValueType which actually just checks whether the
object is a ByteArray, so this change introduces a new annotation which
allows abstract type declarations to state that they should use their
parent type during verification.

This change also adds new test cases to help avoid future regressions of
this logic.

Bug: v8:7793
Change-Id: Ie5046d742fd45e0e0f6c2ba387d909e9f2ac6df1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2469960
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#70698}
This commit is contained in:
Seth Brenith 2020-10-21 12:33:52 -07:00 committed by Commit Bot
parent bcfb8e3fc5
commit 50d474a268
10 changed files with 143 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/builtins/base.tq
View File

@ -112,7 +112,7 @@ type bint generates 'TNode<BInt>' constexpr 'BInt';
type string constexpr 'const char*';
// A Smi value containing a bitfield struct as its integer data.
type SmiTagged<T : type extends uint31> extends Smi;
@useParentTypeChecker type SmiTagged<T : type extends uint31> extends Smi;
// WARNING: The memory representation (i.e., in class fields and arrays) of
// float64_or_hole is just a float64 that may be the hole-representing

18
src/torque/ast.h
View File

@ -855,16 +855,22 @@ struct InstanceTypeConstraints {
struct AbstractTypeDeclaration : TypeDeclaration {
DEFINE_AST_NODE_LEAF_BOILERPLATE(AbstractTypeDeclaration)
AbstractTypeDeclaration(SourcePosition pos, Identifier* name, bool transient,
AbstractTypeDeclaration(SourcePosition pos, Identifier* name,
AbstractTypeFlags flags,
base::Optional<TypeExpression*> extends,
base::Optional<std::string> generates)
: TypeDeclaration(kKind, pos, name),
is_constexpr(IsConstexprName(name->value)),
transient(transient),
flags(flags),
extends(extends),
generates(std::move(generates)) {}
bool is_constexpr;
bool transient;
generates(std::move(generates)) {
CHECK_EQ(IsConstexprName(name->value),
!!(flags & AbstractTypeFlag::kConstexpr));
}
bool IsConstexpr() const { return flags & AbstractTypeFlag::kConstexpr; }
bool IsTransient() const { return flags & AbstractTypeFlag::kTransient; }
AbstractTypeFlags flags;
base::Optional<TypeExpression*> extends;
base::Optional<std::string> generates;
};

8
src/torque/constants.h
View File

@ -76,6 +76,7 @@ static const char* const UNINITIALIZED_ITERATOR_TYPE_STRING =
static const char* const GENERIC_TYPE_INSTANTIATION_NAMESPACE_STRING =
"_generic_type_instantiation_namespace";
static const char* const FIXED_ARRAY_BASE_TYPE_STRING = "FixedArrayBase";
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";
@ -96,8 +97,10 @@ static const char* const ANNOTATION_IF = "@if";
static const char* const ANNOTATION_IFNOT = "@ifnot";
static const char* const ANNOTATION_GENERATE_BODY_DESCRIPTOR =
"@generateBodyDescriptor";
static const char* const ANNOTATION_EXPORT_CPP_CLASS = "@export";
static const char* const ANNOTATION_EXPORT = "@export";
static const char* const ANNOTATION_DO_NOT_GENERATE_CAST = "@doNotGenerateCast";
static const char* const ANNOTATION_USE_PARENT_TYPE_CHECKER =
"@useParentTypeChecker";
inline bool IsConstexprName(const std::string& name) {
return name.substr(0, std::strlen(CONSTEXPR_TYPE_PREFIX)) ==
@ -117,7 +120,8 @@ inline std::string GetConstexprName(const std::string& name) {
enum class AbstractTypeFlag {
kNone = 0,
kTransient = 1 << 0,
kConstexpr = 1 << 1
kConstexpr = 1 << 1,
kUseParentTypeChecker = 1 << 2,
};
using AbstractTypeFlags = base::Flags<AbstractTypeFlag>;

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

@ -3840,15 +3840,15 @@ std::string GenerateRuntimeTypeCheck(const Type* type,
type_check << value << ".IsCleared()";
at_start = false;
}
for (const RuntimeType& runtime_type : type->GetRuntimeTypes()) {
for (const TypeChecker& runtime_type : type->GetTypeCheckers()) {
if (!at_start) type_check << " || ";
at_start = false;
if (maybe_object) {
bool strong = runtime_type.weak_ref_to.empty();
if (strong && runtime_type.type == "MaybeObject") {
// Rather than a generic Weak<T>, this is a basic type Tagged or
// WeakHeapObject. We can't validate anything more about the type of
// the object pointed to, so just check that it's weak.
if (strong && runtime_type.type == WEAK_HEAP_OBJECT) {
// Rather than a generic Weak<T>, this is the basic type WeakHeapObject.
// We can't validate anything more about the type of the object pointed
// to, so just check that it's weak.
type_check << value << ".IsWeak()";
} else {
type_check << "(" << (strong ? "!" : "") << value << ".IsWeak() && "

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

@ -580,18 +580,23 @@ base::Optional<ParseResult> MakeIntrinsicDeclaration(
}
namespace {
bool HasExportAnnotation(ParseResultIterator* child_results,
const char* declaration) {
bool HasAnnotation(ParseResultIterator* child_results, const char* annotation,
const char* declaration) {
auto annotations = child_results->NextAs<std::vector<Annotation>>();
if (annotations.size()) {
if (annotations.size() > 1 || annotations[0].name->value != "@export") {
Error(declaration,
" declarations only support a single @export annotation");
if (annotations.size() > 1 || annotations[0].name->value != annotation) {
Error(declaration, " declarations only support a single ", annotation,
" annotation");
}
return true;
}
return false;
}
bool HasExportAnnotation(ParseResultIterator* child_results,
const char* declaration) {
return HasAnnotation(child_results, ANNOTATION_EXPORT, declaration);
}
} // namespace
base::Optional<ParseResult> MakeTorqueMacroDeclaration(
@ -685,6 +690,8 @@ base::Optional<ParseResult> MakeTypeAliasDeclaration(
base::Optional<ParseResult> MakeAbstractTypeDeclaration(
ParseResultIterator* child_results) {
bool use_parent_type_checker = HasAnnotation(
child_results, ANNOTATION_USE_PARENT_TYPE_CHECKER, "abstract type");
auto transient = child_results->NextAs<bool>();
auto name = child_results->NextAs<Identifier*>();
if (!IsValidTypeName(name->value)) {
@ -693,8 +700,11 @@ base::Optional<ParseResult> MakeAbstractTypeDeclaration(
auto generic_parameters = child_results->NextAs<GenericParameters>();
auto extends = child_results->NextAs<base::Optional<TypeExpression*>>();
auto generates = child_results->NextAs<base::Optional<std::string>>();
AbstractTypeFlags flags(AbstractTypeFlag::kNone);
if (transient) flags |= AbstractTypeFlag::kTransient;
if (use_parent_type_checker) flags |= AbstractTypeFlag::kUseParentTypeChecker;
TypeDeclaration* type_decl = MakeNode<AbstractTypeDeclaration>(
name, transient, extends, std::move(generates));
name, flags, extends, std::move(generates));
Declaration* decl = type_decl;
if (!generic_parameters.empty()) {
decl = MakeNode<GenericTypeDeclaration>(generic_parameters, type_decl);
@ -715,7 +725,8 @@ base::Optional<ParseResult> MakeAbstractTypeDeclaration(
constexpr_extends = AddConstexpr(*extends);
}
TypeDeclaration* constexpr_decl = MakeNode<AbstractTypeDeclaration>(
constexpr_name, transient, constexpr_extends, constexpr_generates);
constexpr_name, flags | AbstractTypeFlag::kConstexpr, constexpr_extends,
constexpr_generates);
constexpr_decl->pos = name->pos;
Declaration* decl = constexpr_decl;
if (!generic_parameters.empty()) {
@ -879,7 +890,7 @@ base::Optional<ParseResult> MakeClassDeclaration(
{ANNOTATION_GENERATE_PRINT, ANNOTATION_NO_VERIFIER, ANNOTATION_ABSTRACT,
ANNOTATION_HAS_SAME_INSTANCE_TYPE_AS_PARENT,
ANNOTATION_GENERATE_CPP_CLASS, ANNOTATION_GENERATE_BODY_DESCRIPTOR,
ANNOTATION_EXPORT_CPP_CLASS, ANNOTATION_DO_NOT_GENERATE_CAST,
ANNOTATION_EXPORT, ANNOTATION_DO_NOT_GENERATE_CAST,
ANNOTATION_HIGHEST_INSTANCE_TYPE_WITHIN_PARENT,
ANNOTATION_LOWEST_INSTANCE_TYPE_WITHIN_PARENT},
{ANNOTATION_RESERVE_BITS_IN_INSTANCE_TYPE,
@ -904,7 +915,7 @@ base::Optional<ParseResult> MakeClassDeclaration(
if (annotations.Contains(ANNOTATION_GENERATE_BODY_DESCRIPTOR)) {
flags |= ClassFlag::kGenerateBodyDescriptor;
}
if (annotations.Contains(ANNOTATION_EXPORT_CPP_CLASS)) {
if (annotations.Contains(ANNOTATION_EXPORT)) {
flags |= ClassFlag::kExport;
}
if (annotations.Contains(ANNOTATION_HIGHEST_INSTANCE_TYPE_WITHIN_PARENT)) {
@ -972,8 +983,10 @@ base::Optional<ParseResult> MakeClassDeclaration(
MakeNode<Identifier>(CONSTEXPR_TYPE_PREFIX + name->value);
constexpr_name->pos = name->pos;
TypeExpression* constexpr_extends = AddConstexpr(extends);
AbstractTypeFlags abstract_type_flags(AbstractTypeFlag::kConstexpr);
if (transient) abstract_type_flags |= AbstractTypeFlag::kTransient;
TypeDeclaration* constexpr_decl = MakeNode<AbstractTypeDeclaration>(
constexpr_name, transient, constexpr_extends, name->value);
constexpr_name, abstract_type_flags, constexpr_extends, name->value);
constexpr_decl->pos = name->pos;
result.push_back(constexpr_decl);
@ -1280,7 +1293,8 @@ base::Optional<ParseResult> MakeEnumDeclaration(
// type kEntryN extends Enum;
// }
auto type_decl = MakeNode<AbstractTypeDeclaration>(
name_identifier, false, base_type_expression, base::nullopt);
name_identifier, AbstractTypeFlag::kNone, base_type_expression,
base::nullopt);
TypeExpression* name_type_expression =
MakeNode<BasicTypeExpression>(name_identifier->value);
@ -1289,8 +1303,8 @@ base::Optional<ParseResult> MakeEnumDeclaration(
std::vector<Declaration*> entry_decls;
for (const auto& entry : entries) {
entry_decls.push_back(MakeNode<AbstractTypeDeclaration>(
entry.name, false, entry.type.value_or(name_type_expression),
base::nullopt));
entry.name, AbstractTypeFlag::kNone,
entry.type.value_or(name_type_expression), base::nullopt));
}
result.push_back(type_decl);
@ -1309,8 +1323,8 @@ base::Optional<ParseResult> MakeEnumDeclaration(
std::vector<Declaration*> entry_decls;
for (const auto& entry : entries) {
entry_decls.push_back(MakeNode<AbstractTypeDeclaration>(
entry.name, false, entry.type.value_or(*base_type_expression),
base::nullopt));
entry.name, AbstractTypeFlag::kNone,
entry.type.value_or(*base_type_expression), base::nullopt));
auto entry_type = MakeNode<BasicTypeExpression>(
std::vector<std::string>{name}, entry.name->value,
@ -1348,8 +1362,8 @@ base::Optional<ParseResult> MakeEnumDeclaration(
base_constexpr_type_expression = AddConstexpr(*base_type_expression);
}
result.push_back(MakeNode<AbstractTypeDeclaration>(
constexpr_type_identifier, false, base_constexpr_type_expression,
constexpr_generates));
constexpr_type_identifier, AbstractTypeFlag::kConstexpr,
base_constexpr_type_expression, constexpr_generates));
TypeExpression* type_expr = nullptr;
Identifier* fromconstexpr_identifier = nullptr;
@ -1386,8 +1400,9 @@ base::Optional<ParseResult> MakeEnumDeclaration(
"::" + entry_name);
entry_decls.push_back(MakeNode<AbstractTypeDeclaration>(
MakeNode<Identifier>(entry_constexpr_type), false,
constexpr_type_expression, constexpr_generates));
MakeNode<Identifier>(entry_constexpr_type),
AbstractTypeFlag::kConstexpr, constexpr_type_expression,
constexpr_generates));
bool generate_typed_constant = entry.type.has_value();
if (generate_typed_constant) {
@ -2535,7 +2550,7 @@ struct TorqueGrammar : Grammar {
Token("{"), List<BitFieldDeclaration>(&bitFieldDeclaration),
Token("}")},
AsSingletonVector<Declaration*, MakeBitFieldStructDeclaration>()),
Rule({CheckIf(Token("transient")), Token("type"), &name,
Rule({annotations, CheckIf(Token("transient")), Token("type"), &name,
TryOrDefault<GenericParameters>(&genericParameters),
Optional<TypeExpression*>(Sequence({Token("extends"), &type})),
Optional<std::string>(

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

@ -77,7 +77,7 @@ std::string ComputeGeneratesType(base::Optional<std::string> opt_gen,
const AbstractType* TypeVisitor::ComputeType(
AbstractTypeDeclaration* decl, MaybeSpecializationKey specialized_from) {
std::string generates =
ComputeGeneratesType(decl->generates, !decl->is_constexpr);
ComputeGeneratesType(decl->generates, !decl->IsConstexpr());
const Type* parent_type = nullptr;
if (decl->extends) {
@ -90,25 +90,21 @@ const AbstractType* TypeVisitor::ComputeType(
}
}
if (decl->is_constexpr && decl->transient) {
if (decl->IsConstexpr() && decl->IsTransient()) {
ReportError("cannot declare a transient type that is also constexpr");
}
const Type* non_constexpr_version = nullptr;
if (decl->is_constexpr) {
if (decl->IsConstexpr()) {
QualifiedName non_constexpr_name{GetNonConstexprName(decl->name->value)};
if (auto type = Declarations::TryLookupType(non_constexpr_name)) {
non_constexpr_version = *type;
}
}
AbstractTypeFlags flags = AbstractTypeFlag::kNone;
if (decl->transient) flags |= AbstractTypeFlag::kTransient;
if (decl->is_constexpr) flags |= AbstractTypeFlag::kConstexpr;
return TypeOracle::GetAbstractType(parent_type, decl->name->value, flags,
generates, non_constexpr_version,
specialized_from);
return TypeOracle::GetAbstractType(parent_type, decl->name->value,
decl->flags, generates,
non_constexpr_version, specialized_from);
}
void DeclareMethods(AggregateType* container_type,

11
src/torque/types.cc
View File

@ -173,13 +173,14 @@ std::string AbstractType::GetGeneratedTNodeTypeNameImpl() const {
return generated_type_;
}
std::vector<RuntimeType> AbstractType::GetRuntimeTypes() const {
std::string type_name = GetGeneratedTNodeTypeName();
std::vector<TypeChecker> AbstractType::GetTypeCheckers() const {
if (UseParentTypeChecker()) return parent()->GetTypeCheckers();
std::string type_name = name();
if (auto strong_type =
Type::MatchUnaryGeneric(this, TypeOracle::GetWeakGeneric())) {
auto strong_runtime_types = (*strong_type)->GetRuntimeTypes();
std::vector<RuntimeType> result;
for (const RuntimeType& type : strong_runtime_types) {
auto strong_runtime_types = (*strong_type)->GetTypeCheckers();
std::vector<TypeChecker> result;
for (const TypeChecker& type : strong_runtime_types) {
// Generic parameter in Weak<T> should have already been checked to
// extend HeapObject, so it couldn't itself be another weak type.
DCHECK(type.weak_ref_to.empty());

27
src/torque/types.h
View File

@ -95,7 +95,10 @@ struct SpecializationKey {
using MaybeSpecializationKey = base::Optional<SpecializationKey<GenericType>>;
struct RuntimeType {
struct TypeChecker {
// The type of the object. This string is not guaranteed to correspond to a
// C++ class, but just to a type checker function: for any type "Foo" here,
// the function Object::IsFoo must exist.
std::string type;
// If {type} is "MaybeObject", then {weak_ref_to} indicates the corresponding
// strong object type. Otherwise, {weak_ref_to} is empty.
@ -135,7 +138,7 @@ class V8_EXPORT_PRIVATE Type : public TypeBase {
std::string GetConstexprGeneratedTypeName() const;
base::Optional<const ClassType*> ClassSupertype() const;
base::Optional<const StructType*> StructSupertype() const;
virtual std::vector<RuntimeType> GetRuntimeTypes() const { return {}; }
virtual std::vector<TypeChecker> GetTypeCheckers() const { return {}; }
virtual std::string GetRuntimeType() const;
static const Type* CommonSupertype(const Type* a, const Type* b);
void AddAlias(std::string alias) const { aliases_.insert(std::move(alias)); }
@ -279,7 +282,7 @@ class AbstractType final : public Type {
return nullptr;
}
std::vector<RuntimeType> GetRuntimeTypes() const override;
std::vector<TypeChecker> GetTypeCheckers() const override;
size_t AlignmentLog2() const override;
@ -315,6 +318,10 @@ class AbstractType final : public Type {
return flags_ & AbstractTypeFlag::kTransient;
}
bool UseParentTypeChecker() const {
return flags_ & AbstractTypeFlag::kUseParentTypeChecker;
}
AbstractTypeFlags flags_;
const std::string name_;
const std::string generated_type_;
@ -349,7 +356,7 @@ class V8_EXPORT_PRIVATE BuiltinPointerType final : public Type {
}
size_t function_pointer_type_id() const { return function_pointer_type_id_; }
std::vector<RuntimeType> GetRuntimeTypes() const override {
std::vector<TypeChecker> GetTypeCheckers() const override {
return {{"Smi", ""}};
}
@ -461,10 +468,10 @@ class V8_EXPORT_PRIVATE UnionType final : public Type {
return union_type ? UnionType(*union_type) : UnionType(t);
}
std::vector<RuntimeType> GetRuntimeTypes() const override {
std::vector<RuntimeType> result;
std::vector<TypeChecker> GetTypeCheckers() const override {
std::vector<TypeChecker> result;
for (const Type* member : types_) {
std::vector<RuntimeType> sub_result = member->GetRuntimeTypes();
std::vector<TypeChecker> sub_result = member->GetTypeCheckers();
result.insert(result.end(), sub_result.begin(), sub_result.end());
}
return result;
@ -498,8 +505,8 @@ class V8_EXPORT_PRIVATE BitFieldStructType final : public Type {
return parent()->GetGeneratedTNodeTypeName();
}
std::vector<RuntimeType> GetRuntimeTypes() const override {
return {{parent()->GetGeneratedTNodeTypeName(), ""}};
std::vector<TypeChecker> GetTypeCheckers() const override {
return parent()->GetTypeCheckers();
}
void SetConstexprVersion(const Type*) const override { UNREACHABLE(); }
@ -559,7 +566,7 @@ class AggregateType : public Type {
std::vector<Method*> Methods(const std::string& name) const;
std::vector<const AggregateType*> GetHierarchy() const;
std::vector<RuntimeType> GetRuntimeTypes() const override {
std::vector<TypeChecker> GetTypeCheckers() const override {
return {{name_, ""}};
}

6
src/wasm/wasm-objects.tq
View File

@ -2,10 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@useParentTypeChecker
type PodArrayOfWasmValueType extends ByteArray
constexpr 'PodArray<wasm::ValueType>';
constexpr 'PodArray<wasm::ValueType>';
@useParentTypeChecker
type ManagedWasmNativeModule extends Foreign
constexpr 'Managed<wasm::NativeModule>';
constexpr 'Managed<wasm::NativeModule>';
type WasmValueType extends uint8 constexpr 'wasm::ValueType::Kind';
extern class WasmInstanceObject extends JSObject;

51
test/cctest/test-verifiers.cc
View File

@ -128,6 +128,57 @@ TEST_PAIR(TestWrongWeakTypeInIndexedStructField) {
TaggedField<Object>::store(*descriptors, offset, *original_value);
}
TEST_PAIR(TestWrongOddball) {
CcTest::InitializeVM();
v8::Isolate* isolate = CcTest::isolate();
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
v8::HandleScope scope(isolate);
v8::Local<v8::Value> v = CompileRun("new Date()");
Handle<JSDate> date = Handle<JSDate>::cast(v8::Utils::OpenHandle(*v));
Handle<Object> original_hour(
TaggedField<Object>::load(*date, JSDate::kHourOffset), i_isolate);
// There must be no GC (and therefore no verifiers running) until we can
// restore the modified data.
DisallowHeapAllocation no_gc;
// Hour is Undefined|Smi|NaN. Other oddballs like null should cause a failure.
TaggedField<Object>::store(*date, JSDate::kHourOffset,
*i_isolate->factory()->null_value());
if (should_fail) {
TorqueGeneratedClassVerifiers::JSDateVerify(*date, i_isolate);
}
// Put back the original value in case verifiers run on test shutdown.
TaggedField<Object>::store(*date, JSDate::kHourOffset, *original_hour);
}
TEST_PAIR(TestWrongNumber) {
CcTest::InitializeVM();
v8::Isolate* isolate = CcTest::isolate();
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
v8::HandleScope scope(isolate);
v8::Local<v8::Value> v = CompileRun("new Date()");
Handle<JSDate> date = Handle<JSDate>::cast(v8::Utils::OpenHandle(*v));
Handle<Object> original_hour(
TaggedField<Object>::load(*date, JSDate::kHourOffset), i_isolate);
v8::Local<v8::Value> v2 = CompileRun("1.1");
Handle<Object> float_val = v8::Utils::OpenHandle(*v2);
// There must be no GC (and therefore no verifiers running) until we can
// restore the modified data.
DisallowHeapAllocation no_gc;
// Hour is Undefined|Smi|NaN. Other doubles like 1.1 should cause a failure.
TaggedField<Object>::store(*date, JSDate::kHourOffset, *float_val);
if (should_fail) {
TorqueGeneratedClassVerifiers::JSDateVerify(*date, i_isolate);
}
// Put back the original value in case verifiers run on test shutdown.
TaggedField<Object>::store(*date, JSDate::kHourOffset, *original_hour);
}
#endif // VERIFY_HEAP
#undef TEST_PAIR