Violating C++ Standard?

Hello,

I've got a piece of code that's causing strange problems when compiled in release (using Visual Studio 2010). Am I violating some rule in the C++ standard by doing this?

Psuedocode:

Code:

---

Object* obj = new Object( ++m_id, name );
m_hashmap->add( m_id, obj );

---

This code is being run inside a function in a factory object, and m_id is a signed int initialized to -1 in the factory's constructor.

When compiled with optimizations turned off (debug build), this code works fine (m_id is incremented to 0 before being passed to the constructor of Object, and then the object is added to the hash map with an id of 0).

When compiled with optimizations turned on (release build), I see very strange things: 0 is passed to the constructor of Object, which is correct. However, the add function is then called with -1 being passed in as the id, which results in an assert (and using the debugger is mostly worthless, the values of the ints being passed to the functions show up grayed out and as some large negative number).

The "fix" is simple - just increment m_id on the previous line and all works like it should (I'm guessing that declaring m_id as volatile would also work, but I've never actually had to use the volatile keyword before).

So my question is, am I actually violating the standard? Or is this an optimization bug? Or is something else going on?

Thanks!

Kelly

Optimizers are known for switching the order of instructions in order to make things run faster. However, this switching should only occur if it does not affect the outcome of the instructions. What you are seeing may very well be a bug with the optimizer, but viewing the generated assembly code would be needed to verify that. Even if you were not seeing this problem, I would recommend moving the m_id increment to its own line before the other two lines. This would make your code more readable. Remember, your goal is not to write your code in as few lines as possible.

Also, I am wondering why you would make the first valid m_id be 0. m_id is like a handle and 0 is NULL, which is often used to indicate the absence of a handle or an error.

Hi Dave,

Thanks for the response!

I agree, moving the increment to the previous line is more readable. And I'll look at digging into the assembly as some point in the near future to see exactly what's going on.

As for your thoughts on whether 0 should be valid or not, the short answer is I agree with you, but the person who designed the system long ago felt that 0 should be a valid id. For some reason all ids are signed, and a valid id is anything >= 0 (although, oddly enough in some cases, negative values are also valid ids - don't ask...).

Kelly

Quote:

---

Originally Posted by Runt888

Hello,

I've got a piece of code that's causing strange problems when compiled in release (using Visual Studio 2010). Am I violating some rule in the C++ standard by doing this?

Psuedocode:

---

Well, if you post psuedo-code, then we won't know what is really happening. So is the code your interpretation of what you're doing, or actual code that is being executed?

Code:

---

Object* obj = new Object( ++m_id, name );
m_hashmap->add( m_id, obj );

---

Not enough information.

How do we know if the creation of Object is not corrupting memory or is using a wild/uninitialized pointer that happens to point to m_id? What is the second parameter "name"? Is m_id a global variable that Object may be using or pointing to?

Quote:

---

So my question is, am I actually violating the standard? Or is this an optimization bug? Or is something else going on?

---

Post a compilable, full example that shows the issue, so that we can see everything that you're doing. I could easily code up an Object class that has the same issue you're seeing, and it has nothing to do with optimization bugs, but with just plain old faulty coding.

If you were to report a bug to Microsoft, they would have you recreate the problem with a real example -- same here. It is highly doubtful that such simple code would cause an optimization bug. If it were the case, then thousands of programmers would have reported it to MS or have documented this issue by now.

Regards,

Paul McKenzie

Quote:

---

The "fix" is simple - just increment m_id on the previous line and all works like it should (I'm guessing that declaring m_id as volatile would also work, but I've never actually had to use the volatile keyword before).

---

No fix is simple where there is no solid explanation for the fix.

Declaring a variable as volatile is a hint to the compiler that the contents of that variable can change without the knowledge of the compiler so the value shouldn't be cached in a register etc. It's usually used for variables pointing to memory that is changed directly by hardware or where a variable can be altered independently by multiple threads (together with syncronisation).

Does your program use multiple threads? as this is showing a classic sign of threading issues where the contents of a variable doesn't update correctly.

Quote:

---

Originally Posted by Runt888

So my question is, am I actually violating the standard?

---

I'd say you aren't violating the standard. And almost without exception when a seemingly unexplainable bug occurs it turns out to be something the programmer did after all.

Since the difference is between the debug and release modes it's most likely related to the use of assertions (or some other code that's present or missing depending on build mode). Are you sure you're not doing something in the nature of this for example,

assert (m_id=-1);

Completely remove all differences between the build modes in the add method and I'm sure it will work, alternatively the bug will manifest itself more clearly.

Also be aware that sometimes compilers will initialise variables when in 'debug mode' but leave them untouched when in 'release mode'.