From 1400a3cb8d53c8c10e23ecdd4f241ea9cff404b5 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 14 Jul 2022 12:49:35 +0200 Subject: [PATCH] [lldb] Always use APFloat for FP dumping The DumpDataExtractor function had two branches for printing floating point values. One branch (APFloat) was used if we had a Target object around and could query it for the appropriate semantics. If we didn't have a Target, we used host operations to read and format the value. This patch changes second path to use APFloat as well. To make it work, I pick reasonable defaults for different byte size. Notably, I did not include x87 long double in that list (as it is ambibuous and architecture-specific). This exposed a bug where we were printing register values using the target-less branch, even though the registers definitely belong to a target, and we had it available. Fixing this prompted the update of several tests for register values due to slightly different floating point outputs. The most dubious aspect of this patch is the change in TypeSystemClang::GetFloatTypeSemantics to recognize `10` as a valid size for x87 long double. This was necessary because because sizeof(long double) on x86_64 is 16 even though it only holds 10 bytes of useful data. This generalizes the hackaround present in the target-free branch of the dumping function. Differential Revision: https://reviews.llvm.org/D129750 --- lldb/include/lldb/Core/DumpRegisterValue.h | 4 +- lldb/source/Commands/CommandObjectRegister.cpp | 3 +- lldb/source/Core/DumpDataExtractor.cpp | 130 ++++++++------------- lldb/source/Core/DumpRegisterValue.cpp | 6 +- .../Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 4 +- .../postmortem/elf-core/TestLinuxCore.py | 34 +++--- lldb/unittests/Core/DumpDataExtractorTest.cpp | 31 +++-- 7 files changed, 92 insertions(+), 120 deletions(-) diff --git a/lldb/include/lldb/Core/DumpRegisterValue.h b/lldb/include/lldb/Core/DumpRegisterValue.h index 443fdb3..655003a 100644 --- a/lldb/include/lldb/Core/DumpRegisterValue.h +++ b/lldb/include/lldb/Core/DumpRegisterValue.h @@ -14,6 +14,7 @@ namespace lldb_private { +class ExecutionContextScope; class RegisterValue; struct RegisterInfo; class Stream; @@ -23,7 +24,8 @@ class Stream; bool DumpRegisterValue(const RegisterValue ®_val, Stream *s, const RegisterInfo *reg_info, bool prefix_with_name, bool prefix_with_alt_name, lldb::Format format, - uint32_t reg_name_right_align_at = 0); + uint32_t reg_name_right_align_at = 0, + ExecutionContextScope *exe_scope = nullptr); } // namespace lldb_private diff --git a/lldb/source/Commands/CommandObjectRegister.cpp b/lldb/source/Commands/CommandObjectRegister.cpp index 56cbacb..12f8971 100644 --- a/lldb/source/Commands/CommandObjectRegister.cpp +++ b/lldb/source/Commands/CommandObjectRegister.cpp @@ -94,7 +94,8 @@ public: bool prefix_with_altname = (bool)m_command_options.alternate_name; bool prefix_with_name = !prefix_with_altname; DumpRegisterValue(reg_value, &strm, reg_info, prefix_with_name, - prefix_with_altname, m_format_options.GetFormat(), 8); + prefix_with_altname, m_format_options.GetFormat(), 8, + exe_ctx.GetBestExecutionContextScope()); if ((reg_info->encoding == eEncodingUint) || (reg_info->encoding == eEncodingSint)) { Process *process = exe_ctx.GetProcessPtr(); diff --git a/lldb/source/Core/DumpDataExtractor.cpp b/lldb/source/Core/DumpDataExtractor.cpp index c48250b..8154444 100644 --- a/lldb/source/Core/DumpDataExtractor.cpp +++ b/lldb/source/Core/DumpDataExtractor.cpp @@ -50,25 +50,6 @@ using namespace lldb; #define NON_PRINTABLE_CHAR '.' -static float half2float(uint16_t half) { - union { - float f; - uint32_t u; - } u; - // Sign extend to 4 byte. - int32_t sign_extended = static_cast(half); - uint32_t v = static_cast(sign_extended); - - if (0 == (v & 0x7c00)) { - u.u = v & 0x80007FFFU; - return u.f * ldexpf(1, 125); - } - - v <<= 13; - u.u = v | 0x70000000U; - return u.f * ldexpf(1, -112); -} - static llvm::Optional GetAPInt(const DataExtractor &data, lldb::offset_t *offset_ptr, lldb::offset_t byte_size) { @@ -336,6 +317,27 @@ printMemoryTags(const DataExtractor &DE, Stream *s, lldb::addr_t addr, s->PutCString(")"); } +static const llvm::fltSemantics &GetFloatSemantics(const TargetSP &target_sp, + size_t byte_size) { + if (target_sp) { + if (auto type_system_or_err = + target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC)) + return type_system_or_err->GetFloatTypeSemantics(byte_size); + else + llvm::consumeError(type_system_or_err.takeError()); + } + // No target, just make a reasonable guess + switch(byte_size) { + case 2: + return llvm::APFloat::IEEEhalf(); + case 4: + return llvm::APFloat::IEEEsingle(); + case 8: + return llvm::APFloat::IEEEdouble(); + } + return llvm::APFloat::Bogus(); +} + lldb::offset_t lldb_private::DumpDataExtractor( const DataExtractor &DE, Stream *s, offset_t start_offset, lldb::Format item_format, size_t item_byte_size, size_t item_count, @@ -645,70 +647,38 @@ lldb::offset_t lldb_private::DumpDataExtractor( case eFormatFloat: { TargetSP target_sp; - bool used_upfloat = false; if (exe_scope) target_sp = exe_scope->CalculateTarget(); - if (target_sp) { - auto type_system_or_err = - target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC); - if (!type_system_or_err) { - llvm::consumeError(type_system_or_err.takeError()); - } else { - auto &type_system = *type_system_or_err; - llvm::SmallVector sv; - // Show full precision when printing float values - const unsigned format_precision = 0; - const unsigned format_max_padding = - target_sp->GetMaxZeroPaddingInFloatFormat(); - - const auto &semantics = - type_system.GetFloatTypeSemantics(item_byte_size); - - // Recalculate the byte size in case of a difference. This is possible - // when item_byte_size is 16 (128-bit), because you could get back the - // x87DoubleExtended semantics which has a byte size of 10 (80-bit). - const size_t semantics_byte_size = - (llvm::APFloat::getSizeInBits(semantics) + 7) / 8; - llvm::Optional apint = - GetAPInt(DE, &offset, semantics_byte_size); - if (apint) { - llvm::APFloat apfloat(semantics, apint.value()); - apfloat.toString(sv, format_precision, format_max_padding); - if (!sv.empty()) { - s->Printf("%*.*s", (int)sv.size(), (int)sv.size(), sv.data()); - used_upfloat = true; - } - } - } - } - if (!used_upfloat) { - std::ostringstream ss; - if (item_byte_size == sizeof(float) || item_byte_size == 2) { - float f; - if (item_byte_size == 2) { - uint16_t half = DE.GetU16(&offset); - f = half2float(half); - } else { - f = DE.GetFloat(&offset); - } - ss.precision(std::numeric_limits::digits10); - DumpFloatingPoint(ss, f); - } else if (item_byte_size == sizeof(double)) { - ss.precision(std::numeric_limits::digits10); - DumpFloatingPoint(ss, DE.GetDouble(&offset)); - } else if (item_byte_size == sizeof(long double) || - item_byte_size == 10) { - ss.precision(std::numeric_limits::digits10); - DumpFloatingPoint(ss, DE.GetLongDouble(&offset)); - } else { - s->Printf("error: unsupported byte size (%" PRIu64 - ") for float format", - (uint64_t)item_byte_size); - return offset; - } - ss.flush(); - s->Printf("%s", ss.str().c_str()); + llvm::Optional format_max_padding; + if (target_sp) + format_max_padding = target_sp->GetMaxZeroPaddingInFloatFormat(); + + // Show full precision when printing float values + const unsigned format_precision = 0; + + const llvm::fltSemantics &semantics = + GetFloatSemantics(target_sp, item_byte_size); + + // Recalculate the byte size in case of a difference. This is possible + // when item_byte_size is 16 (128-bit), because you could get back the + // x87DoubleExtended semantics which has a byte size of 10 (80-bit). + const size_t semantics_byte_size = + (llvm::APFloat::getSizeInBits(semantics) + 7) / 8; + llvm::Optional apint = + GetAPInt(DE, &offset, semantics_byte_size); + if (apint) { + llvm::APFloat apfloat(semantics, apint.value()); + llvm::SmallVector sv; + if (format_max_padding) + apfloat.toString(sv, format_precision, *format_max_padding); + else + apfloat.toString(sv, format_precision); + s->AsRawOstream() << sv; + } else { + s->Format("error: unsupported byte size ({0}) for float format", + item_byte_size); + return offset; } } break; diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp index 2018d61..0417ea9 100644 --- a/lldb/source/Core/DumpRegisterValue.cpp +++ b/lldb/source/Core/DumpRegisterValue.cpp @@ -19,7 +19,8 @@ bool lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s, const RegisterInfo *reg_info, bool prefix_with_name, bool prefix_with_alt_name, Format format, - uint32_t reg_name_right_align_at) { + uint32_t reg_name_right_align_at, + ExecutionContextScope *exe_scope) { DataExtractor data; if (reg_val.GetData(data)) { bool name_printed = false; @@ -71,7 +72,8 @@ bool lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s, UINT32_MAX, // num_per_line LLDB_INVALID_ADDRESS, // base_addr 0, // item_bit_size - 0); // item_bit_offset + 0, // item_bit_offset + exe_scope); return true; } return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index c6eb693..2b03afd 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -4670,7 +4670,9 @@ TypeSystemClang::GetFloatTypeSemantics(size_t byte_size) { return ast.getFloatTypeSemantics(ast.FloatTy); else if (bit_size == ast.getTypeSize(ast.DoubleTy)) return ast.getFloatTypeSemantics(ast.DoubleTy); - else if (bit_size == ast.getTypeSize(ast.LongDoubleTy)) + else if (bit_size == ast.getTypeSize(ast.LongDoubleTy) || + bit_size == llvm::APFloat::semanticsSizeInBits( + ast.getFloatTypeSemantics(ast.LongDoubleTy))) return ast.getFloatTypeSemantics(ast.LongDoubleTy); else if (bit_size == ast.getTypeSize(ast.HalfTy)) return ast.getFloatTypeSemantics(ast.HalfTy); diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py index 0ac0d92..3f00940 100644 --- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py +++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py @@ -315,20 +315,20 @@ class LinuxCoreTestCase(TestBase): values["s5"] = "5.5" values["s6"] = "6.5" values["s7"] = "7.5" - values["s8"] = "1.14437e-28" + values["s8"] = "1.14437421E-28" values["s30"] = "0" - values["s31"] = "6.40969e-10" + values["s31"] = "6.40969056E-10" values["d0"] = "0.5" values["d1"] = "1.5" values["d2"] = "2.5" values["d3"] = "3.5" - values["d4"] = "5.35161536149201e-315" - values["d5"] = "5.36197666906508e-315" - values["d6"] = "5.37233797663815e-315" - values["d7"] = "5.38269928421123e-315" - values["d8"] = "1.80107573659442e-226" + values["d4"] = "5.3516153614920076E-315" + values["d5"] = "5.3619766690650802E-315" + values["d6"] = "5.3723379766381528E-315" + values["d7"] = "5.3826992842112254E-315" + values["d8"] = "1.8010757365944223E-226" values["d30"] = "0" - values["d31"] = "1.39804328609529e-76" + values["d31"] = "1.3980432860952889E-76" values["fpsr"] = "0x00000000" values["fpcr"] = "0x00000000" @@ -373,20 +373,20 @@ class LinuxCoreTestCase(TestBase): values["s5"] = "5.5" values["s6"] = "6.5" values["s7"] = "7.5" - values["s8"] = "1.14437e-28" + values["s8"] = "1.14437421E-28" values["s30"] = "0" - values["s31"] = "6.40969e-10" + values["s31"] = "6.40969056E-10" values["d0"] = "0.5" values["d1"] = "1.5" values["d2"] = "2.5" values["d3"] = "3.5" - values["d4"] = "5.35161536149201e-315" - values["d5"] = "5.36197666906508e-315" - values["d6"] = "5.37233797663815e-315" - values["d7"] = "5.38269928421123e-315" - values["d8"] = "1.80107573659442e-226" + values["d4"] = "5.3516153614920076E-315" + values["d5"] = "5.3619766690650802E-315" + values["d6"] = "5.3723379766381528E-315" + values["d7"] = "5.3826992842112254E-315" + values["d8"] = "1.8010757365944223E-226" values["d30"] = "0" - values["d31"] = "1.39804328609529e-76" + values["d31"] = "1.3980432860952889E-76" values["fpsr"] = "0x00000000" values["fpcr"] = "0x00000000" values["vg"] = "0x0000000000000004" @@ -449,7 +449,7 @@ class LinuxCoreTestCase(TestBase): values["d0"] = "65536.0158538818" values["d1"] = "1572864.25476074" values["d2"] = "0" - values["d3"] = "25165828.0917969" + values["d3"] = "25165828.091796875" values["vg"] = "0x0000000000000004" values["z0"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40}" values["z1"] = "{0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41}" diff --git a/lldb/unittests/Core/DumpDataExtractorTest.cpp b/lldb/unittests/Core/DumpDataExtractorTest.cpp index f229dac..226f2b7 100644 --- a/lldb/unittests/Core/DumpDataExtractorTest.cpp +++ b/lldb/unittests/Core/DumpDataExtractorTest.cpp @@ -138,7 +138,7 @@ TEST(DumpDataExtractorTest, Formats) { TestDump(-1, lldb::Format::eFormatEnum, "-1"); TestDump(0xcafef00d, lldb::Format::eFormatHex, "0xcafef00d"); TestDump(0xcafef00d, lldb::Format::eFormatHexUppercase, "0xCAFEF00D"); - TestDump(0.456, lldb::Format::eFormatFloat, "0.456"); + TestDump(0.456, lldb::Format::eFormatFloat, "0.45600000000000002"); TestDump(9, lldb::Format::eFormatOctal, "011"); // Chars packed into an integer. TestDump(0x4C4C4442, lldb::Format::eFormatOSType, "'LLDB'"); @@ -180,44 +180,39 @@ TEST(DumpDataExtractorTest, Formats) { lldb::Format::eFormatVectorOfFloat16, "{0 -0}"); // Some subnormal numbers. TestDump(std::vector{0x0001, 0x8001}, - lldb::Format::eFormatVectorOfFloat16, "{5.96046e-08 -5.96046e-08}"); + lldb::Format::eFormatVectorOfFloat16, "{5.9605E-8 -5.9605E-8}"); // A full mantisse and empty expontent. TestDump(std::vector{0x83ff, 0x03ff}, - lldb::Format::eFormatVectorOfFloat16, "{-6.09756e-05 6.09756e-05}"); + lldb::Format::eFormatVectorOfFloat16, "{-6.0976E-5 6.0976E-5}"); // Some normal numbers. TestDump(std::vector{0b0100001001001000}, - lldb::Format::eFormatVectorOfFloat16, -#if defined(_MSC_VER) && _MSC_VER < 1920 - // FIXME: This should print the same on all platforms. - "{3.14063}"); -#else - "{3.14062}"); -#endif + lldb::Format::eFormatVectorOfFloat16, "{3.1406}"); // Largest and smallest normal number. TestDump(std::vector{0x0400, 0x7bff}, - lldb::Format::eFormatVectorOfFloat16, "{6.10352e-05 65504}"); + lldb::Format::eFormatVectorOfFloat16, "{6.1035E-5 65504}"); TestDump(std::vector{0xabcd, 0x1234}, - lldb::Format::eFormatVectorOfFloat16, "{-0.0609436 0.000757217}"); + lldb::Format::eFormatVectorOfFloat16, "{-0.060944 7.5722E-4}"); // quiet/signaling NaNs. TestDump(std::vector{0xffff, 0xffc0, 0x7fff, 0x7fc0}, - lldb::Format::eFormatVectorOfFloat16, "{-nan -nan nan nan}"); + lldb::Format::eFormatVectorOfFloat16, "{NaN NaN NaN NaN}"); // +/-Inf. TestDump(std::vector{0xfc00, 0x7c00}, - lldb::Format::eFormatVectorOfFloat16, "{-inf inf}"); + lldb::Format::eFormatVectorOfFloat16, "{-Inf +Inf}"); TestDump(std::vector{std::numeric_limits::min(), std::numeric_limits::max()}, - lldb::Format::eFormatVectorOfFloat32, "{1.17549e-38 3.40282e+38}"); + lldb::Format::eFormatVectorOfFloat32, + "{1.17549435E-38 3.40282347E+38}"); TestDump(std::vector{std::numeric_limits::quiet_NaN(), std::numeric_limits::signaling_NaN(), -std::numeric_limits::quiet_NaN(), -std::numeric_limits::signaling_NaN()}, - lldb::Format::eFormatVectorOfFloat32, "{nan nan -nan -nan}"); + lldb::Format::eFormatVectorOfFloat32, "{NaN NaN NaN NaN}"); TestDump(std::vector{std::numeric_limits::min(), std::numeric_limits::max()}, lldb::Format::eFormatVectorOfFloat64, - "{2.2250738585072e-308 1.79769313486232e+308}"); + "{2.2250738585072014E-308 1.7976931348623157E+308}"); TestDump( std::vector{ std::numeric_limits::quiet_NaN(), @@ -225,7 +220,7 @@ TEST(DumpDataExtractorTest, Formats) { -std::numeric_limits::quiet_NaN(), -std::numeric_limits::signaling_NaN(), }, - lldb::Format::eFormatVectorOfFloat64, "{nan nan -nan -nan}"); + lldb::Format::eFormatVectorOfFloat64, "{NaN NaN NaN NaN}"); // Not sure we can rely on having uint128_t everywhere so emulate with // uint64_t. -- 2.7.4