> On Apr 6, 2018, at 1:52 PM, Greg Clayton <clayb...@gmail.com> wrote:
> 
> 
> 
>> 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.

I agree, I don't think reverting was necessary / the right action in this case. 
We should move to make the interface of the LLDB DWARF parser more like the 
LLVM one. If the LLVM interface is not good for our needs we should evolve both 
at the same time. Since there is a very large overlap between LLDB developers 
and LLVM libDebugInfo developers this should not cause any problems. At this 
point we should avoid any changes that bring the two further apart.

-- adrian

> 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
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
              • ... Pavel Labath via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Greg Clayton via lldb-commits
              • ... Greg Clayton via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Greg Clayton via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Adrian Prantl via lldb-commits
  • [Lldb-commits] [PATCH] D45... Jan Kratochvil via Phabricator via lldb-commits

Reply via email to