Skip to content

[msan] Reland: Increase k num stack origin descrs (limited to non-PowerPC) #93117

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 3 commits into from
May 28, 2024

Conversation

thurstond
Copy link
Contributor

The original pull request (#92838) was reverted due to a PowerPC buildbot breakage (df626dd).
This reland limits the scope of the change to non-PowerPC platforms. I am unaware of any PowerPC use cases that would benefit from a larger kNumStackOriginDescrs constant.

Original CL description: This increases the constant size of kNumStackOriginDescrs to 4M (64GB of BSS across two arrays), which ought to be enough for anybody.

This is the easier alternative suggested by eugenis@ in #92826.

thurstond added 3 commits May 20, 2024 23:56
This increases the constant size of kNumStackOriginDescrs to
64M (1GB of BSS across two arrays), which ought to be enough for
anybody.

This is the easier alternative suggested by eugenis@ in
llvm#92826.
…d the PowerPC buildbot breakage (https://lab.llvm.org/buildbot/#/builders/57/builds/35160). I am unaware of any PowerPC use cases that would benefit from a larger kNumStackOriginDescrs constant.
@llvmbot
Copy link
Member

llvmbot commented May 23, 2024

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

Author: Thurston Dang (thurstond)

Changes

The original pull request (#92838) was reverted due to a PowerPC buildbot breakage (df626dd).
This reland limits the scope of the change to non-PowerPC platforms. I am unaware of any PowerPC use cases that would benefit from a larger kNumStackOriginDescrs constant.

Original CL description: This increases the constant size of kNumStackOriginDescrs to 4M (64GB of BSS across two arrays), which ought to be enough for anybody.

This is the easier alternative suggested by eugenis@ in #92826.


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

1 Files Affected:

  • (modified) compiler-rt/lib/msan/msan.cpp (+11-1)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index a2fc27de1901b..9375e27d4f4d2 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -100,7 +100,17 @@ int msan_report_count = 0;
 
 // Array of stack origins.
 // FIXME: make it resizable.
-static const uptr kNumStackOriginDescrs = 1024 * 1024;
+// Although BSS memory doesn't cost anything until used, it is limited to 2GB
+// in some configurations (e.g., "relocation R_X86_64_PC32 out of range:
+// ... is not in [-2147483648, 2147483647]; references section '.bss'").
+// We use kNumStackOriginDescrs * (sizeof(char*) + sizeof(uptr)) == 64MB.
+#ifdef SANITIZER_PPC
+// soft_rss_limit test (release_origin.c) fails on PPC if kNumStackOriginDescrs
+// is too high
+static const uptr kNumStackOriginDescrs = 1 * 1024 * 1024;
+#else
+static const uptr kNumStackOriginDescrs = 4 * 1024 * 1024;
+#endif  // SANITIZER_PPC
 static const char *StackOriginDescr[kNumStackOriginDescrs];
 static uptr StackOriginPC[kNumStackOriginDescrs];
 static atomic_uint32_t NumStackOriginDescrs;

@thurstond thurstond requested a review from kstoimenov May 28, 2024 17:39
@thurstond thurstond merged commit 0e96eeb into llvm:main May 28, 2024
8 checks passed
vg0204 pushed a commit to vg0204/llvm-project that referenced this pull request May 29, 2024
…erPC) (llvm#93117)

The original pull request
(llvm#92838) was reverted due to a
PowerPC buildbot breakage
(llvm@df626dd).
This reland limits the scope of the change to non-PowerPC platforms. I
am unaware of any PowerPC use cases that would benefit from a larger
kNumStackOriginDescrs constant.

Original CL description: This increases the constant size of
kNumStackOriginDescrs to 4M (64GB of BSS across two arrays), which ought
to be enough for anybody.

This is the easier alternative suggested by eugenis@ in
llvm#92826.
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.

3 participants