[PATCH resend] s390/purgatory: Make sure we fail the build if purgatory has missing symbols

[PATCH resend] s390/purgatory: Make sure we fail the build if purgatory has missing symbols

[Hans de Goede]

Since we link purgatory with -r aka we enable "incremental linking"
no checks for unresolved symbols are done while linking the purgatory.

This commit adds an extra check for unresolved symbols by calling ld
without -r before running objcopy to generate purgatory.ro.

This will help us catch missing symbols in the purgatory sooner.

Note this commit also removes --no-undefined from LDFLAGS_purgatory
as that has no effect.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Using 2 if_changed lines under a single rule does not work, then
  1 of the 2 will always execute each build.
  Instead add a new (unused) purgatory.chk intermediate which gets
  linked from purgatory.ro without -r to do the missing symbols check
- This also fixes the check generating an a.out file (oops)
---
 arch/s390/purgatory/.gitignore |  1 +
 arch/s390/purgatory/Makefile   | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/s390/purgatory/.gitignore b/arch/s390/purgatory/.gitignore
index 04a03433c720..c82157f46b18 100644
--- a/arch/s390/purgatory/.gitignore
+++ b/arch/s390/purgatory/.gitignore
@@ -1,3 +1,4 @@
 purgatory
+purgatory.chk
 purgatory.lds
 purgatory.ro
diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
index bc0d7a0d0394..13e9a5dc0a07 100644
--- a/arch/s390/purgatory/Makefile
+++ b/arch/s390/purgatory/Makefile
@@ -4,7 +4,7 @@ OBJECT_FILES_NON_STANDARD := y
 
 purgatory-y := head.o purgatory.o string.o sha256.o mem.o
 
-targets += $(purgatory-y) purgatory.lds purgatory purgatory.ro
+targets += $(purgatory-y) purgatory.lds purgatory purgatory.chk purgatory.ro
 PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y))
 
 $(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
@@ -26,15 +26,22 @@ KBUILD_CFLAGS += $(CLANG_FLAGS)
 KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
 KBUILD_AFLAGS := $(filter-out -DCC_USING_EXPOLINE,$(KBUILD_AFLAGS))
 
-LDFLAGS_purgatory := -r --no-undefined -nostdlib -z nodefaultlib -T
+# Since we link purgatory with -r unresolved symbols are not checked, so we
+# also link a purgatory.chk binary without -r to check for unresolved symbols.
+PURGATORY_LDFLAGS := -nostdlib -z nodefaultlib
+LDFLAGS_purgatory := -r $(PURGATORY_LDFLAGS) -T
+LDFLAGS_purgatory.chk := -e purgatory_start $(PURGATORY_LDFLAGS)
 $(obj)/purgatory: $(obj)/purgatory.lds $(PURGATORY_OBJS) FORCE
 		$(call if_changed,ld)
 
+$(obj)/purgatory.chk: $(obj)/purgatory FORCE
+		$(call if_changed,ld)
+
 OBJCOPYFLAGS_purgatory.ro := -O elf64-s390
 OBJCOPYFLAGS_purgatory.ro += --remove-section='*debug*'
 OBJCOPYFLAGS_purgatory.ro += --remove-section='.comment'
 OBJCOPYFLAGS_purgatory.ro += --remove-section='.note.*'
-$(obj)/purgatory.ro: $(obj)/purgatory FORCE
+$(obj)/purgatory.ro: $(obj)/purgatory $(obj)/purgatory.chk FORCE
 		$(call if_changed,objcopy)
 
 $(obj)/kexec-purgatory.o: $(obj)/kexec-purgatory.S $(obj)/purgatory.ro FORCE
-- 
2.23.0

Re: [PATCH resend] s390/purgatory: Make sure we fail the build if purgatory has missing symbols

[Christian Borntraeger]

Thanks applied.

Now that the build fixes have been merged I do not see a reasons to
defer this any longer.
I also added a Tested-by: Philipp Rudo <prudo@linux.ibm.com>
as he successfully tested v2 in October.



On 12.12.19 21:53, Hans de Goede wrote:
> Since we link purgatory with -r aka we enable "incremental linking"
> no checks for unresolved symbols are done while linking the purgatory.
> 
> This commit adds an extra check for unresolved symbols by calling ld
> without -r before running objcopy to generate purgatory.ro.
> 
> This will help us catch missing symbols in the purgatory sooner.
> 
> Note this commit also removes --no-undefined from LDFLAGS_purgatory
> as that has no effect.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - Using 2 if_changed lines under a single rule does not work, then
>   1 of the 2 will always execute each build.
>   Instead add a new (unused) purgatory.chk intermediate which gets
>   linked from purgatory.ro without -r to do the missing symbols check
> - This also fixes the check generating an a.out file (oops)
> ---
>  arch/s390/purgatory/.gitignore |  1 +
>  arch/s390/purgatory/Makefile   | 13 ++++++++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/purgatory/.gitignore b/arch/s390/purgatory/.gitignore
> index 04a03433c720..c82157f46b18 100644
> --- a/arch/s390/purgatory/.gitignore
> +++ b/arch/s390/purgatory/.gitignore
> @@ -1,3 +1,4 @@
>  purgatory
> +purgatory.chk
>  purgatory.lds
>  purgatory.ro
> diff --git a/arch/s390/purgatory/Makefile b/arch/s390/purgatory/Makefile
> index bc0d7a0d0394..13e9a5dc0a07 100644
> --- a/arch/s390/purgatory/Makefile
> +++ b/arch/s390/purgatory/Makefile
> @@ -4,7 +4,7 @@ OBJECT_FILES_NON_STANDARD := y
>  
>  purgatory-y := head.o purgatory.o string.o sha256.o mem.o
>  
> -targets += $(purgatory-y) purgatory.lds purgatory purgatory.ro
> +targets += $(purgatory-y) purgatory.lds purgatory purgatory.chk purgatory.ro
>  PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y))
>  
>  $(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
> @@ -26,15 +26,22 @@ KBUILD_CFLAGS += $(CLANG_FLAGS)
>  KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
>  KBUILD_AFLAGS := $(filter-out -DCC_USING_EXPOLINE,$(KBUILD_AFLAGS))
>  
> -LDFLAGS_purgatory := -r --no-undefined -nostdlib -z nodefaultlib -T
> +# Since we link purgatory with -r unresolved symbols are not checked, so we
> +# also link a purgatory.chk binary without -r to check for unresolved symbols.
> +PURGATORY_LDFLAGS := -nostdlib -z nodefaultlib
> +LDFLAGS_purgatory := -r $(PURGATORY_LDFLAGS) -T
> +LDFLAGS_purgatory.chk := -e purgatory_start $(PURGATORY_LDFLAGS)
>  $(obj)/purgatory: $(obj)/purgatory.lds $(PURGATORY_OBJS) FORCE
>  		$(call if_changed,ld)
>  
> +$(obj)/purgatory.chk: $(obj)/purgatory FORCE
> +		$(call if_changed,ld)
> +
>  OBJCOPYFLAGS_purgatory.ro := -O elf64-s390
>  OBJCOPYFLAGS_purgatory.ro += --remove-section='*debug*'
>  OBJCOPYFLAGS_purgatory.ro += --remove-section='.comment'
>  OBJCOPYFLAGS_purgatory.ro += --remove-section='.note.*'
> -$(obj)/purgatory.ro: $(obj)/purgatory FORCE
> +$(obj)/purgatory.ro: $(obj)/purgatory $(obj)/purgatory.chk FORCE
>  		$(call if_changed,objcopy)
>  
>  $(obj)/kexec-purgatory.o: $(obj)/kexec-purgatory.S $(obj)/purgatory.ro FORCE
> 

Re: [PATCH resend] s390/purgatory: Make sure we fail the build if purgatory has missing symbols

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3288 bytes --]

Hi Hans,

I love your patch! Yet something to improve:

[auto build test ERROR on s390/features]
[also build test ERROR on v5.5-rc2 next-20191217]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/s390-purgatory-Make-sure-we-fail-the-build-if-purgatory-has-missing-symbols/20191216-045305
base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
config: s390-randconfig-a001-20191217 (attached as .config)
compiler: s390-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=s390 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/s390/purgatory/purgatory: In function `verify_sha256_digest':
>> (.text+0x20c6): undefined reference to `__asan_report_load8_noabort'
   (.text+0x20e8): undefined reference to `__asan_report_load8_noabort'
   (.text+0x212a): undefined reference to `memcmp'
   arch/s390/purgatory/purgatory: In function `sha256_update':
   (.text+0x21e4): undefined reference to `__asan_report_load8_noabort'
>> (.text+0x22b6): undefined reference to `__asan_report_load4_noabort'
>> (.text+0x2308): undefined reference to `__asan_report_store4_noabort'
   (.text+0x236e): undefined reference to `__asan_report_load4_noabort'
   (.text+0x23d8): undefined reference to `__asan_report_load4_noabort'
   (.text+0x2432): undefined reference to `__asan_report_load4_noabort'
   (.text+0x2498): undefined reference to `__asan_report_load4_noabort'
   (.text+0x24f2): undefined reference to `__asan_report_store4_noabort'
   (.text+0x254c): undefined reference to `__asan_report_load4_noabort'
   (.text+0x25a2): undefined reference to `__asan_report_load4_noabort'
   (.text+0x25f8): undefined reference to `__asan_report_load4_noabort'
   (.text+0x264e): undefined reference to `__asan_report_load4_noabort'
   (.text+0x26a4): undefined reference to `__asan_report_load4_noabort'
   arch/s390/purgatory/purgatory:(.text+0x26fa): more undefined references to `__asan_report_load4_noabort' follow
   arch/s390/purgatory/purgatory: In function `__sha256_final':
>> sha256.c:(.text+0x4062): undefined reference to `__asan_report_load8_noabort'
>> sha256.c:(.text+0x4120): undefined reference to `__asan_report_load4_noabort'
>> sha256.c:(.text+0x416e): undefined reference to `__asan_report_store4_noabort'
   arch/s390/purgatory/purgatory: In function `_GLOBAL__sub_D_65535_0_sha256_update':
   sha256.c:(.text+0x4444): undefined reference to `__asan_unregister_globals'
   arch/s390/purgatory/purgatory: In function `_GLOBAL__sub_I_65535_1_sha256_update':
   sha256.c:(.text+0x4454): undefined reference to `__asan_register_globals'

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28155 bytes --]

Re: [PATCH resend] s390/purgatory: Make sure we fail the build if purgatory has missing symbols

[Christian Borntraeger]

On 18.12.19 01:25, kbuild test robot wrote:
> Hi Hans,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on s390/features]
> [also build test ERROR on v5.5-rc2 next-20191217]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> 
> url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/s390-purgatory-Make-sure-we-fail-the-build-if-purgatory-has-missing-symbols/20191216-045305
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
> config: s390-randconfig-a001-20191217 (attached as .config)
> compiler: s390-linux-gcc (GCC) 7.5.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.5.0 make.cross ARCH=s390 
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>

This actually looks like the patch is doing exactly what it should.
It seems we must not build the purgatory with kasan. Will have a look
> 
> All errors (new ones prefixed by >>):
> 
>    arch/s390/purgatory/purgatory: In function `verify_sha256_digest':
>>> (.text+0x20c6): undefined reference to `__asan_report_load8_noabort'
>    (.text+0x20e8): undefined reference to `__asan_report_load8_noabort'
>    (.text+0x212a): undefined reference to `memcmp'
>    arch/s390/purgatory/purgatory: In function `sha256_update':
>    (.text+0x21e4): undefined reference to `__asan_report_load8_noabort'
>>> (.text+0x22b6): undefined reference to `__asan_report_load4_noabort'
>>> (.text+0x2308): undefined reference to `__asan_report_store4_noabort'
>    (.text+0x236e): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x23d8): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x2432): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x2498): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x24f2): undefined reference to `__asan_report_store4_noabort'
>    (.text+0x254c): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x25a2): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x25f8): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x264e): undefined reference to `__asan_report_load4_noabort'
>    (.text+0x26a4): undefined reference to `__asan_report_load4_noabort'
>    arch/s390/purgatory/purgatory:(.text+0x26fa): more undefined references to `__asan_report_load4_noabort' follow
>    arch/s390/purgatory/purgatory: In function `__sha256_final':
>>> sha256.c:(.text+0x4062): undefined reference to `__asan_report_load8_noabort'
>>> sha256.c:(.text+0x4120): undefined reference to `__asan_report_load4_noabort'
>>> sha256.c:(.text+0x416e): undefined reference to `__asan_report_store4_noabort'
>    arch/s390/purgatory/purgatory: In function `_GLOBAL__sub_D_65535_0_sha256_update':
>    sha256.c:(.text+0x4444): undefined reference to `__asan_unregister_globals'
>    arch/s390/purgatory/purgatory: In function `_GLOBAL__sub_I_65535_1_sha256_update':
>    sha256.c:(.text+0x4454): undefined reference to `__asan_register_globals'
> 
> ---
> 0-DAY kernel test infrastructure                 Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
>