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>

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
 

src/resolver/intrinsic_test.cc

@@ -824,8 +824,8 @@ TEST_F(ResolverIntrinsicDataTest, FrexpVector) {
 }
 
 TEST_F(ResolverIntrinsicDataTest, Frexp_Error_FirstParamInt) {
-  Global("exp", ty.i32(), ast::StorageClass::kWorkgroup);
+  Global("v", ty.i32(), ast::StorageClass::kWorkgroup);
-  auto* call = Call("frexp", 1, AddressOf("exp"));
+  auto* call = Call("frexp", 1, AddressOf("v"));
   WrapInFunction(call);
 
   EXPECT_FALSE(r()->Resolve());
@@ -841,8 +841,8 @@ TEST_F(ResolverIntrinsicDataTest, Frexp_Error_FirstParamInt) {
 }
 
 TEST_F(ResolverIntrinsicDataTest, Frexp_Error_SecondParamFloatPtr) {
-  Global("exp", ty.f32(), ast::StorageClass::kWorkgroup);
+  Global("v", ty.f32(), ast::StorageClass::kWorkgroup);
-  auto* call = Call("frexp", 1.0f, AddressOf("exp"));
+  auto* call = Call("frexp", 1.0f, AddressOf("v"));
   WrapInFunction(call);
 
   EXPECT_FALSE(r()->Resolve());
@@ -872,8 +872,8 @@ TEST_F(ResolverIntrinsicDataTest, Frexp_Error_SecondParamNotAPointer) {
 }
 
 TEST_F(ResolverIntrinsicDataTest, Frexp_Error_VectorSizesDontMatch) {
-  Global("exp", ty.vec4<i32>(), ast::StorageClass::kWorkgroup);
+  Global("v", ty.vec4<i32>(), ast::StorageClass::kWorkgroup);
-  auto* call = Call("frexp", vec2<f32>(1.0f, 2.0f), AddressOf("exp"));
+  auto* call = Call("frexp", vec2<f32>(1.0f, 2.0f), AddressOf("v"));
   WrapInFunction(call);
 
   EXPECT_FALSE(r()->Resolve());

src/resolver/intrinsic_validation_test.cc

@@ -75,6 +75,52 @@ TEST_F(ResolverIntrinsicValidationTest, InvalidPipelineStageIndirect) {
 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;

src/resolver/resolver.cc

@@ -1960,7 +1960,7 @@ bool Resolver::VariableDeclStatement(const ast::VariableDeclStatement* stmt) {
 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 @@ sem::Array* Resolver::Array(const ast::Array* arr) {
   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)) {

src/resolver/resolver.h

@@ -215,6 +215,7 @@ class Resolver {
 
   // 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 @@ class Resolver {
   /// @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

src/resolver/resolver_validation.cc

@@ -665,6 +665,19 @@ bool Resolver::ValidateVariable(const sem::Variable* var) {
   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::ValidateInterpolateDecoration(
 
 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 @@ bool Resolver::ValidateArrayStrideDecoration(const ast::StrideDecoration* deco,
   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 @@ bool Resolver::ValidateLocationDecoration(
   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();