[Lldb-commits] [lldb] r307454 - The x86 instruction unwinder can be asked to disassemble non-instruction

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Fri Jul 7 17:12:15 PDT 2017


Author: jmolenda
Date: Fri Jul  7 17:12:15 2017
New Revision: 307454

URL: http://llvm.org/viewvc/llvm-project?rev=307454&view=rev
Log:
The x86 instruction unwinder can be asked to disassemble non-instruction 
blocks of memory, and if the final bytes of that block look like a long
x86 instruction, it can cause the llvm disassembler to read past the end
of the buffer.  Use the maximum allowed instruction length that we pass
to the llvm disassembler as a way to limit this to the size of the buffer.

An example of how to trigger this is when lldb does a function call, it
puts a breakpoint on the beginning of main() and uses that as the return
address from the function call.  When we stop at that location, lldb may
try to find the first frame up the stack.  Because this is on the first
instruction of a function, it will get the word-size value at the stack
pointer and assume that this was the caller's pc value.  But this is random
stack memory and could point to anything - an object in memory, something
in the data section, whatever.  And if we have a symbol for that thing,
we'll try to disassemble it.

This was leading to infrequent crashes in customer scenarios; figured out
what was happening with address sanitizer.

<rdar://problem/30463256> 


Modified:
    lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
    lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h
    lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp

Modified: lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp?rev=307454&r1=307453&r2=307454&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp (original)
+++ lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp Fri Jul  7 17:12:15 2017
@@ -604,9 +604,10 @@ uint32_t x86AssemblyInspectionEngine::ex
 }
 
 bool x86AssemblyInspectionEngine::instruction_length(uint8_t *insn_p,
-                                                     int &length) {
+                                                     int &length, 
+                                                     uint32_t buffer_remaining_bytes) {
 
-  const uint32_t max_op_byte_size = m_arch.GetMaximumOpcodeByteSize();
+  uint32_t max_op_byte_size = std::min(buffer_remaining_bytes, m_arch.GetMaximumOpcodeByteSize());
   llvm::SmallVector<uint8_t, 32> opcode_data;
   opcode_data.resize(max_op_byte_size);
 
@@ -698,8 +699,9 @@ bool x86AssemblyInspectionEngine::GetNon
     bool row_updated = false; // The UnwindPlan::Row 'row' has been updated
 
     m_cur_insn = data + current_func_text_offset;
-    if (!instruction_length(m_cur_insn, insn_len) || insn_len == 0 ||
-        insn_len > kMaxInstructionByteSize) {
+    if (!instruction_length(m_cur_insn, insn_len, size - current_func_text_offset)
+        || insn_len == 0 
+        || insn_len > kMaxInstructionByteSize) {
       // An unrecognized/junk instruction
       break;
     }
@@ -1002,8 +1004,9 @@ bool x86AssemblyInspectionEngine::Augmen
   while (offset < size) {
     m_cur_insn = data + offset;
     int insn_len;
-    if (!instruction_length(m_cur_insn, insn_len) || insn_len == 0 ||
-        insn_len > kMaxInstructionByteSize) {
+    if (!instruction_length(m_cur_insn, insn_len, size - offset)
+        || insn_len == 0 
+        || insn_len > kMaxInstructionByteSize) {
       // An unrecognized/junk instruction.
       break;
     }
@@ -1214,8 +1217,9 @@ bool x86AssemblyInspectionEngine::FindFi
     int scratch;
 
     m_cur_insn = data + offset;
-    if (!instruction_length(m_cur_insn, insn_len) ||
-        insn_len > kMaxInstructionByteSize || insn_len == 0) {
+    if (!instruction_length(m_cur_insn, insn_len, size - offset) 
+        || insn_len > kMaxInstructionByteSize 
+        || insn_len == 0) {
       // An error parsing the instruction, i.e. probably data/garbage - stop
       // scanning
       break;

Modified: lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h?rev=307454&r1=307453&r2=307454&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h (original)
+++ lldb/trunk/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.h Fri Jul  7 17:12:15 2017
@@ -113,7 +113,7 @@ private:
   bool ret_pattern_p();
   uint32_t extract_4(uint8_t *b);
 
-  bool instruction_length(uint8_t *insn, int &length);
+  bool instruction_length(uint8_t *insn, int &length, uint32_t buffer_remaining_bytes);
 
   bool machine_regno_to_lldb_regno(int machine_regno, uint32_t &lldb_regno);
 

Modified: lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp?rev=307454&r1=307453&r2=307454&view=diff
==============================================================================
--- lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp (original)
+++ lldb/trunk/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp Fri Jul  7 17:12:15 2017
@@ -2415,3 +2415,28 @@ TEST_F(Testx86AssemblyInspectionEngine,
   EXPECT_EQ(rsp_plus_8,
             plan.GetRowForFunctionOffset(sizeof(data) - 1)->GetCFAValue());
 }
+
+// Give the disassembler random bytes to test that it doesn't exceed
+// the bounds of the array when run under clang's address sanitizer.
+TEST_F(Testx86AssemblyInspectionEngine, TestDisassemblyJunkBytes) {
+  AddressRange sample_range;
+  UnwindPlan unwind_plan(eRegisterKindLLDB);
+  std::unique_ptr<x86AssemblyInspectionEngine> engine32 = Geti386Inspector();
+  std::unique_ptr<x86AssemblyInspectionEngine> engine64 = Getx86_64Inspector();
+
+  uint8_t data[] = {
+      0x10, 0x10, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+      0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 };
+
+  sample_range = AddressRange(0x1000, sizeof(data));
+
+  EXPECT_TRUE(engine32->GetNonCallSiteUnwindPlanFromAssembly(
+      data, sizeof(data), sample_range, unwind_plan));
+
+  unwind_plan.Clear();
+
+  EXPECT_TRUE(engine64->GetNonCallSiteUnwindPlanFromAssembly(
+      data, sizeof(data), sample_range, unwind_plan));
+
+}
+




More information about the lldb-commits mailing list