Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clang can't read back a PCH just produced: Assertion failed: (D && "Cannot get layout of forward declarations!"), function getASTRecordLayout #31211

Closed
joker-eph opened this issue Feb 4, 2017 · 18 comments
Closed

Clang can't read back a PCH just produced: Assertion failed: (D && "Cannot get layout of forward declarations!"), function getASTRecordLayout #31211

joker-eph opened this issue Feb 4, 2017 · 18 comments
Assignees
@zygoloid
Labels
bugzilla Issues migrated from bugzilla clang:modules C++20 modules and Clang Header Modules

Comments

@joker-eph
Copy link
Collaborator

Bugzilla Link 31863
Resolution FIXED
Resolved on Jun 05, 2017 15:21
Version 4.0
OS All
Blocks #31409
Attachments Repro reduced
CC @bcardosolopes,@dexonsmith,@DougGregor,@gnzlbg,@zmodem,@zygoloid,@tstellar,@vgvassilev

Extended Description

Clang-4.0 (and trunk) are crashing when building Unreal Engine, a PCH can't be read back we hit this assertion. I believe this is a regression from clang-3.9 introduced in r276159:

Author: Richard Smith richard-llvm@metafoo.co.uk
Date: Wed Jul 20 14:10:16 2016

[modules] Don't emit initializers for VarDecls within a module eagerly whenever
we first touch any part of that module. Instead, defer them until the first
time that module is (transitively) imported. The initializer step for a module
then recursively initializes modules that its own headers imported.

For example, this avoids running the <iostream> global initializer in programs
that don't actually use iostreams, but do use other parts of the standard
library.

Reduced repro attached. Run with:

clang -c -x objective-c++-header pre.h -std=c++14 -o pre.gch
clang -c -x objective-c++ /dev/null -std=c++14 -include-pch pre.gch
@joker-eph
Copy link
Collaborator Author

assigned to @zygoloid

@dexonsmith
Copy link
Mannequin

dexonsmith mannequin commented Feb 4, 2017

clang -c -x objective-c++-header pre.h -std=c++14 -o pre.gch
clang -c -x objective-c++ /dev/null -std=c++14 -include-pch pre.gch

Note: the same repro works with -x c++-header and -x c++.

@bcardosolopes
Copy link
Member

@​rsmith, I've attempted a fix, can you take a look? https://reviews.llvm.org/D29753

@bcardosolopes
Copy link
Member

A stopgap fix for PCH was committed in r296656.

The problem still happens with modules though. Richard triggered the issue with:

$ cat test/PCH/empty-def-fwd-struct.modulemap
module M { header "empty-def-fwd-struct.h" }
$ clang -cc1 -x c++ -std=c++14 -fmodules -emit-module -fmodule-name=M test/PCH/empty-def-fwd-struct.modulemap -o tmp.pcm
$ cat use.cpp
#include "test/PCH/empty-def-fwd-struct.h"
$ clang -cc1 -x c++ -std=c++14 -fmodules -emit-llvm -fmodule-file=tmp.pcm use.cpp -o -
clang: [...]/src/tools/clang/lib/AST/RecordLayoutBuilder.cpp:2933: const clang::ASTRecordLayout &clang::ASTContext::getASTRecordLayout(const clang::RecordDecl *) const: Assertion `D && "Cannot get layout of forward declarations!"' failed.

Leaving this open until we find a more complete solution.

@zmodem
Copy link
Collaborator

zmodem commented Mar 1, 2017

A stopgap fix for PCH was committed in r296656.

Thanks! I will merge it to the release branch once it's been in tree for a bit.

@zmodem
Copy link
Collaborator

zmodem commented Mar 2, 2017

A stopgap fix for PCH was committed in r296656.

Thanks! I will merge it to the release branch once it's been in tree for a
bit.

Merged to 4.0 in r296762.

If we can fix this properly for 4.0.1 that would be great, so marking it a blocker of that.

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Mar 9, 2017

*** Bug llvm/llvm-bugzilla-archive#32186 has been marked as a duplicate of this bug. ***

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Mar 9, 2017

llvm/llvm-bugzilla-archive#32186 has a totally unrelated symptom with the same root cause (the call to DeclMustBeEmitted in a context where AST invariants are not met during deserialization).

@gnzlbg
Copy link
Mannequin

gnzlbg mannequin commented Mar 10, 2017

*** Bug llvm/llvm-bugzilla-archive#32144 has been marked as a duplicate of this bug. ***

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Apr 26, 2017

This was mostly addressed by https://reviews.llvm.org/D30793 / r300110.
A follow-up fix is under review in https://reviews.llvm.org/D32499.

@vgvassilev
Copy link
Contributor

Second part of the fix landed in r303432.

@tstellar
Copy link
Collaborator

Richard, now that the fixes have been merged, are https://reviews.llvm.org/rL300110 and https://reviews.llvm.org/rL303432 OK to merge to the release_40 branch?

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented May 19, 2017

r303432 is very new; would you prefer to merge it now and revert it from the branch if people have problems with it on trunk, or wait a few days and merge it if there are no complications? (I'm not anticipating any problems, just cautious about merging a patch that's not really been field-tested.)

I'm fine with merging r300110 now.

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Jun 5, 2017

*** Bug llvm/llvm-bugzilla-archive#33313 has been marked as a duplicate of this bug. ***

@jsonn
Copy link
Contributor

jsonn commented Nov 26, 2021

mentioned in issue #31409

@gnzlbg
Copy link
Mannequin

gnzlbg mannequin commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#32144

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#32186

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#33313

@jsonn jsonn mentioned this issue Feb 24, 2017
Closed
@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zygoloid zygoloid

Labels
bugzilla Issues migrated from bugzilla clang:modules C++20 modules and Clang Header Modules
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@tstellar @jsonn @zmodem @joker-eph @vgvassilev @bcardosolopes