Skip to content

Commit c227192

Browse files
committed
Make clang-format fuzz through Lexing with asserts enabled.
Makes clang-format bail out if an in-memory source file with an unsupported BOM is handed in instead of creating source locations that are violating clang's assumptions. In the future, we should add support to better transport error messages like this through clang-format instead of printing to stderr and not creating any changes.
1 parent 30b27ec commit c227192

File tree

5 files changed

+85
-23
lines changed

5 files changed

+85
-23
lines changed

clang/lib/Format/Format.cpp

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2989,8 +2989,10 @@ reformat(const FormatStyle &Style, StringRef Code,
29892989
if (Style.isJson()) {
29902990
std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
29912991
auto Env =
2992-
std::make_unique<Environment>(Code, FileName, Ranges, FirstStartColumn,
2992+
Environment::make(Code, FileName, Ranges, FirstStartColumn,
29932993
NextStartColumn, LastStartColumn);
2994+
if (!Env)
2995+
return {};
29942996
// Perform the actual formatting pass.
29952997
tooling::Replacements Replaces =
29962998
Formatter(*Env, Style, Status).process().first;
@@ -3046,9 +3048,10 @@ reformat(const FormatStyle &Style, StringRef Code,
30463048
return TrailingCommaInserter(Env, Expanded).process();
30473049
});
30483050

3049-
auto Env =
3050-
std::make_unique<Environment>(Code, FileName, Ranges, FirstStartColumn,
3051-
NextStartColumn, LastStartColumn);
3051+
auto Env = Environment::make(Code, FileName, Ranges, FirstStartColumn,
3052+
NextStartColumn, LastStartColumn);
3053+
if (!Env)
3054+
return {};
30523055
llvm::Optional<std::string> CurrentCode = None;
30533056
tooling::Replacements Fixes;
30543057
unsigned Penalty = 0;
@@ -3061,10 +3064,12 @@ reformat(const FormatStyle &Style, StringRef Code,
30613064
Penalty += PassFixes.second;
30623065
if (I + 1 < E) {
30633066
CurrentCode = std::move(*NewCode);
3064-
Env = std::make_unique<Environment>(
3067+
Env = Environment::make(
30653068
*CurrentCode, FileName,
30663069
tooling::calculateRangesAfterReplacements(Fixes, Ranges),
30673070
FirstStartColumn, NextStartColumn, LastStartColumn);
3071+
if (!Env)
3072+
return {};
30683073
}
30693074
}
30703075
}
@@ -3090,7 +3095,10 @@ tooling::Replacements cleanup(const FormatStyle &Style, StringRef Code,
30903095
// cleanups only apply to C++ (they mostly concern ctor commas etc.)
30913096
if (Style.Language != FormatStyle::LK_Cpp)
30923097
return tooling::Replacements();
3093-
return Cleaner(Environment(Code, FileName, Ranges), Style).process().first;
3098+
auto Env = Environment::make(Code, FileName, Ranges);
3099+
if (!Env)
3100+
return {};
3101+
return Cleaner(*Env, Style).process().first;
30943102
}
30953103

30963104
tooling::Replacements reformat(const FormatStyle &Style, StringRef Code,
@@ -3107,7 +3115,10 @@ tooling::Replacements fixNamespaceEndComments(const FormatStyle &Style,
31073115
StringRef Code,
31083116
ArrayRef<tooling::Range> Ranges,
31093117
StringRef FileName) {
3110-
return NamespaceEndCommentsFixer(Environment(Code, FileName, Ranges), Style)
3118+
auto Env = Environment::make(Code, FileName, Ranges);
3119+
if (!Env)
3120+
return {};
3121+
return NamespaceEndCommentsFixer(*Env, Style)
31113122
.process()
31123123
.first;
31133124
}
@@ -3116,7 +3127,10 @@ tooling::Replacements sortUsingDeclarations(const FormatStyle &Style,
31163127
StringRef Code,
31173128
ArrayRef<tooling::Range> Ranges,
31183129
StringRef FileName) {
3119-
return UsingDeclarationsSorter(Environment(Code, FileName, Ranges), Style)
3130+
auto Env = Environment::make(Code, FileName, Ranges);
3131+
if (!Env)
3132+
return {};
3133+
return UsingDeclarationsSorter(*Env, Style)
31203134
.process()
31213135
.first;
31223136
}

clang/lib/Format/QualifierAlignmentFixer.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ QualifierAlignmentFixer::QualifierAlignmentFixer(
6161
std::pair<tooling::Replacements, unsigned> QualifierAlignmentFixer::analyze(
6262
TokenAnnotator &Annotator, SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
6363
FormatTokenLexer &Tokens) {
64-
65-
auto Env =
66-
std::make_unique<Environment>(Code, FileName, Ranges, FirstStartColumn,
67-
NextStartColumn, LastStartColumn);
64+
auto Env = Environment::make(Code, FileName, Ranges, FirstStartColumn,
65+
NextStartColumn, LastStartColumn);
66+
if (!Env)
67+
return {};
6868
llvm::Optional<std::string> CurrentCode = None;
6969
tooling::Replacements Fixes;
7070
for (size_t I = 0, E = Passes.size(); I < E; ++I) {
@@ -75,10 +75,12 @@ std::pair<tooling::Replacements, unsigned> QualifierAlignmentFixer::analyze(
7575
Fixes = Fixes.merge(PassFixes.first);
7676
if (I + 1 < E) {
7777
CurrentCode = std::move(*NewCode);
78-
Env = std::make_unique<Environment>(
78+
Env = Environment::make(
7979
*CurrentCode, FileName,
8080
tooling::calculateRangesAfterReplacements(Fixes, Ranges),
8181
FirstStartColumn, NextStartColumn, LastStartColumn);
82+
if (!Env)
83+
return {};
8284
}
8385
}
8486
}

clang/lib/Format/SortJavaScriptImports.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,10 @@ tooling::Replacements sortJavaScriptImports(const FormatStyle &Style,
550550
ArrayRef<tooling::Range> Ranges,
551551
StringRef FileName) {
552552
// FIXME: Cursor support.
553-
return JavaScriptImportSorter(Environment(Code, FileName, Ranges), Style)
553+
auto Env = Environment::make(Code, FileName, Ranges);
554+
if (!Env)
555+
return {};
556+
return JavaScriptImportSorter(*Env, Style)
554557
.process()
555558
.first;
556559
}

clang/lib/Format/TokenAnalyzer.cpp

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,26 +26,61 @@
2626
#include "clang/Basic/SourceManager.h"
2727
#include "clang/Format/Format.h"
2828
#include "llvm/ADT/STLExtras.h"
29+
#include "llvm/ADT/SmallVector.h"
2930
#include "llvm/Support/Debug.h"
31+
#include <type_traits>
3032

3133
#define DEBUG_TYPE "format-formatter"
3234

3335
namespace clang {
3436
namespace format {
3537

38+
// FIXME: Instead of printing the diagnostic we should store it and have a
39+
// better way to return errors through the format APIs.
40+
class FatalDiagnosticConsumer: public DiagnosticConsumer {
41+
public:
42+
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
43+
const Diagnostic &Info) override {
44+
if (DiagLevel == DiagnosticsEngine::Fatal) {
45+
Fatal = true;
46+
llvm::SmallVector<char, 128> Message;
47+
Info.FormatDiagnostic(Message);
48+
llvm::errs() << Message << "\n";
49+
}
50+
}
51+
52+
bool fatalError() const { return Fatal; }
53+
54+
private:
55+
bool Fatal = false;
56+
};
57+
58+
std::unique_ptr<Environment>
59+
Environment::make(StringRef Code, StringRef FileName,
60+
ArrayRef<tooling::Range> Ranges, unsigned FirstStartColumn,
61+
unsigned NextStartColumn, unsigned LastStartColumn) {
62+
auto Env = std::make_unique<Environment>(Code, FileName, FirstStartColumn,
63+
NextStartColumn, LastStartColumn);
64+
FatalDiagnosticConsumer Diags;
65+
Env->SM.getDiagnostics().setClient(&Diags, /*ShouldOwnClient=*/false);
66+
SourceLocation StartOfFile = Env->SM.getLocForStartOfFile(Env->ID);
67+
for (const tooling::Range &Range : Ranges) {
68+
SourceLocation Start = StartOfFile.getLocWithOffset(Range.getOffset());
69+
SourceLocation End = Start.getLocWithOffset(Range.getLength());
70+
Env->CharRanges.push_back(CharSourceRange::getCharRange(Start, End));
71+
}
72+
// Validate that we can get the buffer data without a fatal error.
73+
Env->SM.getBufferData(Env->ID);
74+
if (Diags.fatalError()) return nullptr;
75+
return Env;
76+
}
77+
3678
Environment::Environment(StringRef Code, StringRef FileName,
37-
ArrayRef<tooling::Range> Ranges,
3879
unsigned FirstStartColumn, unsigned NextStartColumn,
3980
unsigned LastStartColumn)
4081
: VirtualSM(new SourceManagerForFile(FileName, Code)), SM(VirtualSM->get()),
4182
ID(VirtualSM->get().getMainFileID()), FirstStartColumn(FirstStartColumn),
4283
NextStartColumn(NextStartColumn), LastStartColumn(LastStartColumn) {
43-
SourceLocation StartOfFile = SM.getLocForStartOfFile(ID);
44-
for (const tooling::Range &Range : Ranges) {
45-
SourceLocation Start = StartOfFile.getLocWithOffset(Range.getOffset());
46-
SourceLocation End = Start.getLocWithOffset(Range.getLength());
47-
CharRanges.push_back(CharSourceRange::getCharRange(Start, End));
48-
}
4984
}
5085

5186
TokenAnalyzer::TokenAnalyzer(const Environment &Env, const FormatStyle &Style)

clang/lib/Format/TokenAnalyzer.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "clang/Format/Format.h"
3030
#include "llvm/ADT/STLExtras.h"
3131
#include "llvm/Support/Debug.h"
32+
#include <memory>
3233

3334
namespace clang {
3435
namespace format {
@@ -40,8 +41,7 @@ class Environment {
4041
// that the next lines of \p Code should start at \p NextStartColumn, and
4142
// that \p Code should end at \p LastStartColumn if it ends in newline.
4243
// See also the documentation of clang::format::internal::reformat.
43-
Environment(StringRef Code, StringRef FileName,
44-
ArrayRef<tooling::Range> Ranges, unsigned FirstStartColumn = 0,
44+
Environment(StringRef Code, StringRef FileName, unsigned FirstStartColumn = 0,
4545
unsigned NextStartColumn = 0, unsigned LastStartColumn = 0);
4646

4747
FileID getFileID() const { return ID; }
@@ -62,6 +62,14 @@ class Environment {
6262
// environment should end if it ends in a newline.
6363
unsigned getLastStartColumn() const { return LastStartColumn; }
6464

65+
// Returns nullptr and prints a diagnostic to stderr if the environment
66+
// can't be created.
67+
static std::unique_ptr<Environment> make(StringRef Code, StringRef FileName,
68+
ArrayRef<tooling::Range> Ranges,
69+
unsigned FirstStartColumn = 0,
70+
unsigned NextStartColumn = 0,
71+
unsigned LastStartColumn = 0);
72+
6573
private:
6674
// This is only set if constructed from string.
6775
std::unique_ptr<SourceManagerForFile> VirtualSM;

0 commit comments

Comments
 (0)