Created attachment 240032 [details] Patch sysutils/jdupes to 1.21.3 Updates sysutils/jdupes to 1.21.3. Stripped builds with the non-default LOW_MEMORY option produce an executable which segfaults in FreeBSD's runtime linker. I suspect this to be a FreeBSD bug, but would appreciate more eyes on that.
^Triage: If there is a changelog or release notes URL available for this version, please add it to the URL field. ^Triage: Please set the maintainer-approval attachment flag (to +) on patches for ports you maintain to signify approval. -- Attachment -> Details -> maintainer-approval [+] Thanks!
Holding back on approval until the LOW_MEMORY crash has been investigated further. It didn't occur in 1.21.1.
Does this also occur if you use GCC?
(In reply to Daniel Engberg from comment #3) Yup. Same error with USE_GCC: Program received signal SIGSEGV, Segmentation fault. Invalid permissions for mapped object. 0x000000080060ec90 in init_rtld (mapbase=0x800607000 "\177ELF\002\001\001\t", aux_info=0x7fffffffe040) at /usr/src/libexec/rtld-elf/rtld.c:2406 2406 memset(&objtmp, 0, sizeof(objtmp));
(In reply to Thomas Hurst from comment #4) Can you provide an example of failure? I can't reproduce it: $ jdupes -R /usr/local/poudriere/jails/12_4amd64/bin/ /usr/local/poudriere/jails/12_4i386/bin/ Scanning: 88 files, 2 items (in 2 specified) /usr/local/poudriere/jails/12_4i386/bin/rm /usr/local/poudriere/jails/12_4i386/bin/unlink /usr/local/poudriere/jails/12_4i386/bin/ed /usr/local/poudriere/jails/12_4i386/bin/red /usr/local/poudriere/jails/12_4i386/bin/link /usr/local/poudriere/jails/12_4i386/bin/ln /usr/local/poudriere/jails/12_4i386/bin/test /usr/local/poudriere/jails/12_4i386/bin/[ /usr/local/poudriere/jails/12_4i386/bin/pgrep /usr/local/poudriere/jails/12_4i386/bin/pkill /usr/local/poudriere/jails/12_4i386/bin/csh /usr/local/poudriere/jails/12_4i386/bin/tcsh /usr/local/poudriere/jails/12_4amd64/bin/pgrep /usr/local/poudriere/jails/12_4amd64/bin/pkill /usr/local/poudriere/jails/12_4amd64/bin/ed /usr/local/poudriere/jails/12_4amd64/bin/red /usr/local/poudriere/jails/12_4amd64/bin/csh /usr/local/poudriere/jails/12_4amd64/bin/tcsh /usr/local/poudriere/jails/12_4amd64/bin/rm /usr/local/poudriere/jails/12_4amd64/bin/unlink /usr/local/poudriere/jails/12_4amd64/bin/test /usr/local/poudriere/jails/12_4amd64/bin/[ /usr/local/poudriere/jails/12_4amd64/bin/freebsd-version /usr/local/poudriere/jails/12_4i386/bin/freebsd-version /usr/local/poudriere/jails/12_4amd64/bin/link /usr/local/poudriere/jails/12_4amd64/bin/ln $ pkg info jdupes | grep LOW LOW_MEMORY : on I tried with bigger directories and it didn't crash either. This is in 13.1 amd64.
(In reply to Fernando Apesteguía from comment #5) No special actions should be necessary. Just run it with no arguments, if it doesn't crash in rtld you're failing to reproduce the problem. ❯ poudriere testport -I -j RELENG_13_1_amd64 sysutils/jdupes ... build started at Tue Feb 14 13:17:18 UTC 2023 port directory: /usr/ports/sysutils/jdupes package name: jdupes-1.21.3 building for: FreeBSD voi.aagh.net 13.1-RELEASE-p6 FreeBSD 13.1-RELEASE-p6 amd64 maintained by: tom@hur.st Makefile ident: Poudriere version: 3.3.7_1 Host OSVERSION: 1301000 Jail OSVERSION: 1301000 ... ---Begin OPTIONS List--- ===> The following configuration options are available for jdupes-1.21.3: LOW_MEMORY=on: Build for lower memory usage instead of speed ===> Use 'make config' to modify these settings ---End OPTIONS List--- ... ====> Running Q/A tests (stage-qa) /wrkdirs/usr/ports/sysutils/jdupes/work/stage/usr/local/bin/jdupes: signal 11 ====> Checking for pkg-plist issues (check-plist) ===> Parsing plist ===> Checking for items in STAGEDIR missing from pkg-plist Error: Orphaned: /jdupes.core This crash doesn't occur if I use GNU binutils strip.
(In reply to Thomas Hurst from comment #6) This is weird. Do you have any specific options in make.conf?
(In reply to Fernando Apesteguía from comment #7) It's certainly weird if you can't reproduce it! To eliminate as much as possible, I knocked up this script and ran it on a FreeBSD 13.1-RELEASE/amd64 VPS which uses stock FreeBSD binary upgrades and packages instead of my own local builds: #!/bin/sh set -e test -e jdupes || git clone --depth=1 https://github.com/jbruchon/jdupes.git cd jdupes git checkout v1.21.3 gmake clean all LOW_MEMORY=1 strip ./jdupes lldb ./jdupes # fin With this I observe the same result (lldb) target create "./jdupes" Current executable set to '/home/freaky/jdupes/jdupes' (x86_64). (lldb) run Process 14470 launched: '/home/freaky/jdupes/jdupes' (x86_64) Process 14470 stopped * thread #1, name = 'jdupes', stop reason = signal SIGSEGV: address access protected (fault address: 0x7fffffffc948) frame #0: 0x0000000800211c90 ld-elf.so.1 -> 0x800211c90: callq 0x800224af0 ; ___lldb_unnamed_symbol71 0x800211c95: movq $0x0, -0x730(%rbp) 0x800211ca0: orb $0x2, -0x514(%rbp) 0x800211ca7: movq %r15, -0x710(%rbp)
(In reply to Thomas Hurst from comment #8) That script without bonkers whitespace. Also available here: https://gist.github.com/Freaky/1929f3d832317f1d2c31ab6f6de12a0c #!/bin/sh set -e test -e jdupes || git clone --depth=1 https://github.com/jbruchon/jdupes.git cd jdupes git checkout v1.21.3 gmake clean all LOW_MEMORY=1 strip ./jdupes lldb ./jdupes
(In reply to Thomas Hurst from comment #9) I can reproduce it with your script. However, having a look at the compilation output log from both poudriere and in host, the outputs are far from being identical. For instance: From your script (ie, 13.1 amd64 host): cc -Wall -Wwrite-strings -Wcast-align -Wstrict-aliasing -Wstrict-prototypes -Wpointer-arith -Wundef -Wshadow -Wfloat-equal -Waggregate-return -Wcast-qual -Wswitch-default -Wswitch-enum -Wconversion -Wunreachable-code -Wformat=2 -std=gnu99 -O2 -g -D_FILE_OFFSET_BITS=64 -fstrict-aliasing -pipe -DSMA_MAX_FREE=11 -DNO_ATIME -DNDEBUG -Wl,-z,stack-size=16777216 -c -o jdupes.o jdupes.c From poudriere jail for 13.1 amd64: cc -O2 -pipe -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -Wall -Wwrite-strings -Wcast-align -Wstrict-aliasing -Wstrict-prototypes -Wpointer-arith -Wundef -Wshadow -Wfloat-equal -Waggregate-return -Wcast-qual -Wswitch-default -Wswitch-enum -Wconversion -Wunreachable-code -Wformat=2 -std=gnu99 -D_FILE_OFFSET_BITS=64 -fstrict-aliasing -pipe -DSMA_MAX_FREE=11 -DNO_ATIME -Wextra -Wstrict-overflow=5 -Winit-self -DNDEBUG -Wl,-z,stack-size=16777216 -isystem /usr/local/include -c -o jdupes.o jdupes.c That's why I asked before if you might have any specific options in make.conf or something. So the output from poudriere has all the flags it has in the hosted compilation except for "-g". In addition, the ports framework adds the following: -Wextra -Winit-self -Wstrict-overflow=5 -fno-strict-aliasing -fstack-protector-strong -isystem One question would be, why would you want to strip an executable that has been compiled with debug info (-g). At least in the porter's handbook we say "When WITH_DEBUG is defined, elf files must not be stripped." Do you have WITH_DEBUG in make.conf? As a workaround, I think we could add a build dependency on devel/binutils and then overwrite ${STRIP_CMD}.
(In reply to Fernando Apesteguía from comment #10) Yes, building with ports and directly is going to have different flags, and both trigger the problem. My flags within poudriere are identical to yours, aside from yours not having the low-memory settings? cc -O2 -pipe -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -Wall -Wwrite-strings -Wcast-align -Wstrict-aliasing -Wstrict-prototypes -Wpointer-arith -Wundef -Wshadow -Wfloat-equal -Waggregate-return -Wcast-qual -Wswitch-default -Wswitch-enum -Wconversion -Wunreachable-code -Wformat=2 -std=gnu99 -D_FILE_OFFSET_BITS=64 -fstrict-aliasing -pipe -DSMA_MAX_FREE=11 -DNO_ATIME -Wextra -Wstrict-overflow=5 -Winit-self -DNDEBUG -Wl,-z,stack-size=16777216 -DLOW_MEMORY -DSMA_PAGE_SIZE=32768 -DNO_HARDLINKS -DNO_SYMLINKS -DNO_USER_ORDER -DNO_PERMS -DNO_ATIME -DNO_JSON -DNO_EXTFILTER -DNO_CHUNKSIZE -DUSE_JODY_HASH -DNO_NUMSORT -DCHUNK_SIZE=16384 -isystem /usr/local/include -c -o jdupes.o jdupes.c If yours is somehow not built with LOW_MEMORY despite the config option being set, that would explain why it isn't crashing for you, but raises further questions! > One question would be, why would you want to strip an executable that has been compiled with debug info (-g). At least in the porter's handbook we say "When WITH_DEBUG is defined, elf files must not be stripped." -g just happens to be what's specified in the upstream source. I patch it out in the port so it's under the control of the framework. > Do you have WITH_DEBUG in make.conf? No, I just have a few targeted WITH_DEBUG_PORTS. The port builds fine if I add it. > As a workaround, I think we could add a build dependency on devel/binutils and then overwrite ${STRIP_CMD}. Either that, or just clobber STRIP_CMD and let it be installed unstripped. It seems kind of counterproductive - particularly to anyone building their own ports - to build and install the whole of binutils for the sake of reducing the installed LOW_MEMORY executable from 48k to 33k. Either way, I think it's time to open a new issue against base.
Does it also occur if you use llvm-strip?
(In reply to Daniel Engberg from comment #12) Nope, llvm-strip also works fine.
(In reply to Thomas Hurst from comment #13) So, can we use llvm-strip and update this port?
(In reply to Fernando Apesteguía from comment #14) Apparently not? Setting STRIP_CMD doesn't appear to do anything. Looking through make debug output suggests it's always setting install(1)'s STRIPBIN to 'strip'. This seems to be doubly-wrong, since that isn't even the stock fully-qualified STRIP_CMD of '/usr/bin/strip'. It works if I modify the do-install like so: STRIPBIN=${STRIP_CMD} ${INSTALL_PROGRAM} ${WRKSRC}/${PORTNAME} ${STAGEDIR}${PREFIX}/bin Does this look like a ports system bug, and is this an acceptable workaround in the meantime?
Created attachment 240295 [details] Patch for jdupes INSTALL_BIN is coded to use ${STRIP} https://cgit.freebsd.org/ports/tree/Mk/bsd.port.mk#n2138 Here's a variant that works around the limitation.
(In reply to Daniel Engberg from comment #16) ${STRIP} is just '-s' or the empty string, depending on if the port is being built with debug symbols or not - the path for the executable it uses for stripping comes from ${STRIPBIN}, which I now see is set by /usr/share/mk/sys.mk: https://cgit.freebsd.org/src/tree/share/mk/sys.mk#n276 It seems to me the ports system should override this. Your patch looks reasonable, thanks. Though should the path to llvm-strip not be qualified to be in /usr/bin?
We should probably use a full path as you pointed out. I can fix that when committing. Anything else?
Comment on attachment 240295 [details] Patch for jdupes Sounds good. Thanks!
(In reply to Thomas Hurst from comment #19) That was supposed to set maintainer-approval, but Bugzilla silently ignored it - I think it's supposed to have my email address set so I can change it? You can pretend I set it if that's easier.
Hi! I'm the creator of jdupes. LOW_MEMORY exists to build a special binary for extremely RAM-limited (read: embedded, likely under 32 MiB of RAM) systems without removing core functionality. I don't know why you're building with LOW_MEMORY but this variant of the program is NOT intended for general distribution to end users as a default jdupes binary. Notice all of the things that get removed, reduced, or turned off: -DSMA_PAGE_SIZE=32768 -DNO_HARDLINKS -DNO_SYMLINKS -DNO_USER_ORDER -DNO_PERMS -DNO_ATIME -DNO_JSON -DNO_EXTFILTER -DNO_CHUNKSIZE -DUSE_JODY_HASH -DNO_NUMSORT -DCHUNK_SIZE=16384 The options -CHjlLOpsX are made unavailable, name sorting behavior becomes "dumber," and smaller memory blocks are allocated to sacrifice performance in favor of reduced total memory usage. The removed options reduce the size of several structs but several workflows exist that expect things like hard linking, JSON output, parameter order preference, or the myriad of -X extfilter options. Also: if you ever have a problem that I can fix upstream, please don't hesitate to file an issue or pull request on Github. If I can make a small change that reduces the overhead of maintaining the program for your distribution packaging then I'm happy to do so.
(In reply to Jody Bruchon from comment #21) LOW_MEMORY is off by default, so nobody's getting a package built with it set unless they explicitly tweaked the knob and built their own. To be fair I'm probably not being explicit enough about how much it strips out in the option description - it just sounds like it makes it slower. I'll update Daniel's patch to fix that. Regarding changes you could make upstream, we do apply a small Makefile patch: https://cgit.freebsd.org/ports/tree/sysutils/jdupes/files/patch-Makefile COMPILER_OPTIONS is stripped of '-O2 -g' so the values are taken from the port system's CFLAGS. Perhaps the Makefile could specify this like 'CFLAGS ?= -O2 -g'. The gcc version check was removed as we don't build with gcc (or indeed have a 'gcc' binary) and these flags are supported by clang. Perhaps this can be made conditional on CC=gcc - or it might be considered old enough at this point to remove outright? xxhash.o is removed from the list of objects to link, as we link to libxxhash from devel/xxhash instead. You rejected unbundling this in #217, and I wouldn't argue the point - you're distributing for many more platforms!
Created attachment 240465 [details] Update to 1.21.3, with strip workaround and updated LOW_MEMORY description I made the STRIP_CMD change conditional on WITH_DEBUG, and fully-qualified the path to llvm-strip.
(In reply to Thomas Hurst from comment #22) I think I've managed to include all of your patch considerations into the jdupes Makefile. Please review these commits (all dated 2023-03-01) and let me know if you have any additional suggestions. 523a0e8 Makefile: add EXTERNAL_HASH_LIB for linking a hash shared library a1624a1 Makefile: move -O2 -g to CFLAGS; remove cruft f45e036 Makefile: only check for an old GCC version on Darwin (Mac)
(In reply to Jody Bruchon from comment #24) Thanks for taking the time to do that, I appreciate it! I think you have the conditional reversed for f45e036, it only checks gcc version if UNAME_S is *not* Darwin - I get an 'expr' error during build unless I change ifneq to ifeq here: ifneq ($(UNAME_S), Darwin) GCCVERSION = $(shell expr `LC_ALL=C gcc -v 2>&1 | grep 'gcc version ' | cut -d\ -f3 | cut -d. -f1` \>= 5) else GCCVERSION = 1 endif
(In reply to Thomas Hurst from comment #25) You're right; I had inverted that to run a test and forgot to flip it back when I removed the test overrides. I just pushed a fix. Thanks for catching that.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=11db49e26787c408dcc391462e2ad98dc7f4519e commit 11db49e26787c408dcc391462e2ad98dc7f4519e Author: Thomas Hurst <tom@hur.st> AuthorDate: 2023-03-13 09:41:03 +0000 Commit: Daniel Engberg <diizzy@FreeBSD.org> CommitDate: 2023-03-13 10:02:55 +0000 sysutils/jdupes: Update to 1.21.3 Changelog: https://github.com/jbruchon/jdupes/releases/tag/v1.21.3 PR: 269443 sysutils/jdupes/Makefile | 17 ++++++++++++++--- sysutils/jdupes/distinfo | 6 +++--- sysutils/jdupes/files/patch-Makefile | 8 ++++---- 3 files changed, 21 insertions(+), 10 deletions(-)
Committed, thanks!