r281057 - [DebugInfo] Ensure complete type is emitted with -fstandalone-debug

[David Blaikie via cfe-commits]

Looks like this test could be a bit simpler:

struct foo;
foo *f; // produces a forward decl
struct foo {
  virtual ~foo();
};
foo f; // requires the type to be complete

A cursory glance looks like this reproduces the issue, but I may've missed
something.

Also the bracing in the new condition you added is inconsistent with the
existing bracing - could you fix that? (add braces to the middle if,
following the "any block more than one line should have braces", or remove
them from the outer one you added (following the "any block of a single
statement should omit braces"))

(idle thought: It'd be good to make the code common between the first time
a type is encountered, and the later callbacks when the type is completed,
required to be complete, vtable is emitted, etc - the inconsistency here (&
it's spread out through several functions: complete[Required]Type,
shouldOmitDefinition, isDefinedInClangModule, etc) is a bit
problematic/error prone)

On Fri, Sep 9, 2016 at 10:12 AM Reid Kleckner via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rnk
> Date: Fri Sep  9 12:03:53 2016
> New Revision: 281057
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281057&view=rev
> Log:
> [DebugInfo] Ensure complete type is emitted with -fstandalone-debug
>
> The logic for upgrading a class from a forward decl to a complete type
> was not checking the debug info emission level before applying the
> vtable optimization. This meant we ended up without debug info for a
> class which was required to be complete. I noticed it because it
> triggered an assertion during CodeView emission, but that's a separate
> issue.
>
> Modified:
>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>     cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=281057&r1=281056&r2=281057&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Sep  9 12:03:53 2016
> @@ -1648,9 +1648,13 @@ void CGDebugInfo::completeRequiredType(c
>    if (DebugKind <= codegenoptions::DebugLineTablesOnly)
>      return;
>
> -  if (const auto *CXXDecl = dyn_cast<CXXRecordDecl>(RD))
> -    if (CXXDecl->isDynamicClass())
> -      return;
> +  // If this is a dynamic class and we're emitting limited debug info,
> wait
> +  // until the vtable is emitted to complete the class debug info.
> +  if (DebugKind <= codegenoptions::LimitedDebugInfo) {
> +    if (const auto *CXXDecl = dyn_cast<CXXRecordDecl>(RD))
> +      if (CXXDecl->isDynamicClass())
> +        return;
> +  }
>
>    if (DebugTypeExtRefs && RD->isFromASTFile())
>      return;
>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp?rev=281057&r1=281056&r2=281057&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-class-nolimit.cpp Fri Sep  9
> 12:03:53 2016
> @@ -1,8 +1,29 @@
> -// RUN: %clang_cc1 -triple x86_64-unk-unk -debug-info-kind=standalone -o
> - -emit-llvm %s | FileCheck %s
> -// On Darwin, "full" debug info is the default, so really these tests are
> -// identical, as cc1 no longer chooses the effective value of
> DebugInfoKind.
>  // RUN: %clang_cc1 -triple x86_64-apple-darwin
> -debug-info-kind=standalone -o - -emit-llvm %s | FileCheck %s
>
> +// We had a bug in -fstandalone-debug where UnicodeString would not be
> completed
> +// when it was required to be complete. This orginally manifested as an
> +// assertion in CodeView emission on Windows with some dllexport stuff,
> but it's
> +// more general than that.
> +
> +struct UnicodeString;
> +struct GetFwdDecl {
> +  static UnicodeString format;
> +};
> +GetFwdDecl force_fwd_decl;
> +struct UnicodeString {
> +private:
> +  virtual ~UnicodeString();
> +};
> +struct UseCompleteType {
> +  UseCompleteType();
> +  ~UseCompleteType();
> +  UnicodeString currencySpcAfterSym[1];
> +};
> +UseCompleteType require_complete;
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name:
> "UnicodeString"
> +// CHECK-NOT:              DIFlagFwdDecl
> +// CHECK-SAME:             ){{$}}
> +
>  namespace rdar14101097_1 { // see also PR16214
>  // Check that we emit debug info for the definition of a struct if the
>  // definition is available, even if it's used via a pointer wrapped in a
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160912/e8edfd51/attachment-0001.html>