Remove reinterpret_casts with undefined behavior from stable/library.h (#151595)

swolchok · pytorchmergebot · commit 2f74cffab2bd · 2025-04-22T20:24:47.000Z
There is a list of valid uses of `reinterpret_cast` (see https://en.cppreference.com/w/cpp/language/reinterpret_cast), and the use here was not on the list, hence undefined behavior. Implement what we meant using memcpy, which is well-defined. Differential Revision: [D73200791](https://our.internmc.facebook.com/intern/diff/D73200791/) Pull Request resolved: #151595 Approved by: https://github.com/janeyx99
diff --git a/docs/source/notes/libtorch_stable_abi.md b/docs/source/notes/libtorch_stable_abi.md
@@ -8,15 +8,15 @@ This note will eventually contain more details on how to use the APIs in torch/c
 
 |  type in custom extension    |   StableIValue representation   |   type in libtorch  |   Schema Type  |
 | -------- | ------- | ------- | ------- |
-| std::optional\<S> | \*reinterpret_cast\<(StableIValue\*)\*>, pointer to a StableIValue recursively defined | std::optional\<T> | Type? |
-| std::nullopt | \*reinterpret_cast\<nullptr_t\*> | IValue() | None |
-| RAIIATH | \*reinterpret_cast\<uint64_t\*> of AtenTensorHandle | at::Tensor |  Tensor |
-| int32_t | \*reinterpret_cast\<uint64_t\*> | at::ScalarType | ScalarType |
-| int32_t | \*reinterpret_cast\<uint64_t\*> | at::Layout | Layout |
-| int32_t | \*reinterpret_cast\<uint64_t\*> | at::MemoryFormat | MemoryFormat |
-| bool | \*reinterpret_cast\<uint64_t\*> | bool | bool |
-| int64_t | \*reinterpret_cast\<uint64_t\*> | int64_t | int |
-| double | \*reinterpret_cast\<uint64_t\*> | double | float |
+| std::optional\<S> | raw bitwise copy into leading bytes of uint64_t of pointer to a new StableIValue representing S | std::optional\<T> | Type? |
+| std::nullopt | nullptr | IValue() | None |
+| RAIIATH | raw bitwise copy of underlying AtenTensorHandle into leading bytes of uint64_t | at::Tensor |  Tensor |
+| int32_t | raw bitwise copy into leading bytes of uint64_t | at::ScalarType | ScalarType |
+| int32_t | raw bitwise copy into leading bytes of uint64_t | at::Layout | Layout |
+| int32_t | raw bitwise copy into leading bytes of uint64_t | at::MemoryFormat | MemoryFormat |
+| bool | raw bitwise copy into leading bytes of uint64_t | bool | bool |
+| int64_t | raw bitwise copy into leading bytes of uint64_t | int64_t | int |
+| double | raw bitwise copy into leading bytes of uint64_t | double | float |
 | ? | ? | c10::Device | Device |
 | ? | ? | c10::Stream | Stream |
 | ? | ? | c10::complex<double> | complex |
diff --git a/torch/csrc/stable/library.h b/torch/csrc/stable/library.h
@@ -25,7 +25,17 @@ StableIValue from(T val) {
   static_assert(
       sizeof(T) <= sizeof(StableIValue),
       "StableLibrary stack does not support parameter types larger than 64 bits.");
-  return *reinterpret_cast<StableIValue*>(&val);
+  static_assert(std::is_trivially_copyable_v<T>);
+  // Initialization should be cheap enough; let's give people well-specified
+  // reproducible behavior.
+  StableIValue result = 0;
+  // NOTE [-Wclass-memaccess ]: reinterpret_cast to suppress
+  // overzealous -Wclass-memaccess. (see
+  // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107361) We have a
+  // static_assert above that T is trivially copyable, which should be
+  // enough.
+  std::memcpy(&result, reinterpret_cast<void*>(&val), sizeof(val));
+  return result;
 }
 
 // Specialization for std::nullopt_t
@@ -76,7 +86,19 @@ template <
     typename T,
     std::enable_if_t<!detail::is_optional<T>::value, bool> = true>
 T to(StableIValue val) {
-  return *reinterpret_cast<T*>(&val);
+  static_assert(std::is_trivially_copyable_v<T>);
+  // T may not have a default constructor. (For example, it might be
+  // c10::Device.) However, std::memcpy implicitly creates a T at the
+  // destination. So, we can use a union to work around this lack of
+  // default constructor.
+  union Result {
+    Result() {}
+    T t;
+  };
+  Result result;
+  // See NOTE[ -Wclass-memaccess ] above.
+  std::memcpy(reinterpret_cast<void*>(&result.t), &val, sizeof(result));
+  return result.t;
 }
 
 template <
