Skip to content

[compiler-rt] Hardcode uptr/sptr typedefs on Linux Arm #108105

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

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

aeubanks
Copy link
Contributor

@aeubanks aeubanks commented Sep 10, 2024

After #106155, Android arm32 asan builds stopped working with missing definition linker errors. This is due to inconsistent definitions of uptr of either unsigned long or unsigned int even between TUs in compiler-rt. This is caused by Linux arm32 headers redefining __UINTPTR_TYPE__ (see arch/arm/include/uapi/asm/types.h in the Linux kernel repo), meaning include order/whether or not the Linux header is included changes compiler-rt symbol mangling.

As a workaround, this hardcodes uptr/sptr in compiler-rt to unsigned int/int on Linux arm32, matching clang/gcc.

After llvm#106155, Android arm32 asan builds stopped working with missing definition linker errors. This is due to inconsistent definitions of `uptr` of either `unsigned long` or `unsigned int` even between TUs in compiler-rt. This is caused by Linux arm32 headers redefining __UINTPTR_TYPE__ (see arch/arm/include/uapi/asm/types.h in the Linux kernel repo), meaning include order/whether or not the Linux header is included changes compiler-rt symbol mangling.

As a workaround, this hardcodes `uptr`/`sptr` in compiler-rt to `unsigned int`/`int` on Linux arm32, matching clang/gcc.
@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Arthur Eubanks (aeubanks)

Changes

After #106155, Android arm32 asan builds stopped working with missing definition linker errors. This is due to inconsistent definitions of uptr of either unsigned long or unsigned int even between TUs in compiler-rt. This is caused by Linux arm32 headers redefining UINTPTR_TYPE (see arch/arm/include/uapi/asm/types.h in the Linux kernel repo), meaning include order/whether or not the Linux header is included changes compiler-rt symbol mangling.

As a workaround, this hardcodes uptr/sptr in compiler-rt to unsigned int/int on Linux arm32, matching clang/gcc.


Full diff: https://github.com/llvm/llvm-project/pull/108105.diff

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h (+6)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h b/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
index f8f03454ea169a..bf15d6704b4bd8 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
@@ -139,8 +139,14 @@
 namespace __sanitizer {
 
 #if defined(__UINTPTR_TYPE__)
+#if defined(__arm__) && defined(__linux__)
+// Linux Arm headers redefine __UINTPTR_TYPE__ and disagree with clang/gcc.
+typedef unsigned int uptr;
+typedef int sptr;
+#else
 typedef __UINTPTR_TYPE__ uptr;
 typedef __INTPTR_TYPE__ sptr;
+#endif
 #elif defined(_WIN64)
 // 64-bit Windows uses LLP64 data model.
 typedef unsigned long long uptr;

Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, sorry for the breakage.

Copy link

github-actions bot commented Sep 10, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@aeubanks aeubanks merged commit db7e8f2 into llvm:main Sep 11, 2024
5 of 6 checks passed
@aeubanks aeubanks deleted the armasan branch September 11, 2024 00:02
@vitalybuka
Copy link
Collaborator

SANITIZER_ARM && SANITIZER_ANDROID ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants