[llvm] 1e820e8 - [DebugInfo/DWARF] - Do not hang when CFI are truncated.

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 23 04:52:31 PDT 2020


Author: Georgii Rymar
Date: 2020-06-23T14:39:24+03:00
New Revision: 1e820e82b1438a52124512175a0e7c6f8d23e158

URL: https://github.com/llvm/llvm-project/commit/1e820e82b1438a52124512175a0e7c6f8d23e158
DIFF: https://github.com/llvm/llvm-project/commit/1e820e82b1438a52124512175a0e7c6f8d23e158.diff

LOG: [DebugInfo/DWARF] - Do not hang when CFI are truncated.

Currently when the .eh_frame section is truncated so that
CFI instructions can't be read, it is possible to enter
an infinite loop.

It happens because `CFIProgram::parse` does not handle errors properly.
This patch fixes the issue.

Differential revision: https://reviews.llvm.org/D82017

Added: 
    

Modified: 
    llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
    llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index 746ba379c0ce..6ffbf1212d47 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -36,8 +36,9 @@ const uint8_t DWARF_CFI_PRIMARY_OPERAND_MASK = 0x3f;
 
 Error CFIProgram::parse(DWARFDataExtractor Data, uint64_t *Offset,
                         uint64_t EndOffset) {
-  while (*Offset < EndOffset) {
-    uint8_t Opcode = Data.getRelocatedValue(1, Offset);
+  DataExtractor::Cursor C(*Offset);
+  while (C && C.tell() < EndOffset) {
+    uint8_t Opcode = Data.getRelocatedValue(C, 1);
     // Some instructions have a primary opcode encoded in the top bits.
     uint8_t Primary = Opcode & DWARF_CFI_PRIMARY_OPCODE_MASK;
 
@@ -55,7 +56,7 @@ Error CFIProgram::parse(DWARFDataExtractor Data, uint64_t *Offset,
         addInstruction(Primary, Op1);
         break;
       case DW_CFA_offset:
-        addInstruction(Primary, Op1, Data.getULEB128(Offset));
+        addInstruction(Primary, Op1, Data.getULEB128(C));
         break;
       }
     } else {
@@ -74,19 +75,19 @@ Error CFIProgram::parse(DWARFDataExtractor Data, uint64_t *Offset,
         break;
       case DW_CFA_set_loc:
         // Operands: Address
-        addInstruction(Opcode, Data.getRelocatedAddress(Offset));
+        addInstruction(Opcode, Data.getRelocatedAddress(C));
         break;
       case DW_CFA_advance_loc1:
         // Operands: 1-byte delta
-        addInstruction(Opcode, Data.getRelocatedValue(1, Offset));
+        addInstruction(Opcode, Data.getRelocatedValue(C, 1));
         break;
       case DW_CFA_advance_loc2:
         // Operands: 2-byte delta
-        addInstruction(Opcode, Data.getRelocatedValue(2, Offset));
+        addInstruction(Opcode, Data.getRelocatedValue(C, 2));
         break;
       case DW_CFA_advance_loc4:
         // Operands: 4-byte delta
-        addInstruction(Opcode, Data.getRelocatedValue(4, Offset));
+        addInstruction(Opcode, Data.getRelocatedValue(C, 4));
         break;
       case DW_CFA_restore_extended:
       case DW_CFA_undefined:
@@ -95,11 +96,11 @@ Error CFIProgram::parse(DWARFDataExtractor Data, uint64_t *Offset,
       case DW_CFA_def_cfa_offset:
       case DW_CFA_GNU_args_size:
         // Operands: ULEB128
-        addInstruction(Opcode, Data.getULEB128(Offset));
+        addInstruction(Opcode, Data.getULEB128(C));
         break;
       case DW_CFA_def_cfa_offset_sf:
         // Operands: SLEB128
-        addInstruction(Opcode, Data.getSLEB128(Offset));
+        addInstruction(Opcode, Data.getSLEB128(C));
         break;
       case DW_CFA_offset_extended:
       case DW_CFA_register:
@@ -109,56 +110,57 @@ Error CFIProgram::parse(DWARFDataExtractor Data, uint64_t *Offset,
         // Note: We can not embed getULEB128 directly into function
         // argument list. getULEB128 changes Offset and order of evaluation
         // for arguments is unspecified.
-        auto op1 = Data.getULEB128(Offset);
-        auto op2 = Data.getULEB128(Offset);
+        uint64_t op1 = Data.getULEB128(C);
+        uint64_t op2 = Data.getULEB128(C);
+        addInstruction(Opcode, op1, op2);
+        break;
+      }
+      case DW_CFA_offset_extended_sf:
+      case DW_CFA_def_cfa_sf:
+      case DW_CFA_val_offset_sf: {
+        // Operands: ULEB128, SLEB128
+        // Note: see comment for the previous case
+        uint64_t op1 = Data.getULEB128(C);
+        uint64_t op2 = (uint64_t)Data.getSLEB128(C);
         addInstruction(Opcode, op1, op2);
         break;
-        }
-        case DW_CFA_offset_extended_sf:
-        case DW_CFA_def_cfa_sf:
-        case DW_CFA_val_offset_sf: {
-          // Operands: ULEB128, SLEB128
-          // Note: see comment for the previous case
-          auto op1 = Data.getULEB128(Offset);
-          auto op2 = (uint64_t)Data.getSLEB128(Offset);
-          addInstruction(Opcode, op1, op2);
-          break;
-        }
-        case DW_CFA_def_cfa_expression: {
-          uint32_t ExprLength = Data.getULEB128(Offset);
-          addInstruction(Opcode, 0);
-          DataExtractor Extractor(
-              Data.getData().slice(*Offset, *Offset + ExprLength),
-              Data.isLittleEndian(), Data.getAddressSize());
-          // Note. We do not pass the DWARF format to DWARFExpression, because
-          // DW_OP_call_ref, the only operation which depends on the format, is
-          // prohibited in call frame instructions, see sec. 6.4.2 in DWARFv5.
-          Instructions.back().Expression =
-              DWARFExpression(Extractor, Data.getAddressSize());
-          *Offset += ExprLength;
-          break;
-        }
-        case DW_CFA_expression:
-        case DW_CFA_val_expression: {
-          auto RegNum = Data.getULEB128(Offset);
-          auto BlockLength = Data.getULEB128(Offset);
-          addInstruction(Opcode, RegNum, 0);
-          DataExtractor Extractor(
-              Data.getData().slice(*Offset, *Offset + BlockLength),
-              Data.isLittleEndian(), Data.getAddressSize());
-          // Note. We do not pass the DWARF format to DWARFExpression, because
-          // DW_OP_call_ref, the only operation which depends on the format, is
-          // prohibited in call frame instructions, see sec. 6.4.2 in DWARFv5.
-          Instructions.back().Expression =
-              DWARFExpression(Extractor, Data.getAddressSize());
-          *Offset += BlockLength;
-          break;
-        }
+      }
+      case DW_CFA_def_cfa_expression: {
+        uint64_t ExprLength = Data.getULEB128(C);
+        addInstruction(Opcode, 0);
+        StringRef Expression = Data.getBytes(C, ExprLength);
+
+        DataExtractor Extractor(Expression, Data.isLittleEndian(),
+                                Data.getAddressSize());
+        // Note. We do not pass the DWARF format to DWARFExpression, because
+        // DW_OP_call_ref, the only operation which depends on the format, is
+        // prohibited in call frame instructions, see sec. 6.4.2 in DWARFv5.
+        Instructions.back().Expression =
+            DWARFExpression(Extractor, Data.getAddressSize());
+        break;
+      }
+      case DW_CFA_expression:
+      case DW_CFA_val_expression: {
+        uint64_t RegNum = Data.getULEB128(C);
+        addInstruction(Opcode, RegNum, 0);
+
+        uint64_t BlockLength = Data.getULEB128(C);
+        StringRef Expression = Data.getBytes(C, BlockLength);
+        DataExtractor Extractor(Expression, Data.isLittleEndian(),
+                                Data.getAddressSize());
+        // Note. We do not pass the DWARF format to DWARFExpression, because
+        // DW_OP_call_ref, the only operation which depends on the format, is
+        // prohibited in call frame instructions, see sec. 6.4.2 in DWARFv5.
+        Instructions.back().Expression =
+            DWARFExpression(Extractor, Data.getAddressSize());
+        break;
+      }
       }
     }
   }
 
-  return Error::success();
+  *Offset = C.tell();
+  return C.takeError();
 }
 
 namespace {

diff  --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
index a7465a8e3133..fd9a2be8a3e9 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
@@ -10,6 +10,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugFrame.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -119,4 +120,154 @@ TEST(DWARFDebugFrame, DumpEH64FDE) {
                    "cie=1111ab9a000c pc=4444abcdabcd...5555bcdebcde");
 }
 
+// Here we test how truncated Call Frame Instructions are parsed.
+TEST(DWARFDebugFrame, ParseTruncatedCFITest) {
+  auto ParseCFI = [](dwarf::CIE &C, ArrayRef<uint8_t> Instructions,
+                     Optional<uint64_t> Size = None) {
+    DWARFDataExtractor Data(Instructions, /*IsLittleEndian=*/true,
+                            /*AddressSize=*/8);
+    uint64_t Offset = 0;
+    const uint64_t EndOffset = Size ? *Size : (uint64_t)Instructions.size();
+    return C.cfis().parse(Data, &Offset, EndOffset);
+  };
+
+  dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/false,
+                                 /*Offset=*/0x0,
+                                 /*Length=*/0xff);
+
+  // Having an empty instructions list is fine.
+  EXPECT_THAT_ERROR(ParseCFI(TestCIE, {}), Succeeded());
+
+  // Unable to read an opcode, because the instructions list is empty, but we
+  // say to the parser that it is not.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {}, /*Size=*/1),
+      FailedWithMessage(
+          "unexpected end of data at offset 0x0 while reading [0x0, 0x1)"));
+
+  // Unable to read a truncated DW_CFA_offset instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_offset}),
+      FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                        "malformed uleb128, extends past end"));
+
+  // Unable to read a truncated DW_CFA_set_loc instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_set_loc}),
+      FailedWithMessage(
+          "unexpected end of data at offset 0x1 while reading [0x1, 0x9)"));
+
+  // Unable to read a truncated DW_CFA_advance_loc1 instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_advance_loc1}),
+      FailedWithMessage(
+          "unexpected end of data at offset 0x1 while reading [0x1, 0x2)"));
+
+  // Unable to read a truncated DW_CFA_advance_loc2 instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_advance_loc2}),
+      FailedWithMessage(
+          "unexpected end of data at offset 0x1 while reading [0x1, 0x3)"));
+
+  // Unable to read a truncated DW_CFA_advance_loc4 instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_advance_loc4}),
+      FailedWithMessage(
+          "unexpected end of data at offset 0x1 while reading [0x1, 0x5)"));
+
+  // A test for an instruction with a single ULEB128 operand.
+  auto CheckOp_ULEB128 = [&](uint8_t Inst) {
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, Inst),
+        FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                          "malformed uleb128, extends past end"));
+  };
+
+  for (uint8_t Inst :
+       {dwarf::DW_CFA_restore_extended, dwarf::DW_CFA_undefined,
+        dwarf::DW_CFA_same_value, dwarf::DW_CFA_def_cfa_register,
+        dwarf::DW_CFA_def_cfa_offset, dwarf::DW_CFA_GNU_args_size})
+    CheckOp_ULEB128(Inst);
+
+  // Unable to read a truncated DW_CFA_def_cfa_offset_sf instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_def_cfa_offset_sf}),
+      FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                        "malformed sleb128, extends past end"));
+
+  // A test for an instruction with two ULEB128 operands.
+  auto CheckOp_ULEB128_ULEB128 = [&](uint8_t Inst) {
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, Inst),
+        FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                          "malformed uleb128, extends past end"));
+
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, {Inst, /*Op1=*/0}),
+        FailedWithMessage("unable to decode LEB128 at offset 0x00000002: "
+                          "malformed uleb128, extends past end"));
+  };
+
+  for (uint8_t Inst : {dwarf::DW_CFA_offset_extended, dwarf::DW_CFA_register,
+                       dwarf::DW_CFA_def_cfa, dwarf::DW_CFA_val_offset})
+    CheckOp_ULEB128_ULEB128(Inst);
+
+  // A test for an instruction with two operands: ULEB128, SLEB128.
+  auto CheckOp_ULEB128_SLEB128 = [&](uint8_t Inst) {
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, Inst),
+        FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                          "malformed uleb128, extends past end"));
+
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, {Inst, /*Op1=*/0}),
+        FailedWithMessage("unable to decode LEB128 at offset 0x00000002: "
+                          "malformed sleb128, extends past end"));
+  };
+
+  for (uint8_t Inst : {dwarf::DW_CFA_offset_extended_sf,
+                       dwarf::DW_CFA_def_cfa_sf, dwarf::DW_CFA_val_offset_sf})
+    CheckOp_ULEB128_SLEB128(Inst);
+
+  // Unable to read a truncated DW_CFA_def_cfa_expression instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_def_cfa_expression}),
+      FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                        "malformed uleb128, extends past end"));
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_def_cfa_expression,
+                         /*expression length=*/0x1}),
+      FailedWithMessage(
+          "unexpected end of data at offset 0x2 while reading [0x2, 0x3)"));
+  // The DW_CFA_def_cfa_expression can contain a zero length expression.
+  EXPECT_THAT_ERROR(ParseCFI(TestCIE, {dwarf::DW_CFA_def_cfa_expression,
+                                       /*ExprLen=*/0}),
+                    Succeeded());
+
+  // A test for an instruction with three operands: ULEB128, expression length
+  // (ULEB128) and expression bytes.
+  auto CheckOp_ULEB128_Expr = [&](uint8_t Inst) {
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, {Inst}),
+        FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                          "malformed uleb128, extends past end"));
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, {Inst, /*Op1=*/0}),
+        FailedWithMessage("unable to decode LEB128 at offset 0x00000002: "
+                          "malformed uleb128, extends past end"));
+    // A zero length expression is fine
+    EXPECT_THAT_ERROR(ParseCFI(TestCIE, {Inst,
+                                         /*Op1=*/0, /*ExprLen=*/0}),
+                      Succeeded());
+    EXPECT_THAT_ERROR(
+        ParseCFI(TestCIE, {Inst,
+                           /*Op1=*/0, /*ExprLen=*/1}),
+        FailedWithMessage(
+            "unexpected end of data at offset 0x3 while reading [0x3, 0x4)"));
+  };
+
+  for (uint8_t Inst : {dwarf::DW_CFA_expression, dwarf::DW_CFA_val_expression})
+    CheckOp_ULEB128_Expr(Inst);
+}
+
 } // end anonymous namespace


        


More information about the llvm-commits mailing list