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/builtin/features/pty:
- Remove -lm workarounds. (re: d79a34b, dd0d03b, d3cd4cf)

src/cmd/ksh93/Mamfile:
- Instead of passing `-lm` directly, properly use ${mam_libm} as
  this is set by 'bind -lm'. (re: 2b27f63)

Resolves: #715
  • Loading branch information
McDutchie committed Feb 1, 2024
1 parent 385289a commit 00c4d03
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 20 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
6 changes: 3 additions & 3 deletions src/cmd/ksh93/Mamfile
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,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_libm} ${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 +1163,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} ${mam_libm}
done ksh
make shcomp
make shcomp.o
Expand All @@ -1179,7 +1179,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} ${mam_libm}
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 00c4d03

Please sign in to comment.