[PATCH] D40199: llvm-dwarfdump --verify is incorrectly saying all DW_AT_location attributes with locations lists are invalid.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:413
   case DW_AT_location: {
     Optional<ArrayRef<uint8_t>> Expr = AttrValue.Value.getAsBlock();
+    auto VerifyLocation = [&](const void *data, size_t len) {
----------------
Probably move this down into the "if (Expr)" condition below

  if (Optional<ArrayRef<uint8_t>> Expr = AttrValue.Value.getAsBlock()) {
    VerifyLocation(...);


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:426-435
+    if (Expr) {
+      // Inlined location
+      VerifyLocation(Expr->data(), Expr->size());
+    } else {
+      // Location list
+      if (auto LocOffset = AttrValue.Value.getAsUnsignedConstant()) {
+        auto DebugLoc = DCtx.getDebugLoc();
----------------
Alternatively/slightly different to all this, could use early breaks:

  auto X = ...;
  if (!X)
    break;

etc...


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:429-431
+    } else {
+      // Location list
+      if (auto LocOffset = AttrValue.Value.getAsUnsignedConstant()) {
----------------
Could roll this into else-if:

  } else if (auto LocOffset = ... ) {


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:432-435
+        auto DebugLoc = DCtx.getDebugLoc();
+        if (DebugLoc) {
+          auto LocList = DebugLoc->getLocationListAtOffset(*LocOffset);
+          if (LocList) {
----------------
Could roll these variables into the conditions:

  if (auto DebugLoc = DCtx.getDebugLoc())
    if (auto LocList = ... 


Repository:
  rL LLVM

https://reviews.llvm.org/D40199