Skip to content

Commit 97b5b79

Browse files
McDutchieperette
andcommitted
autoload: Add subshell scoping to loop detection (re: a410bc4)
@perette reports: > Korn Shell erroneously reports there is an autoload loop when: > > * I have a main program that autoloads a helper function. That > function is only called during shell captures. > * That function autoloads a third function. > > The first invocation doing a shell capture succeeds. Successive > ones report "autoload loop" in the helper function. Analysis: In the referenced commit, I failed to account for loading functions in virtual (non-forking) subshells, such as command substitutions ('shell captures'), without them being loaded in the parent shell envioronment. The loop detection state leaks out of those subshells, whereas the functions themselves do not. src/cmd/ksh93/sh/path.c, src/cmd/ksh93/sh/subshell.c, src/cmd/ksh93/include/defs.h, src/cmd/ksh93/include/shell.h: - Implements subshell scoping for the Cdt tree that keeps track of the names of functions that are currently being autoloaded. This works analogously to the subshell scoping for the variable, function, and hash table trees, but is much simpler, because the autoload loop detection tree only has nodes without values. The sh_subloopdetecttree function is used to create a new subshell tree. The sfaldt field of the subshell data is used to save it and clean it up when the subshell exits. In funload(), when adding a note, we now also need to pass the NV_NOSCOPE flag to nv_search to add it to the subshell tree (if any) and not the parent tree. - Fix another problem that I discovered while fixing this. When the script invoked by autoload contains an error that causes the shell context to exit, such as an error in a special builtin, the shell longjmps away from funload(), the state never gets restored, and there is a memory leak as free() is not called. Fix this by adding a sh_pushcontext/sigsetjmp procedure of its own so the shell longjmps back to the correct point in funload() to restore state correctly before continuing down the longjmp chain. src/cmd/ksh93/data/msg.c, src/cmd/ksh93/include/shell.h, src/cmd/ksh93/sh/xec.c: - Improve the error message given when an autoloadable function is not found, by including the name of the not-found function in it. This fix is by Perette Barella (thanks!). Resolves: #971 Co-authored-by: Perette Barella <perette@barella.org>
1 parent 53e2123 commit 97b5b79

8 files changed

Lines changed: 118 additions & 22 deletions

File tree

‎NEWS‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@ This documents significant changes in the 1.0 branch of ksh 93u+m.
22
For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
33
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
44

5+
2026-04-16:
6+
7+
- Fixed breakage in autoload loop detection when first invoking an
8+
autoloadable function on FPATH from within a subshell.
9+
510
2026-04-14:
611

712
- Ksh is now capable of allocating memory within a 64-bit address space. This

‎src/cmd/ksh93/data/msg.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ const char e_logout[] = "Use 'exit' to terminate this shell";
7777
const char e_exec[] = "%s: cannot execute";
7878
const char e_pwd[] = "cannot determine present working directory";
7979
const char e_found[] = "%s: not found";
80+
const char e_autoloadnotfound[] = "autoload: %s: function not found";
8081
#ifdef ENAMETOOLONG
8182
const char e_toolong[] = "%s: file name too long";
8283
#endif

‎src/cmd/ksh93/include/defs.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ extern void sh_setmatch(const char*,ptrdiff_t,ssize_t,ssize_t[],int);
129129
extern void sh_scope(struct argnod*, int);
130130
extern Namval_t *sh_scoped(Namval_t*);
131131
extern Dt_t *sh_subtracktree(int);
132+
extern Dt_t *sh_subloopdetecttree(int);
132133
extern Dt_t *sh_subfuntree(int);
133134
extern void sh_subjobcheck(pid_t);
134135
extern int sh_subsavefd(int);

‎src/cmd/ksh93/include/shell.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ typedef union Shnode_u Shnode_t;
166166

167167
/* error messages */
168168
extern const char e_found[];
169+
extern const char e_autoloadnotfound[];
169170
#ifdef ENAMETOOLONG
170171
extern const char e_toolong[];
171172
#endif
@@ -314,7 +315,7 @@ struct Shell_s
314315
char intrap_exit_n; /* set if 'exit n' within trap */
315316
char forked;
316317
char binscript;
317-
char funload;
318+
char funload; /* set while autoloading a function via funload() */
318319
char used_pos; /* used positional parameter */
319320
char universe;
320321
char winch; /* set upon window size change or 'set -b' notification */
@@ -380,6 +381,7 @@ struct Shell_s
380381
Dt_t *prev_root;
381382
Dt_t *fpathdict;
382383
Dt_t *typedict;
384+
Dt_t *funload_loopdetect_tree; /* for function autoload loop detection */
383385
char ifstable[256];
384386
Shopt_t offoptions; /* options that were explicitly disabled by the user on the command line */
385387
Shopt_t glob_options;

‎src/cmd/ksh93/sh/path.c‎

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -540,11 +540,14 @@ char *path_fullname(const char *name)
540540
static void funload(int fno, const char *name)
541541
{
542542
char *pname,*oldname=sh.st.filename, buff[IOBSIZE+1];
543-
Namval_t *np, *np_loopdetect;
544-
static Dt_t *loopdetect_tree;
543+
Namval_t *np_loopdetect;
544+
Dt_t *loopdetect_tree;
545545
struct Ufunction *rp,*rpfirst;
546546
int savestates = sh_getstate(), savelineno = sh.inlineno;
547547
char oldload = sh.funload;
548+
volatile char *errorname = NULL;
549+
volatile int jmpval;
550+
struct checkpt checkpoint;
548551
pname = path_fullname(stkptr(sh.stk,PATH_OFFSET));
549552
if(sh.fpathdict && (rp = dtmatch(sh.fpathdict,pname)))
550553
{
@@ -558,6 +561,7 @@ static void funload(int fno, const char *name)
558561
}
559562
do
560563
{
564+
Namval_t *np;
561565
if((np = dtsearch(funtree,rp->np)) && is_afunction(np))
562566
{
563567
if(np->nvalue)
@@ -572,42 +576,55 @@ static void funload(int fno, const char *name)
572576
free(pname);
573577
return;
574578
}
575-
if(!loopdetect_tree)
576-
loopdetect_tree = dtopen(&_Nvdisc,Dtoset);
577-
else if(nv_search(pname,loopdetect_tree,0))
579+
loopdetect_tree = sh_subloopdetecttree(1);
580+
if(nv_search(pname,loopdetect_tree,0))
578581
{
582+
dtclear(loopdetect_tree);
579583
errormsg(SH_DICT,ERROR_exit(ERROR_NOEXEC),"autoload loop: %s in %s",name,pname);
580584
UNREACHABLE();
581585
}
582-
np_loopdetect = nv_search(pname,loopdetect_tree,NV_ADD);
586+
np_loopdetect = nv_search(pname,loopdetect_tree,NV_ADD|NV_NOSCOPE);
583587
sh_onstate(SH_NOALIAS);
584588
sh.readscript = (char*)name;
585589
sh.st.filename = pname;
586590
sh.funload = 1;
587591
sh.inlineno = 1;
588592
error_info.line = 0;
589-
sh_eval(sfnew(NULL,buff,IOBSIZE,fno,SFIO_READ),SH_FUNEVAL);
590-
sh_close(fno);
591-
sh.readscript = 0;
593+
/* sh_eval may longjmp if the script throws an error; to restore
594+
* state, we must include this function in the longjmp chain */
595+
sh_pushcontext(&checkpoint,1);
596+
jmpval = sigsetjmp(checkpoint.buff,0);
597+
if(!jmpval)
598+
{
599+
Namval_t *np;
600+
/* run the function script */
601+
sh_eval(sfnew(NULL,buff,IOBSIZE,fno,SFIO_READ),SH_FUNEVAL);
602+
/* check if the expected function was loaded */
592603
#if SHOPT_NAMESPACE
593-
if(sh.namespace)
594-
np = sh_fsearch(name,0);
595-
else
604+
if(sh.namespace)
605+
np = sh_fsearch(name,0);
606+
else
596607
#endif /* SHOPT_NAMESPACE */
597-
np = nv_search(name,sh.fun_tree,0);
598-
if(!np || !np->nvalue)
599-
pname = stkcopy(sh.stk,sh.st.filename);
600-
else
601-
pname = 0;
602-
free(sh.st.filename);
608+
np = nv_search(name,sh.fun_tree,0);
609+
if(!np || !np->nvalue)
610+
errorname = stkcopy(sh.stk,sh.st.filename);
611+
}
612+
/* restore state */
613+
sh_close(fno);
614+
sh.readscript = 0;
615+
free(pname);
603616
sh.funload = oldload;
604617
sh.inlineno = savelineno;
605618
sh.st.filename = oldname;
606619
sh_setstate(savestates);
607620
nv_delete(np_loopdetect,loopdetect_tree,0);
608-
if(pname)
621+
/* continue the longjmp chain on error */
622+
sh_popcontext(&checkpoint);
623+
if(jmpval)
624+
siglongjmp(*sh.jmplist,jmpval);
625+
if(errorname)
609626
{
610-
errormsg(SH_DICT,ERROR_exit(ERROR_NOEXEC),e_funload,name,pname);
627+
errormsg(SH_DICT,ERROR_exit(ERROR_NOEXEC),e_funload,name,errorname);
611628
UNREACHABLE();
612629
}
613630
}

‎src/cmd/ksh93/sh/subshell.c‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ static struct subshell
5757
struct Link *svar; /* save shell variable table */
5858
Dt_t *sfun; /* function scope for subshell */
5959
Dt_t *strack;/* tracked alias scope for subshell */
60+
Dt_t *sfaldt;/* subshell function autoload loop detection tree */
6061
Pathcomp_t *pathlist; /* for PATH variable */
6162
Shopt_t options;/* save shell options */
6263
pid_t subpid; /* child process ID */
@@ -425,6 +426,27 @@ Dt_t *sh_subfuntree(int create)
425426
return sh.fun_tree;
426427
}
427428

429+
/*
430+
* Return pointer to the function autoload loop detection tree.
431+
* Create new one if in a subshell and one doesn't exist and 'create' is non-zero.
432+
*/
433+
Dt_t *sh_subloopdetecttree(int create)
434+
{
435+
struct subshell *sp = subshell_data;
436+
if(!sh.funload_loopdetect_tree)
437+
sh.funload_loopdetect_tree = dtopen(&_Nvdisc,Dtoset);
438+
if(create && sh.subshell && !sh.subshare)
439+
{
440+
if(sp && !sp->sfaldt)
441+
{
442+
sp->sfaldt = dtopen(&_Nvdisc,Dtoset);
443+
dtview(sp->sfaldt, sh.funload_loopdetect_tree);
444+
sh.funload_loopdetect_tree = sp->sfaldt;
445+
}
446+
}
447+
return sh.funload_loopdetect_tree;
448+
}
449+
428450
int sh_subsavefd(int fd)
429451
{
430452
struct subshell *sp = subshell_data;
@@ -840,6 +862,12 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, char comsub)
840862
}
841863
dtclose(sp->sfun);
842864
}
865+
/* Clean up subshell autoload loop detection tree. */
866+
if(sp->sfaldt)
867+
{
868+
sh.funload_loopdetect_tree = dtview(sp->sfaldt,0);
869+
dtclose(sp->sfaldt);
870+
}
843871
n = sh.st.trapmax-savst.trapmax;
844872
sh_sigreset(1);
845873
if(n>0)

‎src/cmd/ksh93/sh/xec.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1270,7 +1270,7 @@ int sh_exec(const Shnode_t *t, int flags)
12701270
}
12711271
else
12721272
{
1273-
errormsg(SH_DICT,ERROR_exit(0),e_found,"function");
1273+
errormsg(SH_DICT,ERROR_exit(0),e_autoloadnotfound,com0);
12741274
sh.exitval = ERROR_NOENT;
12751275
}
12761276
goto setexit;

‎src/cmd/ksh93/tests/functions.sh‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,5 +1574,47 @@ do got=$(unset -f "$b"; PATH=/dev/null; "$b" --version 2>&1)
15741574
"(expected status 2 and ERE match of $(printf %q "$exp"), got status $e and $(printf %q "$got"))"
15751575
done
15761576

1577+
# ======
1578+
# test autoloading a function from within a command substitutino
1579+
# https://github.com/ksh93/ksh/issues/971
1580+
1581+
print -r >test_program \
1582+
'autoload my_function
1583+
1584+
# If I called this function directly, all problems go away:
1585+
# my_function
1586+
1587+
# Without that, this first command substitution works:
1588+
functions my_function third_function
1589+
print "$(my_function)"
1590+
1591+
# But when I do it a second time:
1592+
functions my_function third_function
1593+
print "$(my_function)"
1594+
# ./test_program: line 4: autoload loop: my_function in /home/perette/bin/shfuncs/my_function'
1595+
1596+
print -r >my_function \
1597+
'autoload third_function
1598+
function my_function {
1599+
third_function
1600+
print "my_function was called."
1601+
}'
1602+
1603+
print -r >third_function \
1604+
'function third_function {
1605+
print "Third function got called."
1606+
}'
1607+
1608+
got=$(FPATH=$PWD; set +x; . ./test_program 2>&1)
1609+
exp='typeset -fu my_function
1610+
Third function got called.
1611+
my_function was called.
1612+
typeset -fu my_function
1613+
Third function got called.
1614+
my_function was called.'
1615+
1616+
[[ $got == "$exp" ]] || err_exit "autoloading a function from a command substitution" \
1617+
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
1618+
15771619
# ======
15781620
exit $((Errors<125?Errors:125))

0 commit comments

Comments
 (0)