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
  • To add to confusion, replacing b.Foo with bar::FOO works as expected: [Ideone](http://ideone.com/QnIEXp) – Anomander Rake Mar 28 '14 at 21:33
  • 1
    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. – Andy Mar 28 '14 at 21:39
  • 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: [Ideone](http://ideone.com/3mBIoO) – Anomander Rake Mar 28 '14 at 21:52
  • Andy: good catch! Such are the vagaries of reckless copying and pasting. – Anomander Rake Mar 28 '14 at 21:54
  • 1
    Submitted a bug report: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60702 – Anomander Rake Mar 28 '14 at 22:52
  • Yeah, I think that's the way to go. Couldn't make sense of it in clang or gcc 4.8.1. Good luck! – Andy Mar 28 '14 at 23:05
  • Here's the shorter test case demonstrating the bug in clang http://coliru.stacked-crooked.com/a/959f4c60a9f90116 – amdn Mar 29 '14 at 18:04
  • 1
    Clang was doing the wrong thing due to a [bug](http://llvm.org/PR19254) that I've now [fixed](http://llvm.org/viewvc/llvm-project?view=revision&revision=204869). We missed the 'initialize a `thread_local` variable' codepath entirely when emitting code for a member-access expression. – Richard Smith Mar 31 '14 at 05:38

1 Answers1

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));
}
amdn
  • 11,314
  • 33
  • 45
  • It would be great, if you could add your findings to the bug [report](http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60702). Thanks! – Anomander Rake Apr 02 '14 at 01:28
  • 1
    @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. – amdn Apr 02 '14 at 02:38
  • 1
    @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. – amdn Apr 02 '14 at 22:46
  • 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). – rustyx Aug 17 '16 at 08:58
  • seems the bug is still not fixed util gcc 7.1 – Tinro Nov 14 '17 at 02:04