Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static initialization order of variables in a single TU #55804

Closed
tchatow opened this issue Jun 1, 2022 · 7 comments
Closed

Static initialization order of variables in a single TU #55804

tchatow opened this issue Jun 1, 2022 · 7 comments
Assignees

Comments

@tchatow
Copy link
Contributor

tchatow commented Jun 1, 2022

I left a question on stack overflow detailing the issue: https://stackoverflow.com/q/72286507/5336393, but no answers yet. I'm not sure whether it is a Clang bug or if there is no initialization order guarantee.

The example is:

#include <cstdio>

struct TestValue;

inline const TestValue* v_ptr = nullptr;

struct TestValue {
    static const TestValue v1;

    TestValue() {
        v_ptr = this;
        printf("TestValue Initialized at %p\n", this);
    }
};

struct CallTest {
    CallTest() {
        printf("CallTest Initalized at %p\n", this);
        printf("v_ptr = %p\n", v_ptr);
    }
};

const inline TestValue TestValue::v1{};
const inline CallTest ct{};


int main() {}

Surprisingly, ct is initialized before v1

Using Clang 14.0.3, but this issue goes back further. Is this a bug or are there no guarantees about the initialization order?

https://clang.godbolt.org/z/M9e757r46

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 1, 2022

@llvm/issue-subscribers-clang-codegen

@yuanfang-chen
Copy link
Collaborator

yuanfang-chen commented Jun 2, 2022

@zygoloid @rnk

Based on the discussion https://reviews.llvm.org/D126341/new/#3541259, presume the test case here is valid (which I think so), this is due to our using COMDAT for inline variables (and we can't rely on the order in llvm.global_ctors), right?

@rnk
Copy link
Collaborator

rnk commented Jun 2, 2022

It's not clear to me if static data member is an inline variable which has the same ordering requirements from the standard, but essentially, yes, that's the issue.

@yuanfang-chen yuanfang-chen self-assigned this Jun 2, 2022
@yuanfang-chen
Copy link
Collaborator

It's not clear to me if static data member is an inline variable which has the same ordering requirements from the standard, but essentially, yes, that's the issue.

Thanks for the confirmation. I'll take a look at this.

@zygoloid
Copy link
Collaborator

zygoloid commented Jun 2, 2022

Based on the discussion https://reviews.llvm.org/D126341/new/#3541259, presume the test case here is valid (which I think so), this is due to our using COMDAT for inline variables (and we can't rely on the order in llvm.global_ctors), right?

I don't think the use of COMDATs is relevant here. That might cause similar problems in multi-translation-unit cases if the linker selects COMDATs inconsistently (but I don't think we know whether any real linkers do so). The entries in llvm.global_ctors are emitted in the wrong order, so even if LLVM preserves the order (which it seems to), we will perform initialization in the wrong order. I'm not sure why we're doing that; it looks to me like CodeGen should be emitting these initializers eagerly and in order.

Certainly switching away from using separate initialization functions and COMDATs for inline variables would solve this. I'm still optimistic that we can avoid doing so.

@zygoloid
Copy link
Collaborator

zygoloid commented Jun 2, 2022

Certainly switching away from using separate initialization functions and COMDATs for inline variables would solve this. I'm still optimistic that we can avoid doing so.

If not, I think we should assign mangled names to the initialization functions for inline variables so we can at least deduplicate them across TUs, even if we're going to end up invoking them more than once each.

@yuanfang-chen
Copy link
Collaborator

Thanks for the suggestions. I've put up a fix here https://reviews.llvm.org/D127233.

yuanfang-chen pushed a commit to yuanfang-chen/llvm-project that referenced this issue Jun 27, 2022
Fixes llvm#55804

The lexing order is already bookkept in DelayedCXXInitPosition but we
were not using it based on the wrong assumption that inline variable is
unordered. This patch fixes it by ordering entries in llvm.global_ctors
by orders in DelayedCXXInitPosition.

for llvm.global_ctors entries without a lexing order, ordering them by
the insertion order.

(This *mostly* orders the template instantiation in
https://reviews.llvm.org/D126341 intuitively, minus one tweak for which I'll
submit a separate patch.)

Differential Revision: https://reviews.llvm.org/D127233
thilinarmtb pushed a commit to nomp-org/llvm-project that referenced this issue Aug 24, 2022
Fixes llvm/llvm-project#55804

The lexing order is already bookkept in DelayedCXXInitPosition but we
were not using it based on the wrong assumption that inline variable is
unordered. This patch fixes it by ordering entries in llvm.global_ctors
by orders in DelayedCXXInitPosition.

for llvm.global_ctors entries without a lexing order, ordering them by
the insertion order.

(This *mostly* orders the template instantiation in
https://reviews.llvm.org/D126341 intuitively, minus one tweak for which I'll
submit a separate patch.)

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D127233
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants