Add verifyfunction for generated llvm function.

This closes #836
Signed-off-by: NMarc Spehlmann <cramja@gmail.com>

reorder header file

src/backend/codegen/include/codegen/base_codegen.h

@@ -12,12 +12,19 @@
 #ifndef GPCODEGEN_BASE_CODEGEN_H_  // NOLINT(build/header_guard)
 #define GPCODEGEN_BASE_CODEGEN_H_
 
+extern "C" {
+#include <utils/elog.h>
+}
+
 #include <string>
 #include <vector>
 #include "codegen/utils/codegen_utils.h"
 #include "codegen/codegen_interface.h"
 
 #include "llvm/IR/Function.h"
+#include "llvm/IR/Verifier.h"
+
+extern bool codegen_validate_functions;
 
 namespace gpcodegen {
 
@@ -44,15 +51,31 @@ class BaseCodegen: public CodegenInterface {
   }
 
   bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils) final {
-    is_generated_ = GenerateCodeInternal(codegen_utils);
-    if (!is_generated_) {
-      // If failed to generate, make sure we do clean up
-      // by erasing all the llvm functions.
+    bool valid_generated_functions = true;
+    valid_generated_functions &= GenerateCodeInternal(codegen_utils);
+
+    // Do this check only if it enabled by guc
+    if (codegen_validate_functions && valid_generated_functions) {
+      for (llvm::Function* function : uncompiled_generated_functions_) {
+        assert(nullptr != function);
+        // Verify function returns true if there are errors.
+        valid_generated_functions &= !llvm::verifyFunction(*function);
+        if (!valid_generated_functions) {
+          std::string func_name = function->getName();
+          elog(WARNING, "Broken function found '%s'", func_name.c_str());
+          break;
+        }
+      }
+    }
+    if (!valid_generated_functions) {
+      // If failed to generate, or have invalid functions, make sure we
+      // do clean up by erasing all the llvm functions.
       for (llvm::Function* function : uncompiled_generated_functions_) {
         assert(nullptr != function);
         function->eraseFromParent();
       }
     }
+    is_generated_ = valid_generated_functions;
     // We don't need to keep these pointers any more
     std::vector<llvm::Function*>().swap(uncompiled_generated_functions_);
     return is_generated_;

src/backend/codegen/tests/codegen_framework_unittest.cc

@@ -44,6 +44,12 @@
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/Casting.h"
 
+extern "C" {
+    #include "utils/elog.h"
+    #undef elog
+    #define elog
+}
+
 #include "codegen/utils/codegen_utils.h"
 #include "codegen/utils/utility.h"
 #include "codegen/codegen_manager.h"
@@ -51,7 +57,7 @@
 #include "codegen/codegen_interface.h"
 #include "codegen/base_codegen.h"
 
-
+extern bool codegen_validate_functions;
 namespace gpcodegen {
 
 typedef int (*SumFunc) (int x, int y);
@@ -168,6 +174,7 @@ class CodegenManagerTest : public ::testing::Test {
  protected:
   virtual void SetUp() {
     manager_.reset(new CodegenManager("CodegenManagerTest"));
+    codegen_validate_functions = true;
   }
 
   template <typename ClassType, typename FuncType>
@@ -225,7 +232,7 @@ TEST_F(CodegenManagerTest, GenerateCodeTest) {
   uncompilable_func_ptr = nullptr;
   EnrollCodegen<UncompilableCodeGenerator<true>, UncompilableFunc>(
       UncompilableFuncRegular, &uncompilable_func_ptr);
-  EXPECT_EQ(2, manager_->GenerateCode());
+  EXPECT_EQ(1, manager_->GenerateCode());
 }
 
 TEST_F(CodegenManagerTest, PrepareGeneratedFunctionsNoCompilationErrorTest) {
@@ -338,16 +345,17 @@ TEST_F(CodegenManagerTest, UnCompilablePassedGenerationTest) {
   EnrollCodegen<UncompilableCodeGenerator<true>, UncompilableFunc>(
       UncompilableFuncRegular, &uncompilable_func_ptr);
 
-  EXPECT_EQ(2, manager_->GenerateCode());
+  EXPECT_EQ(1, manager_->GenerateCode());
 
   // Make sure both the function pointers refer to regular versions
   ASSERT_TRUE(SumFuncRegular == sum_func_ptr);
   ASSERT_TRUE(SumFuncRegular == failed_func_ptr);
   ASSERT_TRUE(UncompilableFuncRegular == uncompilable_func_ptr);
 
-  // This should cause program to exit because of
-  // broken function
-  EXPECT_DEATH(manager_->PrepareGeneratedFunctions(), "");
+  EXPECT_EQ(1, manager_->PrepareGeneratedFunctions());
+
+  ASSERT_TRUE(SumFuncRegular == failed_func_ptr);
+  ASSERT_TRUE(UncompilableFuncRegular == uncompilable_func_ptr);
 
   // Reset the manager, so that all the code generators go away
   manager_.reset(nullptr);
@@ -392,3 +400,4 @@ int main(int argc, char **argv) {
   AddGlobalTestEnvironment(new gpcodegen::CodegenManagerTestEnvironment);
   return RUN_ALL_TESTS();
 }
+

src/backend/utils/misc/guc_gp.c

@@ -567,6 +567,7 @@ bool		optimizer_prefer_scalar_dqa_multistage_agg;
  **/
 bool		init_codegen;
 bool		codegen;
+bool		codegen_validate_functions;
 
 /* Security */
 bool		gp_reject_internal_tcp_conn = true;
@@ -3397,6 +3398,20 @@ struct config_bool ConfigureNamesBool_gp[] =
 		false, NULL, NULL
 	},
 
+	{
+		{"codegen_validate_functions", PGC_USERSET, DEVELOPER_OPTIONS,
+			gettext_noop("Perform verify for generated functions to catch any error before compiling"),
+			NULL,
+			GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE
+		},
+		&codegen_validate_functions,
+#ifdef USE_ASSERT_CHECKING
+		true, NULL, NULL	/* true by default on debug builds. */
+#else
+		false, NULL, NULL
+#endif
+
+	},
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL

src/include/codegen/codegen_wrapper.h

@@ -28,6 +28,7 @@ struct TupleTableSlot;
 struct ProjectionInfo;
 struct List;
 struct ExprContext;
+struct PlanState;
 
 typedef void (*ExecVariableListFn) (struct ProjectionInfo *projInfo, Datum *values, bool *isnull);
 typedef bool (*ExecQualFn) (struct List *qual, struct ExprContext *econtext, bool resultForNull);
@@ -144,12 +145,12 @@ ExecVariableListCodegenEnroll(ExecVariableListFn regular_func_ptr,
                               struct TupleTableSlot* slot);
 
 /*
- * returns the pointer to the ExecQual
+ * returns the pointer to the ExecQual generator
  */
 void*
 ExecQualCodegenEnroll(ExecQualFn regular_func_ptr,
-                              ExecQualFn* ptr_to_regular_func_ptr,
-                              struct PlanState *planstate);
+					  ExecQualFn* ptr_to_regular_func_ptr,
+					  struct PlanState *planstate);
 
 #ifdef __cplusplus
 }  // extern "C"

src/include/utils/guc.h

@@ -465,6 +465,7 @@ extern bool optimizer_prefer_scalar_dqa_multistage_agg;
  **/
 extern bool init_codegen;
 extern bool codegen;
+extern bool codegen_validate_functions;
 
 /**
  * Enable logging of DPE match in optimizer.

src/test/unit/mock/gpcodegen_mock.c

@@ -24,7 +24,7 @@ unsigned int
 CodeGeneratorManagerGenerateCode(void* manager)
 {
 	elog(ERROR, "mock implementation of CodeGeneratorManager_GenerateCode called");
-	return true;
+	return 1;
 }
 
 // compiles and prepares all the code gened function pointers
@@ -32,7 +32,7 @@ unsigned int
 CodeGeneratorManagerPrepareGeneratedFunctions(void* manager)
 {
 	elog(ERROR, "mock implementation of CodeGeneratorManager_PrepareGeneratedFunctions called");
-	return true;
+	return 1;
 }
 
 // notifies a manager that the underlying operator has a parameter change
@@ -40,7 +40,7 @@ unsigned int
 CodeGeneratorManagerNotifyParameterChange(void* manager)
 {
 	elog(ERROR, "mock implementation of CodeGeneratorManager_NotifyParameterChange called");
-	return true;
+	return 1;
 }
 
 // destroys a manager for an operator
@@ -65,7 +65,7 @@ SetActiveCodeGeneratorManager(void* manager)
 	elog(ERROR, "mock implementation of SetActiveCodeGeneratorManager called");
 }
 
-// returns the pointer to the SlotDeformTupleCodegen
+// returns the pointer to the ExecVariableListGenerator
 void*
 ExecVariableListCodegenEnroll(ExecVariableListFn regular_func_ptr,
                               ExecVariableListFn* ptr_to_regular_func_ptr,
@@ -77,3 +77,14 @@ ExecVariableListCodegenEnroll(ExecVariableListFn regular_func_ptr,
 	return NULL;
 }
 
+// returns the pointer to the ExecQualGenerator
+void*
+ExecQualCodegenEnroll(ExecQualFn regular_func_ptr,
+					  ExecQualFn* ptr_to_regular_func_ptr,
+					  struct PlanState *planstate)
+{
+  *ptr_to_regular_func_ptr = regular_func_ptr;
+  elog(ERROR, "mock implementation of ExecQualCodegenEnroll called");
+  return NULL;
+}
+