> On Apr 29, 2015, at 9:50 AM, David Blaikie <[email protected]> wrote: > > > > On Wed, Apr 29, 2015 at 8:16 AM, Adrian Prantl <[email protected]> wrote: > > > On Apr 28, 2015, at 6:26 PM, David Blaikie <[email protected]> wrote: > > > > > > > > On Tue, Apr 28, 2015 at 4:01 PM, Adrian Prantl <[email protected]> wrote: > > Author: adrian > > Date: Tue Apr 28 18:01:24 2015 > > New Revision: 236059 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=236059&view=rev > > Log: > > 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. > > > > 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. > > > > Seems we're not so lucky: > > http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/21650/steps/gdb-75-check/logs/gdb.cp__anon-union.exp > > > > Could you revert & I'll look into getting you a reduced test > > case/demonstration of the issue? (can you run GDB 7.5? Perhaps a simple > > test case would demonstrate the issue if you're lucky, otherwise I can > > reduce one from the failing test case) > > I reverted the commit in r236110. I probably won’t need a reduction — my > guess from the log is that gdb expects a local variable to be present. > > OK > > My suggestion is to emit local artificial shadow variables and then weaken > the Verifier to not verify artificial variables. In a next step, we could use > the new debugger tuning target feature to make the artificial local variables > and the weakened verifier a gdb-specific behavior, file a bug against gdb, > and eventually remove it altogether. > > Sounds reasonable to me
Committed as LLVM r236124, CFE r236125. Let’s hope this works better. > > > -- adrian > > > > > > > rdar://problem/20730771 > > > > Modified: > > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > > cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp > > > > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=236059&r1=236058&r2=236059&view=diff > > ============================================================================== > > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) > > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Apr 28 18:01:24 2015 > > @@ -2835,31 +2835,6 @@ void CGDebugInfo::EmitDeclare(const VarD > > 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. > > > > Modified: cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp?rev=236059&r1=236058&r2=236059&view=diff > > ============================================================================== > > --- cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp (original) > > +++ cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp Tue Apr 28 > > 18:01:24 2015 > > @@ -21,8 +21,26 @@ int test_it() { > > 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]], > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
