r236059 - Debug Info: Represent local anonymous unions as anonymous unions
Robinson, Paul
Paul_Robinson at playstation.sony.com
Wed Apr 29 10:49:29 PDT 2015
Things like the existence of extra artificial entries do seem appropriate for putting under "tuning" control.
I suppose I should ping/refresh the tuning patch… been letting it slide due to a particularly snarky merge and needing to get the depfile thing sorted out for an imminent release. I'll be back on it Real Soon Now.
--paulr
From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
Sent: Wednesday, April 29, 2015 10:04 AM
To: Adrian Prantl; David Blaikie
Cc: llvm cfe
Subject: Re: r236059 - Debug Info: Represent local anonymous unions as anonymous unions
On Wed, Apr 29, 2015 at 9:58 AM Eric Christopher <echristo at gmail.com<mailto:echristo at gmail.com>> wrote:
On Wed, Apr 29, 2015 at 8:20 AM Adrian Prantl <aprantl at apple.com<mailto:aprantl at apple.com>> wrote:
> On Apr 28, 2015, at 6:26 PM, David Blaikie <dblaikie at gmail.com<mailto:dblaikie at gmail.com>> wrote:
>
>
>
> On Tue, Apr 28, 2015 at 4:01 PM, Adrian Prantl <aprantl at apple.com<mailto:aprantl at apple.com>> 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.
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.
FWIW I don't want to use the "tuning" parameters to also affect correctness.
OK. Dave and I debated this a little in person, here's a proposal:
by default it will have the gdb specific behavior, but if you're tuning for lldb (or any other debugger I guess?) it won't be there.
Thoughts?
-eric
-eric
-- 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
> cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150429/b034498c/attachment.html>
More information about the cfe-commits
mailing list