Skip to content

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
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