Author: Jordan Rupprecht Date: 2022-11-30T13:20:13-08:00 New Revision: 3c51ea3619e488db19cd26840ed46d58cfc7062f
URL: https://github.com/llvm/llvm-project/commit/3c51ea3619e488db19cd26840ed46d58cfc7062f DIFF: https://github.com/llvm/llvm-project/commit/3c51ea3619e488db19cd26840ed46d58cfc7062f.diff LOG: [DataFormatter] Fix variant npos with `_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` enabled. This data formatter should print "No Value" if a variant is unset. It does so by checking if `__index` has a value of `-1`, however it does so by interpreting it as a signed int. By default, `__index` has type `unsigned int`. When `_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` is enabled, the type of `__index` is either `unsigned char`, `unsigned short`, or `unsigned int`, depending on how many fields there are -- as small as possible. For example, when `std::variant` has only a few types, the index type is `unsigned char`, and the npos value will be interpreted by LLDB as `255` when it should be `-1`. This change does not special case the variant optimization; it just reads the type instead of assuming it's `unsigned int`. Reviewed By: labath Differential Revision: https://reviews.llvm.org/D138892 Added: Modified: lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp index 8f90c56890e43..464425e291a6d 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp @@ -9,6 +9,8 @@ #include "LibCxxVariant.h" #include "LibCxx.h" #include "lldb/DataFormatters/FormattersHelpers.h" +#include "lldb/Symbol/CompilerType.h" +#include "lldb/Utility/LLDBAssert.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/ScopeExit.h" @@ -66,6 +68,19 @@ namespace { // 3) NPos, its value is variant_npos which means the variant has no value enum class LibcxxVariantIndexValidity { Valid, Invalid, NPos }; +uint64_t VariantNposValue(uint64_t index_byte_size) { + switch (index_byte_size) { + case 1: + return static_cast<uint8_t>(-1); + case 2: + return static_cast<uint16_t>(-1); + case 4: + return static_cast<uint32_t>(-1); + } + lldbassert(false && "Unknown index type size"); + return static_cast<uint32_t>(-1); // Fallback to stable ABI type. +} + LibcxxVariantIndexValidity LibcxxVariantGetIndexValidity(ValueObjectSP &impl_sp) { ValueObjectSP index_sp( @@ -74,9 +89,23 @@ LibcxxVariantGetIndexValidity(ValueObjectSP &impl_sp) { if (!index_sp) return LibcxxVariantIndexValidity::Invalid; - int64_t index_value = index_sp->GetValueAsSigned(0); + // In the stable ABI, the type of __index is just int. + // In the unstable ABI, where _LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION is + // enabled, the type can either be unsigned char/short/int depending on + // how many variant types there are. + // We only need to do this here when comparing against npos, because npos is + // just `-1`, but that translates to diff erent unsigned values depending on + // the byte size. + CompilerType index_type = index_sp->GetCompilerType(); + + llvm::Optional<uint64_t> index_type_bytes = index_type.GetByteSize(nullptr); + if (!index_type_bytes) + return LibcxxVariantIndexValidity::Invalid; + + uint64_t npos_value = VariantNposValue(*index_type_bytes); + uint64_t index_value = index_sp->GetValueAsUnsigned(0); - if (index_value == -1) + if (index_value == npos_value) return LibcxxVariantIndexValidity::NPos; return LibcxxVariantIndexValidity::Valid; diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py index fbd7c8b21b9e4..a2bb31260a85b 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py @@ -76,3 +76,6 @@ def test_with_run_command(self): self.expect("frame variable v_no_value", substrs=['v_no_value = No Value']) + + self.expect("frame variable v_300_types_no_value", + substrs=['v_300_types_no_value = No Value']) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp index c0bc4ae12c1ae..560ec692f30ed 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp @@ -29,6 +29,34 @@ int main() std::variant<int, double, char> v3; std::variant<std::variant<int,double,char>> v_v1 ; std::variant<int, double, char> v_no_value; + // The next variant has 300 types, meaning the type index does not fit in + // a byte and must be `unsigned short` instead of `unsigned char` when + // using the unstable libc++ ABI. With stable libc++ ABI, the type index + // is always just `unsigned int`. + std::variant< + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int> + v_300_types_no_value; v1 = 12; // v contains int v_v1 = v1 ; @@ -54,6 +82,13 @@ int main() } catch( ... ) {} printf( "%zu\n", v_no_value.index() ) ; + + try { + v_300_types_no_value.emplace<0>(S()); + } catch (...) { + } + + printf("%zu\n", v_300_types_no_value.index()); #endif return 0; // break here _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
