aprantl added inline comments.
================ Comment at: lldb/source/Expression/DWARFExpression.cpp:936 /// Insertion point for evaluating multi-piece expression. - uint64_t op_piece_offset = 0; - Value pieces; // Used for DW_OP_piece + llvm::BitVector bit_pieces; + llvm::BitVector bit_mask; ---------------- Is this only going to be used for DW_OP_bit_piece? Otherwise I would have expected a name like `top(_of_stack)` or `result`. ================ Comment at: lldb/source/Expression/DWARFExpression.cpp:937 + llvm::BitVector bit_pieces; + llvm::BitVector bit_mask; ---------------- This should perhaps be called undef_mask? And should have a comment what it is used for. ================ Comment at: lldb/source/Expression/DWARFExpression.cpp:1267 case DW_OP_const2u: - stack.push_back(Scalar((uint16_t)opcodes.GetU16(&offset))); + stack.push_back(Scalar((unsigned int)opcodes.GetU16(&offset))); break; ---------------- This is not portable. The size of int could be anything depending on the host. Why not always use APIInt? ================ Comment at: lldb/source/Expression/DWARFExpression.cpp:2164 + unsigned int curr_scalar = curr_piece.GetScalar().UInt(); + curr_scalar = curr_scalar << curr_width; + bit_pieces.resize(curr_width + piece_byte_size * 8); ---------------- how do you know the `unsigned int` is large enough for this operation? ================ Comment at: lldb/source/Utility/Scalar.cpp:200 case e_uint512: - return (m_integer.getBitWidth() / 8); + return ceil(m_integer.getBitWidth() / 8.0); case e_float: ---------------- ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76449/new/ https://reviews.llvm.org/D76449 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits