Re: [PATCH 6/6] max_tree_depth: lower it for clangarm64 on Windows
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 22, 2025, 07:49 UTC
- Message-ID
- <17780ada-bb64-c780-939f-0702f53dd3ca@gmx.de>
- In-Reply-To
- <aAdImbydzMUkCFqB@pks.im>
Hi Patrick,
On Tue, 22 Apr 2025, Patrick Steinhardt wrote:
Show 14 quoted lines
> On Mon, Apr 21, 2025 at 12:39:10PM +0000, Johannes Schindelin via GitGitGadget wrote: > > diff --git a/environment.c b/environment.c > > index 9e4c7781be0..cc853950bb2 100644 > > --- a/environment.c > > +++ b/environment.c > > @@ -82,9 +82,21 @@ int max_allowed_tree_depth = > > * the stack overflow can occur. > > */ > > 512; > > +#else > > +#if defined(GIT_WINDOWS_NATIVE) && defined(__clang__) && defined(__aarch64__) > > Tiny nit, only because it puzzled me for a second: this should probably > be `#elif`.
I will change it.
Show 18 quoted lines
> > + /* > > + * Similar to Visual C, it seems that on Windows/ARM64 the clang-based > > + * builds have a smaller stack space available. When running out of > > + * that stack space, a `STATUS_STACK_OVERFLOW` is produced. When the > > + * Git command was run from an MSYS2 Bash, this unfortunately results > > + * in an exit code 127. Let's prevent that by lowering the maximal > > + * tree depth; This value seems to be low enough. > > + */ > > + 1280; > > #else > > 2048; > > #endif > > +#endif > > Hm. This whole construct feels rather awful, if you ask me. Instead of > papering over the issue it would be nice if we eventually fixed the root > cause, which is that we use recursion on a data structure that has an > unbounded depth in theory.
True.
It is also quite awful that I cannot find a way to represent `STATUS_STACK_OVERFLOW` by anything else than exit code 127, which always misleads me into thinking that an executable or a DLL might be missing. But I did not find any.
> Anyway, that is clearly outside of the scope of this patch series, so > the bandaid is good enough for now.
True enough!
Thank you, Johannes