chkobjdump.awk: no default action when using llvm-objdump

[CdeMills]

Hello,
context: CentOS8-Stream, spec file extracted from
https://elrepo.org/linux/kernel/el8/SRPMS/kernel-ml-5.12.0-1.el8.elrepo.nosrc.rpm
associated to linux-5.12.tar.xz

kernel-ml packages built using
env MAKEFLAGS="LLVM=1" rpmbuild --noclean --nocheck --nodebuginfo --without bpftool --without perf --without tools -ba SPECS/kernel-ml-5.12.spec

This add "LLVM=1" to each make invocation, leading to a kernel built with CLANG
This generates a message:
llvm-objdump: Unknown command line argument '-v'. Try: 'llvm-objdump --help'
llvm-objdump: Did you mean '-C'?

The source is in arch/x86/tools/Makefile, line 20:
cmd_posttest = ($(OBJDUMP) -v | $(AWK) -f $(chkobjdump)) || $(OBJDUMP) -d -j .text $(objtree)/vmlinux | [ deleted ]

First issue: llvm-objdump does not accept the '-v' flag as objdump does. Fix: replace
$(OBJDUMP) -v
by
$(OBJDUMP) --version
which is understood, the same way, by objdump and llvm-objdump

Second issue: $(chkobjdump), which is resolved to chkobjdump.awk in the same dir, contains a single directive:
/^GNU objdump/ { }
This script has no directive for LLVM ... meaning it exits a value of 0, meaning the second part of the || is executed systematically with clang, i.e. $(OBJDUMP) -d -j .text $(objtree)/vmlinux

I have no idea is that second command can be skipped or not. Could you please check ?

TIA

Pascal Dupuis

[nathanchance]

The first issue is being dealt with in LLVM and it is tracked at #1130. That change will still probably be needed for older versions of LLVM.

chkobjdump.awk script can be dropped, as it was added by https://git.kernel.org/linus/6f5f67267dc4faecd9cba63894de92ca92a608b8 to check for older binutils, but the kernel requires binutils 2.23 or newer. I will send a patch for this.

@ihalip seemed to indicated in this comment that there might be other issues to uncover here. I will test it out when I send the patch.

[nathanchance]

Actually, dropping the chkobjdump.awk script means that we can drop the $(OBJDUMP) -v altogether so there does not need to be a --version change.

[nathanchance]

Tentative patch:

From 219c9eaf7d0c9c8e46ced1af85dd526a20a4adc3 Mon Sep 17 00:00:00 2001
From: Nathan Chancellor <nathan@kernel.org>
Date: Thu, 29 Apr 2021 12:29:49 -0700
Subject: [PATCH] x86/tools: Remove chkobjdump.awk

This check is superfluous now that the minimum version of binutils to
build the kernel is 2.23. This also fixes an error seen with
llvm-objdump because it does not support '-v' prior to LLVM 13:

llvm-objdump: error: unknown argument '-v'

Link: https://reviews.llvm.org/D101483
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/x86/tools/Makefile       |  3 +--
 arch/x86/tools/chkobjdump.awk | 33 ---------------------------------
 2 files changed, 1 insertion(+), 35 deletions(-)
 delete mode 100644 arch/x86/tools/chkobjdump.awk

diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index bddfc9a46645..157c96491b94 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -14,10 +14,9 @@ else
 endif
 
 reformatter = $(srctree)/arch/x86/tools/objdump_reformat.awk
-chkobjdump = $(srctree)/arch/x86/tools/chkobjdump.awk
 
 quiet_cmd_posttest = TEST    $@
-      cmd_posttest = ($(OBJDUMP) -v | $(AWK) -f $(chkobjdump)) || $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(reformatter) | $(obj)/insn_decoder_test $(posttest_64bit) $(posttest_verbose)
+      cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(reformatter) | $(obj)/insn_decoder_test $(posttest_64bit) $(posttest_verbose)
 
 quiet_cmd_sanitytest = TEST    $@
       cmd_sanitytest = $(obj)/insn_sanity $(posttest_64bit) -m 1000000
diff --git a/arch/x86/tools/chkobjdump.awk b/arch/x86/tools/chkobjdump.awk
deleted file mode 100644
index fd1ab80be0de..000000000000
--- a/arch/x86/tools/chkobjdump.awk
+++ /dev/null
@@ -1,33 +0,0 @@
-# GNU objdump version checker
-#
-# Usage:
-# objdump -v | awk -f chkobjdump.awk
-BEGIN {
-	# objdump version 2.19 or later is OK for the test.
-	od_ver = 2;
-	od_sver = 19;
-}
-
-/^GNU objdump/ {
-	verstr = ""
-	for (i = 3; i <= NF; i++)
-		if (match($(i), "^[0-9]")) {
-			verstr = $(i);
-			break;
-		}
-	if (verstr == "") {
-		printf("Warning: Failed to find objdump version number.\n");
-		exit 0;
-	}
-	split(verstr, ver, ".");
-	if (ver[1] > od_ver ||
-	    (ver[1] == od_ver && ver[2] >= od_sver)) {
-		exit 1;
-	} else {
-		printf("Warning: objdump version %s is older than %d.%d\n",
-		       verstr, od_ver, od_sver);
-		print("Warning: Skipping posttest.");
-		# Logic is inverted, because we just skip test without error.
-		exit 0;
-	}
-}
-- 
2.31.1.362.g311531c9de

I need to wait to send it because of #1364.

[kees]

I've tested with the patches from #1364 and everything works happily if all 3 patches land. Please consider this patch:

Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Kees Cook <keescook@chromium.org>

[nathanchance]

I sent a v3 of Sam's series and included that change as the final patch: https://lore.kernel.org/20231129-objdump-reformat-llvm-v3-0-0d855e79314d@kernel.org/

[nathanchance]

Patch accepted: https://git.kernel.org/tip/5225952d74d43e4c054731c74b8afd700b23a94a

[nathanchance]

Merged into mainline: https://git.kernel.org/linus/5225952d74d43e4c054731c74b8afd700b23a94a