pngquant sigbus error on ppc64le with clang-17

[tstellar]

I've run into a regression with Clang 17, and I've bisected it to ffc1205. @gchatelet

One of the test cases from the pngquant project fails with a SIGBUS error when built with Clang 17 on ppc64le. The SIGBUS is coming from this OpenMP loop

Below is a script to build pngquant and reproduce the error. I've also attached the good and bad disassembly of the binary. Let me know what other information would help resolve this bug.

Stack Trace:

#0  0x00007ffff7b5b7fc in __kmp_init_indirect_csptr (crit=0x100455dc <gomp_critical_user_libpng[var]>, loc=0x10034cf0, gtid=<optimized out>, tag=locktag_queuing) at /root/dev/llvm-project/openmp/runtime/src/kmp_csupport.cpp:1066
#1  0x00007ffff7b5b2f4 in __kmpc_critical_with_hint (loc=0x10034cf0, global_tid=0, crit=0x100455dc <gomp_critical_user_libpng[var]>, hint=0) at /root/dev/llvm-project/openmp/runtime/src/kmp_csupport.cpp:1503
#2  0x00007ffff7b5b114 in __kmpc_critical (loc=<optimized out>, global_tid=0, crit=<optimized out>) at /root/dev/llvm-project/openmp/runtime/src/kmp_csupport.cpp:1257
#3  0x0000000010015c90 in read_image (options=0x1005de20, filename=0x7ffffffff154 "/tmp/pngquantfGwdZc/skiptest.png", using_stdin=0, input_image_p=0x7fffffffd5d0, liq_image_p=0x7fffffffd8a8, keep_input_pixels=<optimized out>, strip=<optimized out>, verbose=<optimized out>) at pngquant.c:690
#4  0x0000000010016b38 in pngquant_file_internal (filename=0x7ffffffff154 "/tmp/pngquantfGwdZc/skiptest.png", options=<optimized out>, liq=0x1005de20, outname=<optimized out>) at pngquant.c:436
#5  .omp_outlined._debug__ (options=@0x7fffffffe3e8: 0x7fffffffe0a0, .global_tid.=<optimized out>, .bound_tid.=<optimized out>, liq=<optimized out>, latest_error=<optimized out>, skipped_count=<optimized out>, error_count=<optimized out>, file_count=<optimized out>) at pngquant.c:386
#6  .omp_outlined. (.global_tid.=<optimized out>, .bound_tid.=<optimized out>, options=<optimized out>, liq=@0x7fffffffe3e0: 0x100502b0, latest_error=@0x7fffffffe108: SUCCESS, skipped_count=@0x7fffffffe110: 0, error_count=@0x7fffffffe3f0: 0, file_count=@0x7fffffffe10c: 0) at pngquant.c:352
#7  0x00007ffff7c26e64 in __kmp_invoke_microtask () at /root/dev/llvm-project/openmp/runtime/src/z_Linux_asm.S:1701
#8  0x00007ffff7b7adf8 in __kmp_invoke_task_func (gtid=0) at /root/dev/llvm-project/openmp/runtime/src/kmp_runtime.cpp:7674
#9  0x00007ffff7b706d4 in __kmp_fork_call (loc=0x10034cd8, gtid=0, call_context=<optimized out>, argc=<optimized out>, microtask=<optimized out>, invoker=0x7ffff7b7ab40 <__kmp_invoke_task_func(int)>, ap=<optimized out>) at /root/dev/llvm-project/openmp/runtime/src/kmp_runtime.cpp:2337
#10 0x00007ffff7b59268 in __kmpc_fork_call (loc=0x10034cd8, argc=6, microtask=0x10016180 <.omp_outlined.>) at /root/dev/llvm-project/openmp/runtime/src/kmp_csupport.cpp:300
#11 0x0000000010015824 in pngquant_main_internal (options=0x7fffffffe0a0, liq=0x100502b0) at pngquant.c:352
#12 main (argc=<optimized out>, argv=<optimized out>) at pngquant.c:318
[bad.s.txt](https://github.com/llvm/llvm-project/files/11456711/bad.s.txt)

Script to Reproduce:

libomp_installdir=/root/dev/llvm-project/install/lib/

# dnf install libpng-devel lcms2-devel

git clone https://github.com/pornel/pngquant
git checkout 2.18.0
pushd pngquant

export CFLAGS='-O2 -flto -g'
export LDFLAGS="-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now    -Wl,--build-id=sha1  -flto -fuse-ld=lld -Wl,-rpath=$libomp_installdir"
export CC=clang
export CXX=/root/dev/llvm-project/install/bin/clang++

./configure --with-openmp --with-libimagequant

make

./configure --with-openmp --with-libimagequant

make
make -j1 test V=1

good.s.txt
bad.s.txt

[llvmbot]

@llvm/issue-subscribers-backend-powerpc

[llvmbot]

@llvm/issue-subscribers-openmp

[tstellar]

Looking at the bitcode after LTO optimization, there are a few variables that gained an align 1 after this commit:

Before:

@.gomp_critical_user_.var = internal global [8 x i32] zeroinitializer
@.gomp_critical_user_.reduction.var = internal global [8 x i32] zeroinitializer
@.gomp_critical_user_libpng.var = internal global [8 x i32] zeroinitializer

After:

@.gomp_critical_user_.var = internal global [8 x i32] zeroinitializer, align 1
@.gomp_critical_user_.reduction.var = internal global [8 x i32] zeroinitializer, align 1
@.gomp_critical_user_libpng.var = internal global [8 x i32] zeroinitializer, align 1

[shiltian]

It looks like unrelated to OpenMP here.

[jhuber6]

We make those variables in https://github.com/llvm/llvm-project/blob/main/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp#L4216. We could probably set the alignment manually here based off of the type.

[gchatelet]

So either we fix this in OpenMP as @jhuber6 suggests or we make sure that creating a GlobalVariable in a Module also sets a natural alignment.

[jhuber6]

So either we fix this in OpenMP as @jhuber6 suggests or we make sure that creating a GlobalVariable in a Module also sets a natural alignment.

I'll make a patch for OpenMP at least, don't see a reason why we shouldn't naturally align these.

[jhuber6]

Let me know if this patch resolves it https://reviews.llvm.org/D150461

[tstellar]

Even with this patch, I'm still seeing a crash in the same place. I checked the IR and the variables now have align 4 instead of align 1. The generated assembly is unchanged, though.

[jhuber6]

Even with this patch, I'm still seeing a crash in the same place. I checked the IR and the variables now have align 4 instead of align 1. The generated assembly is unchanged, though.

This is 64-bit, right? I'm curious why the alignment would be four instead of eight. Would it be possible for you to manually change them to eight to make sure that this isn't an alignment problem?

[tstellar]

@jhuber6 The alignment is based on the element type of the array, so I believe four is correct. Hard-coding the alignment to eight does fix the test. This makes me think it might be a codegen bug, where load alignment is being ignored.

[llvmbot]

@llvm/issue-subscribers-backend-powerpc

[tstellar]

I think the bug is here. It's casting a int[8] type (with alignment 4) to a pointer type (with alignment 8).

[jhuber6]

I'm not too familiar with libomp, maybe @jpeyton52 can help?

[jpeyton52]

Still seems like a codegen issue where the gomp_critical_user_* symbols need to be aligned to: max(sizeof(int32), sizeof(void*)). i.e., The runtime expects the compiler to generate 32 byte blobs with at least sizeof(pointer) alignment. The runtime defines the unnamed symbol, .gomp_critical_user_ in openmp/runtime/src/z_Linux_asm.S with these attributes.

It seems like alignment should be added here (I don't know the exact proper way to phrase max(sizeof(int), sizeof(void*) in clang classes/terminology) or if simply sizeof(void*) is good enough, etc.

diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index a74f63cd1fac..026994d837c1 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -2143,7 +2143,9 @@ Address CGOpenMPRuntime::emitThreadIDAddress(CodeGenFunction &CGF,
 llvm::Value *CGOpenMPRuntime::getCriticalRegionLock(StringRef CriticalName) {
   std::string Prefix = Twine("gomp_critical_user_", CriticalName).str();
   std::string Name = getName({Prefix, "var"});
-  return OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
+  llvm::GlobalVariable *G = OMPBuilder.getOrCreateInternalVariable(KmpCriticalNameTy, Name);
+  G->setAlignment(llvm::Align(8));
+  return G;
 }

[tstellar]

@jpeyton52 Proposed fix based on your suggestion: https://reviews.llvm.org/D150723