Author: Yao Qi Date: 2026-08-22T08:13:07+01:00 New Revision: 8add96929a868a4a56ad4b715dcbf30b8836d471
URL: https://github.com/llvm/llvm-project/commit/8add96929a868a4a56ad4b715dcbf30b8836d471 DIFF: https://github.com/llvm/llvm-project/commit/8add96929a868a4a56ad4b715dcbf30b8836d471.diff LOG: [lldb] Clamp out-of-range shift amounts in Scalar (#217027) `Scalar::operator>>=` and `Scalar::ExtractBitfield` pass their shift amount straight to `APSInt::operator>>=(unsigned)`, which asserts when the amount exceeds the operand's bit width: ``` Assertion failed: (ShiftAmt <= BitWidth && "Invalid shift amount"), function ashrInPlace, file APInt.h, line 842. #7 llvm::APInt::ashrInPlace(unsigned int) #8 lldb_private::Scalar::ExtractBitfield(unsigned int, unsigned int) #9 lldb_private::DWARFExpression::Evaluate(...) #11 LLVMFuzzerTestOneInput ``` The DWARF expression evaluator reaches both from untrusted operands (`DW_OP_shra` shift counts and `DW_OP_bit_piece` bit offsets), so `lldb-dwarf-expression-fuzzer` aborts the process on a shift wider than the value. A shift amount at or beyond the width is well-defined as a sign or zero fill, so clamp it rather than reject it. `operator>>=` uses `getLimitedValue`, which also avoids the `getZExtValue` assertion when the amount itself needs more than 64 bits. `operator<<=` and `ShiftRightLogical` (`DW_OP_shl` and `DW_OP_shr`) already pass an `APInt` amount, and those `APInt` overloads clamp internally the same way, so they need no change. --------- Co-authored-by: Jonas Devlieghere <[email protected]> Added: Modified: lldb/source/Utility/Scalar.cpp lldb/unittests/Expression/DWARFExpressionTest.cpp lldb/unittests/Utility/ScalarTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp index f2c18cdd896da..d332c5f7fa06c 100644 --- a/lldb/source/Utility/Scalar.cpp +++ b/lldb/source/Utility/Scalar.cpp @@ -18,6 +18,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" +#include <algorithm> #include <cinttypes> #include <cstdio> @@ -471,10 +472,12 @@ bool Scalar::ShiftRightLogical(const Scalar &rhs) { } Scalar &Scalar::operator>>=(const Scalar &rhs) { - if (m_type == e_int && rhs.m_type == e_int) - m_integer >>= rhs.m_integer.getZExtValue(); - else + if (m_type == e_int && rhs.m_type == e_int) { + // Avoid APSInt assertion when exceeding the width is a sign or zero fill. + m_integer >>= rhs.m_integer.getLimitedValue(m_integer.getBitWidth()); + } else { m_type = e_void; + } return *this; } @@ -820,7 +823,8 @@ bool Scalar::ExtractBitfield(uint32_t bit_size, uint32_t bit_offset) { break; case Scalar::e_int: - m_integer >>= bit_offset; + // Avoid APSInt assertion when exceeding the width. + m_integer >>= std::min(bit_offset, m_integer.getBitWidth()); m_integer = m_integer.extOrTrunc(bit_size).extOrTrunc(8 * GetByteSize()); return true; } diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp index 96239ca16f40f..094cfaf790a9a 100644 --- a/lldb/unittests/Expression/DWARFExpressionTest.cpp +++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp @@ -827,6 +827,13 @@ TEST(DWARFExpression, DW_OP_piece) { ExpectHostAddress(expected_host_buffer)); } +TEST(DWARFExpression, DW_OP_bit_piece) { + // Extract the high 16 bits of a 32-bit value. + EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const4u, 0x44, 0x33, 0x22, 0x11, + DW_OP_bit_piece, 16, 16}), + ExpectScalar(0x1122)); +} + TEST(DWARFExpression, DW_OP_implicit_value) { unsigned char bytes = 4; @@ -1215,6 +1222,14 @@ TEST(DWARFExpression, DW_OP_shl_overflow_count) { SUCCEED(); } +TEST(DWARFExpression, DW_OP_shra_overflow_count) { + // Shift count exceeding scalar bit width must not crash. The arithmetic + // shift sign-fills, so -1 stays -1. + EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const1s, static_cast<uint8_t>(-1), + DW_OP_const1u, 99, DW_OP_shra}), + ExpectScalar(static_cast<int32_t>(-1))); +} + TEST(DWARFExpression, DW_OP_push_object_address_no_object) { // Without an object_address_ptr, must fail cleanly. EXPECT_THAT_EXPECTED(Evaluate({DW_OP_push_object_address}), llvm::Failed()); @@ -1271,6 +1286,13 @@ TEST(DWARFExpression, DW_OP_bit_piece_overflow) { ExpectScalar(5)); } +TEST(DWARFExpression, DW_OP_bit_piece_offset_overflow) { + // A bit offset beyond the scalar width must not crash; shifting the whole + // value out leaves zero. ULEB128(1000) = {0xE8, 0x07}. + EXPECT_THAT_EXPECTED(Evaluate({DW_OP_lit1, DW_OP_bit_piece, 32, 0xE8, 0x07}), + ExpectScalar(0)); +} + TEST(DWARFExpression, DW_OP_bit_piece_empty_stack) { EXPECT_THAT_EXPECTED(Evaluate({DW_OP_bit_piece, 0x08, 0x00}), llvm::Failed()); } diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp index 869a5809e6d14..671659859cdc8 100644 --- a/lldb/unittests/Utility/ScalarTest.cpp +++ b/lldb/unittests/Utility/ScalarTest.cpp @@ -131,6 +131,29 @@ TEST(ScalarTest, RightShiftOperator) { ASSERT_EQ(e >> c, e_scalar >> c_scalar); } +TEST(ScalarTest, RightShiftOutOfRange) { + // A shift of exactly the width is a sign fill for a signed value. This is + // in range for the APSInt shift, which asserts only past the width. + Scalar a(static_cast<int32_t>(-1)); + a >>= Scalar(static_cast<uint32_t>(32)); + EXPECT_EQ(a, Scalar(static_cast<int32_t>(-1))); + + // A shift past the width fills the same way. + Scalar b(static_cast<int32_t>(0x12345678)); + b >>= Scalar(static_cast<uint32_t>(1000)); + EXPECT_EQ(b, Scalar(static_cast<int32_t>(0))); + + // An unsigned value is zero filled instead. + Scalar c(static_cast<uint32_t>(0xFFFFFFFF)); + c >>= Scalar(static_cast<uint32_t>(33)); + EXPECT_EQ(c, Scalar(static_cast<uint32_t>(0))); + + // The shift amount itself can need more than 64 bits to represent. + Scalar d(static_cast<int32_t>(-1)); + d >>= Scalar(APInt::getOneBitSet(128, 70)); + EXPECT_EQ(d, Scalar(static_cast<int32_t>(-1))); +} + TEST(ScalarTest, GetBytes) { uint8_t Storage[256]; int a = 0x01020304; @@ -293,6 +316,26 @@ TEST(ScalarTest, ExtractBitfield) { EXPECT_EQ(u_scalar, b2); } +TEST(ScalarTest, ExtractBitfieldOutOfRange) { + uint32_t len = sizeof(int32_t) * 8; + + // A bit offset of exactly the width is a sign fill for a signed value. This + // is in range for the APSInt shift, which asserts only past the width. + Scalar s_scalar(static_cast<int32_t>(-1)); + ASSERT_TRUE(s_scalar.ExtractBitfield(len, len)); + EXPECT_EQ(s_scalar, Scalar(static_cast<int32_t>(-1))); + + // A bit offset past the width fills the same way. + Scalar s_far_scalar(static_cast<int32_t>(-1)); + ASSERT_TRUE(s_far_scalar.ExtractBitfield(len, 1000)); + EXPECT_EQ(s_far_scalar, Scalar(static_cast<int32_t>(-1))); + + // An unsigned value is zero filled instead. + Scalar u_scalar(static_cast<uint32_t>(0xFFFFFFFF)); + ASSERT_TRUE(u_scalar.ExtractBitfield(len, 1000)); + EXPECT_EQ(u_scalar, Scalar(static_cast<uint32_t>(0))); +} + template <typename T> static std::string ScalarGetValue(T value) { StreamString stream; Scalar(value).GetValue(stream, false); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
