linux-kernel - Re: [PATCH] selftests/arm64: fix build failure during the "emit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jul 2023 17:58:30 -0700
From:   John Hubbard <jhubbard@...dia.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
CC:     Anders Roxell <anders.roxell@...aro.org>,
        David Hildenbrand <david@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Muhammad Usama Anjum <usama.anjum@...labora.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Peter Xu <peterx@...hat.com>, Shuah Khan <shuah@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-mm@...ck.org>,
        <linux-kselftest@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "Mark Brown" <broonie@...nel.org>
Subject: Re: [PATCH] selftests/arm64: fix build failure during the
 "emit_tests" step

On 7/10/23 17:56, John Hubbard wrote:
> The build failure reported in [1] occurred because commit 9fc96c7c19df
> ("selftests: error out if kernel header files are not yet built") added
> a new "kernel_header_files" dependency to "all", and that triggered
> another, pre-existing problem. Specifically, the arm64 selftests
> override the emit_tests target, and that override improperly declares
> itself to depend upon the "all" target.

I also have a corresponding fix for selftests/riscv, which has copied the
arm64 selftest approach. But I'll wait to see how reviews go here,
before sending that out.

thanks,
-- 
John Hubbard
NVIDIA

> 
> This is a problem because the "emit_tests" target in lib.mk was not
> intended to be overridden. emit_tests is a very simple, sequential build
> target that was originally invoked from the "install" target, which in
> turn, depends upon "all".
> 
> That approach worked for years. But with 9fc96c7c19df in place,
> emit_tests failed, because it does not set up all of the elaborate
> things that "install" does. And that caused the new
> "kernel_header_files" target (which depends upon $(KBUILD_OUTPUT) being
> correct) to fail.
> 
> Some detail: The "all" target is .PHONY. Therefore, each target that
> depends on "all" will cause it to be invoked again, and because
> dependencies are managed quite loosely in the selftests Makefiles, many
> things will run, even "all" is invoked several times in immediate
> succession. So this is not a "real" failure, as far as build steps go:
> everything gets built, but "all" reports a problem when invoked a second
> time from a bad environment.
> 
> To fix this, simply remove the unnecessary "all" dependency from the
> overridden emit_tests target. The dependency is still effectively
> honored, because again, invocation is via "install", which also depends
> upon "all".
> 
> An alternative approach would be to harden the emit_tests target so that
> it can depend upon "all", but that's a lot more complicated and hard to
> get right, and doesn't seem worth it, especially given that emit_tests
> should probably not be overridden at all.
> 
> [1] https://lore.kernel.org/20230710-kselftest-fix-arm64-v1-1-48e872844f25@kernel.org
> 
> Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built")
> Reported-by: Mark Brown <broonie@...nel.org>
> Signed-off-by: John Hubbard <jhubbard@...dia.com>
> ---
>   tools/testing/selftests/arm64/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> index 9460cbe81bcc..ace8b67fb22d 100644
> --- a/tools/testing/selftests/arm64/Makefile
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -42,7 +42,7 @@ run_tests: all
>   	done
>   
>   # Avoid any output on non arm64 on emit_tests
> -emit_tests: all
> +emit_tests:
>   	@for DIR in $(ARM64_SUBTARGETS); do				\
>   		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
>   		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
> 
> base-commit: d5fe758c21f4770763ae4c05580be239be18947d