[v2] Makefile: respect strip exclusions for special libraries

ld-*.so and libpthread*.so* are not stripped in the same way as other
binaries because some applications need symbols in these libraries in
order to operate correctly.

However, the special handling for these binaries ignores the usual
BR2_STRIP_EXCLUDE_* rules so it is not possible to build an image which
has debugging symbols in these binaries.

Instead of running one-off commands to strip each of these special
binaries, build a single command for everything that needs
$(STRIP_STRIP_DEBUG) and include the normal exclusion rules in this
command.

Signed-off-by: John Keeping <john@metanate.com>
---
On Mon, 2 Jul 2018 18:40:44 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> I guess you forgot to use 'findfileclauses' here, that is:
> 
>     STRIP_SPECIAL_LIBS_CMD += -not \( $(call findfileclauses,$(call
> qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \)
> 
> Otherwise, expansion of the variable would give an invalide find
> pattern, like so:
> 
>     -not \( some-excluded-file some-other-file a-tird-file \)
> 
> Or did I miss something?

You're right, I'm not sure how this worked when I tested it.  Here's an
updated patch.


 Makefile | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

Hello,

On Mon, 2 Jul 2018 17:57:11 +0100, John Keeping wrote:

> diff --git a/Makefile b/Makefile
> index 8d25c8a239..a2d21be243 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -606,6 +606,26 @@ STRIP_FIND_CMD += -type f \( -perm /111 -o -name '*.so*' \)
>  #   done for kernel modules with incorrect permissions.
>  STRIP_FIND_CMD += -not \( $(call findfileclauses,libpthread*.so* ld-*.so* *.ko $(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) -print0
>  
> +# This is the command used to handle the special cases above.
> +STRIP_SPECIAL_LIBS_CMD = find $(TARGET_DIR)/lib/
> +ifneq (,$(filter lib/%,$(call qstrip,$(BR2_STRIP_EXCLUDE_DIRS))))
> +STRIP_SPECIAL_LIBS_CMD += \( $(call finddirclauses,$(TARGET_DIR),$(BR2_STRIP_EXCLUDE_DIRS)) \) -prune -o
> +endif
> +# Valgrind needs ld.so with enough information, so only strip
> +# debugging symbols.
> +STRIP_SPECIAL_LIBS_CMD += \( -name 'ld-*.so*'
> +ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
> +# See http://sourceware.org/gdb/wiki/FAQ, "GDB does not see any threads
> +# besides the one in which crash occurred; or SIGTRAP kills my program when
> +# I set a breakpoint"
> +STRIP_SPECIAL_LIBS_CMD += -o -name 'libpthread*.so*'
> +endif
> +STRIP_SPECIAL_LIBS_CMD += \)
> +ifneq (,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES)))
> +STRIP_SPECIAL_LIBS_CMD += -not \( $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \)
> +endif
> +STRIP_SPECIAL_LIBS_CMD += | xargs -r $(STRIPCMD) $(STRIP_STRIP_DEBUG)

I understand the problem, and I know you will find my comment a bit
useless, but I find this whole thing pretty complex/noisy. Can we do
better ?

Here is some untested initial proposal:

# When stripping, obey to BR2_STRIP_EXCLUDE_DIRS and
# BR2_STRIP_EXCLUDE_FILES
STRIP_FIND_COMMON_CMD = \
	find $(TARGET_DIR) \
	$(if $(call qstrip,$(BR2_STRIP_EXCLUDE_DIRS)),, \
		\( $(call finddirclauses,$(TARGET_DIR),$(call qstrip,$(BR2_STRIP_EXCLUDE_DIRS))) \) -prune -o
	) \
	-not $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) \
	-print0

# Regular stripping for everything, except libpthread, ld-*.so and
# kernel modules.
STRIP_FIND_CMD = \
	$(STRIP_FIND_COMMON_CMD) \
	-type f \( -perm /111 -o -name '*.so*' \) \
	-not \( $(call findfileclauses,libpthread*.so* ld-*.so* *.ko) \)

# Special stripping (only debugging symbols) for libpthread and ld-*.so.
STRIP_FIND_SPECIAL_LIBS_CMD = \
	$(STRIP_FIND_COMMON_CMD) \
	\( -name 'ld-*.so*' -o -name 'libpthread*.so*' \)

And then:

	$(STRIP_FIND_CMD) | xargs -0 $(STRIPCMD) 2>/dev/null || true
	$(STRIP_FIND_SPECIAL_LIBS_CMD) | xargs -0 $(STRIPCMD) $(STRIP_STRIP_DEBUG) || true

Thoughts ?

Best regards,

Thomas

Thomas, John, All,

On 2018-07-02 22:25 +0200, Thomas Petazzoni spake thusly:
> [...] I find this whole thing pretty complex/noisy. Can we do
> better ?
> 
> Here is some untested initial proposal:
> 
> # When stripping, obey to BR2_STRIP_EXCLUDE_DIRS and
> # BR2_STRIP_EXCLUDE_FILES
> STRIP_FIND_COMMON_CMD = \
> 	find $(TARGET_DIR) \
> 	$(if $(call qstrip,$(BR2_STRIP_EXCLUDE_DIRS)),, \
> 		\( $(call finddirclauses,$(TARGET_DIR),$(call qstrip,$(BR2_STRIP_EXCLUDE_DIRS))) \) -prune -o
> 	) \
> 	-not $(call findfileclauses,$(call qstrip,$(BR2_STRIP_EXCLUDE_FILES))) \) \
> 	-print0
> 
> # Regular stripping for everything, except libpthread, ld-*.so and
> # kernel modules.
> STRIP_FIND_CMD = \
> 	$(STRIP_FIND_COMMON_CMD) \
> 	-type f \( -perm /111 -o -name '*.so*' \) \
> 	-not \( $(call findfileclauses,libpthread*.so* ld-*.so* *.ko) \)
> 
> # Special stripping (only debugging symbols) for libpthread and ld-*.so.
> STRIP_FIND_SPECIAL_LIBS_CMD = \
> 	$(STRIP_FIND_COMMON_CMD) \
> 	\( -name 'ld-*.so*' -o -name 'libpthread*.so*' \)
> 
> And then:
> 
> 	$(STRIP_FIND_CMD) | xargs -0 $(STRIPCMD) 2>/dev/null || true
> 	$(STRIP_FIND_SPECIAL_LIBS_CMD) | xargs -0 $(STRIPCMD) $(STRIP_STRIP_DEBUG) || true
> 
> Thoughts ?

Not tested, but it really looks nice.

Regards,
Yann E. MORIN.