Torque 3D's Achilles heel
by Konrad Kiss · in Torque 3D Professional · 03/24/2012 (11:49 am) · 184 replies
Recently I spent some time getting myself familiar with static code analysis and the more common (indie-budget freindly) tools associated with the topic. #AltDevBlogADay had several posts about such tools by Bruce Dawson and John Carmack. Definitely worth checking out.
So, it all began with me having a very elusive bug somewhere in code. I hoped that it would be something that a static code analyzer would pick up, so I lined up what I could from trial versions to open source software, and got some amazing results once these went through the Torque 3D 1.2 codebase.
I can't remember if the actual problem was found by these tools (it's been a while ago). What I recall is being taken aback by the results I got. But first a disclaimer...
I've been reviewing engine code changes for the past 4 years. Torque 3D is huge. One of the most surprising thing was the fact, that these code analyzers could - proportionally - find only a few problems. However, the few they did find were hard to see even when I knew the problem.
The kinds of problems I found primarily were:
- Uninitialized member variables used without initialization
- Checking the existence of an object and using the same object outside the check condition
- Passing local pointers that could become invalid
- Using && instead of &, || instead of | operations
- Bad casts for variables while the code assumes another cast (ie. U32 vs S32 - especially when negative values are used for error codes - common in some of the linked libraries)
- Typos, cut & paste programming mistakes that compile (probably the most vicious of all)
I'm only going to list my favorites, hoping that this will be a big enough motivation for everyone to start using some basic form of static code analysis. The complete list would be way too long for me to list here.
gfxD3D9TextureManager: using boolean evaluation against variables that were clearly intended to be bitmasks.
guiIconButtonCtrl.cpp: find the highlighted button, I dare you
processedFFMaterial.cpp: making sure (though what about blendDest?)
mRectF.cpp: intersectTriangle
winDInputDevice.cpp: POV_down, not much of a choice (twice in the same file)
rectClipper: clipRect (near eof)
win32Window.cpp: equilibrium
Finally - I need to clarify - by all means, no disrespect was intended towards Torque 3D developers. I would have done far worse most likely. I can't count the many new things in the source that made me go "Wow" and "Yey" - it is clearly visible that the team is bigger and better at what they do. Torque 1.2 is the best Torque yet - and I mean it by comparing the overall usability, the number of bugs, etc..
With this post I'm only trying to point at a solution that can save everyone from such blatant bugs in the future, while - perhaps - also nudging GG a little to take a step in the right direction. ;)
Cheers
--Konrad
So, it all began with me having a very elusive bug somewhere in code. I hoped that it would be something that a static code analyzer would pick up, so I lined up what I could from trial versions to open source software, and got some amazing results once these went through the Torque 3D 1.2 codebase.
I can't remember if the actual problem was found by these tools (it's been a while ago). What I recall is being taken aback by the results I got. But first a disclaimer...
I've been reviewing engine code changes for the past 4 years. Torque 3D is huge. One of the most surprising thing was the fact, that these code analyzers could - proportionally - find only a few problems. However, the few they did find were hard to see even when I knew the problem.
The kinds of problems I found primarily were:
- Uninitialized member variables used without initialization
- Checking the existence of an object and using the same object outside the check condition
- Passing local pointers that could become invalid
- Using && instead of &, || instead of | operations
- Bad casts for variables while the code assumes another cast (ie. U32 vs S32 - especially when negative values are used for error codes - common in some of the linked libraries)
- Typos, cut & paste programming mistakes that compile (probably the most vicious of all)
I'm only going to list my favorites, hoping that this will be a big enough motivation for everyone to start using some basic form of static code analysis. The complete list would be way too long for me to list here.
gfxD3D9TextureManager: using boolean evaluation against variables that were clearly intended to be bitmasks.
if (retTex->mProfile->isRenderTarget() && mslevel != 0 && (mDeviceCaps.Caps2 && D3DDEVCAPS2_CAN_STRETCHRECT_FROM_TEXTURES))
guiIconButtonCtrl.cpp: find the highlighted button, I dare you
ColorI fontColor = mActive ? (highlight ? mProfile->mFontColor : mProfile->mFontColor) : mProfile->mFontColorNA;
processedFFMaterial.cpp: making sure (though what about blendDest?)
result.blendSrc = GFXBlendOne;
result.blendSrc = GFXBlendOne;mRectF.cpp: intersectTriangle
if(contains(a) || contains(b) || contains(b))
winDInputDevice.cpp: POV_down, not much of a choice (twice in the same file)
newEvent.objInst = ( objInst == 0 ) ? SI_DPOV : SI_DPOV
rectClipper: clipRect (near eof)
out_rRect.extent.x = bottomR.x - out_rRect.point.x + 1; out_rRect.extent.x = bottomR.y - out_rRect.point.y + 1;
win32Window.cpp: equilibrium
newLeft = mClamp(newLeft, 0, newLeft);
newTop = mClamp(newLeft, 0, newTop);Finally - I need to clarify - by all means, no disrespect was intended towards Torque 3D developers. I would have done far worse most likely. I can't count the many new things in the source that made me go "Wow" and "Yey" - it is clearly visible that the team is bigger and better at what they do. Torque 1.2 is the best Torque yet - and I mean it by comparing the overall usability, the number of bugs, etc..
With this post I'm only trying to point at a solution that can save everyone from such blatant bugs in the future, while - perhaps - also nudging GG a little to take a step in the right direction. ;)
Cheers
--Konrad
About the author
http://about.me/konrad.kiss
#182
I just happened to notice this thread had been updated -- I haven't received an email alert for it in a while even though I'm subscribed to it.
If there is anyone else who has requested to join the CE and hasn't been processed yet, let us know over on the CE Activation thread. Let's try and put this thread back on track for discussing the actual issues.
07/17/2012 (9:11 am)
@Luis: you're added but delivery of the email failed to that address saying that the account does not exist.I just happened to notice this thread had been updated -- I haven't received an email alert for it in a while even though I'm subscribed to it.
If there is anyone else who has requested to join the CE and hasn't been processed yet, let us know over on the CE Activation thread. Let's try and put this thread back on track for discussing the actual issues.
#183
07/19/2012 (10:31 am)
@Luis: attempts to send you the CE login information to that email address continues to fail.
#184
it should be
as1114144@sapo.pt
Notice the missing 4 - my bad - typed too fast and didn't check
Sorry i didn't answer much earlier
07/22/2012 (1:58 am)
@Michael - sorry anout that - mail was wrong - my mistakeit should be
as1114144@sapo.pt
Notice the missing 4 - my bad - typed too fast and didn't check
Sorry i didn't answer much earlier
Torque Owner Luis Rodrigues
WinterLeaf Entertainment
Would like to chim in too
as111414@sapo.pt
thanks