[llvm] r176882 - Debug Info: use DW_FORM_ref_addr instead of DW_FORM_ref4 if the referenced DIE

[Eric Christopher]

Hi Manman,

Coming back to this it's definitely going to need to be reimplemented
in a different way and since I don't think much of the patch will be
salvagable from it I'm going to go ahead and revert this as well. I
think based on our discussions in other threads and on IRC that you
have an idea how to implement DW_FORM_ref_addr now, but please go
ahead and send out a patch when you've got something or let me know if
you have any questions.

Thanks for all of your work on this, it's very valuable!

-eric


On Fri, Oct 4, 2013 at 11:43 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Mar 13, 2013 at 10:19 AM, Manman Ren <mren at apple.com> wrote:
>>
>>
>> On Mar 13, 2013, at 1:23 AM, Eric Christopher wrote:
>>
>>
>>
>>>
>>> +/// Climb up the parent chain to get the compile unit DIE this DIE
>>> belongs to.
>>> +DIE *DIE::getCompileUnit() const{
>>> +  DIE *p = getParent();
>>> +  while (p) {
>>> +    if (p->getTag() == dwarf::DW_TAG_compile_unit)
>>> +      return p;
>>> +    p = p->getParent();
>>> +  }
>>> +  return NULL;
>>> +}
>>> +
>>
>>
>> Shouldn't this be unreachable or is there some case where we have orphaned
>> DIEs?
>>
>> We should not generate orphaned DIEs, and should be able to change "return
>> NULL" to unreachable.
>> But I fixed an issue before where we generated orphaned DIEs. I can check
>> in a separate change for this.
>>
>>
>>>
>>> +/// For a given compile unit DIE, returns offset from beginning of debug
>>> info.
>>> +unsigned DwarfUnits::getCUOffset(DIE *Die) {
>>> +  for (SmallVector<CompileUnit *, 1>::iterator I = CUs.begin(),
>>> +       E = CUs.end(); I != E; ++I) {
>>> +    CompileUnit *TheCU = *I;
>>> +    if (TheCU->getCUDie() == Die)
>>> +      return TheCU->getDebugInfoOffset();
>>> +  }
>>> +  return 0;
>>> +}
>>> +
>>
>>
>> Can you make this a little more safe? Assert that it's a compile unit on
>> entry and that'd hopefully make the return 0 also be unreachable?
>>
>> Sure.
>>
>>
>> Also can you show at least some of the debug info for how you made your
>> determination? This seems odd (at least the abstract origin part) and I'd
>> like to understand how we lto'd something together and got this.
>>
>> As I said in the commit message, I only saw this when building the kernel
>> with LTO and rdynamic, and this only happened 6 times and we got 6
>> verification errors.
>> With this fix, verification runs fine.
>> Is it strange to you that abstract_origin points to a DIE in a different
>> CU?
>
>
> Yes, that is strange. Please provide a test case for this (I know it's a
> while ago, but really... I don't think anyone intended this & that sounds
> like a bug that this change is working around, not addressing correctly).
>
>>
>>
>> Manman
>>
>>
>> Thanks!
>>
>> -eric
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>