lcn2
commented
1 year ago As mentioned in comment 1675767366, we plan to work on this issue next now that issue #89 has been resolved.
Closed TurtleWilly closed 1 year ago
lcn2
commented
1 year ago As mentioned in comment 1675767366, we plan to work on this issue next now that issue #89 has been resolved.
lcn2
commented
1 year ago We are also in the process of reworking the ALLOW_CUSTOM process (well fixing and improving it) as part of this bug fix.
While this remains the default in the Makefile:
ALLOW_CUSTOM= -DCUSTOMTo compile calc without support of CUSTOM (even when -C is used), one will have to use:
make ... ALLOW_CUSTOM=Or someone prefers:
make ... ALLOW_CUSTOM=-UCUSTOMOr someone may modify the Makefile to use:
ALLOW_CUSTOM= -UCUSTOMAnd yes, @TurtleWilly we are still working on making calc work if someone uses, under the current version of macOS:
LDFLAGS+= -Wl,-dead_strip_dylibs -Wl,-dead_stripWe say "current version of macOS" because we do not have access to the older Mac OS X 10.10.5 that say you are using and we don't have the details your calc build. Nevertheless we are fairly comfortable that the changes we are implementing will work in your environment as well: as the changes we are making should be generic to modern linkers in general.
lcn2
commented
1 year ago No patch, sorry. I do not understand the Makefile nor how to deal with it. :)
Fair point, @TurtleWilly.
As a side note:
We are considering dropping the support of Makefile.simple (see calc discussion 90. A nice side effect of doing that will be to somewhat simplify the calc Makefile.
Doing that will require the use of a make(1) that supports conditional statements. See Conditional Syntax as an example.
Removing this historical baggage will help a little to simplify the Makefile.
Calc does not use autoconf(1). Calc predates such systems. While we have looked at the autoconf system, we have found it to be more problematic that beneficial. So the historic use of the Makefile creating "have_foo.h" remains as our custom alternative to autoconf(1).
TurtleWilly
commented
1 year ago @lcn2
Looks like the team is hard at work. I'm looking forward to the updates. 👍
While a Makefile-only build system is certainly an outlier these days with all the "autoconf/ configure", "cmake", "meson", and other kind of hip build systems folks come up with on a whim, I personally like it. An "old-school" Makefile can do all that too, why bother adding extra layers? My personal projects are Makefile-only too, albeit way less complex than your Makefile which seems to have grown over the decades (which only has a minor downside of making it a bit less accessible for new folks to make quick changes. It simply takes a bit of extra time to read and understand the setup first. For you as maintainer this is no issue though: you would know where the things are handled.)
I occasionally work on some really old/retro systems (not updated since the early 90s basically) and using Conditional Syntax in Make is no problem there, so personally I wouldn't see a big problem of making a Make with Conditional Syntax support a mandatory requirement.
lcn2
commented
1 year ago My personal projects are Makefile-only too, albeit way less complex than your Makefile which seems to have grown over the decades (which only has a minor downside of making it a bit less accessible for new folks to make quick changes. It simply takes a bit of extra time to read and understand the setup first. For you as maintainer this is no issue though: you would know where the things are handled.)
Fair points.
Calc has a long history going back to at least 1984 and running on a wide variety of operating systems, compilers and hardware. That complicates things, but the some cases those older complications are no longer necessary and need to be removed or updated.
lcn2
commented
1 year ago Hello @TurtleWilly,
With commit 92c8d89ed1922093058a1cd97987dbb6ddca039b, under macOS, the linker is run with "-dead_strip" by default.
Regarding "-dead_strip_dylibs":
The macOS ld documentation states that option should not be used when linking against a dylib which is required at runtime for some indirect reason such as the dylib has an important initializer. The libcustcalc appears to have such an initializer.
We added an explicit reference in libcalc (custom() function in custom.c) to a symbol (custtbl_allowed in custom/custtbl.c) in libcustcalc, thereby forcing an explicit link between libcalc and libcustcalc. Unfortunately the use of "-dead_strip_dylibs" seems to eliminate the libcustcalc initializer and the causes calc to fail to load with the error:
dyld[79538]: symbol not found in flat namespace '_cust'The Makefile system for calc has undergone some major changes. See the CHANGES file for details.
See comment 1678117093.
lcn2
commented
1 year ago Hello @TurtleWilly,
We have not given up (yet :) ). We made an explicit reference from main() in calc.c (thus not part of any library) to custtbl_allowed (in custom/custtbl.c and thus in libcustcalc) in order to force the linker to see that the libcustcalc library is required. But now, calc fails with:
dyld[19204]: Symbol not found: _custtbl_allowed
Referenced from: <341612FA-C677-3D4F-BF6A-67A34C601906> /usr/local/src/bin/calc/calc
Expected in: <7FE24095-07FE-3FFA-BF8D-8363981E6238> /usr/local/lib/libcustcalc.2.14.3.1.dylibAnd yet:
$ otool -L ./libcalc.2.14.3.1.dylib ./libcustcalc.2.14.3.1.dylib ./calc
./libcalc.2.14.3.1.dylib:
/usr/local/lib/libcalc.2.14.3.1.dylib (compatibility version 0.0.0, current version 2.14.3)
/opt/homebrew/opt/readline/lib/libreadline.8.dylib (compatibility version 8.2.0, current version 8.2.0)
/opt/homebrew/opt/readline/lib/libhistory.8.dylib (compatibility version 8.2.0, current version 8.2.0)
/usr/lib/libncurses.5.4.dylib (compatibility version 5.4.0, current version 5.4.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.100.3)
./libcustcalc.2.14.3.1.dylib:
/usr/local/lib/libcustcalc.2.14.3.1.dylib (compatibility version 0.0.0, current version 2.14.3)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.100.3)
./calc:
/usr/local/lib/libcalc.2.14.3.1.dylib (compatibility version 0.0.0, current version 2.14.3)
/usr/local/lib/libcustcalc.2.14.3.1.dylib (compatibility version 0.0.0, current version 2.14.3)
/opt/homebrew/opt/readline/lib/libreadline.8.dylib (compatibility version 8.2.0, current version 8.2.0)
/opt/homebrew/opt/readline/lib/libhistory.8.dylib (compatibility version 8.2.0, current version 8.2.0)
/usr/lib/libncurses.5.4.dylib (compatibility version 5.4.0, current version 5.4.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.100.3)Even when we do NOT use "-dead_strip_dylibs" for macOS, only "-dead_strip".
The same problem occurs when neither "-dead_strip_dylibs" nor "-dead_strip" is used for macOS.
Hummm ...
lcn2
commented
1 year ago Yea 🎉‼️
With commit faf3a4fab6638453a3acf75643e32349e02015e1, the use of "-dead_strip_dylibs" will work for calc, however it is NOT used by default when CUSTOM is defined under macOS.
Under macOS, to reduce dependency chains, weDO remove functions and data that are unreachable by the entry point or exported symbols. In particular, the macOS linker is used with "-dead_strip" by default.
While calc on macOS will execute if linker used with "-dead_strip_dylibs"
and CUSTOM is defined, other applications that use libcalc but not
libcustcalc (such as sample_many and sample_rand) will fail to execute
due to missile symbols. Therefore "-dead_strip_dylibs" is not used
by default when ALLOW_CUSTOM is "-DCUSTOM" under macOS.
So by under macOS, when CUSTOM is defined, "-dead_strip_dylib" is not used by default. When CUSTOM is undefined under macOS, , "-dead_strip_dylib" is used by default. And either way, "-dead_strip" is the default under macOS.
In general, use of "-dead_strip_dylib" is highly problematic. It requires applications to make explicit references to symbols to shared libraries that they might not need, just to work around the "-dead_strip_dylib" causing the important initializer to fail.
We looked into adding "-no_dead_strip_inits_and_terms" for the libcustcalc. Unfortunately macOS has disabled this linker option and issues a warning that "-no_dead_strip_inits_and_terms" will be ignored. sigh!
The general use of "-dead_strip_dylib" in general can lead to problems, and so we would not use it unless we had an extremely good reason and the application had explicit symbol references to all shared libraries. And then again, if an application had explicit symbol references to all shared libraries, them "-dead_strip_dylib" becomes useless because there are no dynamic shared libraries that is can strip.
We do not want to force applications that link to the calc libraries to have to do
what calc needed to do (in main() of calc.c, referencing the custtbl_allowed
just so that "-dead_strip_dylib" won't cause calc to crash). So if someone
forces the use of "-dead_strip_dylib", the cannot complain if applications
crash due to symbols being not found due to "-dead_strip_dylib"
causing an important initializer to fail.
When we use the form of your command:
make \
… \
LDFLAGS='-Wl,-dead_strip_dylibs -Wl,-dead_strip' \
…We are able to compile calc, and calc will execute. However other applications such as sample_manyandsample_rand` will fail with:
dyld[2159]: symbol not found in flat namespace '_cust'The Makefile.target, under the Darwin (macOS) target has:
# Do not link with "-dead_strip_dylibs" if CUSTOM is defined
# (i.e., ${ALLOW_CUSTOM} is "-DCUSTOM). This is because unless
# the application makes an explicit symbol reference to some symbol
# in libcustcalc, the application will fail to execute due to
# a missing symbol.
#
# The calc application will run with CUSTOM defined under
# "-dead_strip_dylibs" because calc.o (in main()) makes an explicit
# reference to custtbl_allowed (see custom/custtbl.c) in libcustcalc.
# However applications such as sample_many and sample_rand will
# fail to load when CUSTOM is defined and "-dead_strip_dylibs" is used..
#
ifneq ($(ALLOW_CUSTOM),-DCUSTOM)
COMMON_LDFLAGS+= -Wl,-dead_strip_dylibs
endif # ($(ALLOW_CUSTOM),-DCUSTOM)So you can compile stuff without CUSTOM:
make \
... \
ALLOW_CUSTOM="-UCUSTOM"When you undefine CUSTOM, "-dead_strip_dylibs" will be used by default under macOS.
Or you can use your explicit LDFLAGS setting of "-dead_strip_dylibs", use calc and ignore the fact that other applications will crash due to missing symbols due to "-dead_strip_dylib" causing the important initializer to fail.
It is your choice! :-)
Thank you for the interesting bug fix. As part of the effort to better understand what is going on, we performed a major rework of the calc Makefile system. We even managed to hack the calc source to let you explicitly use "=-dead_strip_dylibs" when CUSTOM is defined.
We plan to release calc version 2.14.3.2 with this code, soon.
lcn2
commented
1 year ago TurtleWilly
commented
1 year ago Maybe there was a small misunderstanding? My bug report wasn't about introducing _dead_stripdylibs into calc's build system. This was merely my vehicle to demonstrate what I believe is the underlying problem in calc's linking setup. 😎
I think, the problem manifests itself in other ways too, as you have noticed. E.g. all programs that want to use libcalc.dylib will also have to link directly with libcustomcalc.dylib, or else they will crash. That shouldn't be a requirement. Programs should just need to link with libcalc.dylib, if they only call some libcalc.dylib functionality.
The key problem remains that libcalc.dylib doesn't link in libcustomcalc.dylib. See your otool -L output, libcustomcalc.dylib is still missing in the list of dependencies for libcalc.dylib. And that's the problem, I believe.
Basically there's now an extra ugly kludge in calc (to create a strong link between calc and libcustomcalc.dylib) to keep the accidental kludge (calc links with libcustom.dylib, even if not needed) in place when a user wants to strip off unneeded dylibs from the final binary. But it only keeps status quo for calc. It doesn't help any other programs that may want to use libcalc.dylib, e.g. your examples.
Lets try to reduce the problem to an absolute minimum for demonstration purposes. Assume we have following 3 source code files representing our 3 separate components (calc, libcalc, and libbcustom[calc]).
calc.c:
#include <stdio.h>
extern const char *calc_func(void); /* libcalc.h */
int main(void) {
printf("Test Value: %s\n", calc_func());
return 0;
}libcalc.c
extern const char *cust_func(void); /* libcustom.h */
const char *calc_func(void) {
return cust_func();
}libcustom.c
const char *cust_func(void) {
return "foobar";
}Now lets build it like calc's build system did things:
# 🔧Build libcustom…
$ gcc -c libcustom.c # creates libcustom.o
$ gcc -dynamiclib -current_version 1.0 libcustom.o -o libcustom.dylib
# 🔧Build libcalc…
$ gcc -c libcalc.c # creates libcalc.o
$ gcc -dynamiclib -current_version 1.0 libcalc.o -o libcalc.dylib
Undefined symbols for architecture x86_64:
"_cust_func", referenced from:
_calc_func in libcalc.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
# Hmm… an interesting and very telling error. But lets do it like calc's Makefile and tell
# the linker to lookup any undefined symbols dynamically (aka way later, not during build).
# Basically this hides our problem.
$ gcc -undefined dynamic_lookup -dynamiclib -current_version 1.0 libcalc.o -o libcalc.dylib
# 🔧Build calc…
$ gcc calc.c -o calc -L. -lcalc -lcustom
# 🏃🏿Run calc…
$ ./calc
Test Value: foobar
# Works! Lucky! 👍
# 💡Dump some information…
$ otool -L libcustom.dylib
libcustom.dylib:
libcustom.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
# Note the absence of libcustom.dylib in the list of required libraries:
$ otool -L libcalc.dylib
libcalc.dylib:
libcalc.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
$ otool -L calc
calc:
libcalc.dylib (compatibility version 0.0.0, current version 1.0.0)
libcustom.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
Now lets do it again, but introduce -Wl,-dead_strip_dylibs like TurtleWilly's default build flags/system does it (we add it everywhere, because it would come via default LDFLAGS, but technically in our example it only matters in the final step when building the "calc" binary.)
# 🔧Build libcustom…
$ gcc -c libcustom.c # creates libcustom.o
$ gcc -Wl,-dead_strip_dylibs -dynamiclib -current_version 1.0 libcustom.o -o libcustom.dylib
# 🔧Build libcalc…
$ gcc -c libcalc.c # creates libcalc.o
$ gcc -Wl,-dead_strip_dylibs -undefined dynamic_lookup -dynamiclib -current_version 1.0 libcalc.o -o libcalc.dylib
# 🔧Build calc…
$ gcc -Wl,-dead_strip_dylibs calc.c -o calc -L. -lcalc -lcustom
# 🏃🏿Run calc…
$ ./calc
dyld: lazy symbol binding failed: Symbol not found: _cust_func
Referenced from: libcalc.dylib
Expected in: flat namespace
dyld: Symbol not found: _cust_func
Referenced from: libcalc.dylib
Expected in: flat namespace
Trace/BPT trap: 5
# CRASH! 👎
# 💡Dump some information…
$ otool -L libcustom.dylib
libcustom.dylib:
libcustom.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
# Note the absence of libcustom.dylib in the list of required libraries:
$ otool -L libcalc.dylib
libcalc.dylib:
libcalc.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
# Note that libcustom is missing here now too (stripped away via dead_strip_dylibs, because
# the calc binary itself doesn't use any symbols from libcustom)
$ otool -L calc
calc:
libcalc.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
Now lets try to link libcalc with libcustom and see if it would fix our problem:
# 🔧Build libcustom…
$ gcc -c libcustom.c # creates libcustom.o
$ gcc -Wl,-dead_strip_dylibs -dynamiclib -current_version 1.0 libcustom.o -o libcustom.dylib
# 🔧Build libcalc…
$ gcc -c libcalc.c # creates libcalc.o
# Note: This is the fix, link in libcustom here:
$ gcc -Wl,-dead_strip_dylibs -undefined dynamic_lookup -dynamiclib \
-current_version 1.0 libcalc.o -o libcalc.dylib -L. -lcustom
# 🔧Build calc…
$ gcc -Wl,-dead_strip_dylibs calc.c -o calc -L. -lcalc -lcustom
# 🏃🏿Run calc…
$ ./calc
Test Value: foobar
# Works again! Whoohoo! 👍 💯
# 💡Dump some information…
$ otool -L libcustom.dylib
libcustom.dylib:
libcustom.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
# Note that libcustom.dylib is now in the list of required libraries:
$ otool -L libcalc.dylib
libcalc.dylib:
libcalc.dylib (compatibility version 0.0.0, current version 1.0.0)
libcustom.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
# calc just needs libcalc.dylib, so a missing libcustom.dylib (automatically stripped away)
# is no longer a problem anymore:
$ otool -L calc
calc:
libcalc.dylib (compatibility version 0.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
At least that's how I understand and would summarize the issue. I may have got some things wrong?
Note: I have not tested the latest versions of calc yet, just referencing your comments here.
lcn2
commented
1 year ago Hello @TurtleWilly,
We will look at your information, thank you.
lcn2
commented
1 year ago Hello @TurtleWilly,
Fixed in commit 28449fd187fa5a44f73cdb7a05518db810e55b66.
Under macOS, to reduce dependency chains, we remove functions and data that are unreachable by the entry point or exported symbols. In particular, the macOS linker is used with both "-dead_strip" and "-dead_strip_dylibs".
The libcalc shared library is now linked with libcustcalc.
lcn2
commented
1 year ago p.s. @TurtleWilly,
Thanks for your comment 1680974295 and taking the time to illustrate.
TurtleWilly
commented
1 year ago I tested the latest ".5" version and libcalc.dylib now properly has libcustcalc.dylib linked in. Of course, calc now uses a symbol of libcustomcalc too, so has it linked too and it's not getting stripped away. It's now double protected. 👍
No worries, I learned something too and looking into the random crash reason like this was very educating and helped me to understand some of those things better too. No wasted time. 😀
While rebuilding and testing I noticed a few other minor things that potentially could be improved, but I'll make separate tickets for that over the weekend.
lcn2
commented
1 year ago p.s. @TurtleWilly,
Our of curiosity, why do you wish to remove weak links?
Calc bug report template version: 1.3 2022-11-27
Describe the bug
libcalc.dylibis not linking inlibcustomcalc.dylibat all. By defaultlibcustomcalc.dylibis only linked to thecalcbinary, even it seems code in libcalc actually depends on libcustomcalc, and not code in calc itself. So things only work accidentally/ by luck in calc's default build setup.Basically what happens is: when we build with an extra LDFLAGS='-Wl,-dead_strip_dylibs' (e.g. to reduce dependency chains) it will end up stripping libcustomcalc.dylib from the calc binary, because the calc binary has no direct symbol reference to that library:
Note the absence of libcustomcalc.dylib here. And once we run calc it crashes of course, because code in libcalc.dylib still wants to use libcustomcalc.dylib, but no longer can find it:
The problem is libcalc.dylib doesn't have libcustomcalc.dylib linked in:
To Reproduce Steps to reproduce the behavior:
Build calc with
then launch calc, e.g.
calc 1+2Expected behavior
libcalc.dylib should have libcustomcalc.dylib linked in, if it uses code from it, e.g. I would expect it to look somewhat like this when inspecting it with
otool:afterwards
calc 1+2should print out "3" w/o crashing.Attach debug.out IMPORTANT: Please run
make debugand then attach thedebug.outfile.Skipped. (It's a ~10,000 lines file. I don't have the time to scan/parse it for potential privacy leaks or issues before sharing it in the public. Too much, sorry.)
Screen shots n/a
Execution environment (please complete the following information):
OS: E.g., Mac OS X
OS version: Darwin 14.5.0 Darwin Kernel Version 14.5.0: Sun Jun 4 21:40:08 PDT 2017; root:xnu-2782.70.3~1/RELEASE_X86_64 x86_64
OS distribution: E.g., Mac OS X 10.10.5 Yosemite
Calc Version 2.14.3.0 (downloaded here from the GitGub releases, bz2 file)
Shell and shell version GNU bash, version 5.2.0(1)-rc1 (x86_64-apple-darwin14.5.0)
Calc mods If you have modified the calc source for some reason, please description what you modified. Please consider attaching a patch (diff -u) between an official calc release and the source you are using.
no modifications, besides passing required arguments for proper setup to the Makefile, here's the full setup for completeness:
Patch If you have a recommended code patch to address the problem, please attach your file
No patch, sorry. I do not understand the Makefile nor how to deal with it. :)
Additional context Add any other context about the problem here.
n/a