#5 uninitialized memory error in CM730

v1.6.0
open-accepted
Framework (4)
5
2012-08-21
2012-07-11
Nick Felt
No

When running a program that uses the MotionManager in the v1.5.0 framework under valgrind, the following error appears:

==5999== Conditional jump or move depends on uninitialised value(s)
==5999== at 0x8050348: Robot::CM730::BulkRead() (CM730.cpp:438)
==5999== by 0x805227B: Robot::MotionManager::Process() (MotionManager.cpp:320)
==5999== by 0x805A954: Robot::LinuxMotionTimer::TimerProc(void*) (LinuxMotionTimer.cpp:35)
==5999== by 0x427FACD: clone (clone.S:130)

This uninitialized value was causing undefined behavior in the program where the MotionManager would sometimes not work at all (i.e. the robot would not move in response to commands such as Head::GetInstance()->MoveToHome()). This would only show up some of the time - it depended on what was allocated on the heap before the MotionManager was constructed via new.

The problematic line in BulkRead() is the line below that reads from m_BulkReadTxPacket[LENGTH]. Since none of the positions in m_BulkReadTxPacket are initialized until MakeBulkReadPacket() is run, the first time BulkRead() is called, m_BulkReadTxPacket[LENGTH] is not initialized and its value will depend on what was present on the heap when the CM730 object was allocated. Presumably the errors occur when this value is not 0.

int CM730::BulkRead()
{
unsigned char rxpacket[MAXNUM_RXPARAM + 10] = {0, };
if(m_BulkReadTxPacket[LENGTH] != 0)
return TxRxPacket(m_BulkReadTxPacket, rxpacket, 0);
else
{
MakeBulkReadPacket();
return TX_FAIL;
}
}

The fix we've used is to add a line initializing m_BulkReadTxPacket[LENGTH] to 0 in the constructor of CM730 - in the following patch, this was line 62 of darwin/Framework/src/CM730.cpp.

diff --git i/darwin/Framework/src/CM730.cpp w/darwin/Framework/src/CM730.cpp
index 2cdfb17..9d4a27f 100644
--- i/darwin/Framework/src/CM730.cpp
+++ w/darwin/Framework/src/CM730.cpp
@@ -59,6 +59,7 @@ CM730::CM730(PlatformCM730 *platform)
{
m_Platform = platform;
DEBUG_PRINT = false;
+ m_BulkReadTxPacket[LENGTH] = 0;
for(int i = 0; i < ID_BROADCAST; i++)
m_BulkReadData[i] = BulkReadData();
}

Discussion

  • Hee-il Kim

    Hee-il Kim - 2012-08-21
    • milestone: --> v1.6.0
    • assigned_to: nobody --> zerom97
    • status: open --> open-accepted
     
  • Hee-il Kim

    Hee-il Kim - 2012-08-21

    Thank you for your help.
    I'll apply this patch next release.

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks