Fix Undefined Behaviour

All caused by calling Castable::As<> on nullptr objects.

Bug: tint:760
Change-Id: I0a408b3cd58086cfeab5a1af34d643f50f304948
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49523
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>

src/castable.h

@@ -173,13 +173,20 @@ inline bool IsAnyOf(FROM* obj) {
 /// @see CastFlags
 template <typename TO, int FLAGS = 0, typename FROM = detail::Infer>
 inline TO* As(FROM* obj) {
-  using castable =
+  auto* as_castable = static_cast<CastableBase*>(obj);
-      typename std::conditional<std::is_const<FROM>::value, const CastableBase,
-                                CastableBase>::type;
-  auto* as_castable = static_cast<castable*>(obj);
   return Is<TO, FLAGS>(obj) ? static_cast<TO*>(as_castable) : nullptr;
 }
 
+/// @returns obj dynamically cast to the type `TO` or `nullptr` if
+/// this object does not derive from `TO`.
+/// @param obj the object to cast from
+/// @see CastFlags
+template <typename TO, int FLAGS = 0, typename FROM = detail::Infer>
+inline const TO* As(const FROM* obj) {
+  auto* as_castable = static_cast<const CastableBase*>(obj);
+  return Is<TO, FLAGS>(obj) ? static_cast<const TO*>(as_castable) : nullptr;
+}
+
 /// CastableBase is the base class for all Castable objects.
 /// It is not encouraged to directly derive from CastableBase without using the
 /// Castable helper template.

src/program_builder.cc

@@ -132,10 +132,10 @@ ast::ConstructorExpression* ProgramBuilder::ConstructValueFilledWith(
 
 typ::Type ProgramBuilder::TypesBuilder::MaybeCreateTypename(
     typ::Type type) const {
-  if (auto* alias = type.ast->As<ast::Alias>()) {
+  if (auto* alias = As<ast::Alias>(type.ast)) {
     return {builder->create<ast::TypeName>(alias->symbol()), type.sem};
   }
-  if (auto* str = type.ast->As<ast::Struct>()) {
+  if (auto* str = As<ast::Struct>(type.ast)) {
     return {builder->create<ast::TypeName>(str->name()), type.sem};
   }
   return type;

src/reader/wgsl/parser_impl.cc

@@ -1396,7 +1396,7 @@ Maybe<ParserImpl::FunctionHeader> ParserImpl::function_header() {
       return_type = type.value;
     }
 
-    if (return_type.ast->Is<ast::Void>()) {
+    if (Is<ast::Void>(return_type.ast)) {
       // crbug.com/tint/677: void has been removed from the language
       deprecated(tok.source(),
                  "omit '-> void' for functions that do not return a value");

src/sem/info.h

@@ -15,6 +15,7 @@
 #ifndef SRC_SEM_INFO_H_
 #define SRC_SEM_INFO_H_
 
+#include <type_traits>
 #include <unordered_map>
 
 #include "src/debug.h"
@@ -26,6 +27,9 @@ namespace sem {
 
 /// Info holds all the resolved semantic information for a Program.
 class Info {
+  /// Placeholder type used by Get() to provide a default value for EXPLICIT_SEM
+  using InferFromAST = std::nullptr_t;
+
  public:
   /// Constructor
   Info();
@@ -44,14 +48,18 @@ class Info {
   /// Get looks up the semantic information for the AST or type node `node`.
   /// @param node the AST or type node
   /// @returns a pointer to the semantic node if found, otherwise nullptr
-  template <typename AST_OR_TYPE,
+  template <typename SEM = InferFromAST,
-            typename SEM = SemanticNodeTypeFor<AST_OR_TYPE>>
+            typename AST_OR_TYPE = CastableBase,
-  const SEM* Get(const AST_OR_TYPE* node) const {
+            typename RESULT =
+                std::conditional_t<std::is_same<SEM, InferFromAST>::value,
+                                   SemanticNodeTypeFor<AST_OR_TYPE>,
+                                   SEM>>
+  const RESULT* Get(const AST_OR_TYPE* node) const {
     auto it = map.find(node);
     if (it == map.end()) {
       return nullptr;
     }
-    return it->second->template As<SEM>();
+    return As<RESULT>(it->second);
   }
 
   /// Add registers the semantic node `sem_node` for the AST or type node

src/transform/decompose_storage_access.cc

@@ -627,8 +627,7 @@ Output DecomposeStorageAccess::Run(const Program* in, const DataMap&) {
   for (auto* node : ctx.src->ASTNodes().Objects()) {
     if (auto* ident = node->As<ast::IdentifierExpression>()) {
       // X
-      auto* expr = sem.Get(ident);
+      if (auto* var = sem.Get<sem::VariableUser>(ident)) {
-      if (auto* var = expr->As<sem::VariableUser>()) {
         if (var->Variable()->StorageClass() == ast::StorageClass::kStorage) {
           // Variable to a storage buffer
           state.AddAccesss(ident, {

src/transform/first_index_offset.cc

@@ -148,19 +148,22 @@ Output FirstIndexOffset::Run(const Program* in, const DataMap& data) {
 
     // Fix up all references to the builtins with the offsets
     ctx.ReplaceAll([=, &ctx](ast::Expression* expr) -> ast::Expression* {
-      auto* sem = ctx.src->Sem().Get(expr);
+      if (auto* sem = ctx.src->Sem().Get(expr)) {
-      if (auto* user = sem->As<sem::VariableUser>()) {
+        if (auto* user = sem->As<sem::VariableUser>()) {
-        auto it = builtin_vars.find(user->Variable());
+          auto it = builtin_vars.find(user->Variable());
-        if (it != builtin_vars.end()) {
+          if (it != builtin_vars.end()) {
-          return ctx.dst->Add(ctx.CloneWithoutTransform(expr),
+            return ctx.dst->Add(
-                              ctx.dst->MemberAccessor(buffer_name, it->second));
+                ctx.CloneWithoutTransform(expr),
+                ctx.dst->MemberAccessor(buffer_name, it->second));
+          }
         }
-      }
+        if (auto* access = sem->As<sem::StructMemberAccess>()) {
-      if (auto* access = sem->As<sem::StructMemberAccess>()) {
+          auto it = builtin_members.find(access->Member());
-        auto it = builtin_members.find(access->Member());
+          if (it != builtin_members.end()) {
-        if (it != builtin_members.end()) {
+            return ctx.dst->Add(
-          return ctx.dst->Add(ctx.CloneWithoutTransform(expr),
+                ctx.CloneWithoutTransform(expr),
-                              ctx.dst->MemberAccessor(buffer_name, it->second));
+                ctx.dst->MemberAccessor(buffer_name, it->second));
+          }
         }
       }
       // Not interested in this experssion. Just clone.