resolver: Forbid module-scope declarations from aliasing a builtin
Fixed: tint:1318
Change-Id: Ifcb1aced6885bebcf3eb883f39bfbd0871dae7b0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/70663
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
diff --git a/docs/origin-trial-changes.md b/docs/origin-trial-changes.md
index d34c2bd..1ad9f85 100644
--- a/docs/origin-trial-changes.md
+++ b/docs/origin-trial-changes.md
@@ -5,6 +5,7 @@
### Breaking Changes
* Taking the address of a vector component is no longer allowed.
+* Module-scope declarations can no longer alias a builtin name. [tint:1318](https://crbug.com/tint/1318)
### Deprecated Features
diff --git a/src/resolver/intrinsic_test.cc b/src/resolver/intrinsic_test.cc
index f54da2c..a2f848a 100644
--- a/src/resolver/intrinsic_test.cc
+++ b/src/resolver/intrinsic_test.cc
@@ -824,8 +824,8 @@
}
TEST_F(ResolverIntrinsicDataTest, Frexp_Error_FirstParamInt) {
- Global("exp", ty.i32(), ast::StorageClass::kWorkgroup);
- auto* call = Call("frexp", 1, AddressOf("exp"));
+ Global("v", ty.i32(), ast::StorageClass::kWorkgroup);
+ auto* call = Call("frexp", 1, AddressOf("v"));
WrapInFunction(call);
EXPECT_FALSE(r()->Resolve());
@@ -841,8 +841,8 @@
}
TEST_F(ResolverIntrinsicDataTest, Frexp_Error_SecondParamFloatPtr) {
- Global("exp", ty.f32(), ast::StorageClass::kWorkgroup);
- auto* call = Call("frexp", 1.0f, AddressOf("exp"));
+ Global("v", ty.f32(), ast::StorageClass::kWorkgroup);
+ auto* call = Call("frexp", 1.0f, AddressOf("v"));
WrapInFunction(call);
EXPECT_FALSE(r()->Resolve());
@@ -872,8 +872,8 @@
}
TEST_F(ResolverIntrinsicDataTest, Frexp_Error_VectorSizesDontMatch) {
- Global("exp", ty.vec4<i32>(), ast::StorageClass::kWorkgroup);
- auto* call = Call("frexp", vec2<f32>(1.0f, 2.0f), AddressOf("exp"));
+ Global("v", ty.vec4<i32>(), ast::StorageClass::kWorkgroup);
+ auto* call = Call("frexp", vec2<f32>(1.0f, 2.0f), AddressOf("v"));
WrapInFunction(call);
EXPECT_FALSE(r()->Resolve());
diff --git a/src/resolver/intrinsic_validation_test.cc b/src/resolver/intrinsic_validation_test.cc
index cb62db1..49cfb07 100644
--- a/src/resolver/intrinsic_validation_test.cc
+++ b/src/resolver/intrinsic_validation_test.cc
@@ -75,6 +75,52 @@
7:8 note: called by entry point 'main')");
}
+TEST_F(ResolverIntrinsicValidationTest, IntrinsicRedeclaredAsFunction) {
+ Func(Source{{12, 34}}, "mix", {}, ty.i32(), {});
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a function)");
+}
+
+TEST_F(ResolverIntrinsicValidationTest, IntrinsicRedeclaredAsGlobalLet) {
+ GlobalConst(Source{{12, 34}}, "mix", ty.i32(), Expr(1));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a module-scope let)");
+}
+
+TEST_F(ResolverIntrinsicValidationTest, IntrinsicRedeclaredAsGlobalVar) {
+ Global(Source{{12, 34}}, "mix", ty.i32(), Expr(1),
+ ast::StorageClass::kPrivate);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a module-scope var)");
+}
+
+TEST_F(ResolverIntrinsicValidationTest, IntrinsicRedeclaredAsAlias) {
+ Alias(Source{{12, 34}}, "mix", ty.i32());
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: 'mix' is a builtin and cannot be redeclared as an alias)");
+}
+
+TEST_F(ResolverIntrinsicValidationTest, IntrinsicRedeclaredAsStruct) {
+ Structure(Source{{12, 34}}, "mix", {Member("m", ty.i32())});
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: 'mix' is a builtin and cannot be redeclared as a struct)");
+}
+
namespace TextureSamplerOffset {
using TextureOverloadCase = ast::intrinsic::test::TextureOverloadCase;
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 3eb96a3..173220c 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1960,7 +1960,7 @@
sem::Type* Resolver::TypeDecl(const ast::TypeDecl* named_type) {
sem::Type* result = nullptr;
if (auto* alias = named_type->As<ast::Alias>()) {
- result = Type(alias->type);
+ result = Alias(alias);
} else if (auto* str = named_type->As<ast::Struct>()) {
result = Structure(str);
} else {
@@ -2142,6 +2142,182 @@
return out;
}
+sem::Type* Resolver::Alias(const ast::Alias* alias) {
+ auto* ty = Type(alias->type);
+ if (!ty) {
+ return nullptr;
+ }
+ if (!ValidateAlias(alias)) {
+ return nullptr;
+ }
+ return ty;
+}
+
+sem::Struct* Resolver::Structure(const ast::Struct* str) {
+ if (!ValidateNoDuplicateDecorations(str->decorations)) {
+ return nullptr;
+ }
+ for (auto* deco : str->decorations) {
+ Mark(deco);
+ }
+
+ sem::StructMemberList sem_members;
+ sem_members.reserve(str->members.size());
+
+ // Calculate the effective size and alignment of each field, and the overall
+ // size of the structure.
+ // For size, use the size attribute if provided, otherwise use the default
+ // size for the type.
+ // For alignment, use the alignment attribute if provided, otherwise use the
+ // default alignment for the member type.
+ // Diagnostic errors are raised if a basic rule is violated.
+ // Validation of storage-class rules requires analysing the actual variable
+ // usage of the structure, and so is performed as part of the variable
+ // validation.
+ uint64_t struct_size = 0;
+ uint64_t struct_align = 1;
+ std::unordered_map<Symbol, const ast::StructMember*> member_map;
+
+ for (auto* member : str->members) {
+ Mark(member);
+ auto result = member_map.emplace(member->symbol, member);
+ if (!result.second) {
+ AddError("redefinition of '" +
+ builder_->Symbols().NameFor(member->symbol) + "'",
+ member->source);
+ AddNote("previous definition is here", result.first->second->source);
+ return nullptr;
+ }
+
+ // Resolve member type
+ auto* type = Type(member->type);
+ if (!type) {
+ return nullptr;
+ }
+
+ // Validate member type
+ if (!IsPlain(type)) {
+ AddError(TypeNameOf(type) +
+ " cannot be used as the type of a structure member",
+ member->source);
+ return nullptr;
+ }
+
+ uint64_t offset = struct_size;
+ uint64_t align = type->Align();
+ uint64_t size = type->Size();
+
+ if (!ValidateNoDuplicateDecorations(member->decorations)) {
+ return nullptr;
+ }
+
+ bool has_offset_deco = false;
+ bool has_align_deco = false;
+ bool has_size_deco = false;
+ for (auto* deco : member->decorations) {
+ Mark(deco);
+ if (auto* o = deco->As<ast::StructMemberOffsetDecoration>()) {
+ // Offset decorations are not part of the WGSL spec, but are emitted
+ // by the SPIR-V reader.
+ if (o->offset < struct_size) {
+ AddError("offsets must be in ascending order", o->source);
+ return nullptr;
+ }
+ offset = o->offset;
+ align = 1;
+ has_offset_deco = true;
+ } else if (auto* a = deco->As<ast::StructMemberAlignDecoration>()) {
+ if (a->align <= 0 || !utils::IsPowerOfTwo(a->align)) {
+ AddError("align value must be a positive, power-of-two integer",
+ a->source);
+ return nullptr;
+ }
+ align = a->align;
+ has_align_deco = true;
+ } else if (auto* s = deco->As<ast::StructMemberSizeDecoration>()) {
+ if (s->size < size) {
+ AddError("size must be at least as big as the type's size (" +
+ std::to_string(size) + ")",
+ s->source);
+ return nullptr;
+ }
+ size = s->size;
+ has_size_deco = true;
+ }
+ }
+
+ if (has_offset_deco && (has_align_deco || has_size_deco)) {
+ AddError(
+ "offset decorations cannot be used with align or size decorations",
+ member->source);
+ return nullptr;
+ }
+
+ offset = utils::RoundUp(align, offset);
+ if (offset > std::numeric_limits<uint32_t>::max()) {
+ std::stringstream msg;
+ msg << "struct member has byte offset 0x" << std::hex << offset
+ << ", but must not exceed 0x" << std::hex
+ << std::numeric_limits<uint32_t>::max();
+ AddError(msg.str(), member->source);
+ return nullptr;
+ }
+
+ auto* sem_member = builder_->create<sem::StructMember>(
+ member, member->symbol, type, static_cast<uint32_t>(sem_members.size()),
+ static_cast<uint32_t>(offset), static_cast<uint32_t>(align),
+ static_cast<uint32_t>(size));
+ builder_->Sem().Add(member, sem_member);
+ sem_members.emplace_back(sem_member);
+
+ struct_size = offset + size;
+ struct_align = std::max(struct_align, align);
+ }
+
+ uint64_t size_no_padding = struct_size;
+ struct_size = utils::RoundUp(struct_align, struct_size);
+
+ if (struct_size > std::numeric_limits<uint32_t>::max()) {
+ std::stringstream msg;
+ msg << "struct size in bytes must not exceed 0x" << std::hex
+ << std::numeric_limits<uint32_t>::max() << ", but is 0x" << std::hex
+ << struct_size;
+ AddError(msg.str(), str->source);
+ return nullptr;
+ }
+ if (struct_align > std::numeric_limits<uint32_t>::max()) {
+ TINT_ICE(Resolver, diagnostics_)
+ << "calculated struct stride exceeds uint32";
+ return nullptr;
+ }
+
+ auto* out = builder_->create<sem::Struct>(
+ str, str->name, sem_members, static_cast<uint32_t>(struct_align),
+ static_cast<uint32_t>(struct_size),
+ static_cast<uint32_t>(size_no_padding));
+
+ for (size_t i = 0; i < sem_members.size(); i++) {
+ auto* mem_type = sem_members[i]->Type();
+ if (mem_type->Is<sem::Atomic>()) {
+ atomic_composite_info_.emplace(out,
+ sem_members[i]->Declaration()->source);
+ break;
+ } else {
+ auto found = atomic_composite_info_.find(mem_type);
+ if (found != atomic_composite_info_.end()) {
+ atomic_composite_info_.emplace(out, found->second);
+ break;
+ }
+ }
+ }
+
+ if (!ValidateStructure(out)) {
+ return nullptr;
+ }
+
+ return out;
+}
+
bool Resolver::Return(const ast::ReturnStatement* ret) {
if (auto* value = ret->value) {
if (!Expression(value)) {
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 37dea95..08a9c94 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -215,6 +215,7 @@
// AST and Type validation methods
// Each return true on success, false on failure.
+ bool ValidateAlias(const ast::Alias*);
bool ValidateArray(const sem::Array* arr, const Source& source);
bool ValidateArrayStrideDecoration(const ast::StrideDecoration* deco,
uint32_t el_size,
@@ -304,11 +305,17 @@
/// @param arr the Array to get semantic information for
sem::Array* Array(const ast::Array* arr);
+ /// Builds and returns the semantic information for the alias `alias`.
+ /// This method does not mark the ast::Alias node, nor attach the generated
+ /// semantic information to the AST node.
+ /// @returns the aliased type, or nullptr if an error is raised.
+ sem::Type* Alias(const ast::Alias* alias);
+
/// Builds and returns the semantic information for the structure `str`.
/// This method does not mark the ast::Struct node, nor attach the generated
/// semantic information to the AST node.
/// @returns the semantic Struct information, or nullptr if an error is
- /// raised. raised, nullptr is returned.
+ /// raised.
sem::Struct* Structure(const ast::Struct* str);
/// @returns the semantic info for the variable `var`. If an error is
diff --git a/src/resolver/resolver_validation.cc b/src/resolver/resolver_validation.cc
index a5b59f1..5698138 100644
--- a/src/resolver/resolver_validation.cc
+++ b/src/resolver/resolver_validation.cc
@@ -665,6 +665,19 @@
auto* decl = var->Declaration();
auto* storage_ty = var->Type()->UnwrapRef();
+ if (var->Is<sem::GlobalVariable>()) {
+ auto name = builder_->Symbols().NameFor(decl->symbol);
+ if (sem::ParseIntrinsicType(name) != sem::IntrinsicType::kNone) {
+ auto* kind = var->Declaration()->is_const ? "let" : "var";
+ AddError(
+ "'" + name +
+ "' is a builtin and cannot be redeclared as a module-scope " +
+ kind,
+ decl->source);
+ return false;
+ }
+ }
+
if (!decl->is_const && !IsStorable(storage_ty)) {
AddError(TypeNameOf(storage_ty) + " cannot be used as the type of a var",
decl->source);
@@ -942,6 +955,15 @@
bool Resolver::ValidateFunction(const sem::Function* func) {
auto* decl = func->Declaration();
+
+ auto name = builder_->Symbols().NameFor(decl->symbol);
+ if (sem::ParseIntrinsicType(name) != sem::IntrinsicType::kNone) {
+ AddError(
+ "'" + name + "' is a builtin and cannot be redeclared as a function",
+ decl->source);
+ return false;
+ }
+
auto workgroup_deco_count = 0;
for (auto* deco : decl->decorations) {
if (deco->Is<ast::WorkgroupDecoration>()) {
@@ -1886,7 +1908,25 @@
return true;
}
+bool Resolver::ValidateAlias(const ast::Alias* alias) {
+ auto name = builder_->Symbols().NameFor(alias->name);
+ if (sem::ParseIntrinsicType(name) != sem::IntrinsicType::kNone) {
+ AddError("'" + name + "' is a builtin and cannot be redeclared as an alias",
+ alias->source);
+ return false;
+ }
+
+ return true;
+}
+
bool Resolver::ValidateStructure(const sem::Struct* str) {
+ auto name = builder_->Symbols().NameFor(str->Declaration()->name);
+ if (sem::ParseIntrinsicType(name) != sem::IntrinsicType::kNone) {
+ AddError("'" + name + "' is a builtin and cannot be redeclared as a struct",
+ str->Declaration()->source);
+ return false;
+ }
+
if (str->Members().empty()) {
AddError("structures must have at least one member",
str->Declaration()->source);
@@ -2032,171 +2072,6 @@
return true;
}
-sem::Struct* Resolver::Structure(const ast::Struct* str) {
- if (!ValidateNoDuplicateDecorations(str->decorations)) {
- return nullptr;
- }
- for (auto* deco : str->decorations) {
- Mark(deco);
- }
-
- sem::StructMemberList sem_members;
- sem_members.reserve(str->members.size());
-
- // Calculate the effective size and alignment of each field, and the overall
- // size of the structure.
- // For size, use the size attribute if provided, otherwise use the default
- // size for the type.
- // For alignment, use the alignment attribute if provided, otherwise use the
- // default alignment for the member type.
- // Diagnostic errors are raised if a basic rule is violated.
- // Validation of storage-class rules requires analysing the actual variable
- // usage of the structure, and so is performed as part of the variable
- // validation.
- uint64_t struct_size = 0;
- uint64_t struct_align = 1;
- std::unordered_map<Symbol, const ast::StructMember*> member_map;
-
- for (auto* member : str->members) {
- Mark(member);
- auto result = member_map.emplace(member->symbol, member);
- if (!result.second) {
- AddError("redefinition of '" +
- builder_->Symbols().NameFor(member->symbol) + "'",
- member->source);
- AddNote("previous definition is here", result.first->second->source);
- return nullptr;
- }
-
- // Resolve member type
- auto* type = Type(member->type);
- if (!type) {
- return nullptr;
- }
-
- // Validate member type
- if (!IsPlain(type)) {
- AddError(TypeNameOf(type) +
- " cannot be used as the type of a structure member",
- member->source);
- return nullptr;
- }
-
- uint64_t offset = struct_size;
- uint64_t align = type->Align();
- uint64_t size = type->Size();
-
- if (!ValidateNoDuplicateDecorations(member->decorations)) {
- return nullptr;
- }
-
- bool has_offset_deco = false;
- bool has_align_deco = false;
- bool has_size_deco = false;
- for (auto* deco : member->decorations) {
- Mark(deco);
- if (auto* o = deco->As<ast::StructMemberOffsetDecoration>()) {
- // Offset decorations are not part of the WGSL spec, but are emitted
- // by the SPIR-V reader.
- if (o->offset < struct_size) {
- AddError("offsets must be in ascending order", o->source);
- return nullptr;
- }
- offset = o->offset;
- align = 1;
- has_offset_deco = true;
- } else if (auto* a = deco->As<ast::StructMemberAlignDecoration>()) {
- if (a->align <= 0 || !utils::IsPowerOfTwo(a->align)) {
- AddError("align value must be a positive, power-of-two integer",
- a->source);
- return nullptr;
- }
- align = a->align;
- has_align_deco = true;
- } else if (auto* s = deco->As<ast::StructMemberSizeDecoration>()) {
- if (s->size < size) {
- AddError("size must be at least as big as the type's size (" +
- std::to_string(size) + ")",
- s->source);
- return nullptr;
- }
- size = s->size;
- has_size_deco = true;
- }
- }
-
- if (has_offset_deco && (has_align_deco || has_size_deco)) {
- AddError(
- "offset decorations cannot be used with align or size decorations",
- member->source);
- return nullptr;
- }
-
- offset = utils::RoundUp(align, offset);
- if (offset > std::numeric_limits<uint32_t>::max()) {
- std::stringstream msg;
- msg << "struct member has byte offset 0x" << std::hex << offset
- << ", but must not exceed 0x" << std::hex
- << std::numeric_limits<uint32_t>::max();
- AddError(msg.str(), member->source);
- return nullptr;
- }
-
- auto* sem_member = builder_->create<sem::StructMember>(
- member, member->symbol, type, static_cast<uint32_t>(sem_members.size()),
- static_cast<uint32_t>(offset), static_cast<uint32_t>(align),
- static_cast<uint32_t>(size));
- builder_->Sem().Add(member, sem_member);
- sem_members.emplace_back(sem_member);
-
- struct_size = offset + size;
- struct_align = std::max(struct_align, align);
- }
-
- uint64_t size_no_padding = struct_size;
- struct_size = utils::RoundUp(struct_align, struct_size);
-
- if (struct_size > std::numeric_limits<uint32_t>::max()) {
- std::stringstream msg;
- msg << "struct size in bytes must not exceed 0x" << std::hex
- << std::numeric_limits<uint32_t>::max() << ", but is 0x" << std::hex
- << struct_size;
- AddError(msg.str(), str->source);
- return nullptr;
- }
- if (struct_align > std::numeric_limits<uint32_t>::max()) {
- TINT_ICE(Resolver, diagnostics_)
- << "calculated struct stride exceeds uint32";
- return nullptr;
- }
-
- auto* out = builder_->create<sem::Struct>(
- str, str->name, sem_members, static_cast<uint32_t>(struct_align),
- static_cast<uint32_t>(struct_size),
- static_cast<uint32_t>(size_no_padding));
-
- for (size_t i = 0; i < sem_members.size(); i++) {
- auto* mem_type = sem_members[i]->Type();
- if (mem_type->Is<sem::Atomic>()) {
- atomic_composite_info_.emplace(out,
- sem_members[i]->Declaration()->source);
- break;
- } else {
- auto found = atomic_composite_info_.find(mem_type);
- if (found != atomic_composite_info_.end()) {
- atomic_composite_info_.emplace(out, found->second);
- break;
- }
- }
- }
-
- if (!ValidateStructure(out)) {
- return nullptr;
- }
-
- return out;
-}
-
bool Resolver::ValidateReturn(const ast::ReturnStatement* ret) {
auto* func_type = current_function_->ReturnType();