Skip to content

Commit 2de6c52

Browse files
zixu-wdaniel-grumberg
authored andcommitted
Revert "Revert "[clang][extract-api] Use relative includes""
Reapply the change after fixing sanitizer errors. The original problem was that `StringRef`s in `Matches` are pointing to temporary local `std::string`s created by `path::convert_to_slash` in the regex match call. This patch does the conversion up front in container `FilePath`. This reverts commit 2966f0f. Differential Revision: https://reviews.llvm.org/D124964
1 parent 5c2b3ab commit 2de6c52

File tree

4 files changed

+375
-189
lines changed

4 files changed

+375
-189
lines changed

clang/include/clang/ExtractAPI/FrontendActions.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ class ExtractAPIAction : public ASTFrontendAction {
4040
std::unique_ptr<llvm::MemoryBuffer> Buffer;
4141

4242
/// The input file originally provided on the command line.
43-
std::vector<std::string> KnownInputFiles;
43+
///
44+
/// This captures the spelling used to include the file and whether the
45+
/// include is quoted or not.
46+
SmallVector<std::pair<SmallString<32>, bool>> KnownInputFiles;
4447

4548
/// Prepare to execute the action on the given CompilerInstance.
4649
///

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Lines changed: 178 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@
3838
#include "llvm/ADT/DenseSet.h"
3939
#include "llvm/ADT/STLExtras.h"
4040
#include "llvm/ADT/SmallVector.h"
41+
#include "llvm/Support/FileSystem.h"
4142
#include "llvm/Support/MemoryBuffer.h"
43+
#include "llvm/Support/Path.h"
44+
#include "llvm/Support/Regex.h"
4245
#include "llvm/Support/raw_ostream.h"
4346
#include <memory>
4447
#include <utility>
@@ -55,10 +58,125 @@ StringRef getTypedefName(const TagDecl *Decl) {
5558
return {};
5659
}
5760

61+
Optional<std::string> getRelativeIncludeName(const CompilerInstance &CI,
62+
StringRef File,
63+
bool *IsQuoted = nullptr) {
64+
assert(CI.hasFileManager() &&
65+
"CompilerInstance does not have a FileNamager!");
66+
67+
using namespace llvm::sys;
68+
// Matches framework include patterns
69+
const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
70+
71+
const auto &FS = CI.getVirtualFileSystem();
72+
73+
SmallString<128> FilePath(File.begin(), File.end());
74+
FS.makeAbsolute(FilePath);
75+
path::remove_dots(FilePath, true);
76+
FilePath = path::convert_to_slash(FilePath);
77+
File = FilePath;
78+
79+
// Checks whether `Dir` is a strict path prefix of `File`. If so returns
80+
// the prefix length. Otherwise return 0.
81+
auto CheckDir = [&](llvm::StringRef Dir) -> unsigned {
82+
llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
83+
FS.makeAbsolute(DirPath);
84+
path::remove_dots(DirPath, true);
85+
Dir = DirPath;
86+
for (auto NI = path::begin(File), NE = path::end(File),
87+
DI = path::begin(Dir), DE = path::end(Dir);
88+
/*termination condition in loop*/; ++NI, ++DI) {
89+
// '.' components in File are ignored.
90+
while (NI != NE && *NI == ".")
91+
++NI;
92+
if (NI == NE)
93+
break;
94+
95+
// '.' components in Dir are ignored.
96+
while (DI != DE && *DI == ".")
97+
++DI;
98+
99+
// Dir is a prefix of File, up to '.' components and choice of path
100+
// separators.
101+
if (DI == DE)
102+
return NI - path::begin(File);
103+
104+
// Consider all path separators equal.
105+
if (NI->size() == 1 && DI->size() == 1 &&
106+
path::is_separator(NI->front()) && path::is_separator(DI->front()))
107+
continue;
108+
109+
// Special case Apple .sdk folders since the search path is typically a
110+
// symlink like `iPhoneSimulator14.5.sdk` while the file is instead
111+
// located in `iPhoneSimulator.sdk` (the real folder).
112+
if (NI->endswith(".sdk") && DI->endswith(".sdk")) {
113+
StringRef NBasename = path::stem(*NI);
114+
StringRef DBasename = path::stem(*DI);
115+
if (DBasename.startswith(NBasename))
116+
continue;
117+
}
118+
119+
if (*NI != *DI)
120+
break;
121+
}
122+
return 0;
123+
};
124+
125+
unsigned PrefixLength = 0;
126+
127+
// Go through the search paths and find the first one that is a prefix of
128+
// the header.
129+
for (const auto &Entry : CI.getHeaderSearchOpts().UserEntries) {
130+
// Note whether the match is found in a quoted entry.
131+
if (IsQuoted)
132+
*IsQuoted = Entry.Group == frontend::Quoted;
133+
134+
if (auto EntryFile = CI.getFileManager().getOptionalFileRef(Entry.Path)) {
135+
if (auto HMap = HeaderMap::Create(*EntryFile, CI.getFileManager())) {
136+
// If this is a headermap entry, try to reverse lookup the full path
137+
// for a spelled name before mapping.
138+
StringRef SpelledFilename = HMap->reverseLookupFilename(File);
139+
if (!SpelledFilename.empty())
140+
return SpelledFilename.str();
141+
142+
// No matching mapping in this headermap, try next search entry.
143+
continue;
144+
}
145+
}
146+
147+
// Entry is a directory search entry, try to check if it's a prefix of File.
148+
PrefixLength = CheckDir(Entry.Path);
149+
if (PrefixLength > 0) {
150+
// The header is found in a framework path, construct the framework-style
151+
// include name `<Framework/Header.h>`
152+
if (Entry.IsFramework) {
153+
SmallVector<StringRef, 4> Matches;
154+
Rule.match(File, &Matches);
155+
// Returned matches are always in stable order.
156+
if (Matches.size() != 4)
157+
return None;
158+
159+
return path::convert_to_slash(
160+
(Matches[1].drop_front(Matches[1].rfind('/') + 1) + "/" +
161+
Matches[3])
162+
.str());
163+
}
164+
165+
// The header is found in a normal search path, strip the search path
166+
// prefix to get an include name.
167+
return path::convert_to_slash(File.drop_front(PrefixLength));
168+
}
169+
}
170+
171+
// Couldn't determine a include name, use full path instead.
172+
return None;
173+
}
174+
58175
struct LocationFileChecker {
59176
bool isLocationInKnownFile(SourceLocation Loc) {
60177
// If the loc refers to a macro expansion we need to first get the file
61178
// location of the expansion.
179+
auto &SM = CI.getSourceManager();
62180
auto FileLoc = SM.getFileLoc(Loc);
63181
FileID FID = SM.getFileID(FileLoc);
64182
if (FID.isInvalid())
@@ -71,20 +189,44 @@ struct LocationFileChecker {
71189
if (KnownFileEntries.count(File))
72190
return true;
73191

192+
if (ExternalFileEntries.count(File))
193+
return false;
194+
195+
StringRef FileName = File->tryGetRealPathName().empty()
196+
? File->getName()
197+
: File->tryGetRealPathName();
198+
199+
// Try to reduce the include name the same way we tried to include it.
200+
bool IsQuoted = false;
201+
if (auto IncludeName = getRelativeIncludeName(CI, FileName, &IsQuoted))
202+
if (llvm::find_if(KnownFiles,
203+
[&IsQuoted, &IncludeName](const auto &KnownFile) {
204+
return KnownFile.first.equals(*IncludeName) &&
205+
KnownFile.second == IsQuoted;
206+
}) != KnownFiles.end()) {
207+
KnownFileEntries.insert(File);
208+
return true;
209+
}
210+
211+
// Record that the file was not found to avoid future reverse lookup for
212+
// the same file.
213+
ExternalFileEntries.insert(File);
74214
return false;
75215
}
76216

77-
LocationFileChecker(const SourceManager &SM,
78-
const std::vector<std::string> &KnownFiles)
79-
: SM(SM) {
80-
for (const auto &KnownFilePath : KnownFiles)
81-
if (auto FileEntry = SM.getFileManager().getFile(KnownFilePath))
217+
LocationFileChecker(const CompilerInstance &CI,
218+
SmallVector<std::pair<SmallString<32>, bool>> &KnownFiles)
219+
: CI(CI), KnownFiles(KnownFiles), ExternalFileEntries() {
220+
for (const auto &KnownFile : KnownFiles)
221+
if (auto FileEntry = CI.getFileManager().getFile(KnownFile.first))
82222
KnownFileEntries.insert(*FileEntry);
83223
}
84224

85225
private:
86-
const SourceManager &SM;
226+
const CompilerInstance &CI;
227+
SmallVector<std::pair<SmallString<32>, bool>> &KnownFiles;
87228
llvm::DenseSet<const FileEntry *> KnownFileEntries;
229+
llvm::DenseSet<const FileEntry *> ExternalFileEntries;
88230
};
89231

90232
/// The RecursiveASTVisitor to traverse symbol declarations and collect API
@@ -743,8 +885,7 @@ ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
743885
CI.getTarget().getTriple(),
744886
CI.getFrontendOpts().Inputs.back().getKind().getLanguage());
745887

746-
auto LCF = std::make_unique<LocationFileChecker>(CI.getSourceManager(),
747-
KnownInputFiles);
888+
auto LCF = std::make_unique<LocationFileChecker>(CI, KnownInputFiles);
748889

749890
CI.getPreprocessor().addPPCallbacks(std::make_unique<MacroCallback>(
750891
CI.getSourceManager(), *LCF, *API, CI.getPreprocessor()));
@@ -758,22 +899,47 @@ bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) {
758899
if (Inputs.empty())
759900
return true;
760901

902+
if (!CI.hasFileManager())
903+
if (!CI.createFileManager())
904+
return false;
905+
761906
auto Kind = Inputs[0].getKind();
762907

763908
// Convert the header file inputs into a single input buffer.
764909
SmallString<256> HeaderContents;
910+
bool IsQuoted = false;
765911
for (const FrontendInputFile &FIF : Inputs) {
766912
if (Kind.isObjectiveC())
767913
HeaderContents += "#import";
768914
else
769915
HeaderContents += "#include";
770-
HeaderContents += " \"";
771-
HeaderContents += FIF.getFile();
772-
HeaderContents += "\"\n";
773916

774-
KnownInputFiles.emplace_back(FIF.getFile());
917+
StringRef FilePath = FIF.getFile();
918+
if (auto RelativeName = getRelativeIncludeName(CI, FilePath, &IsQuoted)) {
919+
if (IsQuoted)
920+
HeaderContents += " \"";
921+
else
922+
HeaderContents += " <";
923+
924+
HeaderContents += *RelativeName;
925+
926+
if (IsQuoted)
927+
HeaderContents += "\"\n";
928+
else
929+
HeaderContents += ">\n";
930+
KnownInputFiles.emplace_back(*RelativeName, IsQuoted);
931+
} else {
932+
HeaderContents += " \"";
933+
HeaderContents += FilePath;
934+
HeaderContents += "\"\n";
935+
KnownInputFiles.emplace_back(FilePath, true);
936+
}
775937
}
776938

939+
if (CI.getHeaderSearchOpts().Verbose)
940+
CI.getVerboseOutputStream() << getInputBufferName() << ":\n"
941+
<< HeaderContents << "\n";
942+
777943
Buffer = llvm::MemoryBuffer::getMemBufferCopy(HeaderContents,
778944
getInputBufferName());
779945

0 commit comments

Comments
 (0)