[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH v4 2/3] build: always produce 2 images: w/ and w/o debug syms


  • To: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>, "minios-devel@xxxxxxxxxxxxx" <minios-devel@xxxxxxxxxxxxx>
  • From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
  • Date: Wed, 29 May 2019 12:14:16 +0000
  • Accept-language: en-GB, gl-ES, de-DE, en-US
  • Delivery-date: Wed, 29 May 2019 12:14:23 +0000
  • List-id: Mini-os development list <minios-devel.lists.xenproject.org>
  • Thread-index: AQHVEh9b8dttYjYtkEKL/aLY1tJDnqaCC9IA
  • Thread-topic: [Minios-devel] [UNIKRAFT PATCH v4 2/3] build: always produce 2 images: w/ and w/o debug syms

Hey Yuri,

thanks for this patch. It looks okay, I have just some minor comments for it. I 
originally would have preferred a non-python solution for your `sect-strip` 
wrapper in order to avoid a build dependency to python. However, we may anyway 
want to update kconfig to the latest version which afaik provides python 
support.

Thanks,

Simon

On 24.05.19, 12:56, "Minios-devel on behalf of Yuri Volchkov" 
<minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx on behalf of 
yuri.volchkov@xxxxxxxxx> wrote:

    On contrary to previously existing scheme, the *.dbg image is a fully
    functional binary now. A user still can run qemu/xen with a stripped
    image, and attach gdb to it, specifying the *.dbg image as a source.
    
    A full dbg images are not making more overhead comparing to
    '--only-keep-debug' ones (apart from disk space), but are bringing
    convenience.
    
    Another effect of this patch - *.dbg file is produced
    always (menuconfig is gone). The 'strip' operation does not cost us
    much, and the stripped image equivalents to image build without debug
    symbols. But even in production systems it is handy to have an image
    with debug symbols by your side to perform a post-mortem analysis of a
    crash.
    
    This approach allows to have strip-able sections, existing only in the
    *.dbg files. Now user can append the variable `STRIP_SECTIONS_FLAGS-y`
    in the library/application makefile, to list all the section he does
    not need in the final image.
    
    Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
    ---
     Config.uk                     | 20 --------
     Makefile.uk                   |  6 ---
     plat/kvm/Linker.uk            | 22 ++++-----
     plat/linuxu/Linker.uk         | 17 +++----
     plat/xen/Linker.uk            | 33 ++++++-------
     support/scripts/sect-strip.py | 87 +++++++++++++++++++++++++++++++++++
     6 files changed, 115 insertions(+), 70 deletions(-)
     create mode 100755 support/scripts/sect-strip.py
    
    diff --git a/Config.uk b/Config.uk
    index d0bfa823..bb791fdb 100644
    --- a/Config.uk
    +++ b/Config.uk
    @@ -86,13 +86,6 @@ config OPTIMIZE_LTO
                will increase overall building time but creates more efficient
                Unikraft binaries.
     
    -config DEBUG_SYMBOLS
    -   bool "Debugging information"
    -   default n
    -   help
    -           Build with debugging information.
    -
    -if DEBUG_SYMBOLS

Since you are removing this option, could you add "No Debug (-g0) " to the 
option list?
For now, it is more for completeness.

     choice
        prompt "Debug information level"
        default DEBUG_SYMBOLS_LVL3
    @@ -121,24 +114,11 @@ config DEBUG_SYMBOLS_LVL3
                support macro expansion.
     endchoice
     
    -config OPTIMIZE_DBGFILE
    -   bool "Create a debugging information file"
    -   default y
    -   help
    -           Create a separate file with debugging information
    -
     config OPTIMIZE_SYMFILE
        bool "Create a symbols file"
        default n
        help
                Create a separate file with all symbol locations
    -endif
    -
    -config OPTIMIZE_STRIP
    -   bool "Strip final image"
    -   default y
    -   help
    -           Strip any extra information from image
     
     config RECORD_BUILDTIME
        bool "Keep track of Building time"
    diff --git a/Makefile.uk b/Makefile.uk
    index c6cf2306..3716fa64 100644
    --- a/Makefile.uk
    +++ b/Makefile.uk
    @@ -40,18 +40,12 @@ CXXFLAGS-$(CONFIG_OPTIMIZE_LTO)           += -flto
     LIBLDFLAGS-$(CONFIG_OPTIMIZE_LTO)         += $(CFLAGS) $(CFLAGS-y)
     LDFLAGS-$(CONFIG_OPTIMIZE_LTO)            += $(CFLAGS) $(CFLAGS-y)
     
    -ifneq ($(CONFIG_DEBUG_SYMBOLS),y)
    -CFLAGS                             += -g0
    -CXXFLAGS                           += -g0
    -LDFLAGS-y                     += -Wl,--strip-debug
    -else

With this menu option we would add
CFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL0)       += -g0
CXXFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL0)     += -g0

     CFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL1)       += -g1
     CXXFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL1)     += -g1
     CFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL2)       += -g2
     CXXFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL2)     += -g2
     CFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL3)       += -g3
     CXXFLAGS-$(CONFIG_DEBUG_SYMBOLS_LVL3)     += -g3
    -endif
     
     ASFLAGS  += -D __Unikraft__ -DUK_CODENAME="$(UK_CODENAME)"
     ASFLAGS  += -DUK_VERSION=$(UK_VERSION).$(UK_SUBVERSION)
    diff --git a/plat/kvm/Linker.uk b/plat/kvm/Linker.uk
    index 8ed25260..a81d3dbf 100644
    --- a/plat/kvm/Linker.uk
    +++ b/plat/kvm/Linker.uk
    @@ -30,28 +30,22 @@ $(KVM_IMAGE): $(KVM_ALIBS) $(KVM_ALIBS-y) $(KVM_OLIBS) 
$(KVM_OLIBS-y) \
        $(call build_cmd,OBJCOPY,,$@.o,\
               $(OBJCOPY) -w -G kvmos_* -G _libkvmplat_entry $@.ld.o $@.o)
     ifneq ($(filter x86_64 arm64,$(CONFIG_UK_ARCH)),)
    -   $(call build_cmd,LD,,$@,\
    +   $(call build_cmd,LD,,$@.dbg,\
               $(LD) $(LDFLAGS) $(LDFLAGS-y) \
                     $(KVM_LDFLAGS) $(KVM_LDFLAGS-y) \
                     -Wl$(comma)-dT$(comma)$(call strip,$(KVM_LDSCRIPT)) \
                     $(KVM_LD_SCRIPT_FLAGS) \
    -                $@.o -o $@)
    -ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
    -   $(call build_cmd,OBJCOPY,,$@.dbg,\
    -          $(OBJCOPY) --only-keep-debug $@ $@.dbg)
    -endif
    +                $@.o -o $@.dbg)
    +   $(call verbose_cmd,sect-strip,$(notdir $@),\

I would prefer a different verbose string than 'sect-strip'. It is 
unfortunately longer than 7 characters, and we usually used upper-case. How 
about 'SSTRIP', 'SESTRIP', or 'SCSTRIP'?

    +           $(SCRIPTS_DIR)/sect-strip.py $(STRIP_SECTIONS_FLAGS-y) \

You named the program `sect-strip.py` but the flags variable is called 
`STRIP_SECTIONS_FLAGS`. Why not calling it SECT_STRIP_FLAGS for consistency?
Usually we also included two variants to the cmdline for convenience (one with 
'-y' and the other one without): $(SECT_STRIP_FLAGS) $(SECT_STRIP_FLAGS-y)

    +                   --with-objcopy=$(OBJCOPY) \
    +                   $@.dbg -o $@)
    +   $(call verbose_cmd,STRIP,$(notdir $@), $(STRIP) $@)
    +
     ifeq ($(CONFIG_OPTIMIZE_SYMFILE),y)
        $(call build_cmd,NM,,$@.sym,\
               $(NM) -n $@ > $@.sym)
     endif
    -# TODO: We have to revisit stripping of KVM binaries. We noticed that 
sometimes
    -#       the images are broken and cannot be boot with QEMU's direct kernel 
boot
    -#       option (fread() error is returned).
    -#

Does this problem not exist anymore? I am asking because we are always 
stripping the image with your patch.

    -#ifeq ($(CONFIG_OPTIMIZE_STRIP),y)
    -#  $(call verbose_cmd,STRIP,$(notdir $@),\
    -#         $(STRIP) -s $@)
    -#endif
        $(call build_cmd,GZ,,$@.gz,\
               $(GZIP) -f -9 -c $@ >$@.gz)
     endif
    diff --git a/plat/linuxu/Linker.uk b/plat/linuxu/Linker.uk
    index 20ebfc05..7ac93edd 100644
    --- a/plat/linuxu/Linker.uk
    +++ b/plat/linuxu/Linker.uk
    @@ -12,7 +12,7 @@ LINUXU_LD_SCRIPT_FLAGS := $(addprefix 
-Wl$(comma)-T$(comma),\
     $(LINUXU_IMAGE): $(LINUXU_ALIBS) $(LINUXU_ALIBS-y) \
                 $(LINUXU_OLIBS) $(LINUXU_OLIBS-y) \
                 $(UK_ALIBS) $(UK_ALIBS-y) $(UK_OLIBS) $(UK_OLIBS-y)
    -   $(call build_cmd,LD,,$@,\
    +   $(call build_cmd,LD,,$@.dbg,\
               $(LD) $(LDFLAGS) $(LDFLAGS-y) \
                     $(LINUXU_LDFLAGS) $(LINUXU_LDFLAGS-y) \
                     $(LINUXU_OLIBS) $(LINUXU_OLIBS-y) \
    @@ -22,19 +22,16 @@ $(LINUXU_IMAGE): $(LINUXU_ALIBS) $(LINUXU_ALIBS-y) \
                     $(UK_ALIBS) $(UK_ALIBS-y) \
                     -Wl$(comma)--end-group \
                     $(LINUXU_LD_SCRIPT_FLAGS) \
    -                -o $@)
    -ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
    -   $(call build_cmd,OBJCOPY,,$@.dbg,\
    -          $(OBJCOPY) --only-keep-debug $@ $@.dbg)
    -endif
    +                -o $@.dbg)
    +   $(call verbose_cmd,sect-strip,$(notdir $@),\
    +           $(SCRIPTS_DIR)/sect-strip.py $(STRIP_SECTIONS_FLAGS-y) \
    +                   --with-objcopy=$(OBJCOPY) \
    +                   $@.dbg -o $@)
    +   $(call verbose_cmd,STRIP,$(notdir $@), $(STRIP)  $@)
     ifeq ($(CONFIG_OPTIMIZE_SYMFILE),y)
        $(call build_cmd,NM,,$@.sym,\
               $(NM) -n $@ > $@.sym)
     endif
    -ifeq ($(CONFIG_OPTIMIZE_STRIP),y)
    -   $(call verbose_cmd,STRIP,$(notdir $@),\
    -          $(STRIP) -s $@)
    -endif
     
     # register image to the build
     UK_IMAGES-$(CONFIG_PLAT_LINUXU) += $(LINUXU_IMAGE)
    diff --git a/plat/xen/Linker.uk b/plat/xen/Linker.uk
    index bcbdc00f..279464e0 100644
    --- a/plat/xen/Linker.uk
    +++ b/plat/xen/Linker.uk
    @@ -28,44 +28,37 @@ $(XEN_IMAGE): $(XEN_ALIBS) $(XEN_ALIBS-y) $(XEN_OLIBS) 
$(XEN_OLIBS-y) \
        $(call build_cmd,OBJCOPY,,$@.o,\
               $(OBJCOPY) -w -G xenos_* -G _libxenplat_start $@.ld.o $@.o)
     ifeq (arm,$(CONFIG_UK_ARCH))
    -   $(call build_cmd,LD,,$@.elf,\
    +   $(call build_cmd,LD,,$@.elf.dbg,\
               $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) \
                     -Wl$(comma)-dT$(comma)$(call strip,$(XEN_LDSCRIPT)) \
                     $(XEN_LD_SCRIPT_FLAGS) \
    -                $@.o -o $@.elf)
    -ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
    -   $(call build_cmd,OBJCOPY,,$@.dbg,\
    -          $(OBJCOPY) --only-keep-debug $@.elf $@.dbg)
    -endif
    +                $@.o -o $@.elf.dbg)
    +   $(call verbose_cmd,sect-strip,$(notdir $@.elf),\
    +           $(SCRIPTS_DIR)/sect-strip.py $(STRIP_SECTIONS_FLAGS-y) \
    +                   --with-objcopy=$(OBJCOPY) \
    +                   $@.elf.dbg -o $@.elf)
    +   $(call verbose_cmd,STRIP,$(notdir $@.elf), $(STRIP) -o $@.elf)
     ifeq ($(CONFIG_OPTIMIZE_SYMFILE),y)
        $(call build_cmd,NM,,$@.sym,\
               $(NM) -n $@.elf > $@.sym)
    -endif
    -ifeq ($(CONFIG_OPTIMIZE_STRIP),y)
    -   $(call verbose_cmd,STRIP,$(notdir $@),\
    -          $(STRIP) -s $@.elf)
     endif
        $(call build_cmd,OBJCOPY,,$@,\
               $(OBJCOPY) -O binary $@.elf $@)
        $(call build_cmd,GZ,,$@.gz,\
               $(GZIP) -f -9 -c $@ >$@.gz)
     else
    -   $(call build_cmd,LD,,$@,\
    +   $(call build_cmd,LD,,$@.dbg,\
               $(LD) $(LDFLAGS) $(LDFLAGS-y) $(XEN_LDFLAGS) $(XEN_LDFLAGS-y) \
                     -Wl$(comma)-dT$(comma)$(call strip,$(XEN_LDSCRIPT)) \
                     $(XEN_LD_SCRIPT_FLAGS) \
    -                $@.o -o $@)
    -ifeq ($(CONFIG_OPTIMIZE_DBGFILE),y)
    -   $(call build_cmd,OBJCOPY,,$@.dbg,\
    -          $(OBJCOPY) --only-keep-debug $@ $@.dbg)
    -endif
    +                $@.o -o $@.dbg)
    +   $(call verbose_cmd,sect-strip,$(notdir $@),\
    +          $(SCRIPTS_DIR)/sect-strip.py $(STRIP_SECTIONS_FLAGS-y) \
    +                   $@.dbg -o $@)
    +   $(call verbose_cmd,STRIP,$(notdir $@), $(STRIP)  $@)
     ifeq ($(CONFIG_OPTIMIZE_SYMFILE),y)
        $(call build_cmd,NM,,$@.sym,\
               $(NM) -n $@ > $@.sym)
    -endif
    -ifeq ($(CONFIG_OPTIMIZE_STRIP),y)
    -   $(call verbose_cmd,STRIP,$(notdir $@),\
    -          $(STRIP) -s $@)
     endif
        $(call build_cmd,GZ,,$@.gz,\
               $(GZIP) -f -9 -c $@ >$@.gz)
    diff --git a/support/scripts/sect-strip.py b/support/scripts/sect-strip.py
    new file mode 100755
    index 00000000..49f821f4
    --- /dev/null
    +++ b/support/scripts/sect-strip.py
    @@ -0,0 +1,87 @@
    +#!/usr/bin/env python3
    +#
    +# Copyright (c) 2019, NEC Laboratories Europe GmbH, NEC Corporation.
    +#
    +# This program is free software; you can redistribute it and/or modify
    +# it under the terms of the GNU General Public License as published by
    +# the Free Software Foundation; either version 2 of the License, or
    +# (at your option) any later version.
    +#
    +# This program is distributed in the hope that it will be useful,
    +# but WITHOUT ANY WARRANTY; without even the implied warranty of
    +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
    +# General Public License for more details.
    +#
    +# This nonsense exists because objcopy and strip are complaining if
    +# the last section within the segment is removed. This is just a
    +# script which suppresses this message.
    +
    +import argparse
    +import subprocess
    +import sys
    +import shutil
    +
    +def format_args(args):
    +    def format_arg(arg):
    +        if ' ' in arg:
    +            return '"%s"' % arg
    +        return arg
    +
    +    return ' '.join(map(format_arg, args))
    +
    +def main():
    +    parser = argparse.ArgumentParser(description="Strip sections 
gracefully")
    +    parser.add_argument("src", help="Source elf to strip")
    +    parser.add_argument("-o", "--output", help="Target elf file", 
required=True)
    +    parser.add_argument("-R", "--remove-section", action='append', 
default=[],
    +                        help="Sections to be removed (patterns allowed)")
    +    parser.add_argument("--with-objcopy", default="objcopy",
    +                        help="Specify if a specific objcopy binary should 
be used")
    +    parser.add_argument("--dry-run", action="store_true",
    +                        default=False,
    +                        help="do not change anything, print objcopy 
command")
    +    opt = parser.parse_args()
    +
    +    cmd = [opt.with_objcopy,
    +           opt.src,
    +           opt.output,
    +           ]
    +
    +    if len(opt.remove_section) == 0:
    +        if not opt.dry_run:
    +            shutil.copy(opt.src, opt.output)
    +        return
    +
    +    for i in opt.remove_section:
    +        cmd += ['-R', i]
    +
    +    if opt.dry_run:
    +        print(format_args(cmd))
    +        return
    +
    +    proc = subprocess.Popen(cmd,
    +                            stdout=subprocess.PIPE,
    +                            stderr=subprocess.STDOUT)
    +    _stdout, _ = proc.communicate()
    +    _stdout = _stdout.decode().strip()
    +
    +    if proc.returncode:
    +        print("Error occurred while objcopy:", file=sys.stderr)
    +        print(_stdout, file=sys.stderr)
    +        sys.exit(1)
    +
    +    filter_str = "warning: Empty loadable segment detected, is this 
intentional"
    +
    +    # If nothing was on stdout, split will produce a list consisting
    +    # of one element - empty string. We an want empty list though.
    +    if len(_stdout) == 0:
    +        lines = []
    +    else:
    +        lines = _stdout.split('\n')
    +
    +    lines = filter(lambda a : filter_str not in a, lines)
    +    for i in lines:
    +        print(i, file=sys.stderr)
    +
    +if __name__ == '__main__':
    +    main()
    -- 
    2.19.2
    
    
    _______________________________________________
    Minios-devel mailing list
    Minios-devel@xxxxxxxxxxxxxxxxxxxx
    https://lists.xenproject.org/mailman/listinfo/minios-devel

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.