Fix memory leak of raw_ostreams in LogDiagnosticPrinter handling.

This is another case of conditional ownership (in this case a raw
reference, plus a boolean to indicate whether the referenced object
should be deleted). While it's not ideal, I prefer to make the ownership
explicit with a unique_ptr than using a boolean flag (though it does
make the reference and the unique_ptr redundant in the sense that they
both refer to the same memory). At some point we might write a reusable
conditional ownership pointer (a stateful custom deleter for a unique_ptr
may be appropriate).

Based on a patch from a patch by Anton Yartsev.

llvm-svn: 217791
This commit is contained in:
David Blaikie 2014-09-15 17:30:56 +00:00
parent 8f45c9cc62
commit 11f8a943bf
3 changed files with 17 additions and 21 deletions

View File

@ -43,13 +43,16 @@ class LogDiagnosticPrinter : public DiagnosticConsumer {
void EmitDiagEntry(llvm::raw_ostream &OS,
const LogDiagnosticPrinter::DiagEntry &DE);
// Conditional ownership (when StreamOwner is non-null, it's keeping OS
// alive). We might want to replace this with a wrapper for conditional
// ownership eventually - it seems to pop up often enough.
raw_ostream &OS;
std::unique_ptr<raw_ostream> StreamOwner;
const LangOptions *LangOpts;
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
SourceLocation LastWarningLoc;
FullSourceLoc LastLoc;
unsigned OwnsOutputStream : 1;
SmallVector<DiagEntry, 8> Entries;
@ -58,8 +61,7 @@ class LogDiagnosticPrinter : public DiagnosticConsumer {
public:
LogDiagnosticPrinter(raw_ostream &OS, DiagnosticOptions *Diags,
bool OwnsOutputStream = false);
virtual ~LogDiagnosticPrinter();
std::unique_ptr<raw_ostream> StreamOwner);
void setDwarfDebugFlags(StringRef Value) {
DwarfDebugFlags = Value;

View File

@ -135,27 +135,27 @@ static void SetUpDiagnosticLog(DiagnosticOptions *DiagOpts,
const CodeGenOptions *CodeGenOpts,
DiagnosticsEngine &Diags) {
std::error_code EC;
bool OwnsStream = false;
std::unique_ptr<raw_ostream> StreamOwner;
raw_ostream *OS = &llvm::errs();
if (DiagOpts->DiagnosticLogFile != "-") {
// Create the output stream.
llvm::raw_fd_ostream *FileOS(new llvm::raw_fd_ostream(
auto FileOS = llvm::make_unique<llvm::raw_fd_ostream>(
DiagOpts->DiagnosticLogFile, EC,
llvm::sys::fs::F_Append | llvm::sys::fs::F_Text));
llvm::sys::fs::F_Append | llvm::sys::fs::F_Text);
if (EC) {
Diags.Report(diag::warn_fe_cc_log_diagnostics_failure)
<< DiagOpts->DiagnosticLogFile << EC.message();
} else {
FileOS->SetUnbuffered();
FileOS->SetUseAtomicWrites(true);
OS = FileOS;
OwnsStream = true;
OS = FileOS.get();
StreamOwner = std::move(FileOS);
}
}
// Chain in the diagnostic client which will log the diagnostics.
LogDiagnosticPrinter *Logger = new LogDiagnosticPrinter(*OS, DiagOpts,
OwnsStream);
LogDiagnosticPrinter *Logger =
new LogDiagnosticPrinter(*OS, DiagOpts, std::move(StreamOwner));
if (CodeGenOpts)
Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
Diags.setClient(new ChainedDiagnosticConsumer(Diags.takeClient(), Logger));

View File

@ -18,17 +18,11 @@
using namespace clang;
using namespace markup;
LogDiagnosticPrinter::LogDiagnosticPrinter(raw_ostream &os,
DiagnosticOptions *diags,
bool _OwnsOutputStream)
: OS(os), LangOpts(nullptr), DiagOpts(diags),
OwnsOutputStream(_OwnsOutputStream) {
}
LogDiagnosticPrinter::~LogDiagnosticPrinter() {
if (OwnsOutputStream)
delete &OS;
}
LogDiagnosticPrinter::LogDiagnosticPrinter(
raw_ostream &os, DiagnosticOptions *diags,
std::unique_ptr<raw_ostream> StreamOwner)
: OS(os), StreamOwner(std::move(StreamOwner)), LangOpts(nullptr),
DiagOpts(diags) {}
static StringRef getLevelName(DiagnosticsEngine::Level Level) {
switch (Level) {