> On Apr 6, 2018, at 10:32 AM, Jan Kratochvil <jan.kratoch...@redhat.com> wrote:
> On Fri, 06 Apr 2018 18:58:27 +0200, Davide Italiano wrote:
>> Yes, I definitely agree. It's clear this needs more discussion, so I
>> don't think it's reasonable if we revert this for now and
>> reconsider.
> I have reverted this https://reviews.llvm.org/D45170 by:
>        https://reviews.llvm.org/rL329423
>> I'll also take a look at the interfaces in LLVM to get a better sense
>> of what should be done. If nobody does this, I'll probably get to the
>> revert by the end of the day.
> I admit my DWARFUnit was in part alignment with LLVM's DWARFUnit (so that
> there is one abstract superclass) but at the same time even more
> differentiating it (as LLVM has no DWARFPartialUnit and DWARFTypeUnit will
> need another intermediate superclass which I call DWARFSomeNameUnit in that
> commit text above).
> I was sure glad with my DWARFUnit https://reviews.llvm.org/D40466 approval to
> be able to commit my DWZ support on top of it making LLDB usable on Red Hat
> systems. But I understand LLDB code unification with LLVM is more important.
> The DWZ patchset now waits on https://reviews.llvm.org/D40467 approval.
> Greg did not like it much so I am now working on new DWARFOneFileOffset being
> type-incompatible to dw_offset_t so that both types can be used safely without
> accidentally interchanging each other; instead of my current relying on unsafe
> type-compatible "dw_offset_t offset" vs. "dw_offset_t file_offset" parameters
> naming.
> Maybe LLDB should really reuse LLVM DWARFUnit first and I will implement DWZ
> already on top of LLVM's DWARFUnit? I have no idea myself now how complex task
> is the reuse of LLVM DWARFUnit. For the LLVM DWARFUnit reusal this Greg's
> commit would be sure fine; just completely unrelated to why+how I made this
> DWARFUnit split myself.

I have run into issues trying to use the LLVM ecosystem for object file parsing 
and for DWARF parsing. If anything in the object file is not exactly right LLVM 
will return an error. A few things I have run into is some ELF files produced 
by some tools don't correctly align their section headers and LLVM refuses to 
open the file. Switching to the LLVM DWARF parser will make us crash more as 
the DWARF classes there have many rules on using them. DWARFDIE will crash if 
you try to ask it to do anything and haven't checked if the object is valid. 
That is different from our classes where we just return an error or invalid 
value if you use and empty DWARFDIE. So we are close, but our code won't assert 
and kill the program. Switching over to the LLVM parser will require some 
detailed work and will take some time.

That being said, I am confused as to why this was reverted. The code I added 
mirrors the LLVM code a bit better, and yes it will require some reworking of 
your patches. The DWARFUnit having an accessor to give out a DWARFCompileUnit 
was really confusing and not the right layering. So I fixed the layering. I 
need to submit .debug_types patches and that patch was needed for this and now 
I am back to square one.

lldb-commits mailing list

Reply via email to