Re: ndb: usage of uninitialized memory

[Jonas Oreland]

Vasil Dimov wrote:
> On Wed, Nov 15, 2006 at 10:38:58AM +0100, Jonas Oreland wrote:
>> Hi,
>>
>> and thx for excellent analysis
> 
> Thanks for the evaluation!
> 
>> the patch that your proposing is "not correct" but do indeed solve your problem.
> 
> Yeah, that's what I suspected... :-)
> 
>> the "correct" patch is to run constructor in DbtupGen
>> (i however have not tested this :-)
>>
>>
>> ===== dbtup/DbtupGen.cpp 1.31 vs edited =====
>> --- 1.31/storage/ndb/src/kernel/blocks/dbtup/DbtupGen.cpp       2006-11-15
> 10:29:28 +01:00
>> +++ dbtup/DbtupGen.cpp  2006-11-15 10:29:13 +01:00
>> @@ -329,6 +329,7 @@
>>  
>>    ScanOpPtr lcp;
>>    ndbrequire(c_scanOpPool.seize(lcp));
>> +  new (lcp.p) ScanOp();
>>    c_lcp_scan_op= lcp.i;
>>  
>>    czero = 0;
>>
> 
> I just tested your patch, ndbd still aborts in the same place but for
> different reason now:
> 
> --- gdb session begins here ---
> //
> // Backtrace looks the same
> //
> (gdb) f 10
> #10 0x00000000005374c4 in Dbtup::scanFirst (this=0xed3000, scanPtr=
>       {p = 0x4378000, i = 0}) at dbtup/DbtupScan.cpp:538
> 538       ptrCheckGuard(fragPtr, cnoOfFragrec, fragrecord);
> (gdb) ins fragPtr.i
> $1 = 4294967040
> (gdb) ins cnoOfFragrec
> $2 = 452
> --- gdb session ends here ---
> 
> This value (4294967040) comes from the constructor:
> 
> --- Dbtup.hpp begins here ---
>  392   struct ScanOp {
>  393     ScanOp() :
>  394       m_state(Undef),
>  395       m_bits(0),
>  396       m_userPtr(RNIL),
>  397       m_userRef(RNIL),
>  398       m_tableId(RNIL),
>  399       m_fragId(~(Uint32)0),
>  400       m_fragPtrI(RNIL),       <---- from here
>  401       m_transId1(0),
>  402       m_transId2(0),
>  403       m_savePointId(0),
>  404       m_accLockOp(RNIL)
>  405     {}
> --- Dbtup.hpp ends here ---
> 
> According to RNIL's definition, this is exactly the same value:
> ndb_limits.h:22:#define RNIL    0xffffff00
> 
> Maybe m_fragPtrI should be initialized with 0?

naaa...it should be initialized as in following patch
but i think the following will fix problem (together with first patch)

===== DbtupScan.cpp 1.15 vs edited =====
--- 1.15/storage/ndb/src/kernel/blocks/dbtup/DbtupScan.cpp      2006-11-15 12:56
:06 +01:00
+++ DbtupScan.cpp       2006-11-15 12:54:50 +01:00
@@ -87,6 +87,7 @@
       
       ndbrequire(frag.m_lcp_scan_op == c_lcp_scan_op);
       c_scanOpPool.getPtr(scanPtr, frag.m_lcp_scan_op);
+      ndbrequire(scanPtr.p->m_fragPtrI == fragPtr.i);
       bits |= ScanOp::SCAN_LCP;
       if (tablePtr.p->m_attributes[MM].m_no_of_varsize > 0) {
         bits |= ScanOp::SCAN_VS;
@@ -1038,6 +1039,7 @@
   {
     ndbrequire(fragPtr.p->m_lcp_scan_op == scanPtr.i);
     fragPtr.p->m_lcp_scan_op = RNIL;
+    scanPtr.p->m_fragPtrI = RNIL;
   }
 }
 
@@ -1064,8 +1066,9 @@
     frag.m_lcp_scan_op = c_lcp_scan_op;
     ScanOpPtr scanPtr;
     c_scanOpPool.getPtr(scanPtr, frag.m_lcp_scan_op);
-    //ndbrequire(scanPtr.p->m_fragPtrI == fragPtr.i); ?
-
+    ndbrequire(scanPtr.p->m_fragPtrI == RNIL);
+    scanPtr.p->m_fragPtrI = fragPtr.i;
+    
     scanFirst(signal, scanPtr);
     scanPtr.p->m_state = ScanOp::First;
   }
> 
>> ps.
>>   I filed a bug report for this, http://bugs.mysql.com/24331, and made patch
> accordingly
> 
> Thanks!
> 
>> ps2.
>>   ndb mails are better to send to cluster@stripped
> 
> Ok, I will have this in mind next time :-)
> 
> Btw ArrayPool<T> is used in many places with different values of T,
> shouldn't their constuctors be called too?

it is the responsibility of the ArrayPool<T> user to initialize it's objects
  before using them (like the missing placement-new for this)

/Jonas

>