[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:103
+    }
+    exit(0);
+  }
----------------
CarlosAlbertoEnciso wrote:
> 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();
> }
> 
> ```
It should read:
```
Error LVReader::doLoad() {
  ...
  if (options().getInternalIntegrity() && !checkIntegrityScopesTree(Root))
    return llvm::make_error<StringError>("Duplicated elements in Scopes Tree",
                                         inconvertibleErrorCode());
  ...
  return Error::success();
}
```


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

https://reviews.llvm.org/D125781