1

I struggle with a bug since hours now. Basically, I do some simple bit operation on an uint64_t array in main.c (no function calls). It works properly on gcc (Ubuntu), MSVS2019 (Windows 10) in Debug, but not in Release. However my target architecture is x64/Windows, so I need to get it work properly with MSVS2019/Release. Besides that, I'm curious what the reason for the problem is. None of the compilers shows errors or warnings.

Now, as soon as I add a totally unrelated command to the loop (commented printf()), it works properly.

...
int q = 5;
uint64_t a[32] = { 0 };
// a[] is filled with data
for (int i = 0; i < 32; i++) {
    a[q] = (a[q] << 2) | 8;
    // printf("%i \n", i); // that's the line which makes it work
}
...

Initially I believed that I messed up the stack somewhere before the for() loop, but I checked it up multiple times ... all fine!

  • all used variables are checked to be initialized
  • no pointer returns of local variables (in scope)
  • array indexing (reads and writes) all within declaration limits (in scope)

All Google/SE posts explain subject UB to some of the above reasons, but none of these apply for my code. Also the fact, that it works in MSVS2019/Debug and gcc shows the code works.

What do I miss?

--- UPDATE (24.08.2021 12:00) ---

I'm completely stuck, since added printf() modifies the result and MSVS/Debug works. So how can I inspect variables?!

@Lev M There are quite some calculations before and after the shown for() loop. That's why I skipped most of the code and just showed the snippet where I could influence the code towards working correctly. I know what should be the final result (it's just a uint64_t), and it's wrong with the Release version of MSVS. I also checked w/o the for() loop. It's not optimized "away". If I leave it out completely, the result is again different.

@tstanisl It's just a matter of an uint64_t number. I know that input A should output B.

@Steve Summit That's why I posted (a bit desperate). I checked in all directions, isolated as much code as I could and yet ... no uninitialized variable or array out of bound. Driving me nuts.

@Craig Estey The code is unfortunately quite extensive. I wonder ... could the error also be in a part of the code which doesn't run?

@Eric Postpischil Agreed!

@Nate Eldredge I tested on valgrind (see below).

...
==13997== HEAP SUMMARY:
==13997==     in use at exit: 0 bytes in 0 blocks
==13997==   total heap usage: 1 allocs, 1 frees, 1,024 bytes allocated
==13997==
==13997== All heap blocks were freed -- no leaks are possible
==13997==
==13997== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

--- UPDATE (24.08.2021 18:00) ---

I found the reason for the problem (after countless trial-and-errors), but no solution yet. I post more of the code.

...
int q = 5;
uint64_t a[32] = { 0 };
// a[] is filled with data
for (int i = 0; i < 32; i++) {
    a[q] = (a[q] << 2) | 8;
    // printf("%i \n", i); // that's the line which makes it work
}
for (int i = 0; i < 32; i++) {
    a[q] = (a[q] << 3) | 3;
}
...

In fact, the MSVS/Release compiler did this:

...
int q = 5;
uint64_t a[32] = { 0 };
// a[] is filled with data
for (int i = 0; i < 32; i++) {
    a[q] = (a[q] << 2) | 8;
    a[q] = (a[q] << 3) | 3;
}
...

... which is not the same. Never seen such a thing!

How can I force the compiler to keep the 2 for() loops separate?

geohei
  • 696
  • 4
  • 15
  • It might be an optimization issue - perhaps VS is somehow optimizing away the loop when you do a release build? What result are you getting in 'release' - data does not change? Random garbage? – Lev M. Aug 23 '21 at 17:48
  • What is the expected behaviour? – tstanisl Aug 23 '21 at 17:49
  • 2
    The symptoms you've described almost always mean you've got an uninitialized local variable, or a local array that you're accessing outside the bounds of. I know you've said you checked those, but I'd keep looking there! – Steve Summit Aug 23 '21 at 17:50
  • 1
    The code you've posted _has_ to work. The issue must be in the code that you _didn't_ post. _What_ does it do [wrong] and _should_ it do? I suspect that the full code is small enough that you could _edit_ your question and post it. – Craig Estey Aug 23 '21 at 17:54
  • Re “Also the fact, that it works in MSVS2019/Debug and gcc shows the code works”: That is not a valid logical inference. – Eric Postpischil Aug 23 '21 at 17:56
  • Have you tried running under valgrind, AddressSanitizer, etc? – Nate Eldredge Aug 23 '21 at 22:05
  • `In fact, the MSVS/Release compiler did this` because it is exactly what should be done. For the code you posted there is no difference between two and one `for` loop versions – 0___________ Aug 24 '21 at 16:49
  • @0___________ You are right I omitted an essential part. The index of array ``a[]`` is outside the ``for()`` loops. I corrected all code snippets by adding ``int q = 5;``. My apologies. However it doesn't change anything to the fact that only MSVS/Release does this attempt to optimize. – geohei Aug 24 '21 at 17:01
  • @geohei When you say "doesn't change anything" do you mean that adding `int q = 5` does not fix the problem, or that it *does* fix the problem, but doesn't fix the puzzle for you that you don't understand what happened when it wasn't there? – Steve Summit Aug 24 '21 at 17:07
  • @geohei editing the question making comments not relevant is another bad habit – 0___________ Aug 24 '21 at 17:09
  • @geohei You're asking about two loops being treated differently. But one loop you showed has `a[q]`, and one loop has `a[i]`. What's going on?!? – Steve Summit Aug 24 '21 at 17:18
  • @geohei You're probably going to need to find another way of solving this particular problem. I don't think you're going to be able to describe it clearly and accurately enough for us to have any hope of understanding it or explaining it to you. – Steve Summit Aug 24 '21 at 17:20
  • Ok ... I try to clean up the mess. ``int q = 5`` should have been in the snippets right from the beginning (my omit). The OP is now correct (after I re-edited it). Snippets are real code and the problem can be simulated. The problem is not **fixed** but just **identified**! The reason for the problem is that MSVS/Release merges the ´´for()´´ loops. I believe the OP should be clear now. IMHO it is a compiler doing too much optimization. Sorry for the generated confusion. – geohei Aug 24 '21 at 18:07
  • @geohei the first loop can be optimized out as it always will be 0xaaaaaaaaaaaaaaa8 despite the initial value. Your code is simply wrong!! – 0___________ Aug 24 '21 at 18:37
  • https://godbolt.org/z/Pbj3K1nrz – 0___________ Aug 24 '21 at 18:44
  • The second loop is also not needed as it will produce the same value every time as first loop will produce the same result. – 0___________ Aug 24 '21 at 18:47
  • BTW discovered a missed gcc optimization\ – 0___________ Aug 24 '21 at 18:54
  • The OP shows the minimum code to demonstrate the problem. Even if the first loop's result is always ``0xaaaaaaaaaaaaaaa8``, the entire code is much more complex (e.g. both loops are not limited to 32, but are variable conforming to % 8). MSVS/Release optimizes where it should not (gcc makes it better). I found the solution in making the array ``volatile uint64_t a[64]``, but this can only be considered as workaround to artificially avoid optimization. I didn't find another solution other than that. A pity the OP is closed. I would have loved to answer in order to explain what went wrong. – geohei Aug 25 '21 at 06:23

1 Answers1

0

Summary:

MSVS/Release (default solution properties) optimization will change this code ...

// Code 1
...
int q = 5;
uint64_t a[32];
// a[] is filled with data
for (int i = 0; i < 32; i++) {
    a[q] = (a[q] << 2) | 8;
    // printf("%i \n", i); // that's the line which makes it work
}
for (int i = 0; i < 32; i++) {
    a[q] = (a[q] << 3) | 3;
}
...

... into the following one, which is not the same as ...

// Code 2
...
int q = 5;
uint64_t a[32];
// a[] is filled with data
for (int i = 0; i < 32; i++) {
    a[q] = (a[q] << 2) | 8;
    a[q] = (a[q] << 3) | 3;
}
...

Above excerpt is slightly simplified, since not limited to constant 32 loops, but kept variable (% 8). Hence 64-bit constants can't be used as commented by a user.

Discoveries:

MSVS/Release - fails
MSVS/Debug - works
gcc/Release - works
gcc/Debug - works

MSVS/Release optimization merges the two for() loops (Code 1) into one for() loop (Code 2).

Fixes:

The commented printf() provides an artificial fix this as the compiler sees the requirement to print an intermediate result.

An alternative fix would be to to use the type qualifier volatile for a[].

The root of the issue is, that MSVS optimization doesn't consider that the index q remains the same in both loops, meaning that the first loop needs to finish before the second loop starts.

geohei
  • 696
  • 4
  • 15