thread_local member variable construction
Asked Answered
A

1

7

I'm facing some strange behavior with thread_local and not sure whether I'm doing something wrong or it's a GCC bug. I have the following minimal repro scenario:

#include <iostream>

using namespace std;

struct bar {
    struct foo {
        foo () {
            cerr << "foo" << endl;
        }
        int i = 42;
    };

    static thread_local foo FOO;
};

static thread_local bar::foo FREE_FOO;
thread_local bar::foo bar::FOO;

int main() {
    bar b;
    cerr << "main" << endl;
    // cerr << FREE_FOO.i << endl;
    cerr << b.FOO.i << endl;
    return 0;
}

With the commented line above the output looks like this:

main
0

Ideone

With it uncommented, it becomes this:

main
foo
foo
42
42

Ideone

Am I just missing something stupid here?

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.8/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 4.8.1-10ubuntu9' --with-bugurl=file:///usr/share/doc/gcc-4.8/README.Bugs --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.8 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.8 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.8-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.8-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.8-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.8.1 (Ubuntu/Linaro 4.8.1-10ubuntu9) 

Update:

This provides unexpected results as well:

#include <iostream>

using namespace std;

template<class T>
struct bar {
    struct foo {
        foo () {
            cerr << "bar::foo" << endl;
        }
        int i = 42;
    };

    void baz() {
        cerr << bar::FOO.i << endl;
    }

    static thread_local foo FOO;
};

struct far {
    struct foo {
        foo () {
            cerr << "far::foo" << endl;
        }
        int i = 42;
    };

    void baz() {
        cerr << far::FOO.i << endl;
    }

    static thread_local foo FOO;
};

template<class T> thread_local typename bar<T>::foo bar<T>::FOO;
thread_local typename far::foo far::FOO;

int main() {
    cerr << "main" << endl;
    bar<int> b;
    b.baz();

    far f;
    f.baz();
    return 0;
}

Result:

main
0
far::foo
bar::foo
42
Adanadana answered 28/3, 2014 at 20:49 Comment(8)
To add to confusion, replacing b.Foo with bar::FOO works as expected: IdeoneAdanadana
Note that the static on static thread_local bar::foo FREE_FOO; has no effect as you are just modifying the linkage there (which defaults to internal). Remove it and you get the same behavior.Acknowledgment
A member of a template class, accessed indirectly, also remains uninitialized, while the same access to the member of a non-template class triggers both initializations: IdeoneAdanadana
Andy: good catch! Such are the vagaries of reckless copying and pasting.Adanadana
Submitted a bug report: gcc.gnu.org/bugzilla/show_bug.cgi?id=60702Adanadana
Yeah, I think that's the way to go. Couldn't make sense of it in clang or gcc 4.8.1. Good luck!Acknowledgment
Here's the shorter test case demonstrating the bug in clang coliru.stacked-crooked.com/a/959f4c60a9f90116Freud
Clang was doing the wrong thing due to a bug that I've now fixed. We missed the 'initialize a thread_local variable' codepath entirely when emitting code for a member-access expression.Marja
F
6

This is too long for a comment, though I don't claim to fully understand it.

I have a shorter version you can run in Coliru

#include <iostream>
using namespace std;

struct foo {
    int i;
    foo() : i{42} {}
};

struct bar {
    static thread_local foo FOO;
};

thread_local foo bar::FOO;

int main() {
    //cerr << string((bar::FOO.i == 42) ? "Ok" : "Bug") << endl; //Ok
    cerr << string((bar().FOO.i == 42) ? "Ok" : "Bug") << endl;  //Bug
}

I think the bug is in this gcc source file

https://chromium.googlesource.com/native_client/nacl-gcc/+/upstream/master/gcc/cp/decl2.c

At this point gcc is trying to decide if FOO, which is a static member of bar, needs a wrapper function to detect if it has been initialized... it decides no wrapper is needed, which is incorrect. It checks

  1. Is it not an error_operand_p ? Yes, it is not. (I guess)
  2. Is it thread_local (DECL_THREAD_LOCAL_P) ? Yes it is thread_local.
  3. Is it not gnu __thread extension (DECL_GNU_TLS_P) ? Yes, it is not.
  4. Is it not declared in function scope (DECL_FUNCTION_SCOPE_P) ? Yes, it is not.
  5. Is the variable not defined in another translation unit (TU)? Yes, it is not. (bug?)
  6. Does it not have a non-trivial destructor? Yes, it does not.
  7. Does it have no initializer or a constant one? It has an initializer, but it is constant.
  8. It doesn't need a wrapper

The flaw is either:

  1. Concluding that if the initializer is constant then it isn't dynamically initialized, or
  2. Failing to properly do the static initialization, or
  3. Failing to notice that even though it is a member variable it could be externally defined

Since the initialization is done by the constructor, I think that is the source of the confusion, a constructor is called, but the value is a constant.

Here's the code

/* Returns true iff we can tell that VAR does not have a dynamic
   initializer.  */

static bool
var_defined_without_dynamic_init (tree var)
{
    /* If it's defined in another TU, we can't tell.  */
    if (DECL_EXTERNAL (var))
        return false;
    /* If it has a non-trivial destructor, registering the destructor
        counts as dynamic initialization.  */
    if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (var)))
        return false;
    /* If it's in this TU, its initializer has been processed.  */
        gcc_assert (DECL_INITIALIZED_P (var));
    /* If it has no initializer or a constant one, it's not dynamic.  */
        return (!DECL_NONTRIVIALLY_INITIALIZED_P (var)
          || DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (var));
}

/* Returns true iff VAR is a variable that needs uses to be
   wrapped for possible dynamic initialization.  */

static bool
var_needs_tls_wrapper (tree var)
{
    return (!error_operand_p (var)
          && DECL_THREAD_LOCAL_P (var)
          && !DECL_GNU_TLS_P (var)
          && !DECL_FUNCTION_SCOPE_P (var)
          && !var_defined_without_dynamic_init (var));
}
Freud answered 29/3, 2014 at 0:8 Comment(5)
It would be great, if you could add your findings to the bug report. Thanks!Adanadana
@AnomanderRake clang had also failed the test you created as well as my shorter version, I submitted that to the clang team and they found it was a duplicate of a bug they recently fixed which had to do with accessing the thread_local static via the this pointer. I'll update your bugreport to point to the fixed clang bug... I just notice that Richard Smith left a comment to that effect here.Freud
@AnomanderRake I've updated your gcc bug report, I've added the shorter testcase and pointed the gcc team to the clang fix, I think that is more productive than my speculation.Freud
Not to say there is no bug in the compiler, but it is bad practice to access static members via this. It makes the code less self-documenting and error-prone (if not for yourself then for those who have to work with your code).Nike
seems the bug is still not fixed util gcc 7.1Orts

© 2022 - 2024 — McMap. All rights reserved.