> On Apr 28, 2015, at 3:50 PM, David Blaikie <[email protected]> wrote: > > > > On Tue, Apr 28, 2015 at 3:45 PM, Adrian Prantl <[email protected] > <mailto:[email protected]>> 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. -- adrian > > > > > REPOSITORY > rL LLVM > > http://reviews.llvm.org/D9329 <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/ > <http://reviews.llvm.org/settings/panel/emailpreferences/> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
