[Lldb-commits] [lldb] 14135f5 - [lldb/Value] Avoid reading more data than the host has available
Vedant Kumar via lldb-commits
lldb-commits at lists.llvm.org
Fri Jan 31 16:33:37 PST 2020
Author: Vedant Kumar
Date: 2020-01-31T16:33:12-08:00
New Revision: 14135f50a036af4d3a64b8e2e0dc2ecda5260533
URL: https://github.com/llvm/llvm-project/commit/14135f50a036af4d3a64b8e2e0dc2ecda5260533
DIFF: https://github.com/llvm/llvm-project/commit/14135f50a036af4d3a64b8e2e0dc2ecda5260533.diff
LOG: [lldb/Value] Avoid reading more data than the host has available
Value::GetValueByteSize() reports the size of a Value as the size of its
underlying CompilerType. However, a host buffer that backs a Value may
be smaller than GetValueByteSize().
This situation arises when the host is only able to partially evaluate a
Value, e.g. because the expression contains DW_OP_piece.
The cleanest fix I've found to this problem is Greg's suggestion, which
is to resize the Value if (after evaluating an expression) it's found to
be too small. I've tried several alternatives which all (in one way or
the other) tried to teach the Value/ValueObjectChild system not to read
past the end of a host buffer, but this was flaky and impractical as it
isn't easy to figure out the host buffer's size (Value::GetScalar() can
point to somewhere /inside/ a host buffer, but you need to walk up the
ValueObject hierarchy to try and find its size).
This fixes an ASan error in lldb seen when debugging a clang binary.
I've added a regression test in test/functionalities/optimized_code. The
point of that test is not specifically to check that DW_OP_piece is
handled a particular way, but rather to check that lldb doesn't crash on
an input that it used to crash on.
Testing: check-lldb, and running the added tests using a sanitized lldb
--
Thanks to Jim for pointing out that an earlier version of this patch,
which simply changed the definition of Value::GetValueByteSize(), would
interact poorly with the ValueObject machinery.
Thanks also to Pavel who suggested a neat way to test this change
(which, incidentally, caught another ASan issue still present in the
original version of this patch).
rdar://58665925
Differential Revision: https://reviews.llvm.org/D73148
Added:
lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/TestNoASanExceptionAfterEvalOP_piece.py
lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/main.cpp
lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s
Modified:
lldb/source/Core/ValueObjectVariable.cpp
Removed:
################################################################################
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/Makefile b/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/Makefile
new file mode 100644
index 000000000000..129330fd9faf
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -O2
+include Makefile.rules
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/TestNoASanExceptionAfterEvalOP_piece.py b/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/TestNoASanExceptionAfterEvalOP_piece.py
new file mode 100644
index 000000000000..c8308c16011e
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/TestNoASanExceptionAfterEvalOP_piece.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals())
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/main.cpp b/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/main.cpp
new file mode 100644
index 000000000000..f1beaf3020d5
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/main.cpp
@@ -0,0 +1,31 @@
+// This is a regression test that checks whether lldb can inspect the variables
+// in this program without triggering an ASan exception.
+
+__attribute__((noinline, optnone)) int use(int x) { return x; }
+
+volatile int sink;
+
+struct S1 {
+ int f1;
+ int *f2;
+};
+
+struct S2 {
+ char a, b;
+ int pad;
+ S2(int x) {
+ a = x & 0xff;
+ b = x & 0xff00;
+ }
+};
+
+int main() {
+ S1 v1;
+ v1.f1 = sink;
+ v1.f2 = nullptr;
+ sink++; //% self.expect("frame variable v1", substrs=["S1"])
+ S2 v2(v1.f1);
+ sink += use(v2.a); //% self.expect("frame variable v2", substrs=["S2"])
+ sink += use(v2.pad); //% self.expect("frame variable v2", substrs=["S2"])
+ return 0;
+}
diff --git a/lldb/source/Core/ValueObjectVariable.cpp b/lldb/source/Core/ValueObjectVariable.cpp
index 945e5c411ec1..d0664276dc17 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -166,6 +166,27 @@ bool ValueObjectVariable::UpdateValue() {
Value::ValueType value_type = m_value.GetValueType();
+ // The size of the buffer within m_value can be less than the size
+ // prescribed by its type. E.g. this can happen when an expression only
+ // partially describes an object (say, because it contains DW_OP_piece).
+ //
+ // In this case, grow m_value to the expected size. An alternative way to
+ // handle this is to teach Value::GetValueAsData() and ValueObjectChild
+ // not to read past the end of a host buffer, but this gets impractically
+ // complicated as a Value's host buffer may be shared with a distant
+ // ancestor or sibling in the ValueObject hierarchy.
+ //
+ // FIXME: When we grow m_value, we should represent the added bits as
+ // undefined somehow instead of as 0's.
+ if (value_type == Value::eValueTypeHostAddress &&
+ compiler_type.IsValid()) {
+ if (size_t value_buf_size = m_value.GetBuffer().GetByteSize()) {
+ size_t value_size = m_value.GetValueByteSize(&m_error, &exe_ctx);
+ if (m_error.Success() && value_buf_size < value_size)
+ m_value.ResizeData(value_size);
+ }
+ }
+
Process *process = exe_ctx.GetProcessPtr();
const bool process_is_alive = process && process->IsAlive();
diff --git a/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s
new file mode 100644
index 000000000000..96e4abe110e6
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s
@@ -0,0 +1,110 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "target variable reset" -b | FileCheck %s
+
+# CHECK: (lldb) target variable reset
+# CHECK: (auto_reset) reset = {
+# CHECK: ptr = 0xdeadbeefbaadf00d
+# Note: We need to find some way to represent "prev" as unknown/undefined.
+# CHECK: prev = false
+# CHECK: }
+
+ .section .debug_abbrev,"", at progbits
+ .byte 1 # Abbreviation Code
+ .byte 17 # DW_TAG_compile_unit
+ .byte 1 # DW_CHILDREN_yes
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 2 # Abbreviation Code
+ .byte 52 # DW_TAG_variable
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 2 # DW_AT_location
+ .byte 24 # DW_FORM_exprloc
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 3 # Abbreviation Code
+ .byte 36 # DW_TAG_base_type
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 62 # DW_AT_encoding
+ .byte 11 # DW_FORM_data1
+ .byte 11 # DW_AT_byte_size
+ .byte 11 # DW_FORM_data1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 4 # Abbreviation Code
+ .byte 19 # DW_TAG_structure_type
+ .byte 1 # DW_CHILDREN_yes
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 11 # DW_AT_byte_size
+ .byte 11 # DW_FORM_data1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 5 # Abbreviation Code
+ .byte 13 # DW_TAG_member
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 56 # DW_AT_data_member_location
+ .byte 11 # DW_FORM_data1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 6 # Abbreviation Code
+ .byte 15 # DW_TAG_pointer_type
+ .byte 0 # DW_CHILDREN_no
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 0 # EOM(3)
+
+ .section .debug_info,"", at progbits
+.Lcu_begin0:
+ .long .Lcu_end-.Lcu_start # Length of Unit
+.Lcu_start:
+ .short 4 # DWARF version number
+ .long .debug_abbrev # Offset Into Abbrev. Section
+ .byte 8 # Address Size (in bytes)
+ .byte 1 # Abbrev [1] 0xb:0x6c DW_TAG_compile_unit
+.Lbool:
+ .byte 3 # Abbrev [3] 0x33:0x7 DW_TAG_base_type
+ .asciz "bool" # DW_AT_name
+ .byte 2 # DW_AT_encoding
+ .byte 1 # DW_AT_byte_size
+ .byte 2 # Abbrev [2] 0x3a:0x15 DW_TAG_variable
+ .asciz "reset" # DW_AT_name
+ .long .Lstruct # DW_AT_type
+ .byte 2f-1f # DW_AT_location
+1:
+ .byte 0xe # DW_OP_constu
+ .quad 0xdeadbeefbaadf00d
+ .byte 0x9f # DW_OP_stack_value
+ .byte 0x93 # DW_OP_piece
+ .uleb128 8
+ # Note: Only the first 8 bytes of the struct are described.
+2:
+.Lstruct:
+ .byte 4 # Abbrev [4] 0x4f:0x22 DW_TAG_structure_type
+ .asciz "auto_reset" # DW_AT_name
+ .byte 16 # DW_AT_byte_size
+ .byte 5 # Abbrev [5] 0x58:0xc DW_TAG_member
+ .asciz "ptr" # DW_AT_name
+ .long .Lbool_ptr # DW_AT_type
+ .byte 0 # DW_AT_data_member_location
+ .byte 5 # Abbrev [5] 0x64:0xc DW_TAG_member
+ .asciz "prev" # DW_AT_name
+ .long .Lbool # DW_AT_type
+ .byte 8 # DW_AT_data_member_location
+ .byte 0 # End Of Children Mark
+.Lbool_ptr:
+ .byte 6 # Abbrev [6] 0x71:0x5 DW_TAG_pointer_type
+ .long .Lbool # DW_AT_type
+ .byte 0 # End Of Children Mark
+.Lcu_end:
More information about the lldb-commits
mailing list