Make equalities with zero-initialized traitrefs raise critical error

include/simics/dmllib.h

@@ -40,22 +40,20 @@ static inline uint64 DML_shlu(uint64 a, uint64 b)
         return b > 63 ? 0 : a << b;
 }
 
+PRINTF_FORMAT(1, 2) UNUSED static void
+_signal_critical_error(const char *restrict format, ...) {
+    va_list va;
+    va_start(va, format);
+    char msg[512];
+    vsnprintf(msg, 512, format, va);
+    va_end(va);
+    VT_critical_error(msg, msg);
+}
+
 UNUSED static void
 _DML_fault(const char *filename, int lineno, const char *msg)
 {
-        // Need SIM_get_api_function indirection to work with old 6 base
-        // packages.
-        // TODO: use VT_critical_error directly in simics 7
-        void (*critical_error)(const char *, const char *)
-                = (void (*)(const char *, const char *))SIM_get_api_function(
-                        "VT_critical_error");
-        if (critical_error) {
-                char long_msg[256];
-                snprintf(long_msg, 256, "%s:%d: %s", filename, lineno, msg);
-                critical_error(msg, long_msg);
-        } else {
-                assert_error(lineno, filename, "", msg);
-        }
+    _signal_critical_error("%s:%d: %s", filename, lineno, msg);
 }
 
 static inline int64 DML_shl(int64 a, int64 b, const char *filename, int lineno)
@@ -774,11 +772,30 @@ _deserialize_object_trait_reference(ht_str_table_t *id_info_ht,
     return Sim_Set_Ok;
 }
 
-UNUSED __attribute__((const)) static inline bool
+UNUSED static inline bool
 _identity_eq(const _identity_t a, const _identity_t b) {
+    if (unlikely(a.id == 0 || b.id == 0)) {
+        _signal_critical_error(
+            "DML object identity equality check where one or both operands "
+            "are not explicitly initialized. This is considered undefined "
+            "behavior.");
+        return false;
+    }
     return a.id == b.id && a.encoded_index == b.encoded_index;
 }
 
+UNUSED static inline bool
+_identity_eq_at_site(const _identity_t a, const _identity_t b,
+                     const char *filename, int lineno) {
+    if (unlikely(a.id == 0 || b.id == 0)) {
+        _DML_fault(filename, lineno,
+                   "object identity equality check where one or both operands "
+                   "are not explicitly initialized. This is considered "
+                   "undefined behavior.");
+        return false;
+    }
+    return a.id == b.id && a.encoded_index == b.encoded_index;
+}
 
 typedef set_error_t (*_deserializer_t)(attr_value_t val, void *dest);
 typedef attr_value_t (*_serializer_t)(const void *addr);
@@ -3157,15 +3174,6 @@ UNUSED static void _free_table(ht_int_table_t *table) {
         ht_clear_int_table(table, true);
 }
 
-UNUSED static void _signal_critical_error(const char *restrict format, ...) {
-    va_list va;
-    va_start(va, format);
-    char msg[512];
-    vsnprintf(msg, 512, format, va);
-    va_end(va);
-    VT_critical_error(msg, msg);
-}
-
 UNUSED static void _memoized_recursion(const char *name) {
     _signal_critical_error("Recursive call to memoized method %s. "
                            "This is considered undefined behavior.", name);

py/dml/ctree-test.h

@@ -7,11 +7,12 @@
 #include <float.h>
 #include <simics/util/help-macros.h>
 
-// Hack: juggle with macros to capture DMLC-generated assert failures without
-// crashing
+// Hack: juggle with macros and function prototypes to capture DMLC-generated
+// assert failures and critical errors without crashing
 #define assert_error capture_assert_error
 static void capture_assert_error(int line, const char *file,
                                  const char *mod_date, const char *message);
+static void VT_critical_error(const char *short_msg, const char *long_msg);
 
 #include <simics/base-types.h>
 #include <simics/dmllib.h>
@@ -52,4 +53,15 @@ capture_assert_error(int line, const char *file,
         }
 }
 
+static void
+VT_critical_error(const char *short_msg, const char *long_msg)
+{
+        if (capturing_assert_errors) {
+            ++captured_assert_errors;
+        } else {
+            fprintf(stderr, "%s\ncritical error: %s\n", py_context, long_msg);
+            exit(1);
+        }
+}
+
 #define EXPECT(x) expect(x, #x, __FILE__, __LINE__)

py/dml/ctree.py

@@ -1443,19 +1443,11 @@ def make(cls, site, lh, rh):
 
         if (isinstance(lhtype, TTrait) and isinstance(rhtype, TTrait)
             and lhtype.trait is rhtype.trait):
-            return mkApply(site,
-                           mkLit(site, '_identity_eq',
-                                 TFunction([TNamed('_identity_t'),
-                                            TNamed('_identity_t')],
-                                           TBool())),
-                           [TraitObjIdentity(lh.site, lh),
-                            TraitObjIdentity(rh.site, rh)])
+            return IdentityEq(site, TraitObjIdentity(lh.site, lh),
+                              TraitObjIdentity(rh.site, rh))
         if (isinstance(lhtype, THook) and isinstance(rhtype, THook)
             and lhtype.cmp(rhtype) == 0):
-            return mkApply(site,
-                           mkLit(site, '_identity_eq',
-                                 TFunction([lhtype, rhtype], TBool())),
-                           [lh, rh])
+            return IdentityEq(site, lh, rh)
 
         raise EILLCOMP(site, lh, lhtype, rh, rhtype)
 
@@ -1465,6 +1457,31 @@ def mkEquals(site, lh, rh):
     else:
         return Equals.make(site, lh, rh)
 
+class IdentityEq(Expression):
+    priority = dml.expr.Apply.priority
+    type = TBool()
+
+    @auto_init
+    def __init__(self, site, lh, rh): pass
+
+    def __str__(self):
+        lh = str(self.lh)
+        rh = str(self.rh)
+        if self.lh.priority <= Equals.priority:
+            lh = f'({lh})'
+        if self.rh.priority <= Equals.priority:
+            rh = f'({rh})'
+        return f'{lh} == {rh}'
+
+    def read(self):
+        if self.site is None:
+            return f'_identity_eq({self.lh.read()}, {self.rh.read()})'
+        else:
+            return (f'_identity_eq_at_site({self.lh.read()}, {self.rh.read()}'
+                    + f', "{quote_filename(self.site.filename())}", '
+                    + f'{self.site.lineno})')
+
+
 class NotEquals_dml12(Compare_dml12):
     priority = 70
     op = '!='

test/1.4/expressions/T_trait_identity.dml

@@ -60,3 +60,11 @@ method init() {
     }
     assert found == 1;
 }
+
+attribute test_bad_eq is write_only_attr {
+    param type = "n";
+    method set(attr_value_t val) throws {
+        local t nullref;
+        assert nullref != nullref;
+    }
+}

test/1.4/expressions/T_trait_identity.py

@@ -0,0 +1,13 @@
+# © 2023 Intel Corporation
+# SPDX-License-Identifier: MPL-2.0
+
+import simics
+import stest
+conf.sim.fail_on_warnings = False
+
+try:
+    obj.test_bad_eq = None
+except simics.CriticalErrors as e:
+    stest.expect_true('object identity equality check' in str(e))
+else:
+    stest.fail('expected critical error')