[DebugInfo] Fix DWARFDebugFrame instruction operand ordering

[igor-laevsky]

Summary

When parsing DWARF instructions we can sometimes end up with instruction operands in reversed order. This was caused by two facts:

1. getULEB128(Offset) changes Offset and
2. Evaluation order of function arguments is unspecified

It means that two getULEB128 calls embedded into function argument list were executed in any arbitrary order. Sometimes this was resulting in a reversed instruction operands.

This issue came up after _eh_frame changes http://reviews.llvm.org/D15535. However they are not directly related.

[dblaikie]

Looks good, with one suggested change. Thanks!

lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

168 ↗

(On Diff #45794)

Roll the declaration/initialization together:

auto op1 = Data.getULEB128(Offset);
auto op2 = Data.getULEB128(Offset);

(using auto is optional - whichever you prefer - but the type information doesn't seem that important (given that the origintal formulation of this code didn't provide explicit type information to the reader))

llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

if (Primary) {
           break;
         case DW_CFA_def_cfa_offset_sf:
           // Operands: SLEB128
           addInstruction(Opcode, Data.getSLEB128(Offset));
           break;
         case DW_CFA_offset_extended:
         case DW_CFA_register:
         case DW_CFA_def_cfa:
-        case DW_CFA_val_offset:
+        case DW_CFA_val_offset: {
           // Operands: ULEB128, ULEB128
-          addInstruction(Opcode, Data.getULEB128(Offset),
-                                 Data.getULEB128(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);
+          addInstruction(Opcode, op1, op2);
           break;
+        }
         case DW_CFA_offset_extended_sf:
         case DW_CFA_def_cfa_sf:
-        case DW_CFA_val_offset_sf:
+        case DW_CFA_val_offset_sf: {
           // Operands: ULEB128, SLEB128
-          addInstruction(Opcode, Data.getULEB128(Offset),
-                                 Data.getSLEB128(Offset));
+          // 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:
         case DW_CFA_expression:
         case DW_CFA_val_expression:
           // TODO: implement this
           report_fatal_error("Values with expressions not implemented yet!");
       }
     }
   }