Re: [PATCH dwarves 0/3] add option to merge more dwarf cu's into

[Yonghong Song <yhs@fb.com>]

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



On 3/25/21 6:10 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 24, 2021 at 11:53:16PM -0700, Yonghong Song escreveu:
>> For vmlinux built with clang thin-lto or lto for latest bpf-next,
>> there exist cross cu debuginfo type references. For example,
>>        compile unit 1:
>>           tag 10:  type A
>>        compile unit 2:
>>           ...
>>             refer to type A (tag 10 in compile unit 1)
>> I only checked a few but have seen type A may be a simple type
>> like "unsigned char" or a complex type like an array of base types.
>> I am using latest llvm trunk and bpf-next. I suspect llvm12 or
>> linus tree >= 5.12 rc2 should be able to exhibit the issue as well.
>> Both thin-lto and lto have the same issues.
>>
>> Current pahole cannot handle this. It will report types cannot
>> be found error. Bill Wendling has attempted to fix the issue
>> with [1] by permitting all tags/types are hashed to the same
>> hash table and then process cu's one by one. This does not
>> really work. The reason is that each cu resolves types locally
>> so for the above example we may have
>>    compile unit 1:
>>      type A : type_id = 10
>>    compile unit 2:
>>      refer to type A : type A will be resolved as type id = 10
>> But id 10 refers to compile unit 1, we will get either out
>> of bound type id or incorrect one.
>>
>> This patch set is a continuation of Bill's work. We still
>> increase the hashtable size and traverse all cu's before
>> recoding and finalization. But instead of creating one-to-one
>> mapping between debuginfo cu and pahole cu, we just create
>> one pahole cu, which should solve the above incorrect type
>> id issue.
>>
>> Patches #1 and #2 are refactoring the existing code
>> and Patch #3 added an option "merge_cus" to permit
>> merging all debuginfo cu's into one pahole cu.
>> For vmlinux built, it can be detected that if LTO or Thin-LTO
>> is enabled, "merge_cus" can be added into pahole
>> command line.
>>
>>    [1] https://www.spinics.net/lists/dwarves/msg00999.html
> 
> Thanks for working on this, I'll look at it today.

Thanks! In case that you want to test with the kernel, I attached a 
patch on top of bpf-next to use --merge_cus when building kernel and 
modules.

> 
> - Arnaldo
> 

[-- Attachment #2: 0001-scripts-bpf-add-pahole-merge_cus-support.patch --]
[-- Type: text/plain, Size: 2300 bytes --]

From 0dfb561a14a9eb1c5bd077fb9b4729455dbb5ec4 Mon Sep 17 00:00:00 2001
From: Yonghong Song <yhs@fb.com>
Date: Thu, 25 Mar 2021 18:15:38 -0700
Subject: [PATCH] scripts: bpf: add pahole --merge_cus support

The following is the command line I used to build the kernel:
  make LLVM=1 LLVM_IAS=1 -j20 && make LLVM=1 LLVM_IAS=1 -j60 vmlinux
Make sure your config has CONFIG_LTO_CLANG_THIN on.
You may also try CONFIG_LTO_CLANG_FULL, but in my box, it takes
quite some time and the llvm linker (ld.lld) takes more than
13 minutes.

The following is the command line to build the bpf selftests:
  make -C tools/testing/selftests/bpf -j60 LLVM=1

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 scripts/Makefile.modfinal | 9 ++++++++-
 scripts/link-vmlinux.sh   | 7 ++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 735e11e9041b..5fc9d91c6976 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -47,6 +47,13 @@ cmd_ld_ko_o +=								\
 endif # SKIP_STACK_VALIDATION
 endif # CONFIG_STACK_VALIDATION
 
+ifdef CONFIG_LTO_CLANG_THIN
+merge_cus = "--merge_cus"
+endif
+ifdef CONFIG_LTO_CLANG_FULL
+merge_cus = "--merge_cus"
+endif
+
 endif # CONFIG_LTO_CLANG
 
 quiet_cmd_ld_ko_o = LD [M]  $@
@@ -59,7 +66,7 @@ quiet_cmd_ld_ko_o = LD [M]  $@
 quiet_cmd_btf_ko = BTF [M] $@
       cmd_btf_ko = 							\
 	if [ -f vmlinux ]; then						\
-		LLVM_OBJCOPY=$(OBJCOPY) $(PAHOLE) -J --btf_base vmlinux $@; \
+		LLVM_OBJCOPY=$(OBJCOPY) $(PAHOLE) -J --btf_base vmlinux $(merge_cus) $@; \
 	else								\
 		printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \
 	fi;
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 3b261b0f74f0..6b52c86acdad 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -227,8 +227,13 @@ gen_btf()
 
 	vmlinux_link ${1}
 
+	merge_cus=
+	if [ -n "${CONFIG_LTO_CLANG_THIN}" -o -n "${CONFIG_LTO_CLANG_FULL}" ]; then
+		merge_cus="--merge_cus"
+	fi
+
 	info "BTF" ${2}
-	LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
+	LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} ${merge_cus}
 
 	# Create ${2} which contains just .BTF section but no symbols. Add
 	# SHF_ALLOC because .BTF will be part of the vmlinux image. --strip-all
-- 
2.30.2