[TRAFODION-2879] Core dump due to reference to deallocated memory for 
EstLogProp object

In the optimizer, we often use a define to refer to an "empty input
logical properties" object. The problem was that this object was
allocated on the statement heap but it was owned by the CmpContext
object, which lives longer than the statement heap. During debugging,
we have seen cases where this led to references of deleted memory.

The fix is to move the empty logical properties to the CmpStatement
object.


Project: http://git-wip-us.apache.org/repos/asf/trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafodion/commit/90f0b665
Tree: http://git-wip-us.apache.org/repos/asf/trafodion/tree/90f0b665
Diff: http://git-wip-us.apache.org/repos/asf/trafodion/diff/90f0b665

Branch: refs/heads/master
Commit: 90f0b665d54f6b04486e290afd118335167b1d18
Parents: bb4edd4
Author: Hans Zeller <[email protected]>
Authored: Tue Jan 9 22:09:29 2018 +0000
Committer: Hans Zeller <[email protected]>
Committed: Tue Jan 9 22:09:29 2018 +0000

----------------------------------------------------------------------
 core/sql/arkcmp/CmpContext.cpp   |  3 ---
 core/sql/arkcmp/CmpContext.h     |  8 --------
 core/sql/arkcmp/CmpStatement.cpp | 23 ++++++++++++-----------
 core/sql/arkcmp/CmpStatement.h   |  6 ++++++
 core/sql/common/CmpCommon.h      |  2 +-
 core/sql/optimizer/RelExpr.h     |  1 +
 6 files changed, 20 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafodion/blob/90f0b665/core/sql/arkcmp/CmpContext.cpp
----------------------------------------------------------------------
diff --git a/core/sql/arkcmp/CmpContext.cpp b/core/sql/arkcmp/CmpContext.cpp
index e5c179e..3108b47 100644
--- a/core/sql/arkcmp/CmpContext.cpp
+++ b/core/sql/arkcmp/CmpContext.cpp
@@ -304,9 +304,6 @@ CmpContext::CmpContext(UInt32 f, CollHeap * h)
   // create dynamic metadata descriptors
   CmpSeabaseDDL cmpSeabaseDDL(heap_);
   cmpSeabaseDDL.createMDdescs(trafMDDescsInfo_);
-
-  emptyInLogProp_ = NULL;
-
 }
 
 // MV

http://git-wip-us.apache.org/repos/asf/trafodion/blob/90f0b665/core/sql/arkcmp/CmpContext.h
----------------------------------------------------------------------
diff --git a/core/sql/arkcmp/CmpContext.h b/core/sql/arkcmp/CmpContext.h
index b875927..5df2eca 100644
--- a/core/sql/arkcmp/CmpContext.h
+++ b/core/sql/arkcmp/CmpContext.h
@@ -449,11 +449,6 @@ public :
   // optimizer cached defaults
   OptDefaults* getOptDefaults() { return optDefaults_; }
 
-  // context global empty input logical property
-  EstLogPropSharedPtr* getGEILP() { return &emptyInLogProp_; }
-  void setGEILP(EstLogPropSharedPtr inLogProp)
-                             { emptyInLogProp_ = inLogProp; }
-    
   MDDescsInfo *getTrafMDDescsInfo() { return trafMDDescsInfo_; }
 
   void setCIClass(CmpContextInfo::CmpContextClassType x) { ciClass_ = x; }
@@ -615,9 +610,6 @@ private:
   // query defaults using during a statement compilation
   OptDefaults* optDefaults_;
 
-  // context global empty input logical property
-  EstLogPropSharedPtr emptyInLogProp_;
-
   MDDescsInfo * trafMDDescsInfo_;
 
   CmpContextInfo::CmpContextClassType ciClass_;

http://git-wip-us.apache.org/repos/asf/trafodion/blob/90f0b665/core/sql/arkcmp/CmpStatement.cpp
----------------------------------------------------------------------
diff --git a/core/sql/arkcmp/CmpStatement.cpp b/core/sql/arkcmp/CmpStatement.cpp
index e16ff5c..4bde511 100644
--- a/core/sql/arkcmp/CmpStatement.cpp
+++ b/core/sql/arkcmp/CmpStatement.cpp
@@ -1515,17 +1515,18 @@ QueryAnalysis* CmpStatement::initQueryAnalysis()
   // do any necessary initialization work here (unless this
   // initialization work fits in the constructor)
 
-  // Initialize the global "empty input logprop".
-  context_->setGEILP(EstLogPropSharedPtr(new (STMTHEAP)
-                              EstLogProp(1,
-                                         NULL,
-                                         EstLogProp::NOT_SEMI_TSJ,
-                                         new (STMTHEAP) CANodeIdSet(),
-                                         TRUE)));
-
-    //++MV
-    // This input cardinality is not estimated , so we keep this knowledge
-    // in a special attribute.
+  // Initialize the global "empty input logprop"
+  if (emptyInLogProp_ == NULL)
+    emptyInLogProp_ = EstLogPropSharedPtr(
+         new (STMTHEAP) EstLogProp(1,
+                                   NULL,
+                                   EstLogProp::NOT_SEMI_TSJ,
+                                   new (STMTHEAP) CANodeIdSet(STMTHEAP),
+                                   TRUE));
+  
+  //++MV
+  // This input cardinality is not estimated , so we keep this knowledge
+  // in a special attribute.
   (*GLOBAL_EMPTY_INPUT_LOGPROP)->setCardinalityEqOne();
 
 #ifdef _DEBUG

http://git-wip-us.apache.org/repos/asf/trafodion/blob/90f0b665/core/sql/arkcmp/CmpStatement.h
----------------------------------------------------------------------
diff --git a/core/sql/arkcmp/CmpStatement.h b/core/sql/arkcmp/CmpStatement.h
index 14b1781..dc0767a 100644
--- a/core/sql/arkcmp/CmpStatement.h
+++ b/core/sql/arkcmp/CmpStatement.h
@@ -230,6 +230,9 @@ public:
   const LIST(CSEInfo *) *getCSEInfoList() const { return cses_; }
   void addCSEInfo(CSEInfo *info);
 
+  // context global empty input logical property
+  EstLogPropSharedPtr* getGEILP() { return &emptyInLogProp_; }
+    
 protected:
   // CmpStatement(const CmpStatement&); please remove this line
   CmpStatement& operator=(const CmpStatement&);
@@ -327,6 +330,9 @@ private:
   // CmpMain::sqlcomp(QueryText, ...
   Int32 numOfCompilationRetries_;
 
+  // context global empty input logical property
+  EstLogPropSharedPtr emptyInLogProp_;
+
 }; // end of CmpStatement
 
 class CmpStatementISP: public CmpStatement

http://git-wip-us.apache.org/repos/asf/trafodion/blob/90f0b665/core/sql/common/CmpCommon.h
----------------------------------------------------------------------
diff --git a/core/sql/common/CmpCommon.h b/core/sql/common/CmpCommon.h
index 74393ad..1caaafa 100644
--- a/core/sql/common/CmpCommon.h
+++ b/core/sql/common/CmpCommon.h
@@ -141,7 +141,7 @@ public:
   #define CURRCONTEXT_OPTDEBUG (CmpCommon::context()->getOptDbg())
   #define CURRCONTEXT_HISTCACHE (CmpCommon::context()->getHistogramCache())
   #define CURRCONTEXT_OPTSIMULATOR 
(CmpCommon::context()->getOptimizerSimulator())
-  #define GLOBAL_EMPTY_INPUT_LOGPROP (CmpCommon::context()->getGEILP())
+  #define GLOBAL_EMPTY_INPUT_LOGPROP (CmpCommon::statement()->getGEILP())
   #define CURRSTMT_OPTDEFAULTS (CmpCommon::context()->getOptDefaults())
 
   // For some routines that do care about the current CmpContext*. 

http://git-wip-us.apache.org/repos/asf/trafodion/blob/90f0b665/core/sql/optimizer/RelExpr.h
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/RelExpr.h b/core/sql/optimizer/RelExpr.h
index 3074aa8..44ed086 100644
--- a/core/sql/optimizer/RelExpr.h
+++ b/core/sql/optimizer/RelExpr.h
@@ -39,6 +39,7 @@
 
 #include "ObjectNames.h"
 #include "CmpContext.h"
+#include "CmpStatement.h"
 #include "RETDesc.h"
 #include "ValueDesc.h"
 #include "Rule.h"

Reply via email to