[ClangImporter] Make sure fake locations are always in /some/ buffer. (#8303)

We synthesize fake source locations for module imports that come from
Swift code; these locations have to be both valid and distinct for
Clang to use. We've mostly been getting away with simply making fake
offsets from the main file, but after the offsets start exceeding the
size of the buffer they start pointing into some /other/ file, and
then if Clang's SourceManager tries to order those locations it gets
/very/ confused.

This commit fixes that by allocating a 256K buffer of zeros and using
offsets into that instead. The hope is that a read-only mmap'd buffer
of zeros that never gets read (except possibly to look for newlines)
will be cheap to allocate.

(Why not just make the main file buffer 256K? Because we actually try
to parse that, and there's really no reason for the lexer to go crawl
through that file eagerly.)

This test case isn't the best because it doesn't actually fail in the
old code, but if only the assertion was added we at least hit that.
I did verify with the reproducing project I have that we no longer
hit this issue.

rdar://problem/30924269
This commit is contained in:
Jordan Rose
2017-03-24 16:00:50 -07:00
committed by GitHub
parent bb870aaf02
commit 0d347ac127
5 changed files with 270 additions and 3 deletions

View File

@@ -58,6 +58,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Memory.h"
#include "llvm/Support/Path.h"
#include <algorithm>
#include <memory>
@@ -206,6 +207,43 @@ namespace {
return MemoryBuffer_Malloc;
}
};
class ZeroFilledMemoryBuffer : public llvm::MemoryBuffer {
const std::string name;
public:
explicit ZeroFilledMemoryBuffer(size_t size, StringRef name)
: name(name.str()) {
assert(size > 0);
std::error_code error;
llvm::sys::MemoryBlock memory =
llvm::sys::Memory::allocateMappedMemory(size, nullptr,
llvm::sys::Memory::MF_READ,
error);
assert(!error && "failed to allocated read-only zero-filled memory");
init(static_cast<char *>(memory.base()),
static_cast<char *>(memory.base()) + memory.size() - 1,
/*null-terminated*/true);
}
~ZeroFilledMemoryBuffer() {
llvm::sys::MemoryBlock memory{const_cast<char *>(getBufferStart()),
getBufferSize()};
std::error_code error = llvm::sys::Memory::releaseMappedMemory(memory);
assert(!error && "failed to deallocate read-only zero-filled memory");
}
ZeroFilledMemoryBuffer(const ZeroFilledMemoryBuffer &) = delete;
ZeroFilledMemoryBuffer(ZeroFilledMemoryBuffer &&) = delete;
void operator=(const ZeroFilledMemoryBuffer &) = delete;
void operator=(ZeroFilledMemoryBuffer &&) = delete;
StringRef getBufferIdentifier() const override {
return name;
}
BufferKind getBufferKind() const override {
return MemoryBuffer_MMap;
}
};
} // end anonymous namespace
namespace {
@@ -1192,11 +1230,22 @@ ModuleDecl *ClangImporter::loadModule(
importLoc);
// FIXME: The source location here is completely bogus. It can't be
// invalid, and it can't be the same thing twice in a row, so we just use
// a counter. Having real source locations would be far, far better.
// invalid, it can't be the same thing twice in a row, and it has to come
// from an actual buffer, so we make a fake buffer and just use a counter.
if (!Impl.DummyImportBuffer.isValid()) {
Impl.DummyImportBuffer = srcMgr.createFileID(
llvm::make_unique<ZeroFilledMemoryBuffer>(
256*1024, StringRef(Implementation::moduleImportBufferName)),
clang::SrcMgr::C_User,
/*LoadedID*/0, /*LoadedOffset*/0,
srcMgr.getLocForStartOfFile(srcMgr.getMainFileID()));
}
clang::SourceLocation clangImportLoc
= srcMgr.getLocForStartOfFile(srcMgr.getMainFileID())
= srcMgr.getLocForStartOfFile(Impl.DummyImportBuffer)
.getLocWithOffset(Impl.ImportCounter++);
assert(srcMgr.isInFileID(clangImportLoc, Impl.DummyImportBuffer) &&
"confused Clang's source manager with our fake locations");
clang::ModuleLoadResult result =
Impl.Instance->loadModule(clangImportLoc, path, visibility,
/*IsInclusionDirective=*/false);

View File

@@ -296,7 +296,13 @@ private:
/// (through the Swift name lookup module file extension).
LookupTableMap LookupTables;
/// \brief The fake buffer used to import modules.
///
/// FIXME: Horrible hack for loadModule().
clang::FileID DummyImportBuffer;
/// \brief A count of the number of load module operations.
///
/// FIXME: Horrible, horrible hack for \c loadModule().
unsigned ImportCounter = 0;

View File

@@ -0,0 +1,100 @@
module Module1 {}
module Module2 {}
module Module3 {}
module Module4 {}
module Module5 {}
module Module6 {}
module Module7 {}
module Module8 {}
module Module9 {}
module Module10 {}
module Module11 {}
module Module12 {}
module Module13 {}
module Module14 {}
module Module15 {}
module Module16 {}
module Module17 {}
module Module18 {}
module Module19 {}
module Module20 {}
module Module21 {}
module Module22 {}
module Module23 {}
module Module24 {}
module Module25 {}
module Module26 {}
module Module27 {}
module Module28 {}
module Module29 {}
module Module30 {}
module Module31 {}
module Module32 {}
module Module33 {}
module Module34 {}
module Module35 {}
module Module36 {}
module Module37 {}
module Module38 {}
module Module39 {}
module Module40 {}
module Module41 {}
module Module42 {}
module Module43 {}
module Module44 {}
module Module45 {}
module Module46 {}
module Module47 {}
module Module48 {}
module Module49 {}
module Module50 {}
module Module51 {}
module Module52 {}
module Module53 {}
module Module54 {}
module Module55 {}
module Module56 {}
module Module57 {}
module Module58 {}
module Module59 {}
module Module60 {}
module Module61 {}
module Module62 {}
module Module63 {}
module Module64 {}
module Module65 {}
module Module66 {}
module Module67 {}
module Module68 {}
module Module69 {}
module Module70 {}
module Module71 {}
module Module72 {}
module Module73 {}
module Module74 {}
module Module75 {}
module Module76 {}
module Module77 {}
module Module78 {}
module Module79 {}
module Module80 {}
module Module81 {}
module Module82 {}
module Module83 {}
module Module84 {}
module Module85 {}
module Module86 {}
module Module87 {}
module Module88 {}
module Module89 {}
module Module90 {}
module Module91 {}
module Module92 {}
module Module93 {}
module Module94 {}
module Module95 {}
module Module96 {}
module Module97 {}
module Module98 {}
module Module99 {}
module Module100 {}

View File

@@ -0,0 +1,2 @@
/// This is a very important doc comment.
void obsoleted() __attribute__((unavailable));

View File

@@ -0,0 +1,110 @@
// RUN: not %target-swift-frontend -typecheck %s -I %S/Inputs/many-imports -import-objc-header %S/Inputs/many-imports/obsoleted.h 2>&1 | %FileCheck %s
// RUN: %target-swift-frontend -emit-pch %S/Inputs/many-imports/obsoleted.h -o %t.pch
// RUN: not %target-swift-frontend -typecheck %s -I %S/Inputs/many-imports -import-objc-header %t.pch 2>&1 | %FileCheck %s
import Module1
import Module2
import Module3
import Module4
import Module5
import Module6
import Module7
import Module8
import Module9
import Module10
import Module11
import Module12
import Module13
import Module14
import Module15
import Module16
import Module17
import Module18
import Module19
import Module20
import Module21
import Module22
import Module23
import Module24
import Module25
import Module26
import Module27
import Module28
import Module29
import Module30
import Module31
import Module32
import Module33
import Module34
import Module35
import Module36
import Module37
import Module38
import Module39
import Module40
import Module41
import Module42
import Module43
import Module44
import Module45
import Module46
import Module47
import Module48
import Module49
import Module50
import Module51
import Module52
import Module53
import Module54
import Module55
import Module56
import Module57
import Module58
import Module59
import Module60
import Module61
import Module62
import Module63
import Module64
import Module65
import Module66
import Module67
import Module68
import Module69
import Module70
import Module71
import Module72
import Module73
import Module74
import Module75
import Module76
import Module77
import Module78
import Module79
import Module80
import Module81
import Module82
import Module83
import Module84
import Module85
import Module86
import Module87
import Module88
import Module89
import Module90
import Module91
import Module92
import Module93
import Module94
import Module95
import Module96
import Module97
import Module98
import Module99
import Module100
// Trigger the diagnostic.
obsoleted()
// CHECK: [[@LINE-1]]:1: error:
// CHECK: explicitly marked unavailable here
// CHECK-NEXT: func obsoleted()