Bug 198611 - Libraries build with libtool are no longer stripped with elftoolchain strip
Summary: Libraries build with libtool are no longer stripped with elftoolchain strip
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
URL:
Keywords:
Depends on: 200350
Blocks:
  Show dependency treegraph
 
Reported: 2015-03-15 15:50 UTC by Antoine Brodin
Modified: 2019-11-08 15:00 UTC (History)
2 users (show)

See Also:


Attachments
Possible patch (519 bytes, patch)
2015-03-15 15:50 UTC, Antoine Brodin
no flags Details | Diff
ELF tool chain update to r3185 (13.13 KB, patch)
2015-04-13 19:29 UTC, Ed Maste
no flags Details | Diff
update ELF Tool Chain to r3212 (from r3197) (28.34 KB, patch)
2015-05-18 16:36 UTC, Ed Maste
no flags Details | Diff
Update ELF Tool Chain to r3222 (from r3197) (40.09 KB, patch)
2015-05-25 18:31 UTC, Ed Maste
no flags Details | Diff
Update to r3222 (from r3197) with GRP_COMDAT backwards compatibility shim restored (39.52 KB, patch)
2015-05-25 19:40 UTC, Ed Maste
no flags Details | Diff
Update to r3223 (from r3197) with GRP_COMDAT backwards compatibility shim (39.53 KB, patch)
2015-05-25 20:42 UTC, Ed Maste
no flags Details | Diff
Possible patch (6.98 KB, patch)
2015-05-27 18:12 UTC, Antoine Brodin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Brodin freebsd_committer 2015-03-15 15:50:23 UTC
Created attachment 154393 [details]
Possible patch

From libtool.m4:

%%%
# _LT_CMD_STRIPLIB
# ----------------
m4_defun([_LT_CMD_STRIPLIB],
[m4_require([_LT_DECL_EGREP])
striplib=
old_striplib=
AC_MSG_CHECKING([whether stripping libraries is possible])
if test -n "$STRIP" && $STRIP -V 2>&1 | $GREP "GNU strip" >/dev/null; then
  test -z "$old_striplib" && old_striplib="$STRIP --strip-debug"
  test -z "$striplib" && striplib="$STRIP --strip-unneeded"
  AC_MSG_RESULT([yes])
else
# FIXME - insert some real tests, host_os isn't really good enough
  case $host_os in
...
  *)
    AC_MSG_RESULT([no])
    ;;
  esac
fi
%%%

With elftoolchain strip:

%%%
% strip -V
strip (elftoolchain r3163M)
%%%
Comment 1 Antoine Brodin freebsd_committer 2015-03-15 15:59:13 UTC
From some configure logs on head:

checking whether stripping libraries is possible... no
Comment 2 Antoine Brodin freebsd_committer 2015-03-15 17:03:58 UTC
Of course,  the proposed patch doesn't work with some versions of libtool

%%%
if test -n "$STRIP" && $STRIP -V 2>&1 | grep "GNU strip" >/dev/null; then
%%%
Comment 3 Ed Maste freebsd_committer 2015-03-15 22:56:57 UTC
Maybe we need something like a strip user agent?

% strip -V
strip (elftoolchain r3136M (like GNU strip))

The libtool commit that originally introduced this is:

commit 86180ef4d5291a193196251621088cb38534e6ca
Author: Thomas Tanner <tanner@ffii.org>
Date:   Tue Jun 29 17:37:06 1999 +0000

It doesn't explicitly explain why, but presumably strip(1) tools then in use did not support stripping libraries (at all, or with the same flags).

We should get the equivalent change submitted upstream as well.
Comment 4 Ed Maste freebsd_committer 2015-03-15 23:03:49 UTC
I'll try to submit a patch like this upstream:

index a3bc337..091adfa 100644
--- a/m4/libtool.m4
+++ b/m4/libtool.m4
@@ -2207,7 +2207,8 @@ m4_defun([_LT_CMD_STRIPLIB],
 striplib=
 old_striplib=
 AC_MSG_CHECKING([whether stripping libraries is possible])
-if test -n "$STRIP" && $STRIP -V 2>&1 | $GREP "GNU strip" >/dev/null; then
+if test -n "$STRIP" && $STRIP -V 2>&1 |
+  $EGREP "GNU strip|elftoolchain" >/dev/null; then
   test -z "$old_striplib" && old_striplib="$STRIP --strip-debug"
   test -z "$striplib" && striplib="$STRIP --strip-unneeded"
   AC_MSG_RESULT([yes])
Comment 7 Antoine Brodin freebsd_committer 2015-03-20 21:03:02 UTC
people.freebsd.org/~antoine/libntop.a
people.freebsd.org/~antoine/libturbojpeg.a
people.freebsd.org/~antoine/vorbis.a
people.freebsd.org/~antoine/libstdc++.a
people.freebsd.org/~antoine/libstdc++-unstripped.a
Comment 8 Ed Maste freebsd_committer 2015-04-09 14:45:19 UTC
It looks like GNU binutils does not handle archive-within-archive on FreeBSD either:

Binutils 2.17.50 (base system):

% strip --strip-debug -o /dev/null libntop.a
strip: Unable to recognise the format of the input file `libntop.a(libndpi.a)'

Binutils 2.25:

% /usr/local/bin/strip --strip-debug -o /dev/null libntop.a 
/usr/local/bin/strip: cannot create tempdir for archive copying (error: Operation not supported)
Comment 9 Ed Maste freebsd_committer 2015-04-13 19:29:41 UTC
Created attachment 155564 [details]
ELF tool chain update to r3185

This patch has a fix for https://sourceforge.net/p/elftoolchain/tickets/486/ which should address gnatdroid-armv7
Comment 10 Ed Maste freebsd_committer 2015-04-14 18:40:43 UTC
I have a change that should address the jpeg-turbo issue in review: https://reviews.freebsd.org/D2292

All five reported issues (on amd64) should be addressed:

deadbeef-0.6.2_3.log
ntop-5.0.1_8.log

Bug in these individual ports, they should not create an ar-within-ar. PRs 199319 and 199427 submitted for these.

jpeg-turbo-1.3.1_2.log
libjpeg-turbo-1.3.1_5.log

Should be addressed by review D2292

gnatdroid-armv7-20141023.log

Should be addressed by patch 155564
Comment 11 Antoine Brodin freebsd_committer 2015-04-18 06:48:19 UTC
Results of the new exp-run are available at:

http://package22.nyi.freebsd.org/jail.html?mastername=headamd64PR198611-default
http://package23.nyi.freebsd.org/jail.html?mastername=headi386PR198611-default

There were 3 new failures (2 are bugs in the software, ar within ar):

+ {"origin"=>"audio/deadbeef", "pkgname"=>"deadbeef-0.6.2_3", "phase"=>"stage", "errortype"=>"???"}
+ {"origin"=>"lang/gnatdroid-armv7", "pkgname"=>"gnatdroid-armv7-20141023", "phase"=>"build", "errortype"=>"???"}
+ {"origin"=>"net/ntop", "pkgname"=>"ntop-5.0.1_8", "phase"=>"stage", "errortype"=>"???"}

The gnatdroid-armv7 failure log:

http://package22.nyi.freebsd.org/data/headamd64PR198611-default/2015-04-18_05h36m14s/logs/errors/gnatdroid-armv7-20141023.log
http://package23.nyi.freebsd.org/data/headi386PR198611-default/2015-04-18_05h12m55s/logs/errors/gnatdroid-armv7-20141023.log
Comment 12 Ed Maste freebsd_committer 2015-05-06 01:47:42 UTC
In the upstream ELF Tool Chain ticket I have mentioned that the SHT_GROUP issue still remains. I'll try to do some more investigation locally to better understand the problem.
Comment 13 Antoine Brodin freebsd_committer 2015-05-06 09:28:40 UTC
Ok.
Can they (elftoolchain) or us (in the version we use) add a "like GNU strip" to the version string, to avoid having to patch hundreds of configure scripts?
Comment 14 Ed Maste freebsd_committer 2015-05-18 16:36:40 UTC
Created attachment 156881 [details]
update ELF Tool Chain to r3212 (from r3197)

Hopefully the SHT_GROUP issue is finally fixed with:
- Fix handling of SHT_GROUP section indices (upstream r3206)
Comment 15 Ed Maste freebsd_committer 2015-05-18 16:41:31 UTC
> Can they (elftoolchain) or us (in the version we use) add a "like GNU strip" to the version string, to avoid having to patch hundreds of configure scripts?

I want to track down the final outstanding failure first, and we can figure out how to go forward after that. We definitely want to get an upstream patch in as well.

I actually proposed the user-agent style "like <foo>" approach as a joke, and think it's quite horrible, but won't object if there's no better way.
Comment 16 Antoine Brodin freebsd_committer 2015-05-18 18:05:07 UTC
I have this compile error:

--- sections.o ---
/poudriere/jails/headi386PR198611/usr/src/usr.bin/elfcopy/../../contrib/elftoolchain/elfcopy/sections.c:619:13: error: use of u
ndeclared identifier 'GRP_COMDAT'
        if ((*ws & GRP_COMDAT) == 0)
                   ^
1 error generated.
*** [sections.o] Error code 1
Comment 17 Ed Maste freebsd_committer 2015-05-18 19:07:13 UTC
This should be a quick workaround:

commit 13d401b072b6c0afddef6719b6167aef1b77d7df
Author: Ed Maste <emaste@freebsd.org>
Date:   Mon May 18 13:53:41 2015 -0400

    Add compat shim for building with older FreeBSD ELF headers

diff --git a/contrib/elftoolchain/elfcopy/sections.c b/contrib/elftoolchain/elfcopy/sections.c
index 1268a9c..d35b425 100644
--- a/contrib/elftoolchain/elfcopy/sections.c
+++ b/contrib/elftoolchain/elfcopy/sections.c
@@ -616,6 +616,9 @@ update_section_group(struct elfcopy *ecp, struct section *s)
        ws = id->d_buf;

        /* We only support COMDAT section. */
+#ifndef GRP_COMDAT
+#define        GRP_COMDAT 0x1
+#endif
        if ((*ws & GRP_COMDAT) == 0)
                return;
Comment 18 Ed Maste freebsd_committer 2015-05-19 14:36:59 UTC
Definition for GRP_COMDAT added in r283110 so the workaround in comment 17 is not necessary with a newer src tree.
Comment 19 Antoine Brodin freebsd_committer 2015-05-20 13:14:00 UTC
Exp-run results:

http://package22.nyi.freebsd.org/jail.html?mastername=headamd64PR198611-default
http://package23.nyi.freebsd.org/jail.html?mastername=headi386PR198611-default

There are the 3 usual new failures (2 ar within ar + gnatdroid-armv7):

+ {"origin"=>"audio/deadbeef", "pkgname"=>"deadbeef-0.6.2_3", "phase"=>"stage", "errortype"=>"???"}
+ {"origin"=>"lang/gnatdroid-armv7", "pkgname"=>"gnatdroid-armv7-20141023", "phase"=>"build", "errortype"=>"configure_error"}
+ {"origin"=>"net/ntop", "pkgname"=>"ntop-5.0.1_8", "phase"=>"stage", "errortype"=>"???"}

For gnatdroid-armv7 the error seems a bit different this time:

http://package22.nyi.freebsd.org/data/headamd64PR198611-default/2015-05-20_07h01m54s/logs/errors/gnatdroid-armv7-20141023.log
http://package23.nyi.freebsd.org/data/headi386PR198611-default/2015-05-20_07h02m11s/logs/errors/gnatdroid-armv7-20141023.log
Comment 20 Ed Maste freebsd_committer 2015-05-25 18:31:29 UTC
Created attachment 157132 [details]
Update ELF Tool Chain to r3222 (from r3197)
Comment 21 Ed Maste freebsd_committer 2015-05-25 18:32:24 UTC
The gnatdroid-armv7 issue should (finally!) be fixed by upstream revision r3216, and kaiw tested building gnatdroid-armv7. ELF Tool Chain strip/elfcopy now also passes the GNU binutils testsuite for SHT_GROUP handling.
Comment 22 Ed Maste freebsd_committer 2015-05-25 19:40:02 UTC
Created attachment 157135 [details]
Update to r3222 (from r3197) with GRP_COMDAT backwards compatibility shim restored

ELF Tool Chain is used in the bootstrap build too, and would fail if the build host's elf.h does not have the GRP_COMDAT define.
Comment 23 Ed Maste freebsd_committer 2015-05-25 20:42:17 UTC
Created attachment 157136 [details]
Update to r3223 (from r3197) with GRP_COMDAT backwards compatibility shim

Fix 32-bit build in upstream rev 3223, discovered by antoine
Comment 24 Antoine Brodin freebsd_committer 2015-05-27 07:09:45 UTC
With this version,  the only new failures are the usual audio/deadbeef and net/ntop.
Comment 25 commit-hook freebsd_committer 2015-05-27 14:28:26 UTC
A commit references this bug:

Author: emaste
Date: Wed May 27 14:28:22 UTC 2015
New revision: 283616
URL: https://svnweb.freebsd.org/changeset/base/283616

Log:
  Update to ELF Tool Chain r3223

  Highlights (upstream revisions):
   - Fix SHT_GROUP handling in elfcopy/strip (3206 3220 3221)
   - Misc elfcopy / strip bug fixes (3215 3216 3217)
   - Many C++ demangler improvements (3199 3200 3201 3202 3203 3204 3205
     3208 3210 3211 3212)
   - Improve GNU binutils compatibility in elfcopy / strip (3213 3214)
   - Add -g option to readelf(1): dump contents of section groups (3219)
   - Add EM_IAMCU 32-bit Intel MCU (3198)

  Also add a compat #define for building with older FreeBSD ELF headers.
  The GRP_COMDAT flag was added to elf_common.h in r283110, but it's not
  available during the bootstrap build.  It is also convenient to be able
  to build on older hosts.

  Thanks to antoine@ for tracking down issues through multiple exp-runs
  and to kaiw@ for fixing.

  PR:		198611 (exp-run), 200350
  Sponsored by:	The FreeBSD Foundation

Changes:
_U  head/contrib/elftoolchain/
  head/contrib/elftoolchain/common/_elftc.h
  head/contrib/elftoolchain/common/elfdefinitions.h
  head/contrib/elftoolchain/elfcopy/elfcopy.h
  head/contrib/elftoolchain/elfcopy/main.c
  head/contrib/elftoolchain/elfcopy/sections.c
  head/contrib/elftoolchain/elfcopy/symbols.c
  head/contrib/elftoolchain/libdwarf/libdwarf_reloc.c
  head/contrib/elftoolchain/libelftc/libelftc_dem_gnu3.c
  head/contrib/elftoolchain/libelftc/os.Linux.mk
  head/contrib/elftoolchain/readelf/readelf.1
  head/contrib/elftoolchain/readelf/readelf.c
  head/lib/libelftc/elftc_version.c
Comment 26 Ed Maste freebsd_committer 2015-05-27 18:01:44 UTC
Upstream discussion:
http://savannah.gnu.org/patch/?8675
http://lists.gnu.org/archive/html/libtool-patches/2015-05/msg00004.html

With luck either delphij@'s or my patch will be accepted shortly, and we can apply the same change to our devel/libtool port.
Comment 27 Antoine Brodin freebsd_committer 2015-05-27 18:12:44 UTC
Created attachment 157201 [details]
Possible patch

Attached is what I have for the ports tree (untested)
Comment 28 Ed Maste freebsd_committer 2015-05-27 18:19:04 UTC
(In reply to Antoine Brodin from comment #27)

> post-patch:		
> 	@${REINPLACE_CMD} -e \
> 		's|"GNU strip"|"strip"|' ${WRKSRC}/configure

That looks good to me, given that any strip binary of interest on any FreeBSD version should be able to handle library stripping.
Comment 29 commit-hook freebsd_committer 2015-06-08 05:59:42 UTC
A commit references this bug:

Author: antoine
Date: Mon Jun  8 05:59:18 UTC 2015
New revision: 388831
URL: https://svnweb.freebsd.org/changeset/ports/388831

Log:
  Make ports using libtool treat elftoolchain's strip the same as GNU strip
  Any strip on any FreeBSD version should be able to handle stripping requested
  by libtool

  PR:		198611
  Reviewed by:	emaste
  Exp-run:	self

Changes:
  head/Mk/Uses/libtool.mk
  head/audio/liblscp/Makefile
  head/audio/mac/files/patch-configure
  head/devel/pcre/files/patch-configure
  head/devel/synfig/Makefile
  head/graphics/gthumb/Makefile
  head/graphics/pixie/files/patch-configure
  head/graphics/synfigstudio/Makefile
  head/math/yacas/Makefile
  head/multimedia/subtitleeditor/Makefile
  head/net/nanomsg/Makefile
  head/net-im/gloox/Makefile
  head/science/gwyddion/Makefile
  head/sysutils/powerman/Makefile
Comment 30 commit-hook freebsd_committer 2019-11-08 15:00:22 UTC
A commit references this bug:

Author: emaste
Date: Fri Nov  8 14:59:41 UTC 2019
New revision: 354544
URL: https://svnweb.freebsd.org/changeset/base/354544

Log:
  elfcopy/strip: Ensure sections have required alignment on output

  Object files may specify insufficient alignment on certain sections, for
  example due to a bug in NASM[1].  When we detect that case in elfcopy or
  strip, emit a warning and increase the alignment to the minimum
  required.

  The NASM bug was fixed in 2015[2], but we might as well have this fixup
  (and warning) in elfcopy in case we encounter such a file for any other
  reason.

  This might be reworked somewhat upstream - see ELF Tool Chain
  ticket 485[3].

  [1] https://bugzilla.nasm.us/show_bug.cgi?id=3392307
  [2] https://repo.or.cz/w/nasm.git/commit/1f0cb0f2c1ba632c0fab02424928cfb756a9160c
  [3] https://sourceforge.net/p/elftoolchain/tickets/485/

  PR:		198611
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D2292

Changes:
  head/contrib/elftoolchain/elfcopy/sections.c