[PATCH] Debug Info: Represent local anonymous unions as anonymous unions.
David Blaikie
dblaikie at gmail.com
Tue Apr 28 15:59:03 PDT 2015
On Tue, Apr 28, 2015 at 3:52 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Apr 28, 2015, at 3:50 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Apr 28, 2015 at 3:45 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>> Hi echristo, dblaikie,
>>
>> Debug Info: Represent local anonymous unions as anonymous unions in the
>> debug info.
>> This patch deletes a hack that emits the members of local anonymous
>> unions as local variables.
>>
>> Why? Besides being morally wrong, the existing representation using local
>> variables breaks internal assumptions about the local variables' storage
>> size.
>>
>> Compiling
>>
>> ```
>> void fn1() {
>> union {
>> int i;
>> char c;
>> };
>> i = c;
>> }
>>
>> ```
>>
>> with -g -O3 -verify will cause the verifier to fail after SROA splits the
>> 32-bit storage for the "local variable" c into two pieces because the
>> second piece is clearly outside the 8-bit range that is expected for a
>> variable of type char. Given the choice I'd rather fix the debug
>> representation than weaken the verifier.
>>
>> Debuggers generally already know how to deal with anonymous unions when
>> they are members of C++ record types, but they may have problems finding
>> the local anonymous struct members in the expression evaluator.
>>
>> Eric, do you know whether the local variable trick is necessary for GDB
>> compatibility?
>>
>
> Throwing it at the GDB buildbot's probably the best way to find out. Don't
> too much mind committing it first & seeing if it fails (unless you guys can
> run it these days, in which case a preliminary run would be nice).
>
> GCC produces something vaguely interesting:
>
> 0x0000002d: DW_TAG_subprogram [2] *
> ...
> 0x0000004e: DW_TAG_lexical_block [3] *
> ...
>
> 0x00000063: DW_TAG_variable [4]
> DW_AT_name [DW_FORM_string] ("i")
> DW_AT_type [DW_FORM_ref4] (cu + 0x0098 =>
> {0x00000098})
> DW_AT_artificial [DW_FORM_flag_present] (true)
> DW_AT_location [DW_FORM_exprloc] (<0x2> 91 60 )
>
> 0x0000006d: DW_TAG_variable [4]
> DW_AT_name [DW_FORM_string] ("c")
> DW_AT_type [DW_FORM_ref4] (cu + 0x009f =>
> {0x0000009f})
> DW_AT_artificial [DW_FORM_flag_present] (true)
> DW_AT_location [DW_FORM_exprloc] (<0x2> 91 60 )
>
> 0x00000077: DW_TAG_variable [5]
> DW_AT_type [DW_FORM_ref4] (cu + 0x0080 =>
> {0x00000080})
> DW_AT_location [DW_FORM_exprloc] (<0x2> 91 60 )
>
> 0x0000007f: NULL
>
> 0x00000080: DW_TAG_union_type [6] *
> DW_AT_byte_size [DW_FORM_data1] (0x04)
> DW_AT_decl_file [DW_FORM_data1]
> ("/tmp/dbginfo/union.c")
> DW_AT_decl_line [DW_FORM_data1] (2)
>
> 0x00000084: DW_TAG_member [7]
> DW_AT_name [DW_FORM_string] ("i")
> DW_AT_decl_file [DW_FORM_data1]
> ("/tmp/dbginfo/union.c")
> DW_AT_decl_line [DW_FORM_data1] (3)
> DW_AT_type [DW_FORM_ref4] (cu + 0x0098 =>
> {0x00000098})
>
> 0x0000008d: DW_TAG_member [7]
> DW_AT_name [DW_FORM_string] ("c")
> DW_AT_decl_file [DW_FORM_data1]
> ("/tmp/dbginfo/union.c")
> DW_AT_decl_line [DW_FORM_data1] (4)
> DW_AT_type [DW_FORM_ref4] (cu + 0x009f =>
> {0x0000009f})
>
> 0x00000096: NULL
>
> 0x00000097: NULL
>
>
> That’s pretty much the output that this patch produces, too (anonymous
> local variable + anonymous union). If we can use GCC as a precedent, I’m
> happy to commit this and see whether it breaks.
>
The interesting thing in GCC's output is that it produced 3 variables in
the function - one is the union, the other two are named variables that are
marked artificial. I hope that's not necessary, but don't rightly know.
The code change + test seem good - feel free to commit & keep an eye on the
GDB buildbot. If it fails for good reasons it's probably best to revert &
I'll try to get you a reduction.
- David
>
> -- adrian
>
>
>
>
>
>>
>> REPOSITORY
>> rL LLVM
>>
>> http://reviews.llvm.org/D9329
>>
>> Files:
>> lib/CodeGen/CGDebugInfo.cpp
>> test/CodeGenCXX/debug-info-anon-union-vars.cpp
>>
>> Index: lib/CodeGen/CGDebugInfo.cpp
>> ===================================================================
>> --- lib/CodeGen/CGDebugInfo.cpp
>> +++ lib/CodeGen/CGDebugInfo.cpp
>> @@ -2835,31 +2835,6 @@
>> return;
>> } else if (isa<VariableArrayType>(VD->getType()))
>> Expr.push_back(llvm::dwarf::DW_OP_deref);
>> - } else if (const RecordType *RT = dyn_cast<RecordType>(VD->getType()))
>> {
>> - // If VD is an anonymous union then Storage represents value for
>> - // all union fields.
>> - const RecordDecl *RD = cast<RecordDecl>(RT->getDecl());
>> - if (RD->isUnion() && RD->isAnonymousStructOrUnion()) {
>> - for (const auto *Field : RD->fields()) {
>> - llvm::MDType *FieldTy = getOrCreateType(Field->getType(), Unit);
>> - StringRef FieldName = Field->getName();
>> -
>> - // Ignore unnamed fields. Do not ignore unnamed records.
>> - if (FieldName.empty() && !isa<RecordType>(Field->getType()))
>> - continue;
>> -
>> - // Use VarDecl's Tag, Scope and Line number.
>> - auto *D = DBuilder.createLocalVariable(
>> - Tag, Scope, FieldName, Unit, Line, FieldTy,
>> - CGM.getLangOpts().Optimize, Flags, ArgNo);
>> -
>> - // Insert an llvm.dbg.declare into the current block.
>> - DBuilder.insertDeclare(Storage, D,
>> DBuilder.createExpression(Expr),
>> - llvm::DebugLoc::get(Line, Column, Scope),
>> - Builder.GetInsertBlock());
>> - }
>> - return;
>> - }
>> }
>>
>> // Create the descriptor for the variable.
>> Index: test/CodeGenCXX/debug-info-anon-union-vars.cpp
>> ===================================================================
>> --- test/CodeGenCXX/debug-info-anon-union-vars.cpp
>> +++ test/CodeGenCXX/debug-info-anon-union-vars.cpp
>> @@ -21,8 +21,26 @@
>> return (c == 1);
>> }
>>
>> +// This is not necessary (and actually harmful because it breaks IR
>> assumptions)
>> +// for local variables.
>> +void foo() {
>> + union {
>> + int i;
>> + char c;
>> + };
>> + i = 8;
>> +}
>> +
>> // CHECK: [[FILE:.*]] = !MDFile(filename:
>> "{{.*}}debug-info-anon-union-vars.cpp",
>> // CHECK: !MDGlobalVariable(name: "c",{{.*}} file: [[FILE]], line:
>> 6,{{.*}} isLocal: true, isDefinition: true
>> // CHECK: !MDGlobalVariable(name: "d",{{.*}} file: [[FILE]], line:
>> 6,{{.*}} isLocal: true, isDefinition: true
>> // CHECK: !MDGlobalVariable(name: "a",{{.*}} file: [[FILE]], line:
>> 6,{{.*}} isLocal: true, isDefinition: true
>> // CHECK: !MDGlobalVariable(name: "b",{{.*}} file: [[FILE]], line:
>> 6,{{.*}} isLocal: true, isDefinition: true
>> +// CHECK: !MDLocalVariable(
>> +// CHECK-NOT: name:
>> +// CHECK: type: ![[UNION:[0-9]+]]
>> +// CHECK: ![[UNION]] = !MDCompositeType(tag: DW_TAG_union_type,
>> +// CHECK-NOT: name:
>> +// CHECK: elements
>> +// CHECK: !MDDerivedType(tag: DW_TAG_member, name: "i", scope:
>> ![[UNION]],
>> +// CHECK: !MDDerivedType(tag: DW_TAG_member, name: "c", scope:
>> ![[UNION]],
>>
>> EMAIL PREFERENCES
>> http://reviews.llvm.org/settings/panel/emailpreferences/
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150428/ce350b9b/attachment.html>
More information about the cfe-commits
mailing list