Breakpad Linux dumper: Make changes requested by Neal Sidhwaney in issue 59002.

- Use manifest constants for 'z' augmentation letters.
- Fix typos and rearrange some code for legibility.

a=jimblandy, r=nealsid


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@560 4c0a9323-5329-0410-9bdc-e9ce6186880e

src/common/dwarf/dwarf2enums.h

@@ -575,6 +575,37 @@ enum DwarfCFI
     DW_CFA_GNU_negative_offset_extended = 0x2f
   };
 
+// Exception handling 'z' augmentation letters.
+enum DwarfZAugmentationCodes {
+  // If the CFI augmentation string begins with 'z', then the CIE and FDE
+  // have an augmentation data area just before the instructions, whose
+  // contents are determined by the subsequent augmentation letters.
+  DW_Z_augmentation_start = 'z',
+
+  // If this letter is present in a 'z' augmentation string, the CIE
+  // augmentation data includes a pointer encoding, and the FDE
+  // augmentation data includes a language-specific data area pointer,
+  // represented using that encoding.
+  DW_Z_has_LSDA = 'L',
+
+  // If this letter is present in a 'z' augmentation string, the CIE
+  // augmentation data includes a pointer encoding, followed by a pointer
+  // to a personality routine, represented using that encoding.
+  DW_Z_has_personality_routine = 'P',
+
+  // If this letter is present in a 'z' augmentation string, the CIE
+  // augmentation data includes a pointer encoding describing how the FDE's
+  // initial location, address range, and DW_CFA_set_loc operands are
+  // encoded.
+  DW_Z_has_FDE_address_encoding = 'R',
+
+  // If this letter is present in a 'z' augmentation string, then code
+  // addresses covered by FDEs that cite this CIE are signal delivery
+  // trampolines. Return addresses of frames in trampolines should not be
+  // adjusted as described in section 6.4.4 of the DWARF 3 spec.
+  DW_Z_is_signal_trampoline = 'S'
+};
+
 // Exception handling frame description pointer formats, as described
 // by the Linux Standard Base Core Specification 4.0, section 11.5,
 // DWARF Extensions.

src/common/dwarf/dwarf2reader.cc

@@ -1870,7 +1870,7 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) {
 
   // If we don't recognize the version, we can't parse any more fields
   // of the CIE. For DWARF CFI, we handle versions 1 through 3 (there
-  // was never a version 2 fo CFI data). For .eh_frame, we handle only
+  // was never a version 2 of CFI data). For .eh_frame, we handle only
   // version 1.
   if (eh_frame_) {
     if (cie->version != 1) {
@@ -1878,7 +1878,7 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) {
       return false;
     }
   } else {
-    if (cie->version < 1 || 3 < cie->version) {
+    if (cie->version < 1 || cie->version > 3) {
       reporter_->UnrecognizedVersion(cie->offset, cie->version);
       return false;
     }
@@ -1893,18 +1893,18 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) {
   // Skip the terminating '\0'.
   cursor++;
 
-  // Is this an augmentation we recognize?
+  // Is this CFI augmented?
-  if (cie->augmentation.empty()) {
+  if (!cie->augmentation.empty()) {
-    ; // Stock DWARF CFI.
+    // Is it an augmentation we recognize?
-  } else if (cie->augmentation[0] == 'z') {
+    if (cie->augmentation[0] == DW_Z_augmentation_start) {
-    // Linux C++ ABI 'z' augmentation, used for exception handling data.
+      // Linux C++ ABI 'z' augmentation, used for exception handling data.
-    cie->has_z_augmentation = true;
+      cie->has_z_augmentation = true;
-  } else {
+    } else {
-    // Not an augmentation we recognize. Augmentations can have
+      // Not an augmentation we recognize. Augmentations can have arbitrary
-    // arbitrary effects on the form of rest of the content, so we
+      // effects on the form of rest of the content, so we have to give up.
-    // have to give up.
+      reporter_->UnrecognizedAugmentation(cie->offset, cie->augmentation);
-    reporter_->UnrecognizedAugmentation(cie->offset, cie->augmentation);
+      return false;
-    return false;
+    }
   }
 
   // Parse the code alignment factor.
@@ -1947,7 +1947,7 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) {
     // augmentation data as the string directs.
     for (size_t i = 1; i < cie->augmentation.size(); i++) {
       switch (cie->augmentation[i]) {
-        case 'L':
+        case DW_Z_has_LSDA:
           // The CIE's augmentation data holds the language-specific data
           // area pointer's encoding, and the FDE's augmentation data holds
           // the pointer itself.
@@ -1965,7 +1965,7 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) {
           // LSDA to use, since it appears in the FDE.
           break;
 
-        case 'P':
+        case DW_Z_has_personality_routine:
           // The CIE's augmentation data holds the personality routine
           // pointer's encoding, followed by the pointer itself.
           cie->has_z_personality = true;
@@ -1992,7 +1992,7 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) {
           data += len;
           break;
 
-        case 'R':
+        case DW_Z_has_FDE_address_encoding:
           // The CIE's augmentation data holds the pointer encoding to use
           // for addresses in the FDE.
           if (data >= data_end) return ReportIncomplete(cie);
@@ -2009,7 +2009,7 @@ bool CallFrameInfo::ReadCIEFields(CIE *cie) {
           }
           break;
 
-        case 'S':
+        case DW_Z_is_signal_trampoline:
           // Frames using this CIE are signal delivery frames.
           cie->has_z_signal_frame = true;
           break;
@@ -2295,7 +2295,8 @@ void CallFrameInfo::Reporter::UnusablePointerEncoding(uint64 offset,
                                                       uint8 encoding) {
   fprintf(stderr,
           "%s: CFI common information entry at offset 0x%llx in '%s':"
-          " 'z' augmentation specifies a pointer encoding for which we have no base address: 0x%02x\n",
+          " 'z' augmentation specifies a pointer encoding for which"
+          " we have no base address: 0x%02x\n",
           filename_.c_str(), offset, section_.c_str(), encoding);
 }
 

src/common/dwarf/dwarf2reader_cfi_unittest.cc

@@ -39,8 +39,8 @@
 // if you #define WRITE_ELF while compiling this file, and add the
 // 'include' directory from the binutils, gcc, or gdb source tree to the
 // #include path, then each test that calls the
-// PERHAPS_WRITE_DEBUG_FRAME_FILE or PERHAPS_WRITE_EH_FRAME_FILE will an
+// PERHAPS_WRITE_DEBUG_FRAME_FILE or PERHAPS_WRITE_EH_FRAME_FILE will write
-// ELF file containing a .debug_frame or .eh_frame section; you can then
+// an ELF file containing a .debug_frame or .eh_frame section; you can then
 // use tools like readelf to examine the test data, and check the tools'
 // interpretation against the test's intentions. Each ELF file is named
 // "cfitest-TEST", where TEST identifies the particular test.