Skip to content

Commit

Permalink
Fix build on systems that require -lm (re: 4edfa45)
Browse files Browse the repository at this point in the history
@JohnoKing reports:
> Ksh now fails to build on the specified operating systems unless
> -lm is added to LDFLAGS when invoking bin/package. Failing to
> manually add -lm results in missing symbol errors for functions
> such as frexpl.

The build is confirmed to fail upon linking the libast version of
mamake on NetBSD, DragonflyBSD and Illumos.

Analysis: The root cause of the problem is that the probe in
src/lib/libast/Mamfile, lines 276-301, fails to add a '-lm' line to
ast.req. This probe is designed to figure out is math functions are
dependent on libm. It compiles/links src/lib/libast/port/astmath.c
for a number of values of the macro N, trying a different math
function each time. If any one of them fails without '-lm' but
succeeds with '-lm', the '-lm' is added to libast's dependencies as
processed by mamake.

The problem is that the probe function calls in astmath.c get
optimised out because their values, and hence the results, are
known at compile time -- so linking succeeds where it should fail.
When compiling with -O0 to disable optimisation, the probe works
correctly, -lm is added to ast.req (so mamake includes it in the
mam_libast MAM variable), and the build succeeds on NetBSD without
any problem.

So, the fix is for astmath.c to work around compiler optimisation.

This fixes an old problem in the build system. With the root cause
fixed, we are now also able to remove a bunch of -lm workarounds.

src/lib/libast/port/astmath.c:
- Replace fixed 0 values with rand() calls to avoid probe call
  results being known at compile time.

src/lib/libast/Mamfile:
- For some reason, the N=7 astmath.c case was not tried; add it.
- Tweaks to error out early in case of failure.

src/cmd/builtin/Mamfile,
src/cmd/ksh93/Mamfile,
src/cmd/builtin/features/pty:
- Remove -lm workarounds. (re: 2b27f63, d79a34b/22e044c3,
  dd0d03b, d3cd4cf, and even AT&T)

Resolves: #715
  • Loading branch information
McDutchie committed Feb 1, 2024
1 parent 385289a commit 67a581d
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 32 deletions.
2 changes: 1 addition & 1 deletion src/cmd/builtin/Mamfile
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ make install virtual
exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} -I. -I${PACKAGE_ast_INCLUDE} -DERROR_CATALOG=\""builtin"\" -DCMD_STANDALONE=b_pty -c pty.c
done pty.o
bind -lutil dontcare
exec - ${CC} ${CCLDFLAGS} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS} ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -o pty pty.o ${mam_libutil} ${mam_libast} -lm ${mam_libcmd}
exec - ${CC} ${CCLDFLAGS} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS} ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -o pty pty.o ${mam_libutil} ${mam_libast} ${mam_libcmd}
done pty
make ${INSTALLROOT}/bin
exec - mkdir -p ${INSTALLROOT}/bin
Expand Down
10 changes: 3 additions & 7 deletions src/cmd/builtin/features/pty
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ lib openpty,_getpty,ptsname -lutil
lib grantpt,unlockpt,posix_openpt stdlib.h
lib cfmakeraw termios.h

# try once with -lm, once without
tst - -lm - - output{
tst output{
#include <fcntl.h>
#if _lib_ptsname
#include <stdlib.h>
Expand Down Expand Up @@ -51,11 +50,8 @@ tst - -lm - - output{
return 0;
}
}end fail{
case ${_features_pty_TRIEDONCE-no} in
no) _features_pty_TRIEDONCE=yes ;;
*) echo "$0: Output block failed to compile. Export IFFEFLAGS=-d1 to debug." >&2
exit 1 ;;
esac
echo "$0: Output block failed to compile. Export IFFEFLAGS=-d1 to debug." >&2
exit 1
}end

extern _getpty char* (int*, int, mode_t, int)
Expand Down
29 changes: 14 additions & 15 deletions src/cmd/ksh93/Mamfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,16 @@ make install virtual
bind -ldll
bind -lcmd
bind -last
bind -lm dontcare
bind -lnsl dontcare
make shell.req
prev ${INSTALLROOT}/bin/mkreq
exec - mkreq ${CC} ${mam_cc_FLAGS} ${CCFLAGS} : ${LDFLAGS} : shell dll cmd ast m jobs i socket nsl secdb network
exec - mkreq ${CC} ${mam_cc_FLAGS} ${CCFLAGS} : ${LDFLAGS} : shell dll cmd ast jobs i socket nsl secdb network
done shell.req
make shopt.h notrace implicit
make FEATURE/options implicit
prev features/options
prev shell.req
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/options
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/options
done FEATURE/options
prev SHOPT.sh
prev Mamfile
Expand Down Expand Up @@ -108,13 +107,13 @@ make install virtual
make sh/pmain.c
make FEATURE/externs implicit
prev features/externs
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/externs
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/externs
done FEATURE/externs
make include/shell.h implicit
make include/fault.h implicit
make FEATURE/sigfeatures implicit
prev features/sigfeatures
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/sigfeatures
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/sigfeatures
done FEATURE/sigfeatures
make ${PACKAGE_ast_INCLUDE}/sfio.h implicit
prev ${PACKAGE_ast_INCLUDE}/sfio_s.h implicit
Expand Down Expand Up @@ -247,7 +246,7 @@ make install virtual
prev include/shtable.h
make FEATURE/dynamic implicit
prev features/dynamic
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/dynamic
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/dynamic
prev ${PACKAGE_ast_INCLUDE}/dlldefs.h
done FEATURE/dynamic
prev ${PACKAGE_ast_INCLUDE}/option.h
Expand Down Expand Up @@ -307,17 +306,17 @@ make install virtual
make include/terminal.h implicit
make FEATURE/ttys implicit
prev features/ttys
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/ttys
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/ttys
done FEATURE/ttys
done include/terminal.h
prev ${PACKAGE_ast_INCLUDE}/sig.h
make FEATURE/locale implicit
prev features/locale
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/locale
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/locale
done FEATURE/locale
make FEATURE/cmds implicit
prev features/cmds
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/cmds
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/cmds
done FEATURE/cmds
done include/edit.h
prev include/builtins.h
Expand All @@ -340,7 +339,7 @@ make install virtual
prev ${PACKAGE_ast_INCLUDE}/times.h
make FEATURE/time implicit
prev features/time
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/time
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/time
make ${PACKAGE_ast_INCLUDE}/times.h implicit dontcare
prev ${PACKAGE_ast_INCLUDE}/ast_time.h implicit dontcare
prev ${PACKAGE_ast_INCLUDE}/ast.h
Expand Down Expand Up @@ -436,7 +435,7 @@ make install virtual
make bltins/sleep.c
make FEATURE/poll implicit
prev features/poll
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/poll
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/poll
done FEATURE/poll
prev FEATURE/time
prev include/builtins.h
Expand Down Expand Up @@ -492,7 +491,7 @@ make install virtual
make include/ulimit.h implicit
make FEATURE/rlimits implicit
prev features/rlimits
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} : run features/rlimits
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/rlimits
done FEATURE/rlimits
prev FEATURE/time
done include/ulimit.h
Expand Down Expand Up @@ -1031,7 +1030,7 @@ make install virtual
make features/math.sh
prev data/math.tab implicit
done features/math.sh
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libm} ${mam_libnsl} -lm : run features/math.sh ${PACKAGEROOT}/src/cmd/ksh93/data/math.tab
exec - iffe ${IFFEFLAGS} -v -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" ref ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -I${PACKAGE_ast_INCLUDE} -I${INSTALLROOT}/include ${mam_libdll} ${mam_libcmd} ${mam_libast} ${mam_libnsl} : run features/math.sh ${PACKAGEROOT}/src/cmd/ksh93/data/math.tab
prev ${PACKAGE_ast_INCLUDE}/ast_standards.h implicit
make ${INSTALLROOT}/src/lib/libast/FEATURE/float implicit
prev ${INSTALLROOT}/src/lib/libast/FEATURE/standards implicit
Expand Down Expand Up @@ -1163,7 +1162,7 @@ make install virtual
exec - (ranlib libshell.a) >/dev/null 2>&1 || true
done libshell.a
bind -lshell
exec - ${CC} ${CCLDFLAGS} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS} ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -o ksh pmain.o ${mam_libshell} ${mam_libnsl} ${mam_libast} -lm
exec - ${CC} ${CCLDFLAGS} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS} ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -o ksh pmain.o ${mam_libshell} ${mam_libnsl} ${mam_libast}
done ksh
make shcomp
make shcomp.o
Expand All @@ -1179,7 +1178,7 @@ make install virtual
exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} -I. -Iinclude -I${PACKAGE_ast_INCLUDE} -DSH_DICT=${SH_DICT} -D_API_ast=20100309 -DERROR_CONTEXT_T=Error_context_t -c sh/shcomp.c
done shcomp.o
prev ksh
exec - ${CC} ${CCLDFLAGS} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS} ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -o shcomp shcomp.o ${mam_libshell} ${mam_libnsl} ${mam_libast} -lm
exec - ${CC} ${CCLDFLAGS} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS} ${mam_cc_L+-L.} ${mam_cc_L+-L${INSTALLROOT}/lib} -o shcomp shcomp.o ${mam_libshell} ${mam_libnsl} ${mam_libast}
done shcomp
make shell virtual
prev libshell.a
Expand Down
9 changes: 7 additions & 2 deletions src/lib/libast/Mamfile
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ make install virtual
note * generate initial ast.req
note *
prev ${INSTALLROOT}/bin/mkreq
exec - set -o errexit
exec - mkreq ${CC} ${mam_cc_FLAGS} ${CCFLAGS} : ${LDFLAGS} : ast iconv w
note *
note * some systems move -lc routines to -lm; see astmath.c for details
Expand All @@ -281,21 +282,24 @@ make install virtual
prev std/endian.h
done port/astmath.c
exec - X=1
exec - for N in 1 2 3 4 5 6 8
exec - for N in 1 2 3 4 5 6 7 8
exec - do if ${CC} ${CCLDFLAGS} -DN=$N -DIS ${mam_cc_FLAGS} ${CCFLAGS} -I. -Istd ${LDFLAGS} -o astmath.exe port/astmath.c 2>/dev/null
exec - then : implicit math function N=$N :
exec - elif ${CC} ${CCLDFLAGS} -DN=$N -DIS ${mam_cc_FLAGS} ${CCFLAGS} -I. -Istd ${LDFLAGS} -o astmath.exe port/astmath.c -lm 2>/dev/null
exec - then : math function N=$N requires -lm :
exec - X=0
exec - break
exec - else : ERROR: astmath.c fails to compile or link, even with -lm :
exec - exit 1
exec - fi
exec - done
exec - ${STDRM} -rf astmath.exe*
exec - echo $X > astmath.out
done astmath.out
prev FEATURE/aso
exec - sed -e '/^#define _REQ_/!d' -e 's/#define _REQ_\([a-z0-9_]*\).*/ -l\1/' FEATURE/aso >> ast.req
exec - case $(${STDCAT} astmath.out) in
exec - read no_libm_needed <astmath.out
exec - case $no_libm_needed in
exec - 0) echo ' -lm' >> ast.req ;;
exec - *) touch ast.req ;;
exec - esac
Expand Down Expand Up @@ -4989,6 +4993,7 @@ make install virtual
exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} -I${INSTALLROOT}/include/ast -D_PACKAGE_ast -c ${PACKAGEROOT}/src/cmd/INIT/mamake.c
done mamake.o
bind -last
exec - set -o errexit
exec - ${CC} ${CCLDFLAGS} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS} ${mam_cc_L+-L${INSTALLROOT}/lib} -o mamake mamake.o ${mam_libast}
note *
note * We purposely do not have a make target of ${INSTALLROOT}/bin/mamake here;
Expand Down
20 changes: 13 additions & 7 deletions src/lib/libast/port/astmath.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* *
* This software is part of the ast package *
* Copyright (c) 1985-2011 AT&T Intellectual Property *
* Copyright (c) 2020-2023 Contributors to ksh 93u+m *
* Copyright (c) 2020-2024 Contributors to ksh 93u+m *
* and is licensed under the *
* Eclipse Public License, Version 2.0 *
* *
Expand All @@ -20,27 +20,33 @@
/*
* used to test if -last requires -lm
*
* arch -last -lm
* ---- ----- ---
* linux.sparc sfdlen,sfputd frexp,ldexp
* This program is compiled and linked from a probe script in libast/Mamfile
* but never actually run. It is only used to check if linking succeeds
* without or with -lm.
*
* For that test to work correctly, we must work around compiler optimization.
* The rand() calls are to stop the result from being considered known at
* compile time, which would cause modern compilers to optimize out the probe
* calls, which would in turn cause linking to succeed where it shouldn't.
*/

#if N >= 8
#define _ISOC99_SOURCE 1
#endif

#include <stdlib.h>
#include <math.h>

int
main(void)
{
#if N & 1
long double value = 0;
long double value = rand();
#else
double value = 0;
double value = rand();
#endif
#if N < 5
int exp = 0;
int exp = rand();
#endif

#if N == 1
Expand Down

0 comments on commit 67a581d

Please sign in to comment.