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
#122
Please setup a PayPal donation link for us to help you pay for your space.
If people can help great, if not I think that is okay. It will be the source code contributions that will be worth the most. That is my opinion.
05/20/2012 (11:02 pm)
@Alfio,Please setup a PayPal donation link for us to help you pay for your space.
If people can help great, if not I think that is okay. It will be the source code contributions that will be worth the most. That is my opinion.
#123
If any of the guys on GG tells me how i can authenticate users with a valid license, i can start adding users to the services.
Edit: For example, users could put the email here and check myself if they are have the license, then i could send a mail with the user data for logging.
05/21/2012 (6:46 pm)
As requested by some users, i added the button for free amount donations, although it is not necessary to make donations. :)If any of the guys on GG tells me how i can authenticate users with a valid license, i can start adding users to the services.
Edit: For example, users could put the email here and check myself if they are have the license, then i could send a mail with the user data for logging.
#124
Incidentally (to GG) it would be nice to have 'contact member' functionality on the profile page, and of course the option to turn it off (because some people like to be left alone) :)
05/22/2012 (3:40 am)
@Alfio. If you right click on a user name ( <-- to the left for instance ) and open a new page, you should see a member profile, with their respective licensed products.Incidentally (to GG) it would be nice to have 'contact member' functionality on the profile page, and of course the option to turn it off (because some people like to be left alone) :)
#125
The way in which users were authenticated for the Torque X CEV was for them to post in a thread dedicated to the purpose of asking for access. Pino would then look at their profile to check their owned Torque products (you can click on a person's badge beside the post to see this information) and send an invite to the users' supplied email address.
Have you started accepting/allowing users yet?
05/28/2012 (10:24 am)
@Alfio: The way in which users were authenticated for the Torque X CEV was for them to post in a thread dedicated to the purpose of asking for access. Pino would then look at their profile to check their owned Torque products (you can click on a person's badge beside the post to see this information) and send an invite to the users' supplied email address.
Have you started accepting/allowing users yet?
#126
05/28/2012 (10:52 am)
Yes Michael, i started to accept people. Just post request with your email in a private forum where i can see, and i proceed with the manual activation. After that, you can change the password assigned by default from your account preferences.
#127
05/28/2012 (10:57 am)
EDIT: Thanks Alfio!
#131
05/29/2012 (4:23 am)
Konrad, i can not see your mail: <edit: address hidden>
#132
05/29/2012 (4:29 am)
Sorry Alfio, I assumed you were getting it in the automatic email. It's <...>. I'll edit it once you give me access.
#133
05/29/2012 (4:34 am)
U are right... i have the notification enaboled... u can delete the mail now... sorry :P
#134
05/29/2012 (4:38 am)
Thanks a lot!
#137
05/29/2012 (11:26 am)
Sorry, Ross. But your account is valid for version 1.2?
#139
05/29/2012 (11:31 am)
@Alfio, whoops, nope I don't have 1.2. Are you planning on putting a 1.1 repo up?
Torque Owner Alfio Saitta
Collateral Studios