(Optimization?) Bug regarding GCC std::thread

Question

While testing some functionality with std::thread, a friend encountered a problem with GCC and we thought it's worth asking if this is a GCC bug or perhaps there's something wrong with this code (the code prints (for example) "7 8 9 10 1 2 3", but we expect every integer in [1,10] to be printed):

#include <algorithm>
#include <iostream>
#include <iterator>
#include <thread>

int main() {
    int arr[10];
    std::iota(std::begin(arr), std::end(arr), 1);
    using itr_t = decltype(std::begin(arr));

    // the function that will display each element
    auto f = [] (itr_t first, itr_t last) {
        while (first != last) std::cout<<*(first++)<<' ';};

    // we have 3 threads so we need to figure out the ranges for each thread to show
    int increment = std::distance(std::begin(arr), std::end(arr)) / 3;
    auto first    = std::begin(arr);
    auto to       = first + increment;
    auto last     = std::end(arr);
    std::thread threads[3] = {
        std::thread{f, first, to},
        std::thread{f, (first = to), (to += increment)},
        std::thread{f, (first = to), last} // go to last here to account for odd array sizes
    };
    for (auto&& t : threads) t.join();
}

The following alternate code works:

int main()
{
    std::array<int, 10> a;
    std::iota(a.begin(), a.end(), 1);
    using iter_t = std::array<int, 10>::iterator;
    auto dist = std::distance( a.begin(), a.end() )/3;
    auto first = a.begin(), to = first + dist, last = a.end();
    std::function<void(iter_t, iter_t)> f =
        []( iter_t first, iter_t last ) {
            while ( first != last ) { std::cout << *(first++) << ' '; }
        };
    std::thread threads[] {
            std::thread { f,  first, to },
            std::thread { f, to, to + dist },
            std::thread { f, to + dist, last }
    };
    std::for_each(
        std::begin(threads),std::end(threads),
        std::mem_fn(&std::thread::join));
    return 0;
}

We thought maybe its got something to do with the unsequenced evaluation of function's arity or its just the way std::thread is supposed to work when copying non-std::ref-qualified arguments. We then tested the first code with Clang and it works (and so started to suspect a GCC bug).

Compiler used: GCC 4.7, Clang 3.2.1

EDIT: The GCC code gives the wrong output with the first version of the code, but with the second version it gives the correct output.

Sarthe

I don't understand the problem. Which code is problematic, the one in the pastebin or the one posted here? In any case, please post the one that is currently on pastebin here. – Zoes

I'm sorry I didn't specify well. The pastebin code is the one with the problem. I'm using my cellphone and its difficult to type with it – Sarthe

And what output did you get, and what did you expect? – Smirch

The pastebin code outputs-> "7 8 9 10 1 2 3" so "4 5 6" went missing – Sarthe

Have you tried to pass the arguments to the thread constructor in the same way as you do it in the posted code? It could be that the comma inside an array constructor is not a sequence point, so it could be that in both the second and the third thread first points to to + dist. – Zoes

I have tried this, this is actually the problem, I'll write up an answer for that. – Zoes

@Zoes There are no sequence points in C++11. The order of evaluation inside a braced-init-list is fully defined though, as if there were a sequence point at each of the (top-level) ,. – Joniejonina

Possibly this bug, which would make this question a rough duplicate of this one – Joniejonina

@Joniejonina is correct -- it's a bug in gcc, it's fixed in the latest release – Goring

I'd also suspect std::cout not being locked when used in f. When you have multiple threads using it for output in parallel, you might get strange results. – Cat

It outputs 1 2 3 4 5 6 7 8 9 10 as exepected when compiled with gcc-4.9.2. – Bench

Answer 1

From this modified program:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <thread>
#include <sstream>

int main()
{
    int arr[10];
    std::iota(std::begin(arr), std::end(arr), 1);
    using itr_t = decltype(std::begin(arr));

    // the function that will display each element
    auto f = [] (itr_t first, itr_t last) {
        std::stringstream ss;
        ss << "**Pointer:" << first  << " | " << last << std::endl;
        std::cout << ss.str();
        while (first != last) std::cout<<*(first++)<<' ';};

    // we have 3 threads so we need to figure out the ranges for each thread to show
    int increment = std::distance(std::begin(arr), std::end(arr)) / 3;
    auto first    = std::begin(arr);
    auto to       = first + increment;
    auto last     = std::end(arr);
    std::thread threads[3] = {
        std::thread{f, first, to},
#ifndef FIX
        std::thread{f, (first = to), (to += increment)},
        std::thread{f, (first = to), last} // go to last here to account for odd array sizes
#else
        std::thread{f,  to,  to+increment},
        std::thread{f,  to+increment, last} // go to last here to account for odd array sizes
#endif
    };
    for (auto&& t : threads) {
        t.join();
    }
}

I add the prints of the first and last pointer for lambda function f, and find this interesting results (when FIX is undefined):

**Pointer:0x28abd8 | 0x28abe4
1 2 3 **Pointer:0x28abf0 | 0x28abf0
**Pointer:0x28abf0 | 0x28ac00
7 8 9 10

Then I add some code for the #ELSE case for the #ifndef FIX. It works well.

- Update: This conclusion, the original post below, is wrong. My fault. See Josh's comment below -

I believe the 2nd line std::thread{f, (first = to), (to += increment)}, of threads[] contains a bug: The assignment inside the two pairs of parenthesis, can be evaluated in any order, by the parser. Yet the assignment order of 1st, 2nd and 3rd argument of the constructor needs to keep the order as given.

--- Update: corrected ---

Thus the above debug printing results suggest that GCC4.8.2 (my version) is still buggy (not to say GCC4.7), but GCC 4.9.2 fixes this bug, as reported by Maxim Yegorushkin (see comment above).

Osset

Similarly, for an expression (a+b)*(c+d), the two parenthesis pairs can be executed in any order as the compiler prefers. – Osset

Robin Hsu, according to the C++ standard and CppReference, it states that In list-initialization, every value computation and side effect of a given initializer clause is sequenced before every value computation and side effect associated with any initializer clause that follows it in the comma-separated list of the initializer list. So, saying The compiler can choose which parenthesis to evaluate first is incorrect. – Sarthe

Josh, you are right. I have corrected it in my answer. Sorry for everyone. – Osset