Build dynamic libraries #357

McDutchie · 2021-12-01T23:43:11Z

This is an attempt to amend the build system to support dynamic libraries. It is mostly working on Linux, macOS, *BSD and Solaris.

My first attempt dropped dynamic libraries in arch/*/lib. But that confused bin/packace so hopelessly it didn't even run.

So instead this commit separates the new functionality cleanly from the existing AT&T mess. It uses a wrapper script, bin/dynamic, that exports some environment variables. New script code is added to relevant Mamfiles that use these variables to build dynamic libraries. It checks if $DYLIB_CCFLAG is set and if not, it does nothing. So nothing changes if this wrapper script is not used.

It also adds an installation feature, bin/dynamic install, that either creates a nice directory tree complete with manual pages and include headers, or it will install into an existing directory tree without overwriting it. By default, ksh and shcomp are installed, but that selection can be overridden by providing command name arguments after the root directory, for example:

$ bin/dynamic install /usr/local ksh shcomp pty suid_exec iffe

Of course, this is not ready. Problems persist.

Most importantly, major regressions appear on various systems when ksh is linked dynamically. On Linux, $PATH search is broken on shell initialisation, causing various regression test failures and a basically unusable shell binary. This a problem I've seen before on some ancient systems, I just chalked it up to them being obsolete, but no, it's a real ksh bug and it needs fixing before any of this is ready for prime time. Help! :-)

~~There are different regression test failures on the BSDs and on macOS.~~ It fails a lot on OpenBSD.

Secondly, I have not succeeded at using standard library versioning on systems other than macOS. macOS uses the version number before the .dylib extension, making it possible to link to specific versions using something like -last.1.0 without trickery. On Linux and other systems the version number follows the extension, making that impossible, and I don't know a way around it. Simply using the path to the library is not an option as that makes the binary dependent on your development directory. So this uses a naming scheme that is similar to macOS on all systems, with an additional 93u+m tag to make it clear this is our version and not AT&T's. This also works on systems that use a .so extension, but is idiosyncratic on those. I don't know if this actually a problem.

Perhaps we need to use something like GNU libtool instead for generating dynamic libraries. That should be quite doable as the Mamfiles can execute any shell script, but I don't understand the legalities of combining GPL'ed software with ours. Plus I'd like to avoid adding bloat if at all possible.

bin/dynamic:

Added.

src/*/*/Mamfile:

Add exec - scripts to handle dynamic building.

src/cmd/ksh93/Mamfile:

The change from setv mam_cc_FLAGS to setv mam_cc_FLAGS ${mam_cc_DLL} is important; this makes libshell compile with -fPIC or whatever flag mamprobe detects to make the compiler generate relocatable code that is usable in dynamic libraries. That mam_cc_DLL variable is what's exported by mamprobe.

Resolves: #302

JohnoKing · 2021-12-03T03:07:50Z

Most importantly, major regressions appear on various systems when
ksh is linked dynamically. On Linux, $PATH search is broken on
shell initialisation, causing various regression test failures and
a basically unusable shell binary. This a problem I've seen before
on some ancient systems, I just chalked it up to them being
obsolete, but no, it's a real ksh bug and it needs fixing before
any of this is ready for prime time. Help! :-)

Some testing on my primary Linux system shows that the build dir somehow gets included in shp->bltin_tree under spurious circumstances:

# Note 1: The full path to the build directory's bin folder must be included in $PATH
# Note 2: The Linux system I test on symlinks /bin to /usr/bin (usrmerge)
$ PATH=/usr/bin:/home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin ksh -c 'whence -a chmod; /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin/chmod --version'
chmod is a tracked alias for /usr/bin/chmod
chmod is a shell builtin version of /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin/chmod
  version         chmod (AT&T Research) 2012-04-20
# Note that there is no chmod command in the build dir bin folder
$ [ -e /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin/chmod ]; echo $?
1

The whence code below shows that the only way the above whence -a call could fail in that manner is if the build directory's bin folder is in shp->bltin_tree:

ksh/src/cmd/ksh93/bltins/whence.c

Lines 290 to 292 in 430e478

    
           /* built-in version of program */ 
        
           if(nv_search(cp,shp->bltin_tree,0)) 
        
           	msg = sh_translate(is_builtver);

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

Line 145 in 430e478

const char is_builtver[] = "is a shell builtin version of";

JohnoKing · 2021-12-03T04:04:58Z

The cause of the regression test failure on my machine is this .paths file the build system generates at arch/*/bin/.paths:

# use { no NO } prefix to permanently disable #
PLUGIN_LIB=cmd
FPATH=../fun

The specific setting that causes shell builtins to become loadable from arch/*/bin is PLUGIN_LIB. Example:

$ mkdir /tmp/bin
$ PATH=/usr/bin:/tmp/bin ksh -c 'whence -a chmod; /tmp/bin/chmod --version'
chmod is a tracked alias for /usr/bin/chmod
ksh: /tmp/bin/chmod: not found
$ echo 'PLUGIN_LIB=cmd' > /tmp/bin/.paths
$ PATH=/usr/bin:/tmp/bin ksh -c 'whence -a chmod; /tmp/bin/chmod --version'   
chmod is a tracked alias for /usr/bin/chmod
chmod is a shell builtin version of /tmp/bin/chmod

Relevant code:

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

Lines 1613 to 1618 in 430e478

    
           else if(m==11 && memcmp((void*)sp,(void*)"PLUGIN_LIB=",m)==0) 
        
           { 
        
           	if(pp->bbuf) 
        
           		free(pp->bbuf); 
        
           	pp->blib = pp->bbuf = sh_strdup(ep); 
        
           }

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

Line 52 in 430e478

#define LIBCMD "cmd"

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

Lines 850 to 860 in 430e478

    
           if(!strcmp(cp,LIBCMD) && 
        
              (addr=(Shbltin_f)dlllook((void*)0,stakptr(n))) && 
        
              (np = sh_addbuiltin(stakptr(PATH_OFFSET),addr,NiL)) && 
        
              nv_isattr(np,NV_BLTINOPT)) 
        
           { 
        
           found: 
        
           	if(fp) 
        
           		free(fp); 
        
           	shp->bltin_dir = 0; 
        
           	return(oldpp); 
        
           }

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

Lines 1547 to 1554 in 430e478

    
           if(strcmp(name,SH_CMDLIB_DIR)==0) 
        
           { 
        
           	pp->dev = 1; 
        
           	pp->flags |= PATH_BUILTIN_LIB; 
        
           	pp->blib = pp->bbuf = sh_malloc(sizeof(LIBCMD)); 
        
           	strcpy(pp->blib,LIBCMD); 
        
           	return(first); 
        
           }

McDutchie · 2021-12-03T16:07:14Z

Nice find. If I move that .paths file out of the way on my Slackware Linux VM, all the regression tests pass with the dynamically linked ksh! However, there must be more to the cause, as this failure only occurs on Linux. On other systems, that .paths file is also there but does not cause failures.

On macOS, after disabling System Integrity Protection so I can use DYLD_LIBRARY_PATH (same as LD_LIBRARY_PATH on Linux), there are no regressions at all with the dynamically linked ksh, with that .paths file present:

$ find arch -name .paths                                                                                            
arch/darwin.i386-64/bin/.paths
$ cat arch/darwin.i386-64/bin/.paths
# use { no NO } prefix to permanently disable #
PLUGIN_LIB=cmd
FPATH=../fun

$ DYLD_LIBRARY_PATH=$PWD/arch/darwin.i386-64/dyn:/lib:/usr/lib bin/shtests -p SHELL=$PWD/arch/darwin.i386-64/dyn/ksh         
#### Regression-testing /usr/local/src/ksh93/ksh/arch/darwin.i386-64/dyn/ksh ####
test alias begins at 2021-12-03+16:58:20
test alias passed at 2021-12-03+16:58:20 [ 24 tests 0 errors ]
<…successful tests snipped…>
test vartree2 begins at 2021-12-03+17:01:16
test vartree2 passed at 2021-12-03+17:01:16 [ 21 tests 0 errors ]
Total errors: 0
CPU time       user:      system:
main:      0m00.076s    0m00.170s
tests:     0m53.520s    1m06.519s

McDutchie · 2021-12-03T21:02:58Z

I've been tracing the execution of this code with debug output commands. On Linux, it looks like the dlllook() call in this if clause:

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

Lines 850 to 860 in 430e478

    
           if(!strcmp(cp,LIBCMD) && 
        
              (addr=(Shbltin_f)dlllook((void*)0,stakptr(n))) && 
        
              (np = sh_addbuiltin(stakptr(PATH_OFFSET),addr,NiL)) && 
        
              nv_isattr(np,NV_BLTINOPT)) 
        
           { 
        
           found: 
        
           	if(fp) 
        
           		free(fp); 
        
           	shp->bltin_dir = 0; 
        
           	return(oldpp); 
        
           }

returns an address. On other systems (FreeBSD, macOS) dlllook() returns NULL.

On Linux, this causes sh_addbuiltin() to be executed, incorrectly adding a built-in path starting with the directory where .paths was found to sh.bltin_tree. sh_addbuiltin() then returns NULL, so the found: clause is not executed straight away, but then this:

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

Lines 871 to 872 in 430e478

    
           if(*stakptr(PATH_OFFSET)=='/' && nv_search(stakptr(PATH_OFFSET),shp->bltin_tree,0)) 
        
           	goto found;

causes that path to be found in sh.bltins_tree so it goes back to found:.

So it looks like we need to investigate why dlllook() returns an address on Linux systems and NULL on others.

McDutchie · 2021-12-03T21:21:30Z

On Linux, this causes sh_addbuiltin() to be executed, incorrectly adding a built-in path starting with the directory where .paths was found to sh.bltin_tree.

Actually, is that incorrect? It seems consistent with what sh.1 describes for the PLUGIN_LIB line in .paths. Perhaps the Linux behaviour is correct, and what's broken is both the regression tests and this lookup on non-Linux systems.

McDutchie · 2021-12-03T21:35:22Z

Yes, the Linux behaviour is correct, even though that's what breaks the regression tests. On Slackware:

$ mkdir -p /tmp/foo/bin
$ echo 'PLUGIN_LIB=cmd' >/tmp/foo/bin/.paths
$ PATH=/tmp/foo/bin:$PATH LD_LIBRARY_PATH=$PWD/arch/linux.i386-64/dyn arch/linux.i386-64/dyn/ksh -c 'whence -a chmod'
chmod is a shell builtin version of /tmp/foo/bin/chmod
chmod is /usr/bin/chmod
chmod is /bin/chmod

The above is consistent with the .paths/PLUGIN_LIB behaviour described in the manual. Whereas, on macOS:

$ mkdir -p /tmp/foo/bin
$ echo 'PLUGIN_LIB=cmd' >/tmp/foo/bin/.paths
$ DYLD_LIBRARY_PATH=/usr/local/src/ksh93/ksh/arch/darwin.i386-64/dyn PATH=/tmp/foo/bin:$PATH arch/darwin.i386-64/dyn/ksh -c 'whence -a chmod'                                   
chmod is a tracked alias for /bin/chmod

…it doesn't seem to work at all.

I think that's a bug for another day. For now, I'm content to just get rid of that .paths file generated by bin/package. At some point we should test this functionality in a distinct set of regression tests instead.

JohnoKing · 2021-12-03T21:44:53Z

It should be noted that PLUGIN_LIB loads all of the libcmd commands, not just the /opt/ast/bin commands in shtab_builtins:

$ mkdir /tmp/bin; echo 'PLUGIN_LIB=cmd' > /tmp/bin/.paths
$ PATH=/usr/bin:/tmp/bin arch/linux.i386-64/dyn/ksh -c 'whence -a tail; /tmp/bin/tail --version; /opt/ast/bin/tail --version'
tail is a tracked alias for /usr/bin/tail
tail is a shell builtin version of /tmp/bin/tail
  version         tail (AT&T Research) 2012-10-10
arch/linux.i386-64/dyn/ksh: /opt/ast/bin/tail: not found

That's not to say the current behavior doesn't have any issues. The vmstate builtin only works with vmalloc, yet becomes available when compiled with -D_std_malloc after setting PLUGIN_LIB:

$ PATH=/usr/bin:/tmp/bin arch/linux.i386-64/dyn/ksh -c 'whence -a vmstate; /tmp/bin/vmstate'
vmstate is a shell builtin version of /tmp/bin/vmstate
region=0x7f2717c23e60 method=best flags=0 size=32768 segments=1 busy=(0,0,0) free=(32640,3,28432)

Additionally, the libcmd builtins don't become accessible unless whence -a is run beforehand:

$ PATH=/usr/bin:/tmp/bin arch/*/dyn/ksh -c '/tmp/bin/tail'   
arch/linux.i386-64/dyn/ksh: /tmp/bin/tail: not found

McDutchie · 2021-12-03T21:58:21Z

So, I pushed a commit to my dyn branch that removes the creation of a .paths file by bin/package.

And look what that exposed: on Linux (CI runner):

test leaks begins at 2021-12-03+21:55:45
	leaks.sh[169]: memory leak on PATH reset before PATH search (leaked approx 143584 bytes after 512 iterations)
	leaks.sh[176]: memory leak on PATH reset (leaked approx 41600 bytes after 512 iterations)
	leaks.sh[381]: set PATH value in main shell (leaked approx 141472 bytes after 512 iterations)
	leaks.sh[388]: run command with preceding PATH assignment in main shell (leaked approx 42432 bytes after 512 iterations)
test leaks failed at 2021-12-03+21:55:45 with exit code 4 [ 41 tests 4 errors ]

…and on macOS:

test leaks begins at 2021-12-03+22:45:42
	leaks.sh[169]: memory leak on PATH reset before PATH search (leaked approx 4636 KiB after 16384 iterations)
	leaks.sh[176]: memory leak on PATH reset (leaked approx 1384 KiB after 16384 iterations)
	leaks.sh[381]: set PATH value in main shell (leaked approx 4164 KiB after 16384 iterations)
	leaks.sh[388]: run command with preceding PATH assignment in main shell (leaked approx 1364 KiB after 16384 iterations)
test leaks failed at 2021-12-03+22:46:26 with exit code 4 [ 41 tests 4 errors ]

~~So, that .paths file has been masking memory leaks all this time. Or (we can hope) just one memory leak manifesting as four test failures.~~

All other tests pass though, on both Linux and macOS.

The odd thing is, the build system currently only regression-tests the ksh with statically linked libast/libdll/libcmd/libshell. It does not test the dynamically linked version at all. Yet, these failures apparently occur because the bin/.paths file was not created in the build directory.

McDutchie · 2021-12-03T22:03:12Z

What I had missed is that src/cmd/INIIT/Mamfile also creates a .paths file, if bin/package does not create one!

The content of that one, on my system, is:

$ cat arch/darwin.i386-64/bin/.paths 
# use { no NO } prefix to permanently disable #
noPLUGIN_LIB=cmd
FPATH=../fun
DYLD_LIBRARY_PATH=../lib

After removing that .paths file, the memory leaks disappear again.

So, that is also to note for a future bugfix. For now, I'll get rid of the creation of that from the Mamfile as well.

JohnoKing · 2021-12-03T22:05:56Z

This is minor, but I have a patch that fixes some typos I saw in the pull request:

typo-fixes.patch

--- a/bin/dynamic
+++ b/bin/dynamic
@@ -104,7 +104,7 @@ install)
 	shift 2
 	# commands to install by default
 	test "$#" -eq 0 && set -- ksh shcomp  # pty suid_exec
-	# check if we need to build, determining soure command paths;
+	# check if we need to build, determining source command paths;
 	# install from dyn/ or, if not found, from bin/
 	for f do
 		shift
--- a/src/cmd/ksh93/Mamfile
+++ b/src/cmd/ksh93/Mamfile
@@ -1390,7 +1390,7 @@ make install
 		make libshell.dyn
 			prev libshell.a archive
 			note *
-			note * Link a dynamic libary if appropriate variables were exported.
+			note * Link a dynamic library if appropriate variables were exported.
 			note * See bin/dynamic in the source directory for more information.
 			note *
 			exec - set -e
--- a/src/lib/libast/Mamfile
+++ b/src/lib/libast/Mamfile
@@ -6103,7 +6103,7 @@ make install
 		make libast.dyn
 			prev libast.a archive
 			note *
-			note * Link a dynamic libary if appropriate variables were exported.
+			note * Link a dynamic library if appropriate variables were exported.
 			note * See bin/dynamic in the source directory for more information.
 			note *
 			exec - set -e
--- a/src/lib/libcmd/Mamfile
+++ b/src/lib/libcmd/Mamfile
@@ -761,7 +761,7 @@ make install
 		make libcmd.dyn
 			prev libcmd.a archive
 			note *
-			note * Link a dynamic libary if appropriate variables were exported.
+			note * Link a dynamic library if appropriate variables were exported.
 			note * See bin/dynamic in the source directory for more information.
 			note *
 			exec - set -e
--- a/src/lib/libdll/Mamfile
+++ b/src/lib/libdll/Mamfile
@@ -270,7 +270,7 @@ make install
 		make libdll.dyn
 			prev libdll.a archive
 			note *
-			note * Link a dynamic libary if appropriate variables were exported.
+			note * Link a dynamic library if appropriate variables were exported.
 			note * See bin/dynamic in the source directory for more information.
 			note *
 			exec - set -e

McDutchie · 2021-12-03T22:29:05Z

That's not to say the current behavior doesn't have any issues. The vmstate builtin only works with vmalloc, yet becomes available when compiled with -D_std_malloc after setting PLUGIN_LIB:
$ PATH=/usr/bin:/tmp/bin arch/linux.i386-64/dyn/ksh -c 'whence -a vmstate; /tmp/bin/vmstate'
vmstate is a shell builtin version of /tmp/bin/vmstate
region=0x7f2717c23e60 method=best flags=0 size=32768 segments=1 busy=(0,0,0) free=(32640,3,28432)

It looks like you did not use LD_LIBRARY_PATH to ensure that dyn/ksh, which is little more than a wrapper around libshell, linked against the recompiled libshell. So I'm guessing the libshell that it found had vmalloc enabled.

Actually, I reproduced the issue, with a libshell that definitely has vmalloc disabled.

I'll have to disable the compilation of that completely.

per ksh93#357 (comment)

McDutchie · 2021-12-03T23:05:11Z

Additionally, the libcmd builtins don't become accessible unless whence -a is run beforehand:
$ PATH=/usr/bin:/tmp/bin arch/*/dyn/ksh -c '/tmp/bin/tail'   
arch/linux.i386-64/dyn/ksh: /tmp/bin/tail: not found

This makes sense, since the PATH search is what adds them, and that is as documented. If you invoke a command using a direct path, then no PATH search is done. The following should work (but currently only seems to work on Linux):

$ cat /tmp/foo/bin/.paths
PLUGIN_LIB=cmd
$ LD_LIBRARY_PATH=$PWD/arch/linux.i386-64/dyn PATH=/tmp/foo/bin:$PATH arch/linux.i386-64/dyn/ksh -c 'tail --version; whence -a tail'
  version         tail (AT&T Research) 2012-10-10
tail is a shell builtin version of /tmp/foo/bin/tail
tail is /usr/bin/tail
tail is /bin/tail

JohnoKing · 2021-12-06T16:59:19Z

I recently found a patch which adds support for DYLD_LIBRARY_PATH to iffe: lkujaw@48ff054. Perhaps this would be a useful addition to your pull request.

McDutchie · 2021-12-06T20:36:22Z

I recently found a patch which adds support for DYLD_LIBRARY_PATH to iffe: lkujaw@48ff054. Perhaps this would be a useful addition to your pull request.

Thanks. It doesn't hurt, so I'll add it for completeness' sake, but it's of limited use since DYLD_LIBRARY_PATH is disabled by default on macOS and to enable it you have to disable System Integrity Protection in a rescue console, which few will do. Things are also working fine without, as all libraries are compiled in static versions regardless. The dynamic libraries in my patch are stored separately so iffe never sees them.

ryandesign · 2021-12-10T17:18:13Z

DYLD_LIBRARY_PATH is disabled by default on macOS and to enable it you have to disable System Integrity Protection

That's not true. When SIP is enabled, DYLD_LIBRARY_PATH still works, but it is not propagated to subprocesses, so you must set it at exactly the right place, rather than trying to set it very early and hoping the system will pass it down to each successive subprocess for you.

JohnoKing · 2021-12-12T09:22:14Z

I've found a bug in shtests caused by the removal of the .paths file in arch/*/bin. The pty regression tests fail to run because the script cannot find pty in the PATH:

$ bin/shtests -p pty
#### Regression-testing /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin/ksh ####
test pty begins at 2021-12-12+01:15:17
	pty.sh[36]: warning: pty command not found -- tests skipped
test pty passed at 2021-12-12+01:15:17 [ 42 tests 0 errors ]
Total errors: 0
CPU time       user:      system:
main:      0m00.003s    0m00.005s
tests:     0m00.028s    0m00.016s

This occurs because the shtests script doesn't set the PATH correctly if arch/*/bin lacks a .paths file:

ksh/src/cmd/ksh93/tests/shtests

Lines 275 to 277 in 5d8ae36

    
           if	[[ $INSTALLROOT && -r $INSTALLROOT/bin/.paths ]] 
        
           then	PATH=$INSTALLROOT/bin:$PATH 
        
           fi

This can be fixed by just detecting if the folder exists:

folder-detect.patch

--- a/src/cmd/ksh93/tests/shtests
+++ b/src/cmd/ksh93/tests/shtests
@@ -272,7 +272,7 @@ if	[[ -d /usr/ucb ]]
 then	PATH=$PATH:/usr/ucb
 fi
 PATH=$PATH:$d
-if	[[ $INSTALLROOT && -r $INSTALLROOT/bin/.paths ]]
+if	[[ $INSTALLROOT && -d $INSTALLROOT/bin ]]
 then	PATH=$INSTALLROOT/bin:$PATH
 fi
 if	[[ ${SHELL%/*} != $INSTALLROOT/bin ]]

Ref.: ksh93#357 (comment)

bin/dynamic

Ref.: ksh93#357 (comment)

McDutchie · 2021-12-13T15:00:11Z

In my testing, this is now working fine on macOS, Linux (Slackware, Debian, Void with musl, NixOS), *BSD (DragonFly, Free, Net, Open), and Solaris (10.1 and 11.4), which covers most of the *nix market. I'd like to offer this functionality in the next beta and I think it's either ready or close to ready. Cygwin can wait. Please test what we've got now.

hyenias · 2021-12-13T18:20:51Z

I could not figure out from all the commentary above on how to compile a dynamic version of ksh let alone how to go about running shtests against it. I tried a few things but all failed.

By running bin/dynamic make, it seemed to complete successfully with both a statically linked normal ksh file in the usual location of arch/*/bin/ksh. I noticed a arch/*/dyn directory having a bin and a lib folder. The arch/*/dyn/bin directory had a smaller ksh executable.

$ arch/*/bin/ksh --version
  version         sh (AT&T Research) 93u+m/1.1.0-alpha+ea3c1526 2021-12-13
$ arch/*/dyn/bin/ksh --version
arch/linux.arm/dyn/bin/ksh: error while loading shared libraries: libshell.so.6: cannot open shared object file: No such file or directory

Can someone please list out the build and test steps so that I may assist?

McDutchie · 2021-12-13T22:56:51Z

Sorry about that @hyenias. Yes, bin/dynamic make is what you do. I'm now thinking I should integrate it in bin/package after all, but I want to clean up a lot of old cruft from that build system first (I'm working on it).

$ arch/*/dyn/bin/ksh --version
arch/linux.arm/dyn/bin/ksh: error while loading shared libraries: libshell.so.6: cannot open shared object file: No such file or directory

This is normal. The libraries are not yet installed in the library search path, so the system can't find them. You can do one of the following:

Install directly: bin/dynamic install /usr/local
Install into a package tree: bin/dynamic install /some/directory/usr/local and then use whatever package manager your OS uses to create a package from /some/directory.
Test it without installing by exporting LD_LIBRARY_PATH=$PWD/arch/linux.arm/dyn/lib.

Hope this helps.

hyenias · 2021-12-14T00:58:32Z

Thanks! As I am testing, I do not want to install anything anywhere just keep all the files right where they are. Unfortunately, when I attempted a simple version display the dynamic ksh immediately had a segmentation fault before I even attempted to run shtests:

# on a Linux Ubuntu 20.04 arm7l box
$ bin/dynamic make

$ arch/*/bin/ksh --version
  version         sh (AT&T Research) 93u+m/1.1.0-alpha+ea3c1526 2021-12-13
$
$ LD_LIBRARY_PATH=$(echo $PWD/arch/*/dyn/lib) arch/*/dyn/bin/ksh --version
Segmentation fault

# tested the arch/*/bin/ksh and completed without any errors
$ bin/shtests

# attempted to run shtests for the dynamic ksh and failed
$ LD_LIBRARY_PATH=$(echo $PWD/arch/*/dyn/lib) KSH=$(echo $PWD/arch/*/dyn/bin/ksh) bin/shtests
#### Regression-testing /dev/shm/ksh/arch/linux.arm/dyn/bin/ksh ####
Segmentation fault
$

hyenias · 2021-12-14T02:18:03Z

Here is the backtrace using gdb after I deleted my arch folder and compiled again using CCFLAGS='-O0 -g -D_std_malloc' bin/dynamic make.

$ LD_LIBRARY_PATH=$(echo $PWD/arch/*/dyn/lib) gdb $PWD/arch/*/dyn/bin/ksh
GNU gdb (Ubuntu 9.2-0ubuntu1~20.04) 9.2
Reading symbols from /dev/shm/ksh/arch/linux.arm/dyn/bin/ksh...
(gdb) run --version
Starting program: /dev/shm/ksh/arch/linux.arm/dyn/bin/ksh --version

Program received signal SIGSEGV, Segmentation fault.
0xb6dad9e2 in regnexec_20120528 (p=0x422c5c, s=0x41f630 "xterm-256color", len=14, nmatch=0, match=0x0, flags=49)
    at /dev/shm/ksh/src/lib/libast/regex/regnexec.c:1864
1864		if (len < env->min)
(gdb) backtrace
#0  0xb6dad9e2 in regnexec_20120528 (p=0x422c5c, s=0x41f630 "xterm-256color", len=14, nmatch=0, match=0x0, flags=49)
    at /dev/shm/ksh/src/lib/libast/regex/regnexec.c:1864
#1  0xb6da9a46 in regexec_20120528 (p=0x422c5c, s=0x41f630 "xterm-256color", nmatch=0, match=0x0, flags=49)
    at /dev/shm/ksh/src/lib/libast/regex/regexec.c:54
#2  0xb6d53d00 in strgrpmatch_20120528 (b=0x41f630 "xterm-256color",
    p=0xb6de8ae0 "(ansi|cons|dtterm|linux|screen|sun|vt???|wsvt|xterm)*", sub=0x0, n=0, flags=7)
    at /dev/shm/ksh/src/lib/libast/string/strmatch.c:144
#3  0xb6d53e1a in strmatch (s=0x41f630 "xterm-256color",
    p=0xb6de8ae0 "(ansi|cons|dtterm|linux|screen|sun|vt???|wsvt|xterm)*")
    at /dev/shm/ksh/src/lib/libast/string/strmatch.c:180
#4  0xb6d60974 in opthelp (
    oopts=0xb6fba018 <sh_optksh> "+[-1?\n@(#)$Id: sh (AT&T Research) 93u+m/1.1.0-alpha+ea3c1526 2021-12-13 $\n][-author?David Korn <dgk@research.att.com>][-author?Contributors to https://github.com/ksh93/ksh][-copyright?(c) 1982-2014 AT"...,
    what=0xb6e12f6c <_opt_info_+36> "--version") at /dev/shm/ksh/src/lib/libast/misc/optget.c:2538
#5  0xb6d6884a in optget (argv=0xbefff504,
    oopts=0xb6fba018 <sh_optksh> "+[-1?\n@(#)$Id: sh (AT&T Research) 93u+m/1.1.0-alpha+ea3c1526 2021-12-13 $\n][-author?David Korn <dgk@research.att.com>][-author?Contributors to https://github.com/ksh93/ksh][-copyright?(c) 1982-2014 AT"...)
    at /dev/shm/ksh/src/lib/libast/misc/optget.c:5570
#6  0xb6f575ee in sh_argopts (argc=2, argv=0xbefff504, context=0xb6fd4120 <sh>)
    at /dev/shm/ksh/src/cmd/ksh93/sh/args.c:147
#7  0xb6f65de0 in sh_init (argc=2, argv=0xbefff504, userinit=0x0) at /dev/shm/ksh/src/cmd/ksh93/sh/init.c:1371
#8  0xb6f4cc1e in sh_main (ac=2, av=0xbefff504, userinit=0x0) at /dev/shm/ksh/src/cmd/ksh93/sh/main.c:142
#9  0x004005d0 in main (argc=2, argv=0xbefff504) at /dev/shm/ksh/src/cmd/ksh93/sh/pmain.c:46
(gdb)

# to allow core dumps
$ ulimit -c unlimited
$ LD_LIBRARY_PATH=$(echo $PWD/arch/*/dyn/lib) KSH=$(echo $PWD/arch/*/dyn/bin/ksh) bin/shtests -pk
#### Regression-testing /dev/shm/ksh/arch/linux.arm/dyn/bin/ksh ####
Segmentation fault (core dumped)
$ ls src/cmd/ksh93/tests/core
src/cmd/ksh93/tests/core
$
$ LD_LIBRARY_PATH=$(echo $PWD/arch/*/dyn/lib) gdb $PWD/arch/*/dyn/bin/ksh src/cmd/ksh93/tests/core
GNU gdb (Ubuntu 9.2-0ubuntu1~20.04) 9.2
Reading symbols from /dev/shm/ksh/arch/linux.arm/dyn/bin/ksh...
[New LWP 3715]
Core was generated by `/dev/shm/ksh/arch/linux.arm/dyn/bin/ksh shtests -pk'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0xb6cf29e2 in regnexec_20120528 (p=0x4d112c, s=0x4d10e0 "shtests", len=7, nmatch=0, match=0x0, flags=49)
    at /dev/shm/ksh/src/lib/libast/regex/regnexec.c:1864
1864		if (len < env->min)
(gdb) backtrace
#0  0xb6cf29e2 in regnexec_20120528 (p=0x4d112c, s=0x4d10e0 "shtests", len=7, nmatch=0, match=0x0, flags=49)
    at /dev/shm/ksh/src/lib/libast/regex/regnexec.c:1864
#1  0xb6ceea46 in regexec_20120528 (p=0x4d112c, s=0x4d10e0 "shtests", nmatch=0, match=0x0, flags=49)
    at /dev/shm/ksh/src/lib/libast/regex/regexec.c:54
#2  0xb6c98d00 in strgrpmatch_20120528 (b=0x4d10e0 "shtests", p=0xb6eef07c <e_devfdNN> "/dev/fd/+([0-9])", sub=0x0, n=0,
    flags=7) at /dev/shm/ksh/src/lib/libast/string/strmatch.c:144
#3  0xb6c98e1a in strmatch (s=0x4d10e0 "shtests", p=0xb6eef07c <e_devfdNN> "/dev/fd/+([0-9])")
    at /dev/shm/ksh/src/lib/libast/string/strmatch.c:180
#4  0xb6e92194 in sh_main (ac=3, av=0xbebe34b4, userinit=0x0) at /dev/shm/ksh/src/cmd/ksh93/sh/main.c:258
#5  0x004ad5d0 in main (argc=3, argv=0xbebe34b4) at /dev/shm/ksh/src/cmd/ksh93/sh/pmain.c:46
(gdb)

posguy99 · 2021-12-14T16:59:08Z

I built @9d9327 on macOS 11.6.1 and it appears to work. I think I'm missing how to run shtests with the dynamically linked ksh vs the static one, though.

Edit: Don't mind me, stupid me hasn't used bin/package test in a long time, I always just run shtests. The dynamic script passes arguments it doesn't understand to the main script.

…140c)

JohnoKing · 2024-03-06T13:54:12Z

The current PR cannot produce a functional dynamically linked executable under ASan because the regcomp function gets overriden by ASan, despite linking with --no-as-needed. Applying the regcomp patch fixes this crash and allows the regression tests to complete with no more failures than a statically linked binary compiled from the dev branch.

ASan stacktrace

$ LD_LIBRARY_PATH=/home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/dyn/lib:$LD_LIBRARY_PATH KSH=/home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/dyn/bin/ksh ASAN_OPTIONS='detect_leaks=0' bin/shtests -u
#### Regression-testing /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/dyn/bin/ksh ####
=================================================================
==71901==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x74f69d652d68 at pc 0x74f6a06a2da5 bp 0x7ffe66251b10 sp 0x7ffe662512b8
WRITE of size 64 at 0x74f69d652d68 thread T0
    #0 0x74f6a06a2da4 in __interceptor_regcomp /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:8119
    #1 0x74f69ff6a3df in glob_dir /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/glob.c:516
    #2 0x74f69ff6cc14 in _ast_glob /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/glob.c:785
    #3 0x74f6a025aa54 in path_expand /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/expand.c:124
    #4 0x74f6a025c334 in path_generate /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/expand.c:370
    #5 0x74f6a02c2c37 in endfield /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:2588
    #6 0x74f6a02aa0ea in sh_macexpand /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:247
    #7 0x74f6a023ff0b in arg_expand /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/args.c:819
    #8 0x74f6a023ebd6 in sh_argbuild /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/args.c:659
    #9 0x74f6a035c6be in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:2014
    #10 0x74f6a02c9ed5 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:613
    #11 0x74f6a02c747d in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:366
    #12 0x6064c01b017d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:42
    #13 0x74f69fa2600f  (/usr/lib/libc.so.6+0x2600f) (BuildId: 9c200c88774e0d02ea4d64ef758822e799a28202)
    #14 0x74f69fa260c9 in __libc_start_main (/usr/lib/libc.so.6+0x260c9) (BuildId: 9c200c88774e0d02ea4d64ef758822e799a28202)
    #15 0x6064c01b0084 in _start (/home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/dyn/bin/ksh+0x1084) (BuildId: 77985521d2b6aa1fd2ab3447e60b765d8f298cc3)

Address 0x74f69d652d68 is located in stack of thread T0 at offset 104 in frame
    #0 0x74f69ff68aa6 in glob_dir /home/johno/GitRepos/KornShell/ksh/src/lib/libast/misc/glob.c:299

  This frame has 4 object(s):
    [32, 36) 't1' (line 313)
    [48, 52) 't2' (line 314)
    [64, 104) 'rec' (line 310)
    [144, 184) 'rei' (line 311) <== Memory access at offset 104 partially underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:8119 in __interceptor_regcomp

src/cmd/INIT/Mamfile: - Move .paths from bin into dyn/bin. - Drastically simplify -- just cat the contents that the old code generated. I have no idea why that was so complicated. bin/package: - Remove redunant .paths generation code. src/cmd/INIT/dylink.sh: - Disable dynamic libraries if AST_NO_DYLIB is exported. - Disable dynamic libraries on untested systems unless AST_DYLIB_TEST is exported. - Tweaks.

Putting the .paths file in dyn/bin somehow makes the linker fail, with lots of undefined symbols, when the shell used to compile is /bin/ksh on Debian 11 (ksh 93u+ 2012-08-01). I think I am going to nix the whole .paths thing from the shell -- in my view it's just a bad idea.

@JohnoKing

@JohnoKing writes: > The current PR cannot produce a functional dynamically linked > executable under ASan because the regcomp function gets overriden > by ASan, despite linking with --no-as-needed. Applying the > regcomp patch fixes this crash and allows the regression tests to > complete with no more failures than a statically linked binary > compiled from the dev branch.

McDutchie · 2024-03-23T21:45:45Z

Alright then, this is now confirmed good on Linux, Android, macOS, FreeBSD, NetBSD, OpenBSD, and illumos (plus presumably Solaris). After nearly 2.5 years, it's time to commit this at long last, so we can subject it to wider testing. Some systems are still untested but they can be tested (and then whitelisted in dylink.sh) later. Many thanks for all the help, everyone.

McDutchie · 2024-03-23T22:33:29Z

Actually, I forgot about one thing: I'd like the file version number to be individually settable for each dynamic library. This is easy enough to do from the Mamfiles, so won't take long.

They can now be set by changing DYLIB_VERSION in each of the four libraries in their respective Mamfiles.

This commit re-adds support for building a dynamically linked ksh with libast, libdll, libcmd, and libshell available as dynamic libraries for other applications to use. Previously, this required nmake; as of this commit, we use a helper script dylink.sh instead, which does all the necessary platform-specific things and can be easily extended to add support for new platforms. Currently tested and supported platforms are Linux, Android, macOS, FreeBSD, NetBSD, OpenBSD, and illumos/Solaris. Summary of important changes: src/cmd/ksh93/SHOPT.sh: - Enable SHOPT_DYNAMIC by default. This enables the '-f' option to the 'builtin' command, making it possible to load builtins from dynamic/shared libraries. This mostly makes sense for a dynamically linked ksh, as that is needed to allow ksh and builtins to share the same libraries such as libast. src/cmd/INIT/dylink.sh: - Added. This is a dynamic linking tool to invoke from Mamfiles. - Disable dynamic libraries if AST_NO_DYLIB is exported. - Disable dynamic libraries on untested systems unless AST_DYLIB_TEST is exported. bin/package: - Add support for $INSTALLROOT/dyn subdirectory. - Set LD_LIBRARY_PATH, etc. to point to dyn/lib. - Prefix dyn/bin to $PATH. - do_install(): - Prefer dynamically linked versions for installing, if they have been built. - If dyn/lib exists, install not only the dynamic libraries but also the dev stuff: AST headers and section 3 manual pages. - Remove generation of .paths file. It causes regression test failures because of how built-ins are bound to different paths when found via a .paths file. This functionality is deprecated and may be removed from ksh 93u+m/1.1 and up. bin/shtests: - Relaunch self via 'bin/package use' to pull in the necessary environment stuff to make the pre-installed dynamically linked binaries work. src/cmd/INIT/make.probe: - Add probe for the --as-needed and --no-as-needed linker options. To make dynamic libraries work on Linux arm64, we need to pass --no-as-needed to the linker on Linux to avoid glibc interfering with libast in identically named functions like AST regcomp(3). Linux systems vary as to whether --as-needed is on by default. Thanks to Lukáš Zaoral at Red Hat for finding the key to the fix. src/cmd/INIT/mamprobe.sh: - Update version number. This change causes mamake to redo the probe (it checks for a change in mamprobe but not in make.probe; I should probably fix that at some point). We need mamake to redo the probe for the --no-as-needed addition to take effect. **/Mamfile: - Use the --no-as-needed probe result (either -Wl,-no-as-needed or empty) from ${mam_cc_LD_NOASNEEDED} whenever linking anything; add it to LDFLAGS before invoking dylink. - ksh and shcomp also need the results of the existing --export-dynamic probe; this option is needed on Linux as without it ksh fails to export symbols to loadable builtins. See: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=821806 Thanks to Johnothan King. src/lib/libast/features/map.c, src/lib/libast/regex/regexec.c, src/lib/libast/regex/regnexec.c, src/lib/libast/regex/regrexec.c, src/lib/libast/regex/regsubexec.c: - Map regcomp and all the other AST regex functions via macros so they get an _ast_ prefix. This allows a functional dynamically linked executable to exist under ASan because the regcomp function gets overriden by ASan, despite linking with --no-as-needed. Thanks to Johnothan King. src/lib/libast/comp/errno.c: - Make one trivial change (copyright year) to force libast to relink -- which then triggers a relink of everything else. Co-authored-by: Johnothan King <johnothanking@protonmail.com> Co-authored-by: Lukáš Zaoral <lzaoral@redhat.com>

McDutchie added a commit to McDutchie/ksh that referenced this pull request Dec 3, 2021

libcmd: do not compile vmstate if vmalloc disabled

451752e

per ksh93#357 (comment)

McDutchie force-pushed the dyn branch from d7b4f1c to fa531ca Compare December 4, 2021 01:42

McDutchie mentioned this pull request Dec 11, 2021

Build failure on macOS with memccpy: error: expected parameter declarator #373

Closed

McDutchie added a commit to McDutchie/ksh that referenced this pull request Dec 12, 2021

[dyn] fix $PATH for regression tests (thanks @JohnoKing)

7e10f01

Ref.: ksh93#357 (comment)

JohnoKing reviewed Dec 12, 2021

View reviewed changes

bin/dynamic Outdated Show resolved Hide resolved

McDutchie added a commit to McDutchie/ksh that referenced this pull request Dec 12, 2021

up AST_DYLIB_VERSION to 6.0

9d8240f

Ref.: ksh93#357 (comment)

McDutchie force-pushed the master branch 2 times, most recently from 11741ff to 2084e97 Compare December 14, 2021 09:13

add missing 'setv CCLDFLAGS' to prevent unexpanded expansion (re: 295…

8378da2

…140c)

McDutchie added 9 commits March 7, 2024 01:17

Merge branch 'dev' into dyn

62e8d0e

Merge branch 'dev' into dyn

20f3e16

Merge branch 'dev' into dyn

e89fd83

Merge branch 'dev' into dyn

06f3f12

Merge branch 'dev' into dyn

b9b5a57

dylink: whitelist android as supported

047f249

McDutchie force-pushed the dyn branch from 18e6e6c to 047f249 Compare March 20, 2024 21:51

McDutchie added 9 commits March 22, 2024 00:58

Merge branch 'dev' into dyn

4570a92

Merge branch 'dev' into dyn

31dd151

Merge branch 'dev' into dyn

f545ab8

package: bump version date

fe23d49

dylink.sh: NetBSD tested successfully

ef7a677

bin/package: remove old .paths to avoid regression test failures

82be346

Merge branch 'dev' into dyn

2afa90f

Merge branch 'dev' into dyn

8d1cc28

README.md: update

bcf28ce

McDutchie added 5 commits March 24, 2024 01:14

Merge branch 'dev' into dyn

55ba88a

dylink, Mamfiles: support individually settable lib versions

5ff4190

They can now be set by changing DYLIB_VERSION in each of the four libraries in their respective Mamfiles.

add NEWS entry; bump ksh version date

cb2a8f8

package: rm AST_DYLIB_VERSION (re: 5ff4190)

05e0ee3

kshrc.sh: better macOS workaround for env(1)

f4f25eb

McDutchie merged commit f2bc1f4 into ksh93:dev Mar 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build dynamic libraries #357

Build dynamic libraries #357

McDutchie commented Dec 1, 2021 •

edited

Loading

JohnoKing commented Dec 3, 2021 •

edited

Loading

JohnoKing commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021

McDutchie commented Dec 3, 2021

McDutchie commented Dec 3, 2021

JohnoKing commented Dec 3, 2021

McDutchie commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021

JohnoKing commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021

JohnoKing commented Dec 6, 2021

McDutchie commented Dec 6, 2021

ryandesign commented Dec 10, 2021

JohnoKing commented Dec 12, 2021 •

edited

Loading

McDutchie commented Dec 13, 2021

hyenias commented Dec 13, 2021

McDutchie commented Dec 13, 2021 •

edited

Loading

hyenias commented Dec 14, 2021

hyenias commented Dec 14, 2021 •

edited

Loading

posguy99 commented Dec 14, 2021 •

edited

Loading

JohnoKing commented Mar 6, 2024 •

edited

Loading

McDutchie commented Mar 23, 2024

McDutchie commented Mar 23, 2024

Build dynamic libraries #357

Build dynamic libraries #357

Conversation

McDutchie commented Dec 1, 2021 • edited Loading

JohnoKing commented Dec 3, 2021 • edited Loading

JohnoKing commented Dec 3, 2021 • edited Loading

McDutchie commented Dec 3, 2021 • edited Loading

McDutchie commented Dec 3, 2021

McDutchie commented Dec 3, 2021

McDutchie commented Dec 3, 2021

JohnoKing commented Dec 3, 2021

McDutchie commented Dec 3, 2021 • edited Loading

McDutchie commented Dec 3, 2021

JohnoKing commented Dec 3, 2021 • edited Loading

McDutchie commented Dec 3, 2021 • edited Loading

McDutchie commented Dec 3, 2021

JohnoKing commented Dec 6, 2021

McDutchie commented Dec 6, 2021

ryandesign commented Dec 10, 2021

JohnoKing commented Dec 12, 2021 • edited Loading

McDutchie commented Dec 13, 2021

hyenias commented Dec 13, 2021

McDutchie commented Dec 13, 2021 • edited Loading

hyenias commented Dec 14, 2021

hyenias commented Dec 14, 2021 • edited Loading

posguy99 commented Dec 14, 2021 • edited Loading

JohnoKing commented Mar 6, 2024 • edited Loading

McDutchie commented Mar 23, 2024

McDutchie commented Mar 23, 2024

McDutchie commented Dec 1, 2021 •

edited

Loading

JohnoKing commented Dec 3, 2021 •

edited

Loading

JohnoKing commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021 •

edited

Loading

JohnoKing commented Dec 3, 2021 •

edited

Loading

McDutchie commented Dec 3, 2021 •

edited

Loading

JohnoKing commented Dec 12, 2021 •

edited

Loading

McDutchie commented Dec 13, 2021 •

edited

Loading

hyenias commented Dec 14, 2021 •

edited

Loading

posguy99 commented Dec 14, 2021 •

edited

Loading

JohnoKing commented Mar 6, 2024 •

edited

Loading