[Lldb-commits] [lldb] r187423 - Updates the DW_AT_data_member_location handlers for the LLDB DWARF plugin

[Thirumurthi, Ashok]

Thanks for the feedback on the format, Greg,

My real question is what code would I put into a test in order to call DWARFDebugInfoEntry::DumpAttribute.  I see that it's called by Dump, but Dump is a common name.  I renamed Dump as DDump and its clear that this is called by DumpAncestry which is dead code.  It's also called by DWARFDebugInfo::DumpCallback, which is only called from DWARFDebugInfo::Dump.  Renaming this routine, I am able to build liblldb.so and run the test suite.  

All that suggests that the code that I modified is dead code.  For instance, it's not called by lang/cpp/bitfields which does a good job of exercising SBType and SBTypeMember...

Regarding testing, what would you say if the test suite ran test/lang with -gdwarf-2 through -gdwarf-4?

- Ashok

-----Original Message-----
From: Greg Clayton [mailto:gclayton at apple.com] 
Sent: Tuesday, July 30, 2013 4:04 PM
To: Thirumurthi, Ashok
Cc: lldb-commits at cs.uiuc.edu
Subject: Re: [Lldb-commits] [lldb] r187423 - Updates the DW_AT_data_member_location handlers for the LLDB DWARF plugin

When the value is a constant, it might make more sense to show the value with a "+<decimal>" format, so instead of:

+                    s.Printf( "0x%8.8" PRIx64, offset);

it would be:

+                    s.Printf( "+%" PRIu64, offset);

This makes it clear that the value isn't a .debug_loc offset.

In order to test this, you might be able to emit assembler using "-g -S" (for clang only) and then hand edit the assembler output to emit the right kind of DWARF. Then we could test this with clang only by checking in the assembler file and compiling it. You could then verify the offset is correct via the SBType and SBTypeMember APIs without having to run this (since the assembler would be arch specific).

Greg


On Jul 30, 2013, at 12:02 PM, "Thirumurthi, Ashok" <ashok.thirumurthi at intel.com> wrote:

> I wanted to follow-up this patch with a fix for 
> DWARFDebugInfoEntry::DumpAttribute such as the attached patch.  
> However, I couldn't find a test or an LLDB command that exercises this 
> code.  Any hints would be welcome :)
> 
> - Ashok
> 
> -----Original Message-----
> From: lldb-commits-bounces at cs.uiuc.edu 
> [mailto:lldb-commits-bounces at cs.uiuc.edu] On Behalf Of Ashok 
> Thirumurthi
> Sent: Tuesday, July 30, 2013 10:59 AM
> To: lldb-commits at cs.uiuc.edu
> Subject: [Lldb-commits] [lldb] r187423 - Updates the 
> DW_AT_data_member_location handlers for the LLDB DWARF plugin
> 
> Author: athirumu
> Date: Tue Jul 30 09:58:39 2013
> New Revision: 187423
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=187423&view=rev
> Log:
> Updates the DW_AT_data_member_location handlers for the LLDB DWARF plugin to handle the case of an integer constant (DWARF 3 and later).
> 
> - Fixes tests that assert in 
> RecordLayoutBuilder::updateExternalFieldOffset
> because LLDB was providing an external AST source with missing member offsets.
> 
> Modified:
>    lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
> 
> Modified: 
> lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFi
> le/DWARF/SymbolFileDWARF.cpp?rev=187423&r1=187422&r2=187423&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
> (original)
> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Tue 
> +++ Jul 30 09:58:39 2013
> @@ -1780,6 +1780,13 @@ SymbolFileDWARF::ParseChildMembers
>                                         member_byte_offset = memberOffset.ResolveValue(NULL).UInt();
>                                     }
>                                 }
> +                                else
> +                                {
> +                                    // With DWARF 3 and later, if the value is an integer constant,
> +                                    // this form value is the offset in bytes from the beginning
> +                                    // of the containing entity. 
> +                                    member_byte_offset = form_value.Unsigned(); 
> +                                }
>                                 break;
> 
>                             case DW_AT_accessibility: accessibility = DW_ACCESS_to_AccessType (form_value.Unsigned()); break; @@ -2164,16 +2171,28 @@ SymbolFileDWARF::ParseChildMembers
>                                         member_byte_offset = memberOffset.ResolveValue(NULL).UInt();
>                                     }
>                                 }
> +                                else
> +                                {
> +                                    // With DWARF 3 and later, if the value is an integer constant,
> +                                    // this form value is the offset in bytes from the beginning
> +                                    // of the containing entity. 
> +                                    member_byte_offset = form_value.Unsigned(); 
> +                                }
>                                 break;
> 
>                             case DW_AT_accessibility:
>                                 accessibility = DW_ACCESS_to_AccessType(form_value.Unsigned());
>                                 break;
> 
> -                            case DW_AT_virtuality: is_virtual = form_value.Boolean(); break;
> -                            default:
> +                            case DW_AT_virtuality:
> +                                is_virtual = form_value.Boolean();
> +                                break;
> +                                                   
>                             case DW_AT_sibling:
>                                 break;
> +
> +                            default:
> +                                break;
>                             }
>                         }
>                     }
> @@ -2620,7 +2639,7 @@ SymbolFileDWARF::ResolveClangOpaqueTypeD
>         assert(false && "not a forward clang type decl!");
>         break;
>     }
> -    return NULL;
> +    return false;
> }
> 
> Type*
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> <data-member-location.patch>__________________________________________
> _____
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits