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

[Greg Clayton via lldb-commits]

> On Mar 31, 2016, at 11:51 AM, Tamas Berghammer <tberghammer at google.com> wrote:
> 
> This assert isn't checking for the correctness of the debug info (what is a program input). It is checking that we passed the DIERef to the correct SymbolFileDWARFDwo instance what is the contract of the API and should be checked by an assert. This assert will only fire if we have a bug in LLDB regarding the usage of SymbolFileDWARFDwo what have to be fixed.

Again, the assertion that we guarantee will go away right now is lldbassert() so use that if you need to. But your code should work correctly if the assert is not in the the code and you current fix of:

DWARFDIE
SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref)
{
    assert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset);
    return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset);
}

Is not doing the right thing. It is still taking an unrelated DIE offset and looking it up in the wrong debug info. So the correct fix when we assume the assert will get compiled out of the code for release mode is:


DWARFDIE
SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref)
{
    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();
}

> 
> I think most people here just want to get rid of all assert in their production builds what is perfectly reasonable but it can be done by disabling asserts what is the only right way considering clang and llvm contain a lot of them.

We are trying to get rid of these. A compiler that claims it is code that can be used as a library should not assert. Otherwise you are saying "if you want to use clang, you must use it out of process because it will kill your program for you at any time when it becomes unhappy".

Greg