[DSLX:type_system] Add check/error for tuple param binding against wr…

…ong kind. Found via fuzzing. PiperOrigin-RevId: 586830656
kammoh · Dec 1, 2023 · 1483bea · 1483bea
1 parent 8ab8632
commit 1483bea
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 2 deletions.
diff --git a/xls/dslx/type_system/BUILD b/xls/dslx/type_system/BUILD
@@ -113,13 +113,15 @@ cc_library(
         "//xls/common/status:status_macros",
         "//xls/dslx:constexpr_evaluator",
         "//xls/dslx:errors",
+        "//xls/dslx:interp_bindings",
         "//xls/dslx:interp_value",
         "//xls/dslx:warning_kind",
         "//xls/dslx/bytecode:bytecode_emitter",
         "//xls/dslx/bytecode:bytecode_interpreter",
         "//xls/dslx/frontend:ast",
         "//xls/dslx/frontend:ast_node",
         "//xls/dslx/frontend:ast_utils",
+        "//xls/dslx/frontend:module",
         "//xls/dslx/frontend:pos",
         "//xls/dslx/frontend:token_utils",
         "//xls/ir:bits",
@@ -489,8 +491,12 @@ cc_library(
     deps = [
         ":concrete_type",
         ":deduce_ctx",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/strings:str_format",
+        "//xls/common/logging",
         "//xls/common/status:ret_check",
         "//xls/common/status:status_macros",
+        "//xls/dslx/frontend:ast",
         "//xls/dslx/frontend:pos",
     ],
 )

diff --git a/xls/dslx/type_system/deduce.cc b/xls/dslx/type_system/deduce.cc
@@ -51,8 +51,10 @@
 #include "xls/dslx/frontend/ast.h"
 #include "xls/dslx/frontend/ast_node.h"
 #include "xls/dslx/frontend/ast_utils.h"
+#include "xls/dslx/frontend/module.h"
 #include "xls/dslx/frontend/pos.h"
 #include "xls/dslx/frontend/token_utils.h"
+#include "xls/dslx/interp_bindings.h"
 #include "xls/dslx/interp_value.h"
 #include "xls/dslx/type_system/concrete_type.h"
 #include "xls/dslx/type_system/concrete_type_zero_value.h"

diff --git a/xls/dslx/type_system/parametric_bind.cc b/xls/dslx/type_system/parametric_bind.cc
@@ -18,8 +18,13 @@
 #include <memory>
 #include <string>
 
+#include "absl/status/status.h"
+#include "absl/strings/str_format.h"
+#include "xls/common/logging/logging.h"
 #include "xls/common/status/ret_check.h"
 #include "xls/common/status/status_macros.h"
+#include "xls/dslx/frontend/ast.h"
+#include "xls/dslx/type_system/concrete_type.h"
 
 namespace xls::dslx {
 namespace {
@@ -144,17 +149,32 @@ absl::Status ParametricBindConcreteTypeDim(const ConcreteType& param_type,
 absl::Status ParametricBind(const ConcreteType& param_type,
                             const ConcreteType& arg_type,
                             ParametricBindContext& ctx) {
+  auto wrong_kind = [&]() {
+    std::string message = absl::StrFormat(
+        "expected argument kind '%s' to match parameter kind '%s'",
+        arg_type.GetDebugTypeName(), param_type.GetDebugTypeName());
+    return ctx.deduce_ctx.TypeMismatchError(ctx.span, nullptr, param_type,
+                                            nullptr, arg_type, message);
+  };
   if (auto* param_bits = dynamic_cast<const BitsType*>(&param_type)) {
     auto* arg_bits = dynamic_cast<const BitsType*>(&arg_type);
-    XLS_RET_CHECK(arg_bits != nullptr);
+    if (arg_bits == nullptr) {
+      return wrong_kind();
+    }
     return ParametricBindBits(*param_bits, *arg_bits, ctx);
   }
   if (auto* param_tuple = dynamic_cast<const TupleType*>(&param_type)) {
     auto* arg_tuple = dynamic_cast<const TupleType*>(&arg_type);
+    if (arg_tuple == nullptr) {
+      return wrong_kind();
+    }
     return ParametricBindTuple(*param_tuple, *arg_tuple, ctx);
   }
   if (auto* param_struct = dynamic_cast<const StructType*>(&param_type)) {
     auto* arg_struct = dynamic_cast<const StructType*>(&arg_type);
+    if (arg_struct == nullptr) {
+      return wrong_kind();
+    }
     const StructDef& param_nominal = param_struct->nominal_type();
     const StructDef& arg_nominal = arg_struct->nominal_type();
     if (&param_nominal != &arg_nominal) {
@@ -168,7 +188,9 @@ absl::Status ParametricBind(const ConcreteType& param_type,
   }
   if (auto* param_array = dynamic_cast<const ArrayType*>(&param_type)) {
     auto* arg_array = dynamic_cast<const ArrayType*>(&arg_type);
-    XLS_RET_CHECK(arg_array != nullptr);
+    if (arg_array == nullptr) {
+      return wrong_kind();
+    }
     return ParametricBindArray(*param_array, *arg_array, ctx);
   }
   if (dynamic_cast<const EnumType*>(&param_type) != nullptr) {

diff --git a/xls/dslx/type_system/typecheck_test.cc b/xls/dslx/type_system/typecheck_test.cc
@@ -415,6 +415,31 @@ fn main() -> sN[0] {
                                  "fit in the bitwidth of a sN[0]")));
 }
 
+TEST(TypecheckErrorTest, ParametricBindArrayToTuple) {
+  EXPECT_THAT(Typecheck(R"(
+fn p<N: u32>(x: (uN[N], uN[N])) -> uN[N] { x.0 }
+
+fn main() -> u32 {
+  p(u32[2]:[0, 1])
+})"),
+              StatusIs(absl::StatusCode::kInvalidArgument,
+                       HasSubstr("Parameter 0 and argument types are different "
+                                 "kinds (tuple vs array)")));
+}
+
+TEST(TypecheckErrorTest, ParametricBindNested) {
+  EXPECT_THAT(
+      Typecheck(R"(
+fn p<N: u32>(x: (u32, u64)[N]) -> u32 { x[0].0 }
+
+fn main() -> u32 {
+  p(u32[1][1]:[[u32:0]])
+})"),
+      StatusIs(absl::StatusCode::kInvalidArgument,
+               HasSubstr("(uN[32], uN[64]) vs uN[32][1]: expected argument "
+                         "kind 'array' to match parameter kind 'tuple'")));
+}
+
 TEST(TypecheckTest, ForBuiltinInBody) {
   XLS_EXPECT_OK(Typecheck(R"(
 fn f() -> u32 {