#449 bad pointer in addCompound SourceFile.cpp

3.2.0
closed
5
2012-08-14
2007-10-19
No

This is on Windows built with Visual Studio 2005, after commit 1008. The interpreter crashes in addCompound() at line 3401 of SourceFile.cpp

With this change:

INT length; / length of tail section /

going to

size_t length; / length of tail on /

length becomes unsigned int.

When length is 0 and then decremented (length--) it becomes 4294967295, at least when compiled with Visual Studio 2005.

Then of course the test at the top of while loop does not fail: ( while (length > 0) ) and we step past the end of the string.

I suppose this could be a pervasive problem with the change to size_t? Since you're working on this, I'll wait for your comments Rick.

Discussion

  • Rick McGuire

    Rick McGuire - 2007-10-19

    Logged In: YES
    user_id=1125291
    Originator: NO

    Yes, that would be problem. This is why I wanted to make these changes right up front, to give us time to catch these sorts of problems. Do you have any idea on what type of statement tripped this up?

     
  • Mark Miesfeld

    Mark Miesfeld - 2007-10-19

    Logged In: YES
    user_id=191588
    Originator: YES

    Rick,

    It is when it is parsing a compound variable narm.

    We have:

    length = name->getLength(); / get the string length /

    And then the main while loop:

    while (length > 0) { / process rest of the variable /

    At the end of the loop, when the end of the compound variable name is reached, length is 0 and there is a decrement:

    length--; / adjust the length /

    That decrement pushes it to 4294967295 instead of -1

    My first thought is to declare length as an int and then do this instead:

    length = (int)name->getLength(); / get the string length /

    Because I don't think we will ever have a compound variable name bigger thatn 2 GB. But, if there are a lot of places like this where length is used, you would have a better grasp of the big picture. I just see a fix in this one small spot. <grin>

     
  • Rick McGuire

    Rick McGuire - 2007-10-19

    Logged In: YES
    user_id=1125291
    Originator: NO

    That's the easy fix, but I don't think it's the "correct" fix. One of the goals I have here is clean up the use of various int types. In general lengths of things should be unsigned values. Trying to do otherwise causes all sorts of problems with signed/unsigned arithmetic warnings and constant casting between types. I've just about got a fix done that's a little more code, but might even be a small performance improvement :-)

     
  • Rick McGuire

    Rick McGuire - 2007-10-19

    Logged In: YES
    user_id=1125291
    Originator: NO

    Committed revision 1021.

     


Anonymous

Cancel  Add attachments