From: Herbert Xu <herbert@gondor.apana.org.au>
To: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: dash@vger.kernel.org
Subject: Re: [PATCH v3] parser: don't keep alloca()ing in a loop for substitutions
Date: Sun, 8 Jan 2023 20:07:08 +0800 [thread overview]
Message-ID: <Y7qx7LJwJUNVd/UD@gondor.apana.org.au> (raw)
In-Reply-To: <20230105134204.e7mczfzt2vbit3oc@tarta.nabijaczleweli.xyz>
On Thu, Jan 05, 2023 at 02:42:04PM +0100, наб wrote:
> When encountering
> printf %010000d | tr 0 \` | sh -n
> printf %09999d | tr 0 \` | sh -n
> you want no output and "Syntax error: EOF in backquote substitution",
> respectively; instead, current dash segfaults.
>
> This is because the alloca for the save buffer is run, naturally,
> in the same function, so first it allocates one byte, then two,
> then ..., then appx. 4000 (for me, depends on the binary),
> then it segfaults on the memcpy (it's even worse, since due to
> alignment, it usually allocates much more for the early stuff).
>
> Nevertheless, the stack frame grows unboundedly, until we completely
> destroy the stack. Instead of squirreling the out block away, then
> letting subsequent allocations override the original, mark it used,
> and just re-copy it to the top of the dash stack. This increases peak
> memory usage somewhat
> (in the most pathological case ‒ the above but with three nines ‒
> from 23.26 to 173.7KiB according to massif,
> in parsing a regular program (ratrun from ratrun 0c)
> from 28.68 to 29.19;
> a simpler program (ibid., rat) stays at 5.422;
> parsing libtoolize, debootstrap, and dkms
> (the biggest shell programs in my /[s]bin by size + by `/$( count)
> likewise stay the same at 12.02, 41.48, and 6.438)
> but it's barely measurable outside of truly pathological conditions
> that were a step away from a segfault previously.
>
> Fixes: https://bugs.debian.org/966156
> ---
> I think this means we also need to turn the USTPUTC() into STPUTC(),
> since the previous code explicitly over-accounted for it in growstackto().
>
> src/parser.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
prev parent reply other threads:[~2023-01-08 12:07 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-14 23:39 [PATCH] parser: don't keep alloca()ing in a loop for substitutions наб
2022-12-15 10:27 ` Herbert Xu
2022-12-15 17:02 ` [PATCH v2] " наб
2023-01-05 9:02 ` Herbert Xu
2023-01-05 13:42 ` [PATCH v3] " наб
2023-01-06 3:15 ` [PATCH] parser: Print CTLBACKQ early in parsesub Herbert Xu
2023-01-06 11:49 ` наб
2023-01-08 12:07 ` Herbert Xu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y7qx7LJwJUNVd/UD@gondor.apana.org.au \
--to=herbert@gondor.apana.org.au \
--cc=dash@vger.kernel.org \
--cc=nabijaczleweli@nabijaczleweli.xyz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).