zturner marked 2 inline comments as done.
zturner added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:25
+ lldb::offset_t *offset_ptr) {
+ assert(debug_info.ValidOffset(*offset_ptr));
+
----------------
clayborg wrote:
> We have error checking now, use it instead of asserting? We are checking it
> with the assert anyways, so might as well check and return an error?
The reason I changed this is because the logic is simpler if we assert, and I
examined all call-sites and ensured that the value is already sanitized before
we call this function.
In this case, the only actual call to this function is here:
```
while (debug_info_data.ValidOffset(offset)) {
llvm::Expected<DWARFUnitSP> cu_sp =
DWARFCompileUnit::extract(m_dwarf2Data, debug_info_data, &offset);
...
}
```
so we have already verified that the offset is valid before we even call the
function. In fact, that is the most natural way to parse compile units anyway,
by running a loop until you're at the end of the data, so it would be hard to
imagine someone could actually intend to call the function
Because of this, I'm considering the assert here to be an enforcement of
internal consistency and not one of user input
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:46
+llvm::Expected<DWARFDebugAranges &> DWARFDebugInfo::GetCompileUnitAranges() {
+ assert(m_cu_aranges_up && m_dwarf2Data);
+
----------------
clayborg wrote:
> This logic is wrong. We cache the m_cu_arange_up. If it is NULL then we
> build. If not, we return what we have. Maybe change this to:
>
> ```
> if (m_cu_aranges_up)
> return *m_cu_aranges_up;
> ```
Yes, you are right. I'm at home today on my Windows box so I couldn't really
run the test suite, but that certainly would have caught this. Thanks for
pointing it out! I'll make sure to fix when I'm back at work before submitting.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59381/new/
https://reviews.llvm.org/D59381
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
