[libcxx-commits] [libcxx] Poison memory in variant destroy (PR #101048)
Chris Cotter via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jul 29 10:51:39 PDT 2024
https://github.com/ccotter created https://github.com/llvm/llvm-project/pull/101048
In the version of `__destroy` that does not destruct any part of `__data`, MSAN is unable to see that `__data` is logically uninitialized. This allows false negatives such as the following to run without any MSAN diagnostic.
```
std::variant<double, int> v;
v.emplace<double>();
double& d = std::get<double>(v);
v.emplace<int>();
if (d) ...
```
With these changes, MSAN reports uninitialized memory:
```
==32202==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5557b64820aa in main /home/ccotter/variant_msan.cpp:19:9
#1 0x7fbfacd88554 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:266
#2 0x5557b63ed40d in _start (/home/ccotter/a.out+0x3140d)
```
I wasn't sure how to add a test for this just yet, but wanted to get the changes up first to review whether this change would make sense in libc++.
>From af5ed533be2fd1729221579e12a081a7f396d8d6 Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotter14 at bloomberg.net>
Date: Mon, 29 Jul 2024 13:25:39 -0400
Subject: [PATCH] Poison memory in variant destroy
In the version of `__destroy` that does not destruct any part of
`__data`, MSAN is unable to see that `__data` is logically
uninitialized. This allows false negatives such as the following to
run without any MSAN diagnostic.
```
std::variant<double, int> v;
v.emplace<double>();
double& d = std::get<double>(v);
v.emplace<int>();
if (d) ...
```
With these changes, MSAN reports uninitialized memory:
```
==32202==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5557b64820aa in main /home/ccotter/variant_msan.cpp:19:9
#1 0x7fbfacd88554 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/../csu/libc-start.c:266
#2 0x5557b63ed40d in _start (/home/ccotter/a.out+0x3140d)
```
---
libcxx/include/variant | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 631ffceab5f68..41c946fc96727 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -261,6 +261,10 @@ namespace std {
#include <new>
#include <version>
+#if __has_feature(memory_sanitizer)
+#include <sanitizer/msan_interface.h>
+#endif
+
// standard-mandated includes
// [variant.syn]
@@ -781,6 +785,9 @@ _LIBCPP_VARIANT_DESTRUCTOR(
_Trait::_TriviallyAvailable,
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__dtor() = default,
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __destroy() noexcept {
+#if __has_feature(memory_sanitizer)
+ __sanitizer_dtor_callback(&this->__data, sizeof(this->__data));
+#endif
this->__index = __variant_npos<__index_t>;
} _LIBCPP_EAT_SEMICOLON);
More information about the libcxx-commits
mailing list