7

First of all: I know that most optimization bugs are due to programming errors or relying on facts which may change depending on optimization settings (floating point values, multithreading issues, ...).

However I experienced a very hard to find bug and am somewhat unsure if there is any way to prevent these kind of errors from happening without turning the optimization off. Am I missing something? Could this really be an optimizer bug? Here's a simplified example:

struct Data {
  int    a;
  int    b;
  double c;
};

struct Test {
  void optimizeMe();

  Data m_data;
};

void Test::optimizeMe() {
  Data * pData; // Note that this pointer is not initialized!

  bool first = true;

  for (int i = 0; i < 3; ++i) {
    if (first) {
      first = false;

      pData = &m_data;

      pData->a = i * 10;
      pData->b = i * pData->a;
      pData->c = pData->b / 2;
    } else {
      pData->a = ++i;
    } // end if
  } // end for
};

int main(int argc, char *argv[]) {
  Test test;
  test.optimizeMe();
  return 0;
}

The real program of course has a lot more to do than this. But it all boils down to the fact that instead of accessing m_data directly, a (previously unitialized) pointer is being used. As soon as I add enough statements to the if (first)-part, the optimizer seems to change the code to something along these lines:

if (first) {
  first = false;

  // pData-assignment has been removed!

  m_data.a = i * 10;
  m_data.b = i * m_data.a;
  m_data.c = m_data.b / m_data.a;
} else {
  pData->a = ++i; // This will crash - pData is not set yet. 
} // end if

As you can see, it replaces the unnecessary pointer dereference with a direct write to the member struct. However it does not do this in the else-branch. It also removes the pData-assignment. Since the pointer is now still unitialized, the program will crash in the else-branch.

Of course there are various things which could be improved here, so you might blame it on the programmer:

  • Forget about the pointer and do what the optimizer does - use m_data directly.
  • Initialize pData to nullptr - that way the optimizer knows that the else-branch will fail if the pointer is never assigned. At least it seems to solve the problem in my test-environment.
  • Move the pointer assignment in front of the loop (effectively initializing pData with &m_data, which then could also be a reference instead of a pointer (for good measure). This makes sense because pData is needed in all cases so there is no reason to do this inside the loop.

The code is obviously smelly, to say the least, and I'm not trying to "blame" the optimizer for doing this. But I'm asking: What am I doing wrong? The program might be ugly, but it's valid code...

I should add that I'm using VS2012 with C++/CLI and v110_xp-Toolset. Optimization is set to /O2. Please also note that if you really want to reproduce the problem (that's not really the point of this question though) you need to play around with the complexity of the program. This is a very simplified example and the optimizer sometimes doesn't remove the pointer assignment. Hiding &m_data behind a function seems to "help".

EDIT:

Q: How do I know that the compiler is optimizing it to something like the example provided?

A: I'm not very good at reading assembler, I have looked at it however and have made 3 observations which make me believe that it's behaving this way:

  1. As soon as optimization kicks in (adding more assignments usually does the trick) the pointer assignment has no associated assembler statement. It also hasn't been moved up to the declaration, so it's really left uninitialized it seems (at least to me).
  2. In cases where the program crashes, the debugger skips the assignment statement. In cases where the program runs without problems, the debugger stops there.
  3. If I watch the content of pData and the content of m_data while debugging, it clearly shows that all assignments in the if-branch have an effect on m_data and m_data receives the correct values. The pointer itself it still pointing to the same uninitialized value it had from the beginning. Therefore I have to assume that it is in fact not using the pointer to make the assignments at all.

Q: Does it have to do anything with i (Loop unrolling)?

A: No, the actual program actually uses do { ... } while() to loop over a SQL SELECT-resultset so the iteration count is completely runtime-specific and cannot be predetermined by the compiler.

24
  • Reduce this to an SSCCE. Commented Apr 22, 2013 at 18:07
  • 7
    @djechlin: Bugs affected by optimization may be hard to reduce to small code samples. The question explicitly states this is already a simplified example. Commented Apr 22, 2013 at 18:09
  • 1
    Have you looked at the assembly or are you just assuming that the optimizer removes the pData assignment? Commented Apr 22, 2013 at 18:11
  • 3
    Yeah... have you verified that this is happening? Or are you just speculating? Commented Apr 22, 2013 at 18:13
  • 1
    Microsoft state to use the highest warning levels to avoid subtle hard to find bugs, I believe this is one of them, init pData to nullptr and its likely the issue will go away. Commented Apr 22, 2013 at 18:19

2 Answers 2

5

It sure looks like an bug to me. It's fine for the optimizer to eliminate the unnecessary redirection, but it should not eliminate the assignment to pData.

Of course, you can work around the problem by assigning to pData before the loop (at least in this simple example). I gather that the problem in your actual code isn't as easily resolved.

Sign up to request clarification or add additional context in comments.

2 Comments

Actually removing the pointer as a whole has been the solution in the actual code. The code is over 15 years old and most bugs like these are easy to resolve because the code simply needs some simple refactoring. Finding the bug is a totally different story.
Since I am fairly sure now that this has been a compiler bug I will accept this as the answer. As soon as I have filed a bugreport with microsoft, I will add the link to the question, so there will be a real reproducable example available. I think it's more of a C++/CLI thing though, the normal compiler didn't mess up.
1

I also vote for an optimizer bug if it is really reproducible in this example. To overrule the optimizer you could try to declare pData as volatile.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.