Merge pull request #15271 from NixOS/faster-type-internal-type

libexpr: Optimise `Value::type()`, `ValueStorage::getInternalType()`

nix-meson-build-support/common/asan-options/meson.build

@@ -9,3 +9,9 @@ endif
 if 'address' in get_option('b_sanitize')
   deps_other += declare_dependency(sources : 'asan-options.cc')
 endif
+
+if 'undefined' in get_option('b_sanitize')
+  add_project_arguments('-DNIX_UBSAN_ENABLED=1', language : 'cpp')
+else
+  add_project_arguments('-DNIX_UBSAN_ENABLED=0', language : 'cpp')
+endif

src/libexpr-tests/value/value.cc

@@ -13,8 +13,7 @@ TEST_F(ValueTest, unsetValue)
 {
     Value unsetValue;
     ASSERT_EQ(false, unsetValue.isValid());
-    ASSERT_EQ(nThunk, unsetValue.type(true));
-    ASSERT_DEATH(unsetValue.type(), "");
+    ASSERT_EQ(nThunk, unsetValue.type</*invalidIsThunk=*/true>());
 }
 
 TEST_F(ValueTest, vInt)

src/libexpr/eval.cc

@@ -466,7 +466,7 @@ void EvalState::addConstant(const std::string & name, Value * v, Constant info)
 
            We might know the type of a thunk in advance, so be allowed
            to just write it down in that case. */
-        if (auto gotType = v->type(true); gotType != nThunk)
+        if (auto gotType = v->type</*invalidIsThunk=*/true>(); gotType != nThunk)
             assert(info.type == gotType);
 
         /* Install value the base environment. */

src/libexpr/include/nix/expr/value.hh

@@ -35,7 +35,7 @@ class BindingsBuilder;
  * about how this is mapped into the alignment bits to save significant memory.
  * This also restricts the number of internal types represented with distinct memory layouts.
  */
-typedef enum {
+enum InternalType {
     tUninitialized = 0,
     /* layout: Single/zero field payload */
     tInt = 1,
@@ -46,16 +46,20 @@ typedef enum {
     tPrimOp,
     tAttrs,
     /* layout: Pair of pointers payload */
-    tListSmall,
+    tFirstPairOfPointers,
+    tListSmall = tFirstPairOfPointers,
     tPrimOpApp,
     tApp,
     tThunk,
     tLambda,
+    tLastPairOfPointers = tLambda,
     /* layout: Single untaggable field */
-    tListN,
+    tFirstSingleUntaggable,
+    tListN = tFirstSingleUntaggable,
     tString,
     tPath,
-} InternalType;
+    tNumberOfInternalTypes, // Must be last
+};
 
 /**
  * This type abstracts over all actual value types in the language,
@@ -633,7 +637,7 @@ class alignas(16)
     template<InternalType type, typename T, typename U>
     void setPairOfPointersPayload(T * firstPtrField, U * secondPtrField) noexcept
     {
-        static_assert(type >= tListSmall && type <= tLambda);
+        static_assert(type >= tFirstPairOfPointers && type <= tLastPairOfPointers);
         {
             auto firstFieldPayload = std::bit_cast<PackedPointer>(firstPtrField);
             assertAligned(firstFieldPayload);
@@ -642,7 +646,7 @@ class alignas(16)
         {
             auto secondFieldPayload = std::bit_cast<PackedPointer>(secondPtrField);
             assertAligned(secondFieldPayload);
-            payload[1] = (type - tListSmall) | secondFieldPayload;
+            payload[1] = (type - tFirstPairOfPointers) | secondFieldPayload;
         }
     }
 
@@ -670,11 +674,11 @@ protected:
         case pdListN:
         case pdString:
         case pdPath:
-            return static_cast<InternalType>(tListN + (pd - pdListN));
+            return static_cast<InternalType>(tFirstSingleUntaggable + (pd - pdListN));
         case pdPairOfPointers:
-            return static_cast<InternalType>(tListSmall + (payload[1] & discriminatorMask));
+            return static_cast<InternalType>(tFirstPairOfPointers + (payload[1] & discriminatorMask));
         [[unlikely]] default:
-            unreachable();
+            nixUnreachableWhenHardened();
         }
     }
 
@@ -1027,7 +1031,7 @@ private:
     T getStorage() const noexcept
     {
         if (getInternalType() != detail::payloadTypeToInternalType<T>) [[unlikely]]
-            unreachable();
+            nixUnreachableWhenHardened();
         T out;
         ValueStorage::getStorage(out);
         return out;
@@ -1079,45 +1083,44 @@ public:
      * Returns the normal type of a Value. This only returns nThunk if
      * the Value hasn't been forceValue'd
      *
-     * @param invalidIsThunk Instead of aborting an an invalid (probably
+     * @param invalidIsThunk Instead of UB an an invalid (probably
      * 0, so uninitialized) internal type, return `nThunk`.
      */
-    inline ValueType type(bool invalidIsThunk = false) const
+    template<bool invalidIsThunk = false>
+    inline ValueType type() const
     {
-        switch (getInternalType()) {
-        case tUninitialized:
-            break;
-        case tInt:
-            return nInt;
-        case tBool:
-            return nBool;
-        case tString:
-            return nString;
-        case tPath:
-            return nPath;
-        case tNull:
-            return nNull;
-        case tAttrs:
-            return nAttrs;
-        case tListSmall:
-        case tListN:
-            return nList;
-        case tLambda:
-        case tPrimOp:
-        case tPrimOpApp:
-            return nFunction;
-        case tExternal:
-            return nExternal;
-        case tFloat:
-            return nFloat;
-        case tThunk:
-        case tApp:
-            return nThunk;
+        /* Explicit lookup table. switch() might compile down (and it does at least with GCC 14)
+           to a jump table. Let's help the compiler a bit here. */
+        static constexpr auto table = [] {
+            std::array<ValueType, tNumberOfInternalTypes> t{};
+            t[tUninitialized] = nThunk;
+            t[tInt] = nInt;
+            t[tBool] = nBool;
+            t[tNull] = nNull;
+            t[tFloat] = nFloat;
+            t[tExternal] = nExternal;
+            t[tAttrs] = nAttrs;
+            t[tPrimOp] = nFunction;
+            t[tLambda] = nFunction;
+            t[tPrimOpApp] = nFunction;
+            t[tApp] = nThunk;
+            t[tThunk] = nThunk;
+            t[tListSmall] = nList;
+            t[tListN] = nList;
+            t[tString] = nString;
+            t[tPath] = nPath;
+            return t;
+        }();
+
+        auto it = getInternalType();
+        if (it == tUninitialized || it >= tNumberOfInternalTypes) [[unlikely]] {
+            if constexpr (invalidIsThunk)
+                return nThunk;
+            else
+                nixUnreachableWhenHardened();
         }
-        if (invalidIsThunk)
-            return nThunk;
-        else
-            unreachable();
+
+        return table[it];
     }
 
     /**

src/libutil/include/nix/util/error.hh

@@ -350,6 +350,15 @@ int handleExceptions(const std::string & programName, std::function<void()> fun)
  */
 [[gnu::noinline, gnu::cold, noreturn]] void unreachable(std::source_location loc = std::source_location::current());
 
+#if NIX_UBSAN_ENABLED == 1
+/* When building with sanitizers, also enable expensive unreachable checks. In
+   optimised builds this explicitly invokes UB with std::unreachable for better
+   optimisations. */
+#  define nixUnreachableWhenHardened ::nix::unreachable
+#else
+#  define nixUnreachableWhenHardened std::unreachable
+#endif
+
 #ifdef _WIN32
 
 namespace windows {