[FE] Manipulate the first byte of guard variable type in both load and store operation

[Xiangling_L]

Summary

As Itanium ABI[http://itanium-cxx-abi.github.io/cxx-abi/abi.html#once-ctor] points out:

The size of the guard variable is 64 bits. The first byte (i.e. the byte at the address of the full variable) shall contain the value 0 prior to initialization of the associated variable, and 1 after initialization is complete.

We should manipulate the first byte of guard variable in both load and store operation in general cases.

[hubert.reinterpretcast]

clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp

193

Okay, so the store here is wrong, but it's got nothing to do with AIX.

The ABI doc (http://itanium-cxx-abi.github.io/cxx-abi/abi.html#once-ctor) says:

the first byte (i.e., the byte with lowest address)

So the bug here seems to be that the Itanium ABI implementation in Clang is incorrect for big-endian systems.

[hubert.reinterpretcast]

clang/test/CodeGenCXX/aix-static-init.cpp

144

What actually does the definition of _ZGVZN5test41fEvE11staticLocal look like? The test seems to be missing something pertinent for the change being proposed here. I will note that the symbol is 8-bytes (and aligned accordingly) from XL.

[Xiangling_L]

clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp

193

That makes sense to me. I will update the patch accordingly.

[Xiangling_L]

Align load and store operation related to guard variable by both using the first byte of the variable.

[abhina.sreeskantharajan]

clang/test/CodeGenCXX/global-init.cpp

84

nit: the lines above omit the end of the line. It's probably better to be consistent

[hubert.reinterpretcast]

guard.patch

1 ↗

(On Diff #320864)

Looks like this file got added by accident.

[Xiangling_L]

clang/test/CodeGenCXX/global-init.cpp

84

It didn't omit the end of the line. It is because before my patch, there was no casting.

guard.patch

1 ↗

(On Diff #320864)

Thanks!

[hubert.reinterpretcast]

clang/test/CodeGenCXX/global-init.cpp

84

Hi @Xiangling_L, I think Abhina meant the lines above in this file. The load has a bitcast too (and stops after the guard variable name).

[Xiangling_L]

Remove redundant file;

[Xiangling_L]

Addressed the comments related to test formatting;

clang/test/CodeGenCXX/global-init.cpp

84

I see. Sorry for the misunderstanding. @abhina.sreeskantharajan

[abhina.sreeskantharajan]

LGTM, thanks!

[hubert.reinterpretcast]

LGTM. I think there would be more concern if the load/store actions were previously symmetric, but this patch is actually needed for the symmetry. Nevertheless, please wait a bit before committing this (since this is a change for other platforms as well).

clang/lib/CodeGen/ItaniumCXXABI.cpp

void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
   if (threadsafe) {
     // Pop the guard-abort cleanup if we pushed one.
     CGF.PopCleanupBlock();
 
     // Call __cxa_guard_release.  This cannot throw.
     CGF.EmitNounwindRuntimeCall(getGuardReleaseFn(CGM, guardPtrTy),
                                 guardAddr.getPointer());
   } else {
-    Builder.CreateStore(llvm::ConstantInt::get(guardTy, 1), guardAddr);
+    // Store 1 into the first byte of the guard variable after initialization is
+    // complete.
+    Builder.CreateStore(llvm::ConstantInt::get(CGM.Int8Ty, 1),
+                        Builder.CreateElementBitCast(guardAddr, CGM.Int8Ty));
   }
 
   CGF.EmitBlock(EndBlock);
 }
 
 /// Register a global destructor using __cxa_atexit.
 static void emitGlobalDtorWithCXAAtExit(CodeGenFunction &CGF,
                                         llvm::FunctionCallee dtor,

clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp

 A<int> &bar() {
   A<int> *a = new A<int>;
   return *a;
 }
 template <>
 A<int> A<int>::instance = bar();
 } // namespace test2
 
+// CHECK: @_ZGVN5test12t2E = linkonce_odr global i64 0, align 8
+// CHECK: @_ZGVN5test21AIvE8instanceE = weak_odr global i64 0, align 8
+// CHECK: @_ZGVN5test12t1IiEE = linkonce_odr global i64 0, align 8
 // CHECK: @llvm.global_ctors = appending global [4 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__cxx_global_var_init.1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @__cxx_global_var_init.2, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @__cxx_global_var_init.4, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I__, i8* null }]
 // CHECK: @llvm.global_dtors = appending global [4 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__finalize__ZN5test12t2E, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @__finalize__ZN5test21AIvE8instanceE, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @__finalize__ZN5test12t1IiEE, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__D_a, i8* null }]
 
 // CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
 // CHECK32: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t0E, i32 2)
 // CHECK64: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t0E, i32 signext 2)
 // CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t0E)

 // CHECK:   br label %destruct.end
 
 // CHECK: destruct.end:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__cxx_global_var_init.1() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load atomic i8, i8* bitcast (i64* @_ZGVN5test12t2E to i8*) acquire
+// CHECK:   %0 = load atomic i8, i8* bitcast (i64* @_ZGVN5test12t2E to i8*) acquire, align 8
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
 // CHECK: init.check:
 // CHECK:   %1 = call i32 @__cxa_guard_acquire(i64* @_ZGVN5test12t2E)
 // CHECK:   %tobool = icmp ne i32 %1, 0
 // CHECK:   br i1 %tobool, label %init, label %init.end
 

 // CHECK:   br label %destruct.end
 
 // CHECK: destruct.end:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__cxx_global_var_init.2() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load i8, i8* bitcast (i64* @_ZGVN5test21AIvE8instanceE to i8*)
+// CHECK:   %0 = load i8, i8* bitcast (i64* @_ZGVN5test21AIvE8instanceE to i8*), align 8
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
 // CHECK: init.check:
 // CHECK:   call void @_ZN5test21AIvEC1Ev(%"struct.test2::A"* {{[^,]*}} @_ZN5test21AIvE8instanceE)
 // CHECK:   %1 = call i32 @atexit(void ()* @__dtor__ZN5test21AIvE8instanceE)
-// CHECK:   store i64 1, i64* @_ZGVN5test21AIvE8instanceE
+// CHECK:   store i8 1, i8* bitcast (i64* @_ZGVN5test21AIvE8instanceE to i8*), align 8
 // CHECK:   br label %init.end
 
 // CHECK: init.end:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__dtor__ZN5test21AIvE8instanceE() [[ATTR:#[0-9]+]] {
 // CHECK: entry:

 // CHECK:   br label %destruct.end
 
 // CHECK: destruct.end:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__cxx_global_var_init.4() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load i8, i8* bitcast (i64* @_ZGVN5test12t1IiEE to i8*)
+// CHECK:   %0 = load i8, i8* bitcast (i64* @_ZGVN5test12t1IiEE to i8*), align 8
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
 // CHECK: init.check:
 // CHECK32: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t1IiEE, i32 2)
 // CHECK64: call void @_ZN5test15Test1C1Ei(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t1IiEE, i32 signext 2)
 // CHECK:   %1 = call i32 @atexit(void ()* @__dtor__ZN5test12t1IiEE)
-// CHECK:   store i64 1, i64* @_ZGVN5test12t1IiEE
+// CHECK:   store i8 1, i8* bitcast (i64* @_ZGVN5test12t1IiEE to i8*), align 8
 // CHECK:   br label %init.end
 
 // CHECK: init.end:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define internal void @__dtor__ZN5test12t1IiEE() [[ATTR:#[0-9]+]] {
 // CHECK: entry:

clang/test/CodeGenCXX/aix-static-init.cpp

struct Test4 {
     ~Test4();
   };
 
   void f() {
     static Test4 staticLocal;
   }
 } // namespace test4
 
+// CHECK: @_ZGVZN5test41fEvE11staticLocal = internal global i64 0, align 8
 // CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I__, i8* null }]
 // CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__D_a, i8* null }]
 
 // CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
 // CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* {{[^,]*}} @_ZN5test12t1E)
 // CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
 // CHECK:   ret void

 // CHECK:   br label %destruct.end
 
 // CHECK: destruct.end:
 // CHECK:   ret void
 // CHECK: }
 
 // CHECK: define void @_ZN5test41fEv() [[ATTR:#[0-9]+]] {
 // CHECK: entry:
-// CHECK:   %0 = load atomic i8, i8* bitcast (i64* @_ZGVZN5test41fEvE11staticLocal to i8*) acquire
+// CHECK:   %0 = load atomic i8, i8* bitcast (i64* @_ZGVZN5test41fEvE11staticLocal to i8*) acquire, align 8
 // CHECK:   %guard.uninitialized = icmp eq i8 %0, 0
 // CHECK:   br i1 %guard.uninitialized, label %init.check, label %init.end
 
 // CHECK: init.check:
 // CHECK:   %1 = call i32 @__cxa_guard_acquire(i64* @_ZGVZN5test41fEvE11staticLocal)
 // CHECK:   %tobool = icmp ne i32 %1, 0
 // CHECK:   br i1 %tobool, label %init, label %init.end
 
 // CHECK: init:
 // CHECK:   call void @_ZN5test45Test4C1Ev(%"struct.test4::Test4"* {{[^,]*}} @_ZZN5test41fEvE11staticLocal)
 // CHECK:   %2 = call i32 @atexit(void ()* @__dtor__ZZN5test41fEvE11staticLocal)
 // CHECK:   call void @__cxa_guard_release(i64* @_ZGVZN5test41fEvE11staticLocal)
 // CHECK:   br label %init.end

clang/test/CodeGenCXX/cxx11-thread-local.cpp

 // LINUX-SAME: comdat($_ZN1VIfE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIfE1mE to i8*)
 // CHECK: %[[VF_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[VF_M_INITIALIZED]],
 // need init:
 // CHECK: call i32 @_Z1gv()
 // CHECK: store i32 %{{.*}}, i32* @_ZN1VIfE1mE, align 4
-// CHECK: store i64 1, i64* @_ZGVN1VIfE1mE
+// CHECK: store i8 1, i8* bitcast (i64* @_ZGVN1VIfE1mE to i8*)
 // CHECK: br label
 
 // LINUX: define internal void @[[XF_M_INIT]]()
 // DARWIN: define internal cxx_fast_tlscc void @[[XF_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1XIfE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIfE1mE to i8*)
 // CHECK: %[[XF_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[XF_M_INITIALIZED]],
 // need init:
 // LINUX: call {{.*}}__cxa_thread_atexit
 // DARWIN: call {{.*}}_tlv_atexit
-// CHECK: store i64 1, i64* @_ZGVN1XIfE1mE
+// CHECK: store i8 1, i8* bitcast (i64* @_ZGVN1XIfE1mE to i8*)
 // CHECK: br label
 
 // LINUX: declare i32 @__cxa_thread_atexit(void (i8*)*, i8*, i8*)
 // DARWIN: declare i32 @_tlv_atexit(void (i8*)*, i8*, i8*)
 
 // DARWIN: declare cxx_fast_tlscc i32* @_ZTWN1VIcE1mE()
 // LINUX: define linkonce_odr hidden i32* @_ZTWN1VIcE1mE() {{#[0-9]+}} comdat {
 // LINUX: br i1 icmp ne (void ()* @_ZTHN1VIcE1mE,

 // LINUX-SAME: comdat($_ZN1VIiE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIiE1mE to i8*)
 // CHECK: %[[V_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[V_M_INITIALIZED]],
 // need init:
 // CHECK: call i32 @_Z1gv()
 // CHECK: store i32 %{{.*}}, i32* @_ZN1VIiE1mE, align 4
-// CHECK: store i64 1, i64* @_ZGVN1VIiE1mE
+// CHECK: store i8 1, i8* bitcast (i64* @_ZGVN1VIiE1mE to i8*)
 // CHECK: br label
 
 // LINUX: define internal void @[[X_M_INIT]]()
 // DARWIN: define internal cxx_fast_tlscc void @[[X_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1XIiE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIiE1mE to i8*)
 // CHECK: %[[X_M_INITIALIZED:.*]] = icmp eq i8 %{{.*}}, 0
 // CHECK: br i1 %[[X_M_INITIALIZED]],
 // need init:
 // LINUX: call {{.*}}__cxa_thread_atexit
 // DARWIN: call {{.*}}_tlv_atexit
-// CHECK: store i64 1, i64* @_ZGVN1XIiE1mE
+// CHECK: store i8 1, i8* bitcast (i64* @_ZGVN1XIiE1mE to i8*)
 // CHECK: br label
 
 // CHECK: define {{.*}}@[[GLOBAL_INIT:.*]]()
 // CHECK: call void @[[C_INIT]]()
 // CHECK: call void @[[E_INIT]]()
 
 
 // CHECK: define {{.*}}@__tls_init()

clang/test/CodeGenCXX/global-init.cpp

 
 namespace test4 {
   struct A {
     A();
   };
   extern int foo();
 
   // This needs an initialization function and guard variables.
-  // CHECK: load i8, i8* bitcast (i64* @_ZGVN5test41xE
+  // CHECK: load i8, i8* bitcast (i64* @_ZGVN5test41xE to i8*)
   // CHECK: [[CALL:%.*]] = call i32 @_ZN5test43fooEv
   // CHECK-NEXT: store i32 [[CALL]], i32* @_ZN5test41xE
-  // CHECK-NEXT: store i64 1, i64* @_ZGVN5test41xE
+  // CHECK-NEXT: store i8 1, i8* bitcast (i64* @_ZGVN5test41xE to i8*)
   __attribute__((weak)) int x = foo();
 }
 
 namespace PR5974 {
   struct A { int a; };
   struct B { int b; };
   struct C : A, B { int c; };
 

clang/test/CodeGenCXX/static-data-member.cpp

namespace test3 {
   // CHECK-LABEL: define internal void @__cxx_global_var_init.1() {{.*}} comdat($_ZN5test31AIiE1xE)
   // MACHO-LABEL: define internal void @__cxx_global_var_init.1()
   // MACHO-NOT: comdat
   // CHECK:      [[GUARDBYTE:%.*]] = load i8, i8* bitcast (i64* @_ZGVN5test31AIiE1xE to i8*)
   // CHECK-NEXT: [[UNINITIALIZED:%.*]] = icmp eq i8 [[GUARDBYTE]], 0
   // CHECK-NEXT: br i1 [[UNINITIALIZED]]
   // CHECK:      [[TMP:%.*]] = call i32 @_ZN5test33fooEv()
   // CHECK-NEXT: store i32 [[TMP]], i32* @_ZN5test31AIiE1xE, align 4
-  // CHECK-NEXT: store i64 1, i64* @_ZGVN5test31AIiE1xE
+  // CHECK-NEXT: store i8 1, i8* bitcast (i64* @_ZGVN5test31AIiE1xE to i8*)
   // CHECK-NEXT: br label
   // CHECK:      ret void
 }
 
 // Test that we can fold member lookup expressions which resolve to static data
 // members.
 namespace test4 {
   struct A {