Fix dune build#34
Open
mgree wants to merge 408 commits into
Open
Conversation
When our own pmatch is used, loc2 is unused in scanleft/right when quotes is true. However, it is still needed when quotes is false. Fix the scanleft/right code so that loc2 is always updated (so it will be garbage when quotes is true) but only returned depending on the value of quotes. Fixes: c5bf970 ("expand: Add multi-byte support to pmatch") Reported-by: Johannes Altmanninger <aclopte@gmail.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
With C23 and LTO, we get the following warning (or error if promoted to such):
```
src/builtins.c:28:5: error: type of ‘timescmd’ does not match original declaration [-Werror=lto-type-mismatch]
28 | int timescmd(int, char **);
| ^
src/bltin/times.c:15:5: note: type mismatch in parameter 1
src/bltin/times.c:15:5: note: type ‘void’ should match type ‘int’
```
Make the two consistent. This didn't show up before because pre-C23
had unprototyped functions.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Johannes Altmanninger <aclopte@gmail.com> wrote: > I noticed another regression in c5bf970 (expand: Add multi-byte > support to pmatch, 2024-06-02). > > This command now prints "abc-def" but used to print "ef". > > x=abc-def > y="${x##*d}" > echo "$y" Fix this by setting s to the correct value in scanright based on FNMATCH_IS_ENABLED. Fixes: c5bf970 ("expand: Add multi-byte support to pmatch") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Jan Pechanec <Jan.Pechanec@oracle.com> wrote: > > thank you for working on dash. I was testing it recently and it worked > really well. > > However, I noticed the dash code from github does filename pattern > matching even for code like "[ x = x ] && echo ok". I believe the > unquoted space after '[' should not trigger pattern matching but rather > only to invoke the test/[ utility, as before. It seems it works fine > though and only doing some extra unneeded work which may not be > immediatelly noticeable. > > dash installed on my Oracle Linux 9: > > janp:len49:~/_INST/dash$ strings /usr/bin/dash | grep dash > dash-0.5.11.5-4.el9.x86_64.debug > janp:len49:~/_INST/dash$ time dash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done' > > real 0m0.752s > user 0m0.748s > sys 0m0.002s > > dash from github (commit b3e38ad) take > way more time to do the same thing: > > janp:len49:~/_INST/dash$ time ./src/dash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done' > > real 0m4.202s > user 0m1.361s > sys 0m2.804s > > For the latter, strace shows open, fstat, getdents*, and close system > calls for each iteration and it depends on number of files in the > current directory. With more files, it takes more time: > > janp:len49:/etc$ time ~/_INST/dash/src/dash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done' > real 0m15.591s > user 0m5.704s > sys 0m9.828s > > If I change [ to test, the dash github version behaves as before, and > possibly even faster: > > janp:len49:~/_INST/dash$ time ~/_INST/dash/src/dash -c 'i=0; while :; do : $((i=i+1)); test $i -eq 500000 && break; done' > > real 0m0.662s > user 0m0.659s > sys 0m0.002s > > Even bash would be faster than the current github version of dash: > > janp:len49:~/_INST/dash$ time bash -c 'i=0; while :; do : $((i=i+1)); [ $i -eq 500000 ] && break; done' > real 0m1.943s > user 0m1.939s > sys 0m0.002s Fix performance regression for idiomatic "[ ... ]" expression by adding a bypass for a literal "]" in pathname expansion. Reported-by: Jan Pechanec <Jan.Pechanec@oracle.com> Fixes: 8d0eca2 ("expand: Rewrite expmeta meta detection") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
…g in pmatch strpbrk() accepts two null-terminated string arguments. stop[] is char array that is not null-terminated but is still passed as a second argument to strpbrk. This causes buffer overread, which is detected by AddressSanitizer. This commit adds an explicit null-terminated to the end of the array. Signed-off-by: Zurab Kvachadze <zurabid2016@gmail.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Move the stop array closer to the strpbrk(3) call in pmatch. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Ensure that the EOF state is reset in reset_input as otherwise the new stdin may be treated as empty. Reported-by: Nathan Royce <nroycea+kernel@gmail.com> Fixes: 69786bc ("input: Fix pungetc on PEOF") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
As pointed out by Denys Vlasenko, we can avoid blocking signals on vfork() by making the signal handler of a vfork child immediately return. This saves a syscall. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Recent versions of VxWorks support fork() and as result can support dash.
For example, to cross compile for IA with this patch applied, and your VSB environment sourced (aka sysroot)
./configure --build=x86_64-pc-linux-gnu --host=x86_64-wrs-vxworks --prefix=/usr \
CC=wr-cc CXX=wr-c++ LD=wr-ld AR=wr-ar NM=wr-nm OBJCOPY=wr-objcopy OBJDUMP=wr-objdump RANLIB=wr-ranlib READELF=wr-readelf SIZE=wr-size STRIP= wr-strip \
ac_cv_func_faccessat=no \
CFLAGS="-DJOBS=0 "
make install DESTDIR=${VSB}/usr/3pp/develop
For other architectures update your <host> appropriately.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
procargs(int argc, char **argv)
argc is used in just one place:
if (argc > 0)
xargv++;
Trivially replaceable by if(xargv[0] != NULL), so can avoid passing
this argument.
char **xargv;
xargv = argv;
xargv is always equal to argv, so why having a separate variable?
const char *xminusc;
xminusc = minusc;
Similar situation with xminusc being equal to minusc
during the range where it is live, they diverge here:
if (xminusc) {
minusc = *xargv++;
but after this, xminusc is not used.
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
When the shell is used to execute another utility, it makes no sense to initialise stdin, which costs up to two system calls. Since the input layer can already handle first-use initialisation because of reset_init, simply make this always the case. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
As the vfork path now requires the use of getpid, cache the current PID in a new global variable mypid. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Set needprompt to zero as it may have been set by the caller. It is safe to do so here as the only calls to expandstr within the parser set needprompt to zero already. Reported-by: Aleksander Ushakov <aushakov@astralinux.ru> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Abort aexpr and oexpr if t_wp hits a NULL at the start. Link: https://lore.kernel.org/dash/52d97ed4-7c78-44ec-8c1b-60569491aa31@astralinux.ru/ Reported-by: Aleksander Ushakov <aushakov@astralinux.ru> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Remove the rmescapes call from expari as the string produced by the recursive argstr call is not escaped. Link: https://lore.kernel.org/dash/fc8ed5da-3024-4d22-b6da-83dccabafe99@astralinux.ru/ Fixes: 3cd5386 ("expand: Do not reprocess data when expanding words") Reported-by: Aleksander Ushakov <aushakov@astralinux.ru> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
On Fri, Aug 29, 2025 at 07:40:16PM -0700, Nathan Mills wrote: > > * Crash #0 is a null-pointer-dereference. I haven't found the fix yet > but it is probably simple. > > redir->nhere.doc is NULL. The global variable **heredoclist** is also > NULL and parseheredoc does nothing when heredoclist is NULL. > > 1. readtoken1 > 2. parseheredoc (heredoclist equals heredoc which is the same as the > value in the next step) > 3. parseheredoc (heredoclist is NULL, heredoc is 0x5555560bb6a8 <stackbase+232>) > 4. parsefname > 5. parseheredoc (heredoclist->here->nhere.doc->narg.text equals the > empty string) > 6. **SIGSEGV** openhere (redir->nhere.doc is **NULL**) > > ** Crash #0: null pointer dereference ** > > Base64'd > > dmVzjHdyPDwAAACAYAAAAHd3cjw8AAAAgGAAAACA/38zZGlsaQ== > > Minimized: > > src/dash -c $(echo -c "<<\`<<\0\`") This is caused by the unnecessary recursive parsing of old-style command substitution which then gets confused by the embedded here-document. Fix this by skipping the recursive parsing of command substitution if it is old-style and a here-document marker. Reported-by: Nathan Mills <the.true.nathan.mills@gmail.com> Fixes: 7a11b3e ("parser: Extend coverage of CHKEOFMARK") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Check for NUL before parsing range expression in pmatch. Reported-by: Nathan Mills <the.true.nathan.mills@gmail.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
…c marker Use STPUTC after aommdntextcont when parsing a new-style command substitution used as a here-document marker. Reported-by: Nathan Mills <the.true.nathan.mills@gmail.com> Fixes: 7a11b3e ("parser: Extend coverage of CHKEOFMARK") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Using memcmp past the end of a string may crash if it hits a page boundary. Fix this by calling strcmp/strncmp instead. Reported-by: Nathan Mills <the.true.nathan.mills@gmail.com> Reported-by: Harald van Dijk <harald@gigawatt.nl> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Remove ERANGE check in atomax for consistency as other direct calls to strtoimax do not perform the range check. Reported-by: Szász Gergely <szaszg@hu.inter.net> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Steffen Nurpmeso <steffen@sdaoden.eu> wrote: > > In an email communication with kre@ on NetBSD's tech-userlevel we > came over > > commit a373e69a196cd8d45f2806d805e548fa65a982ba > Author: kre <kre@NetBSD.org> > AuthorDate: 2017-07-24 12:35:37 +0000 > Commit: kre <kre@NetBSD.org> > CommitDate: 2017-07-24 12:35:37 +0000 > > PR standards/52406 > > Absent other information, the shell should be interactive if reading > from stdin, and stdin and stderr are ttys, not stdin and stdout. > > and i had in mind that dash changed this already (i thought i saw > a patch flying by on the ML?), but looking at procargs() there is > > if (iflag == 2 && sflag == 1 && stdin_istty && isatty(1)) > iflag = 1; > > where that would require > > if (iflag == 2 && sflag == 1 && stdin_istty && isatty(2)) > iflag = 1; Check stderr instead of stdout in procargs for interactivity. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Skip the optimisation for * for only if the next character is ? and [, rather than ? and anything but [. Reported-by: Reilly Brogan <reilly@reillybrogan.com> Fixes: e878137 ("expand: Move stop array closer to strpbrk call") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Ensure the terminating NUL character is included when expanding path names. Reported-by: Reilly Brogan <reilly@reillybrogan.com> Fixes: a9012f4 ("expand: Process multi-byte characters in expmeta") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Only dollar single quote should eat the backslash character before a quote. Make the skipping of the backslash conditional on mbchar in conv_escape. Reported-by: Juergen Daubert <j.daubert@posteo.de> Fixes: 776424a ("parser: Add dollar single quote") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
When a char is promoted to an int, it needs to be signed as otherwise comparisons on it may fail. Alternatively, an integer needs to be truncated to char before comparing it against another char. Reported-by: Juergen Daubert <j.daubert@posteo.de> Fixes: e878137 ("expand: Do not call rmescapes in expari") Fixes: c5bf970 ("expand: Add multi-byte support to pmatch") Fixes: 8f01c37 ("[PARSER] Add FAKEEOFMARK for expandstr") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Call input_init before using stdin_istty as otherwise it will always be true. Reported-by: Juergen Daubert <j.daubert@posteo.de> Fixes: 6abb589 ("input: Call input_init on first use") Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Replace direct uint64_t dereference with memcpy() to avoid SIGBUS on armv7 when p is not 8-byte aligned. Fixes crashes in dash during IFS splitting on strict-alignment architectures. Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
In order to test the fallback path when tee(2) is not available, allow tee(2) to be disabled with --disable-tee. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trying to fix the dune build... and recover from some old squash merges that make rebasing history hard.