[impala] 06/06: IMPALA-7288: Fix Codegen Crash in FinalizeModule() (Addendum)

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git

commit aea18dd08f34caf5c659c4b71f7bc4d70d743739
Author: Bikramjeet Vig <bi...@cloudera.com>
AuthorDate: Fri Jul 13 13:07:54 2018 -0700

    IMPALA-7288: Fix Codegen Crash in FinalizeModule() (Addendum)
    
    In addition to previous fix for IMPALA-7288, this patch would prevent
    impala from crashing in case a code-path generates a malformed
    handcrafted function which it then tries to finalize. Ideally this
    would never happen since the code paths for generating handcrafted IRs
    would never generate a malformed function.
    
    Change-Id: Id09c6f59f677ba30145fb2081715f1a7d89fe20b
    Reviewed-on: http://gerrit.cloudera.org:8080/10944
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/codegen/llvm-codegen.cc | 5 +----
 be/src/codegen/llvm-codegen.h  | 4 ++--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index 7fe4ec1..173dbb2 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -1009,10 +1009,7 @@ llvm::Function* LlvmCodeGen::CloneFunction(llvm::Function* fn) {
 
 llvm::Function* LlvmCodeGen::FinalizeFunction(llvm::Function* function) {
   SetCPUAttrs(function);
-  if (!VerifyFunction(function)) {
-    function->eraseFromParent(); // deletes function
-    return NULL;
-  }
+  if (!VerifyFunction(function)) return NULL;
   finalized_functions_.insert(function);
   if (FLAGS_dump_ir) {
     string fn_name = function->getName();
diff --git a/be/src/codegen/llvm-codegen.h b/be/src/codegen/llvm-codegen.h
index 53569ca..7e9da26 100644
--- a/be/src/codegen/llvm-codegen.h
+++ b/be/src/codegen/llvm-codegen.h
@@ -392,8 +392,8 @@ class LlvmCodeGen {
   /// passed to AddFunctionToJit() otherwise the functions will be deleted from the
   /// module when the module is finalized. Also, all loaded functions that need to be JIT
   /// compiled after modification also need to be finalized.
-  /// If the function does not verify, it will delete the function and return NULL,
-  /// otherwise, it returns the function object.
+  /// If the function does not verify, it returns NULL and the function will eventually
+  /// be deleted in FinalizeModule(), otherwise, it returns the function object.
   llvm::Function* FinalizeFunction(llvm::Function* function);
 
   /// Adds the function to be automatically jit compiled when the codegen object is