[PATCH] D125781: [llvm-debuginfo-analyzer] 06 - Warning and internal options

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVReader.cpp:43
+    else
+      // We found a duplicated.
+      Duplicate.emplace_back(Element, Scope, Iter->second);
----------------
probinson wrote:
> probinson wrote:
> > 
> ping.
Changed to `// We found a duplicate.`.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVReader.cpp:103
+    }
+    exit(0);
+  }
----------------
probinson wrote:
> CarlosAlbertoEnciso wrote:
> > probinson wrote:
> > > `exit` in a library function?
> > This is a good point.
> > 
> > During the development of the CodeView Reader, due to incorrect parsing of the debug information, in some cases the same logical element was added to more than one logical scope.
> > 
> > If this method detects an error, it means the tool will crash as the memory is corrupted: the same logical element being destroyed more than once. That is the reason to force the `exit`.
> Better to have this function return a bool status, and the caller can do the exit or whatever else is appropriate to its context.
```
bool checkIntegrityScopesTree(LVScope *Root) {
  ...
  // Start traversing the scopes root and print any duplicates.
  TraverseScope(Root);
  bool PassIntegrity = true;
  if (Duplicate.size()) {
    ...
    PassIntegrity = false;
  }
  return PassIntegrity;
}
```

If the scopes tree integrity fails, propagate an `Error` condition to the caller `LVReader::doLoad()`.

```
Error LVReader::doLoad() {
  ...
  if (options().getInternalIntegrity() && !checkIntegrityScopesTree(Root))
    return make_error<StringError>("Duplicated elements in Scopes Tree");
  ...
  return Error::success();
}

```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125781/new/

https://reviews.llvm.org/D125781