From 917b14b626c95f6cb8b4374e2b788b6d37a8035b Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 19 Apr 2021 16:50:23 +0000 Subject: [PATCH] CloneContext: Assert objects are owned by the program Check that pre-clone objects are owned by the source program. Check that post-clone object are owned by the target builder. Fixed: tint:469 Change-Id: Idd0eeb8dfb386e295b66b4b6621cc13dc1a30786 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48260 Commit-Queue: Ben Clayton Reviewed-by: Antonio Maiorano Reviewed-by: James Price --- src/ast/node.h | 2 +- src/clone_context.h | 20 ++++++++++++++-- src/clone_context_test.cc | 50 +++++++++++++++++++++++++++++++++++++++ src/program.h | 6 +++++ src/program_builder.h | 6 +++++ src/type/type.h | 7 ++++++ 6 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/ast/node.h b/src/ast/node.h index fe74af83c1..18eee89209 100644 --- a/src/ast/node.h +++ b/src/ast/node.h @@ -81,7 +81,7 @@ class Node : public Castable { /// @param node a pointer to an AST node /// @returns the ProgramID of the given AST node. -inline ProgramID ProgramIDOf(ast::Node* node) { +inline ProgramID ProgramIDOf(const ast::Node* node) { return node ? node->program_id() : ProgramID(); } diff --git a/src/clone_context.h b/src/clone_context.h index 55af18f731..16f8fa1c15 100644 --- a/src/clone_context.h +++ b/src/clone_context.h @@ -32,11 +32,14 @@ namespace tint { class CloneContext; class Program; class ProgramBuilder; - namespace ast { class FunctionList; +class Node; } // namespace ast +ProgramID ProgramIDOf(const Program*); +ProgramID ProgramIDOf(const ast::Node*); + /// Cloneable is the base class for all objects that can be cloned class Cloneable : public Castable { public: @@ -46,6 +49,11 @@ class Cloneable : public Castable { virtual Cloneable* Clone(CloneContext* ctx) const = 0; }; +/// @returns an invalid ProgramID +inline ProgramID ProgramIDOf(const Cloneable*) { + return ProgramID(); +} + /// ShareableCloneable is the base class for Cloneable objects which will only /// be cloned once when CloneContext::Clone() is called with the same object /// pointer. @@ -95,6 +103,8 @@ class CloneContext { return nullptr; } + TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(src, a); + // Have we cloned this object already, or was Replace() called for this // object? auto it = cloned_.find(a); @@ -127,7 +137,11 @@ class CloneContext { cloned_.emplace(a, cloned); } - return CheckedCast(cloned); + auto* out = CheckedCast(cloned); + + TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(dst, out); + + return out; } /// Clones the Node or type::Type `a` into the ProgramBuilder #dst if `a` is @@ -149,6 +163,8 @@ class CloneContext { return nullptr; } + TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(src, a); + // Have we seen this object before? If so, return the previously cloned // version instead of making yet another copy. auto it = cloned_.find(a); diff --git a/src/clone_context_test.cc b/src/clone_context_test.cc index 7337de646a..a038f6b056 100644 --- a/src/clone_context_test.cc +++ b/src/clone_context_test.cc @@ -97,6 +97,23 @@ struct NotANode : public Castable { } }; +struct ProgramNode : public Castable { + ProgramNode(Allocator* alloc, ProgramID id, ProgramID cloned_id) + : allocator(alloc), program_id(id), cloned_program_id(cloned_id) {} + + Allocator* const allocator; + ProgramID const program_id; + ProgramID const cloned_program_id; + + ProgramNode* Clone(CloneContext*) const override { + return allocator->Create(cloned_program_id, cloned_program_id); + } +}; + +ProgramID ProgramIDOf(const ProgramNode* node) { + return node->program_id; +} + struct UniqueTypes { using Node = UniqueNode; using Replaceable = UniqueReplaceable; @@ -642,6 +659,38 @@ TYPED_TEST(CloneContextTest, CloneNewSymbols_AfterCloneSymbols) { EXPECT_EQ(cloned.Symbols().NameFor(new_c), "c"); } +TYPED_TEST(CloneContextTest, ProgramIDs) { + ProgramBuilder dst; + Program src(ProgramBuilder{}); + CloneContext ctx(&dst, &src); + Allocator allocator; + ctx.Clone(allocator.Create(src.ID(), dst.ID())); +} + +TYPED_TEST(CloneContextTest, ProgramIDs_ObjectNotOwnedBySrc) { + EXPECT_FATAL_FAILURE( + { + ProgramBuilder dst; + Program src(ProgramBuilder{}); + CloneContext ctx(&dst, &src); + Allocator allocator; + ctx.Clone(allocator.Create(ProgramID::New(), dst.ID())); + }, + R"(internal compiler error: TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(src, a))"); +} + +TYPED_TEST(CloneContextTest, ProgramIDs_ObjectNotOwnedByDst) { + EXPECT_FATAL_FAILURE( + { + ProgramBuilder dst; + Program src(ProgramBuilder{}); + CloneContext ctx(&dst, &src); + Allocator allocator; + ctx.Clone(allocator.Create(src.ID(), ProgramID::New())); + }, + R"(internal compiler error: TINT_ASSERT_PROGRAM_IDS_EQUAL_IF_VALID(dst, out))"); +} + } // namespace TINT_INSTANTIATE_TYPEINFO(UniqueNode); @@ -651,5 +700,6 @@ TINT_INSTANTIATE_TYPEINFO(ShareableNode); TINT_INSTANTIATE_TYPEINFO(ShareableReplaceable); TINT_INSTANTIATE_TYPEINFO(ShareableReplacement); TINT_INSTANTIATE_TYPEINFO(NotANode); +TINT_INSTANTIATE_TYPEINFO(ProgramNode); } // namespace tint diff --git a/src/program.h b/src/program.h index f2f6eaff3c..815e751459 100644 --- a/src/program.h +++ b/src/program.h @@ -171,6 +171,12 @@ class Program { bool moved_ = false; }; +/// @param program the Program +/// @returns the ProgramID of the Program +inline ProgramID ProgramIDOf(const Program* program) { + return program->ID(); +} + } // namespace tint #endif // SRC_PROGRAM_H_ diff --git a/src/program_builder.h b/src/program_builder.h index 5125419af7..5691c01780 100644 --- a/src/program_builder.h +++ b/src/program_builder.h @@ -1505,6 +1505,12 @@ struct ProgramBuilder::TypesBuilder::CToAST { }; //! @endcond +/// @param builder the ProgramBuilder +/// @returns the ProgramID of the ProgramBuilder +inline ProgramID ProgramIDOf(const ProgramBuilder* builder) { + return builder->ID(); +} + } // namespace tint #endif // SRC_PROGRAM_BUILDER_H_ diff --git a/src/type/type.h b/src/type/type.h index 39bc71093f..95ed3710ff 100644 --- a/src/type/type.h +++ b/src/type/type.h @@ -102,6 +102,13 @@ class Type : public Castable { Type(); }; +/// @returns the ProgramID of the given type. +inline ProgramID ProgramIDOf(const Type*) { + /// TODO(crbug.com/tint/724): Actually implement this once we split the `type` + /// namespace into ast::Type and sem::Type. + return ProgramID(); +} + } // namespace type } // namespace tint