Undefined Behavior (Alignment Error) in `SetVendor`

[mhansen]

Hi, I wrote some of this using AI, but I stand behind it.

There is an undefined behavior (alignment violation) in src/impl_x86__base_implementation.inl when writing the CPU vendor string in the SetVendor function.

In SetVendor, a char* pointer (which is not guaranteed to be 4-byte aligned) is cast to uint32_t* and dereferenced for writing.

According to the C99 Standard (Section 6.3.2.3, paragraph 7):

"A pointer to an object or incomplete type may be converted to a pointer to a different object or incomplete type. If the resulting pointer is not correctly aligned for the pointed-to type, the behavior is undefined."

Since char has an alignment requirement of 1 byte, and uint32_t typically requires 4-byte alignment, casting an arbitrary char* to uint32_t* and dereferencing it is undefined behavior if the address is not a multiple of 4.

cpu_features/src/impl_x86__base_implementation.inl

Lines 240 to 245 in 81d13c4

	static void SetVendor(const Leaf leaf, char* const vendor) {
	*(uint32_t*)(vendor) = leaf.ebx;
	*(uint32_t*)(vendor + 4) = leaf.edx;
	*(uint32_t*)(vendor + 8) = leaf.ecx;
	vendor[12] = '\0';
	}

This code is specific to x86/x86_64 architectures. While x86/x86_64 CPUs generally support unaligned memory access in hardware, it remains undefined behavior at the language level. Compilers can optimize code assuming no UB exists, which can lead to unexpected behavior or compiler-generated crashes.

Furthermore, when compiled with UndefinedBehaviorSanitizer (UBSan) enabled, this triggers a runtime error.

This was originally observed as a crash (SIGILL) in GetX86Info on Android x86 (32-bit) targets during NDK builds where I had enabled UBSan trap mode.

You can reproduce this behavior in google3 using Blaze. The repro files are located in experimental/users/markhansen/cpu_features_ub/.

experimental/users/markhansen/cpu_features_ub/repro_set_vendor.c:

#include <stdint.h>
#include <string.h>

#include "third_party/cpu_features/src/impl_x86__base_implementation.inl"

// Stubs for OS-specific functions declared in impl_x86__base_implementation.inl
// but defined in OS-specific .c files.
static void OverrideOsPreserves(OsPreserves* os_preserves) {
  (void)os_preserves;
}
static void DetectFeaturesFromOs(X86Info* info, X86Features* features) {
  (void)info;
  (void)features;
}

int main() {
  Leaf leaf = {0, 0x756E6547, 0x6C65746E, 0x49656E69};
  char storage[20] __attribute__((aligned(16))) = {0};
  strcpy(storage + 1, "GenuineIntel");
  SetVendor(leaf, storage + 1);
  return 0;
}

Run the repro with Blaze and UBSan diagnostics enabled:

blaze run --config=ubsan-diagnostics //experimental/users/markhansen/cpu_features_ub:repro_set_vendor

Output:

src/impl_x86__base_implementation.inl:241:3: runtime error: store to misaligned address 0x7fff68732ac1 for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
0x7fff68732ac1: note: pointer points here
 7f 00 00  00 47 65 6e 75 69 6e 65  49 6e 74 65 6c 00 00 00  00 00 00 00 66 55 00 00  38 bd 7c c2 00
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/impl_x86__base_implementation.inl:241:3

Replace the direct pointer casts with memcpy to safely handle potentially unaligned memory addresses. Modern compilers (like Clang and GCC) optimize memcpy to direct instructions on architectures that support unaligned access (like x86), so there is no performance penalty.

diff --git a/src/impl_x86__base_implementation.inl b/src/impl_x86__base_implementation.inl
index 0101573..010156f 100644
--- a/src/impl_x86__base_implementation.inl
+++ b/src/impl_x86__base_implementation.inl
@@ -240,9 +240,9 @@
 static void SetVendor(const Leaf leaf, char* const vendor) {
-  *(uint32_t*)(vendor) = leaf.ebx;
-  *(uint32_t*)(vendor + 4) = leaf.edx;
-  *(uint32_t*)(vendor + 8) = leaf.ecx;
+  memcpy(vendor, &leaf.ebx, 4);
+  memcpy(vendor + 4, &leaf.edx, 4);
+  memcpy(vendor + 8, &leaf.ecx, 4);
   vendor[12] = '\0';
 }

[gchatelet]

Fixed in #458 , thx a lot for the patch!