[Lldb-commits] [PATCH] D18646: Fix DWO breakage in r264909

[Tamas Berghammer via lldb-commits]

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);
----------------
clayborg wrote:
> 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);
> else
>     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.


Repository:
  rL LLVM

http://reviews.llvm.org/D18646