[Lldb-commits] [PATCH] D18646: Fix DWO breakage in r264909
Tamas Berghammer via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 31 09:46:31 PDT 2016
tberghammer added inline comments.
Comment at: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:136
@@ +135,3 @@
+ assert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset);
+ return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset);
> I would prefer to not use an assertion here, we can't crash if things go south for some reason. Can we change this to something like:
> lldbassert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset);
> if (m_base_dwarf_cu->GetOffset() == die_ref.cu_offset)
> return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset);
> return DWARFDIE();
> The lldbassert will be in debug builds but not release builds so it can fire during testing, but won't crash a release build. assert() is dangerous as it is us to the builders to ensure DEBUG or NDEBUG is defined and if the assert is left in it can crash your lldb, IDE, or any program directly loading LLDB which isn't acceptable.
I asked Pavel to add the assert in as if the condition isn't met that means a pretty serious problem and will cause incorrect behavior or crash at a later stage. I think it is better to crash at the location where we detect the error instead of crashing at a random point or displaying incorrect data to the user.
If you don't want LLDB to take down you full IDE then I think you have 2 option:
* Compile with assert turned off and hope that we don't crash
* Move LLDB to a separate process and handle the case from the IDE when LLDB crashes
We have a lot of assert in different parts of LLDB and in clang/llvm as well and usually they help a lot in finding strange bugs. I would prefer to continue adding more assert to the code so if somebody start miss-using an API we detect it immediately. Even if we remove every assert from LLDB (I strongly against it) clang and llvm will contain a large number of asserts what can take down your IDE the same way.
More information about the lldb-commits