Bug 255589 - strip(1) leaves empty file when applied to unstrippable file
Summary: strip(1) leaves empty file when applied to unstrippable file
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
Depends on:
Reported: 2021-05-04 10:24 UTC by Adriaan de Groot
Modified: 2021-05-04 13:24 UTC (History)
3 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Adriaan de Groot freebsd_committer 2021-05-04 10:24:24 UTC
When strip(1) is applied to a file that it can't strip, it leaves behind an empty file alongside the thing it was stripping.

To reproduce:

    mkdir /tmp/example-for-strip
    echo "bogus" > /tmp/example-for-strip/mytextfile
    strip /tmp/example-for-strip/mytextfile

Output from strip is
    strip: file format not recognized

The directory /tmp/example-for-strip now contains two files: the mytextfile -- that's intended -- and a 0-byte ecp.<random> file, which is not intended.

This affects ports builds where sometimes strip is applied to scripts -- that's nominally a build-system problem for the port in question, but strip(1) shouldn't be leaving spare files around anyway.
Comment 1 Adriaan de Groot freebsd_committer 2021-05-04 10:50:38 UTC
I'm *fairly* sure this was introduced in upstream https://sourceforge.net/p/elftoolchain/code/3919/ and that it needs cleanup to remove the tempfile on the error-exit path with unrecognized input, something like this:

        switch (elf_kind(ecp->ein)) {
        case ELF_K_NONE:
                if (tempfile != NULL) {
                        if (unlink(tempfile) < 0)
                                err(EXIT_FAILURE, "unlink %s failed", tempfile);
                errx(EXIT_FAILURE, "file format not recognized");

Adding crees@ as upstream-involved.

In the meantime, ports are getting cruft like `${RM} ${STAGEDIR}${LOCALBASE}/bin/ecp.*` to clean up after strip(1).

Note that you don't need to use a full path for the strip command; soemthing like `strip *.png` will bail out on the first png file and leave a single `ecp.<random>` file in the current directory.
Comment 2 Adriaan de Groot freebsd_committer 2021-05-04 11:06:52 UTC
As an example of what happens in a ports build: as part of the install step, for whatever silly reason, a non-ELF file is stripped:

strip /wrkdirs/usr/ports/x11/lumina-core/work/stage/usr/local/share/lumina-desktop/menu-scripts/ls.json.sh
strip: file format not recognized

and now there's an orphan file

Error: Orphaned: share/lumina-desktop/menu-scripts/ecp.bN32GnFD
Comment 3 Chris Rees freebsd_committer 2021-05-04 11:39:26 UTC
Yes, I introduced this fix in 5ac70383, but somehow I didn't manage to get it into 13.0-RELEASE, which is actually a major pain.

It was always a problem, but didn't appear until the default directory for strip(1) temporary files was changed to '.' from /tmp in 1e4896b1.

Portmgr, I'm really sorry, but I think we have no alternative but to put an OSVERSION check and then find ${WRKSRC} -type f -name 'ecp.*' -delete on the do-strip target in bsd.port.mk or wherever, otherwise the tree is going to be totally littered with ad-hoc fixes that only apply to 13.0-R.

Alternatively, we could (ab)use the releng/13.0 branch...
Comment 4 Adriaan de Groot freebsd_committer 2021-05-04 12:16:00 UTC
Thanks Chris for the quick response. Yeah, I've got 5 .. maybe 8 ports commits lined up (thanks to git, just locally) that all are variations on

sysutils/luckybackup: workaround strip(1) leaving behind empty files

With my ports hat on, they're pretty imporant, because otherwise I can't tell the difference between "plist failure because of strip(1)" and "plist failure for some other legitimate reason"; I'll keep them local for the time being.
Comment 5 Antoine Brodin freebsd_committer 2021-05-04 12:43:32 UTC
why not an ERRATA NOTICE in base?
Comment 6 Mark Johnston freebsd_committer 2021-05-04 13:12:31 UTC
If anyone could mail secteam@ with some reference to the commit and ideally a filled out copy of the erratum notice template (https://www.freebsd.org/security/errata-template.txt), we can include it in the next batch.