Skip to content

[C++20][Modules] Fix crash when function and lambda inside loaded from different modules #109167

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions clang/include/clang/Serialization/ASTBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,12 @@ enum ASTRecordTypes {

/// Record code for vtables to emit.
VTABLES_TO_EMIT = 70,

/// Record code for the FunctionDecl to lambdas mapping. These lambdas have to
/// be loaded right after the function they belong to. It is required to have
/// canonical declaration for the lambda class from the same module as
/// enclosing function.
FUNCTION_DECL_TO_LAMBDAS_MAP = 71,
};

/// Record types used within a source manager block.
Expand Down
12 changes: 12 additions & 0 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,18 @@ class ASTReader
/// namespace as if it is not delayed.
DelayedNamespaceOffsetMapTy DelayedNamespaceOffsetMap;

/// Mapping from FunctionDecl IDs to the corresponding lambda IDs.
///
/// These lambdas have to be loaded right after the function they belong to.
/// It is required to have canonical declaration for lambda class from the
/// same module as enclosing function. This is required to correctly resolve
/// captured variables in the lambda. Without this, due to lazy
/// deserialization, canonical declarations for the function and lambdas can
/// be selected from different modules and DeclRefExprs may refer to the AST
/// nodes that don't exist in the function.
llvm::DenseMap<GlobalDeclID, SmallVector<GlobalDeclID, 4>>
FunctionToLambdasMap;

struct PendingUpdateRecord {
Decl *D;
GlobalDeclID ID;
Expand Down
8 changes: 8 additions & 0 deletions clang/include/clang/Serialization/ASTWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,14 @@ class ASTWriter : public ASTDeserializationListener,
/// instead of comparing the result of `getDeclID()` or `GetDeclRef()`.
llvm::SmallPtrSet<const Decl *, 32> PredefinedDecls;

/// Mapping from FunctionDecl to the list of lambda IDs inside the function.
///
/// These lambdas have to be loaded right after the function they belong to.
/// In order to have canonical declaration for lambda class from the same
/// module as enclosing function during deserialization.
llvm::DenseMap<const Decl *, SmallVector<LocalDeclID, 4>>
FunctionToLambdasMap;

/// Offset of each declaration in the bitstream, indexed by
/// the declaration's ID.
std::vector<serialization::DeclOffset> DeclOffsets;
Expand Down
11 changes: 11 additions & 0 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3856,6 +3856,17 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F,
break;
}

case FUNCTION_DECL_TO_LAMBDAS_MAP:
for (unsigned I = 0, N = Record.size(); I != N; /*in loop*/) {
GlobalDeclID ID = ReadDeclID(F, Record, I);
auto &Lambdas = FunctionToLambdasMap[ID];
unsigned NN = Record[I++];
Lambdas.reserve(NN);
for (unsigned II = 0; II < NN; II++)
Lambdas.push_back(ReadDeclID(F, Record, I));
}
break;

case OBJC_CATEGORIES_MAP:
if (F.LocalNumObjCCategoriesInMap != 0)
return llvm::createStringError(
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4351,6 +4351,16 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
reader::ASTDeclContextNameLookupTrait(*this, *Update.Mod));
DC->setHasExternalVisibleStorage(true);
}

// Load any pending lambdas for the function.
if (auto *FD = dyn_cast<FunctionDecl>(D); FD && FD->isCanonicalDecl()) {
if (auto IT = FunctionToLambdasMap.find(ID);
IT != FunctionToLambdasMap.end()) {
for (auto LID : IT->second)
GetDecl(LID);
FunctionToLambdasMap.erase(IT);
}
}
}

void ASTReader::loadPendingDeclChain(Decl *FirstLocal, uint64_t LocalOffset) {
Expand Down
22 changes: 22 additions & 0 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,7 @@ void ASTWriter::WriteBlockInfoBlock() {
RECORD(PENDING_IMPLICIT_INSTANTIATIONS);
RECORD(UPDATE_VISIBLE);
RECORD(DELAYED_NAMESPACE_LEXICAL_VISIBLE_RECORD);
RECORD(FUNCTION_DECL_TO_LAMBDAS_MAP);
RECORD(DECL_UPDATE_OFFSETS);
RECORD(DECL_UPDATES);
RECORD(CUDA_SPECIAL_DECL_REFS);
Expand Down Expand Up @@ -5706,6 +5707,27 @@ void ASTWriter::WriteDeclAndTypes(ASTContext &Context) {
Stream.EmitRecord(DELAYED_NAMESPACE_LEXICAL_VISIBLE_RECORD,
DelayedNamespaceRecord);

if (!FunctionToLambdasMap.empty()) {
// TODO: on disk hash table for function to lambda mapping might be more
// efficent becuase it allows lazy deserialization.
RecordData FunctionToLambdasMapRecord;
for (const auto &Pair : FunctionToLambdasMap) {
FunctionToLambdasMapRecord.push_back(
GetDeclRef(Pair.first).getRawValue());
FunctionToLambdasMapRecord.push_back(Pair.second.size());
for (const auto &Lambda : Pair.second)
FunctionToLambdasMapRecord.push_back(Lambda.getRawValue());
}

auto Abv = std::make_shared<llvm::BitCodeAbbrev>();
Abv->Add(llvm::BitCodeAbbrevOp(FUNCTION_DECL_TO_LAMBDAS_MAP));
Abv->Add(llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Array));
Abv->Add(llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::VBR, 6));
unsigned FunctionToLambdaMapAbbrev = Stream.EmitAbbrev(std::move(Abv));
Stream.EmitRecord(FUNCTION_DECL_TO_LAMBDAS_MAP, FunctionToLambdasMapRecord,
FunctionToLambdaMapAbbrev);
}

const TranslationUnitDecl *TU = Context.getTranslationUnitDecl();
// Create a lexical update block containing all of the declarations in the
// translation unit that do not come from other AST files.
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Serialization/ASTWriterDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,11 @@ void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
} else {
Record.push_back(0);
}
// For lambdas inside canonical FunctionDecl remember the mapping.
if (auto FD = llvm::dyn_cast_or_null<FunctionDecl>(D->getDeclContext());
FD && FD->isCanonicalDecl()) {
Writer.FunctionToLambdasMap[FD].push_back(Writer.GetDeclRef(D));
}
} else {
Record.push_back(CXXRecNotTemplate);
}
Expand Down
76 changes: 76 additions & 0 deletions clang/test/Headers/crash-instantiated-in-scope-cxx-modules.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// RUN: rm -fR %t
// RUN: split-file %s %t
// RUN: cd %t
// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header -Werror=uninitialized folly-conv.h
// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header -Werror=uninitialized thrift_cpp2_base.h
// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header -Werror=uninitialized -fmodule-file=folly-conv.pcm -fmodule-file=thrift_cpp2_base.pcm logger_base.h

//--- Conv.h
#pragma once

template <typename _Tp, typename _Up = _Tp&&>
_Up __declval(int);

template <typename _Tp>
auto declval() noexcept -> decltype(__declval<_Tp>(0));

namespace folly {

template <class Value, class Error>
struct Expected {
template <class Yes>
auto thenOrThrow() -> decltype(declval<Value&>()) {
return 1;
}
};

struct ExpectedHelper {
template <class Error, class T>
static constexpr Expected<T, Error> return_(T) {
return Expected<T, Error>();
}

template <class This, class Fn, class E = int, class T = ExpectedHelper>
static auto then_(This&&, Fn&&)
-> decltype(T::template return_<E>((declval<Fn>()(true), 0))) {
return Expected<int, int>();
}
};

template <class Tgt>
inline Expected<Tgt, const char*> tryTo() {
Tgt result = 0;
// In build with asserts:
// clang/lib/Sema/SemaTemplateInstantiate.cpp: llvm::PointerUnion<Decl *, LocalInstantiationScope::DeclArgumentPack *> *clang::LocalInstantiationScope::findInstantiationOf(const Decl *): Assertion `isa<LabelDecl>(D) && "declaration not instantiated in this scope"' failed.
// In release build compilation error on the line below inside lambda:
// error: variable 'result' is uninitialized when used here [-Werror,-Wuninitialized]
ExpectedHelper::then_(Expected<bool, int>(), [&](bool) { return result; });
return {};
}

} // namespace folly

inline void bar() {
folly::tryTo<int>();
}
// expected-no-diagnostics

//--- folly-conv.h
#pragma once
#include "Conv.h"
// expected-no-diagnostics

//--- thrift_cpp2_base.h
#pragma once
#include "Conv.h"
// expected-no-diagnostics

//--- logger_base.h
#pragma once
import "folly-conv.h";
import "thrift_cpp2_base.h";

inline void foo() {
folly::tryTo<unsigned>();
}
// expected-no-diagnostics
30 changes: 30 additions & 0 deletions clang/test/Headers/crash-instantiated-in-scope-cxx-modules2.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// RUN: rm -fR %t
// RUN: split-file %s %t
// RUN: cd %t
// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header header.h
// RUN: %clang_cc1 -std=c++20 -fmodule-file=header.pcm main.cpp

//--- header.h
template <typename T>
void f(T) {}

class A {
virtual ~A();
};

inline A::~A() {
f([](){});
}

struct B {
void g() {
f([](){
[](){};
});
}
};
// expected-no-diagnostics

//--- main.cpp
import "header.h";
// expected-no-diagnostics
26 changes: 26 additions & 0 deletions clang/test/Headers/crash-instantiated-in-scope-cxx-modules3.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// RUN: %clang_cc1 %s -std=c++11 -emit-pch -o %t
// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -fsyntax-only -verify

// expected-no-diagnostics
#ifndef HEADER
#define HEADER

// No crash or assertion failure on multiple nested lambdas deserialization.
template <typename T>
void b() {
[] {
[]{
[]{
[]{
[]{
}();
}();
}();
}();
}();
}

void foo() {
b<int>();
}
#endif
Loading