[clang] [llvm] Revert "[llvm][clang] Allocate a new stack instead of spawning a new … (PR #135865)

2025-04-16 Thread LLVM Continuous Integration via cfe-commits 

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder `lld-x86_64-win` running on 
`as-worker-93` while building `clang,llvm` at step 7 
"test-build-unified-tree-check-all".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/146/builds/2715


Here is the relevant piece of the build log for the reference

```
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
 TEST 'LLVM-Unit :: Support/./SupportTests.exe/82/95' 
FAILED 
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-15412-82-95.json
 GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=95 GTEST_SHARD_INDEX=82 
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe 
--gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): 
error: Expected equality of these values:
  0
  RC
Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): 
error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied







```



https://github.com/llvm/llvm-project/pull/135865
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Revert "[llvm][clang] Allocate a new stack instead of spawning a new … (PR #135865)

2025-04-15 Thread via cfe-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- 
clang/include/clang/Basic/Stack.h clang/lib/Basic/Stack.cpp 
clang/lib/Frontend/CompilerInstance.cpp 
llvm/include/llvm/Support/CrashRecoveryContext.h 
llvm/include/llvm/Support/thread.h llvm/lib/Support/CrashRecoveryContext.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Basic/Stack.cpp b/clang/lib/Basic/Stack.cpp
index aa15d8e66..98dc2f0d2 100644
--- a/clang/lib/Basic/Stack.cpp
+++ b/clang/lib/Basic/Stack.cpp
@@ -15,7 +15,7 @@
 #include "llvm/Support/CrashRecoveryContext.h"
 
 #ifdef _MSC_VER
-#include   // for _AddressOfReturnAddress
+#include  // for _AddressOfReturnAddress
 #endif
 
 static LLVM_THREAD_LOCAL void *BottomOfStack = nullptr;
@@ -66,9 +66,11 @@ bool clang::isStackNearlyExhausted() {
 void clang::runWithSufficientStackSpaceSlow(llvm::function_ref Diag,
 llvm::function_ref Fn) {
   llvm::CrashRecoveryContext CRC;
-  CRC.RunSafelyOnThread([&] {
-noteBottomOfStack();
-Diag();
-Fn();
-  }, DesiredStackSize);
+  CRC.RunSafelyOnThread(
+  [&] {
+noteBottomOfStack();
+Diag();
+Fn();
+  },
+  DesiredStackSize);
 }

``




https://github.com/llvm/llvm-project/pull/135865
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Revert "[llvm][clang] Allocate a new stack instead of spawning a new … (PR #135865)

2025-04-15 Thread via cfe-commits 

llvmbot wrote:



@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-support

Author: Daniel Thornburgh (mysterymath)


Changes

…thread to get more stack space (#133173)"

This change breaks the Clang build on Mac AArch64.

This reverts commit d0c973a7a0149db3b71767d4c5a20a31e6a8ed5b. This reverts 
commit 429a84f8a4bf559f43f50072747ef49d3e3b2cf1. This reverts commit 
4f64c80d5a23c244f942193e58ecac666c173308.

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


12 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (-4) 
- (modified) clang/include/clang/Basic/Stack.h (+1-4) 
- (modified) clang/lib/Basic/Stack.cpp (+28-12) 
- (modified) clang/lib/Frontend/CompilerInstance.cpp (+1-1) 
- (modified) llvm/include/llvm/Support/CrashRecoveryContext.h (-3) 
- (removed) llvm/include/llvm/Support/ProgramStack.h (-63) 
- (modified) llvm/include/llvm/Support/thread.h (-1) 
- (modified) llvm/lib/Support/CMakeLists.txt (-1) 
- (modified) llvm/lib/Support/CrashRecoveryContext.cpp (-11) 
- (removed) llvm/lib/Support/ProgramStack.cpp (-114) 
- (modified) llvm/unittests/Support/CMakeLists.txt (-1) 
- (removed) llvm/unittests/Support/ProgramStackTest.cpp (-35) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 38142ad32bea0..166f26921cb71 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -195,10 +195,6 @@ Non-comprehensive list of changes in this release
 - Added `__builtin_elementwise_exp10`.
 - For AMDPGU targets, added `__builtin_v_cvt_off_f32_i4` that maps to the 
`v_cvt_off_f32_i4` instruction.
 - Added `__builtin_elementwise_minnum` and `__builtin_elementwise_maxnum`.
-- Clang itself now uses split stacks instead of threads for allocating more
-  stack space when running on Apple AArch64 based platforms. This means that
-  stack traces of Clang from debuggers, crashes, and profilers may look
-  different than before.
 
 New Compiler Flags
 --
diff --git a/clang/include/clang/Basic/Stack.h 
b/clang/include/clang/Basic/Stack.h
index 9674b9d9b62c3..30ebd94aedd1f 100644
--- a/clang/include/clang/Basic/Stack.h
+++ b/clang/include/clang/Basic/Stack.h
@@ -27,10 +27,7 @@ namespace clang {
 
   /// Call this once on each thread, as soon after starting the thread as
   /// feasible, to note the approximate address of the bottom of the stack.
-  ///
-  /// \param ForceSet set to true if you know the call is near the bottom of a
-  /// new stack. Used for split stacks.
-  void noteBottomOfStack(bool ForceSet = false);
+  void noteBottomOfStack();
 
   /// Determine whether the stack is nearly exhausted.
   bool isStackNearlyExhausted();
diff --git a/clang/lib/Basic/Stack.cpp b/clang/lib/Basic/Stack.cpp
index 8cbb84943f8d3..aa15d8e66950f 100644
--- a/clang/lib/Basic/Stack.cpp
+++ b/clang/lib/Basic/Stack.cpp
@@ -13,13 +13,33 @@
 
 #include "clang/Basic/Stack.h"
 #include "llvm/Support/CrashRecoveryContext.h"
-#include "llvm/Support/ProgramStack.h"
 
-static LLVM_THREAD_LOCAL uintptr_t BottomOfStack = 0;
+#ifdef _MSC_VER
+#include   // for _AddressOfReturnAddress
+#endif
 
-void clang::noteBottomOfStack(bool ForceSet) {
-  if (!BottomOfStack || ForceSet)
-BottomOfStack = llvm::getStackPointer();
+static LLVM_THREAD_LOCAL void *BottomOfStack = nullptr;
+
+static void *getStackPointer() {
+#if __GNUC__ || __has_builtin(__builtin_frame_address)
+  return __builtin_frame_address(0);
+#elif defined(_MSC_VER)
+  return _AddressOfReturnAddress();
+#else
+  char CharOnStack = 0;
+  // The volatile store here is intended to escape the local variable, to
+  // prevent the compiler from optimizing CharOnStack into anything other
+  // than a char on the stack.
+  //
+  // Tested on: MSVC 2015 - 2019, GCC 4.9 - 9, Clang 3.2 - 9, ICC 13 - 19.
+  char *volatile Ptr = &CharOnStack;
+  return Ptr;
+#endif
+}
+
+void clang::noteBottomOfStack() {
+  if (!BottomOfStack)
+BottomOfStack = getStackPointer();
 }
 
 bool clang::isStackNearlyExhausted() {
@@ -31,8 +51,7 @@ bool clang::isStackNearlyExhausted() {
   if (!BottomOfStack)
 return false;
 
-  intptr_t StackDiff =
-  (intptr_t)llvm::getStackPointer() - (intptr_t)BottomOfStack;
+  intptr_t StackDiff = (intptr_t)getStackPointer() - (intptr_t)BottomOfStack;
   size_t StackUsage = (size_t)std::abs(StackDiff);
 
   // If the stack pointer has a surprising value, we do not understand this
@@ -47,12 +66,9 @@ bool clang::isStackNearlyExhausted() {
 void clang::runWithSufficientStackSpaceSlow(llvm::function_ref Diag,
 llvm::function_ref Fn) {
   llvm::CrashRecoveryContext CRC;
-  // Preserve the BottomOfStack in case RunSafelyOnNewStack uses split stacks.
-  uintptr_t PrevBottom = BottomOfStack;
-  CRC.RunSafelyOnNewStack([&] {
-noteBottomOfStack(true);
+  CRC.RunSafelyOnThread([&] {
+noteBottomOfStack();
 Diag();
 Fn();
   }, DesiredStackSize);
-  BottomOfStack = PrevBottom;
 }
diff --git a/clang/lib/

[clang] [llvm] Revert "[llvm][clang] Allocate a new stack instead of spawning a new … (PR #135865)

2025-04-15 Thread Daniel Thornburgh via cfe-commits 

https://github.com/mysterymath closed 
https://github.com/llvm/llvm-project/pull/135865
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Revert "[llvm][clang] Allocate a new stack instead of spawning a new … (PR #135865)

2025-04-15 Thread Daniel Thornburgh via cfe-commits 

https://github.com/mysterymath created 
https://github.com/llvm/llvm-project/pull/135865

…thread to get more stack space (#133173)"

This change breaks the Clang build on Mac AArch64.

This reverts commit d0c973a7a0149db3b71767d4c5a20a31e6a8ed5b. This reverts 
commit 429a84f8a4bf559f43f50072747ef49d3e3b2cf1. This reverts commit 
4f64c80d5a23c244f942193e58ecac666c173308.

>From 22202c5ad09ce07b1c44a47d3e659e4dd672cef5 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh 
Date: Tue, 15 Apr 2025 14:44:57 -0700
Subject: [PATCH] Revert "[llvm][clang] Allocate a new stack instead of
 spawning a new thread to get more stack space (#133173)"

This change breaks the Clang build on Mac AArch64.

This reverts commit d0c973a7a0149db3b71767d4c5a20a31e6a8ed5b.
This reverts commit 429a84f8a4bf559f43f50072747ef49d3e3b2cf1.
This reverts commit 4f64c80d5a23c244f942193e58ecac666c173308.
---
 clang/docs/ReleaseNotes.rst   |   4 -
 clang/include/clang/Basic/Stack.h |   5 +-
 clang/lib/Basic/Stack.cpp |  40 --
 clang/lib/Frontend/CompilerInstance.cpp   |   2 +-
 .../llvm/Support/CrashRecoveryContext.h   |   3 -
 llvm/include/llvm/Support/ProgramStack.h  |  63 --
 llvm/include/llvm/Support/thread.h|   1 -
 llvm/lib/Support/CMakeLists.txt   |   1 -
 llvm/lib/Support/CrashRecoveryContext.cpp |  11 --
 llvm/lib/Support/ProgramStack.cpp | 114 --
 llvm/unittests/Support/CMakeLists.txt |   1 -
 llvm/unittests/Support/ProgramStackTest.cpp   |  35 --
 12 files changed, 30 insertions(+), 250 deletions(-)
 delete mode 100644 llvm/include/llvm/Support/ProgramStack.h
 delete mode 100644 llvm/lib/Support/ProgramStack.cpp
 delete mode 100644 llvm/unittests/Support/ProgramStackTest.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 38142ad32bea0..166f26921cb71 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -195,10 +195,6 @@ Non-comprehensive list of changes in this release
 - Added `__builtin_elementwise_exp10`.
 - For AMDPGU targets, added `__builtin_v_cvt_off_f32_i4` that maps to the 
`v_cvt_off_f32_i4` instruction.
 - Added `__builtin_elementwise_minnum` and `__builtin_elementwise_maxnum`.
-- Clang itself now uses split stacks instead of threads for allocating more
-  stack space when running on Apple AArch64 based platforms. This means that
-  stack traces of Clang from debuggers, crashes, and profilers may look
-  different than before.
 
 New Compiler Flags
 --
diff --git a/clang/include/clang/Basic/Stack.h 
b/clang/include/clang/Basic/Stack.h
index 9674b9d9b62c3..30ebd94aedd1f 100644
--- a/clang/include/clang/Basic/Stack.h
+++ b/clang/include/clang/Basic/Stack.h
@@ -27,10 +27,7 @@ namespace clang {
 
   /// Call this once on each thread, as soon after starting the thread as
   /// feasible, to note the approximate address of the bottom of the stack.
-  ///
-  /// \param ForceSet set to true if you know the call is near the bottom of a
-  /// new stack. Used for split stacks.
-  void noteBottomOfStack(bool ForceSet = false);
+  void noteBottomOfStack();
 
   /// Determine whether the stack is nearly exhausted.
   bool isStackNearlyExhausted();
diff --git a/clang/lib/Basic/Stack.cpp b/clang/lib/Basic/Stack.cpp
index 8cbb84943f8d3..aa15d8e66950f 100644
--- a/clang/lib/Basic/Stack.cpp
+++ b/clang/lib/Basic/Stack.cpp
@@ -13,13 +13,33 @@
 
 #include "clang/Basic/Stack.h"
 #include "llvm/Support/CrashRecoveryContext.h"
-#include "llvm/Support/ProgramStack.h"
 
-static LLVM_THREAD_LOCAL uintptr_t BottomOfStack = 0;
+#ifdef _MSC_VER
+#include   // for _AddressOfReturnAddress
+#endif
 
-void clang::noteBottomOfStack(bool ForceSet) {
-  if (!BottomOfStack || ForceSet)
-BottomOfStack = llvm::getStackPointer();
+static LLVM_THREAD_LOCAL void *BottomOfStack = nullptr;
+
+static void *getStackPointer() {
+#if __GNUC__ || __has_builtin(__builtin_frame_address)
+  return __builtin_frame_address(0);
+#elif defined(_MSC_VER)
+  return _AddressOfReturnAddress();
+#else
+  char CharOnStack = 0;
+  // The volatile store here is intended to escape the local variable, to
+  // prevent the compiler from optimizing CharOnStack into anything other
+  // than a char on the stack.
+  //
+  // Tested on: MSVC 2015 - 2019, GCC 4.9 - 9, Clang 3.2 - 9, ICC 13 - 19.
+  char *volatile Ptr = &CharOnStack;
+  return Ptr;
+#endif
+}
+
+void clang::noteBottomOfStack() {
+  if (!BottomOfStack)
+BottomOfStack = getStackPointer();
 }
 
 bool clang::isStackNearlyExhausted() {
@@ -31,8 +51,7 @@ bool clang::isStackNearlyExhausted() {
   if (!BottomOfStack)
 return false;
 
-  intptr_t StackDiff =
-  (intptr_t)llvm::getStackPointer() - (intptr_t)BottomOfStack;
+  intptr_t StackDiff = (intptr_t)getStackPointer() - (intptr_t)BottomOfStack;
   size_t StackUsage = (size_t)std::abs(StackDiff);
 
   // If the stack pointer has a surprising valu