[PATCH] Fixed double-free in case of module loading error.

[David Blaikie]

On Fri, May 8, 2015 at 3:11 PM, Artem Belevich <tra at google.com> wrote:

> Hi echristo, rnk, dblaikie,
>
>
> GetOutputStream() owns the stream it returns pointer to and the
> pointer should never be freed by us. When we fail to load and exit
> early, unique_ptr still holds the pointer and frees it which leads to
> compiler crash when CompilerInstance attempts to free it again.
>
> Added regression test for failed bitcode linking.
>
> http://reviews.llvm.org/D9625
>
> Files:
>   lib/CodeGen/CodeGenAction.cpp
>   test/CodeGen/link-bitcode-file.c
>
> Index: lib/CodeGen/CodeGenAction.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenAction.cpp
> +++ lib/CodeGen/CodeGenAction.cpp
> @@ -629,7 +629,7 @@
>  std::unique_ptr<ASTConsumer>
>  CodeGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
>    BackendAction BA = static_cast<BackendAction>(Act);
> -  std::unique_ptr<raw_pwrite_stream> OS(GetOutputStream(CI, InFile, BA));
> +  raw_pwrite_stream *OS(GetOutputStream(CI, InFile, BA));
>

Use = rather than () where both are valid (such as here):

raw_pwrite_stream *OS = GetOutputStream(...);

Otherwise looks good.

If you wanted to do some cleanup, perhaps GetOutputStream should return by
reference so there's no confusion about ownership passing.


>    if (BA != Backend_EmitNothing && !OS)
>      return nullptr;
>
> @@ -666,7 +666,7 @@
>    std::unique_ptr<BackendConsumer> Result(new BackendConsumer(
>        BA, CI.getDiagnostics(), CI.getCodeGenOpts(), CI.getTargetOpts(),
>        CI.getLangOpts(), CI.getFrontendOpts().ShowTimers, InFile,
> -      LinkModuleToUse, OS.release(), *VMContext, CoverageInfo));
> +      LinkModuleToUse, OS, *VMContext, CoverageInfo));
>    BEConsumer = Result.get();
>    return std::move(Result);
>  }
> Index: test/CodeGen/link-bitcode-file.c
> ===================================================================
> --- test/CodeGen/link-bitcode-file.c
> +++ test/CodeGen/link-bitcode-file.c
> @@ -1,6 +1,9 @@
>  // RUN: %clang_cc1 -triple i386-pc-linux-gnu -DBITCODE -emit-llvm-bc -o
> %t.bc %s
>  // RUN: %clang_cc1 -triple i386-pc-linux-gnu -mlink-bitcode-file %t.bc
> -O3 -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-NO-BC %s
>  // RUN: not %clang_cc1 -triple i386-pc-linux-gnu -DBITCODE
> -mlink-bitcode-file %t.bc -O3 -emit-llvm -o - %s 2>&1 | FileCheck
> -check-prefix=CHECK-BC %s
> +// Make sure we deal with failure to load the file.
> +// RUN: not %clang_cc1 -triple i386-pc-linux-gnu -mlink-bitcode-file
> no-such-file.bc \
> +// RUN:    -emit-llvm -o - %s 2>&1 | FileCheck
> -check-prefix=CHECK-NO-FILE %s
>
>  int f(void);
>
> @@ -22,3 +25,5 @@
>  // CHECK-NO-BC-LABEL: define i32 @f
>
>  #endif
> +
> +// CHECK-NO-FILE: fatal error: cannot open file 'no-such-file.bc'
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150508/d5dffcd5/attachment.html>