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

Build failure with Clang 16.0.6 and 17 -- size of array element of type 'PM128A' (aka '_M128U *') (8 bytes) isn't a multiple of its alignment (16 bytes) #6982

Closed
EmployedRussian opened this issue Apr 21, 2024 · 3 comments

Comments

@EmployedRussian
Copy link
Contributor

clang++ --version
Debian clang version 16.0.6 (19)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
/build.sh --debug --static
Searching for Clang...
Clang++ found at /usr/bin/clang++
Build path: /home/ChakraCore/out/Debug
Compile Target : System Default
Generating Debug build
...
[  0%] Building CXX object pal/src/CMakeFiles/Chakra.Pal.dir/cruntime/file.cpp.o
In file included from /home/ChakraCore/pal/src/cruntime/file.cpp:23:
In file included from /home/ChakraCore/pal/src/include/pal/palinternal.h:323:
/home/ChakraCore/pal/inc/pal.h:2682:31: error: size of array element of type 'PM128A' (aka '_M128U *') (8 bytes) isn't a multiple of its alignment (16 bytes)
        PM128A FloatingContext[16];
                              ^
1 error generated.
pal/src/CMakeFiles/Chakra.Pal.dir/build.make:75: recipe for target 'pal/src/CMakeFiles/Chakra.Pal.dir/cruntime/file.cpp.o' failed
make[2]: *** [pal/src/CMakeFiles/Chakra.Pal.dir/cruntime/file.cpp.o] Error 1
CMakeFiles/Makefile2:643: recipe for target 'pal/src/CMakeFiles/Chakra.Pal.dir/all' failed
make[1]: *** [pal/src/CMakeFiles/Chakra.Pal.dir/all] Error 2
Makefile:90: recipe for target 'all' failed
make: *** [all] Error 2
See error details above. Exit code was 2

As far as I understand, PM128A is a 16-byte aligned pointer (of size 8).

A single pointer can be aligned on 16-byte boundary, but the next pointer in the array will necessarily be 8-byte aligned.

This patch fixes the problem:

diff --git a/pal/inc/pal.h b/pal/inc/pal.h
index a68a6e65a..20ca7a80d 100644
--- a/pal/inc/pal.h
+++ b/pal/inc/pal.h
@@ -2498,7 +2498,8 @@ typedef struct _M128U {
 } M128U, *PM128U;

 // Same as _M128U but aligned to a 16-byte boundary
-typedef DECLSPEC_ALIGN(16) M128U M128A, *PM128A;
+typedef DECLSPEC_ALIGN(16) M128U M128A;
+typedef M128A *PM128A;

 typedef struct _XMM_SAVE_AREA32 {
     WORD   ControlWord;
@ppenzin
Copy link
Member

ppenzin commented Apr 21, 2024

Did the definition get pointer levels mixed up? This is what PAL in dotnet runtime does: https://github.com/dotnet/runtime/blob/a3dc1337a17a3de6aefe76e465f85ebf5ba6ae5e/src/coreclr/pal/inc/pal.h#L1411

The struct should be 16 bytes (two 64-bit integer) in theory.

@ppenzin
Copy link
Member

ppenzin commented Apr 21, 2024

After thinking for a bit it does look right. I guess time to start building with dev clang to get advance notice on issues like this. Another aside is I am personally uncomfortable with PAL because of the code like this - it does win32 things which Windows does internally in user mode and it has been a source of problems.

@rhuanjl
Copy link
Collaborator

rhuanjl commented Apr 22, 2024

Closing as presumably fixed by #6983

@rhuanjl rhuanjl closed this as completed Apr 22, 2024
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

3 participants