2023-10-02 17:05:42

by Will Deacon

[permalink] [raw]
Subject: [PATCH v5 0/3] Fix 'faddr2line' for LLVM arm64 builds

Hello again.

This is version five of the fadd2line fixes that I previously posted at:

v1: https://lore.kernel.org/r/[email protected]
v2: https://lore.kernel.org/r/[email protected]
v3: https://lore.kernel.org/r/[email protected]
v4: https://lore.kernel.org/r/[email protected]

Changes since v4 include:
* Simplify the is_mapping_symbol() regex to reflect the latest version
of the C code.
* Add Suggested-by and Reviewed-by tags to patch 1.

Cheers,

Will

Cc: Masahiro Yamada <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Nicolas Schier <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: John Stultz <[email protected]>
Cc: [email protected]

--->8

Will Deacon (3):
scripts/faddr2line: Don't filter out non-function symbols from readelf
scripts/faddr2line: Use LLVM addr2line and readelf if LLVM=1
scripts/faddr2line: Skip over mapping symbols in output from readelf

scripts/faddr2line | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)

--
2.42.0.582.g8ccd20d70d-goog


2023-10-02 17:11:00

by Will Deacon

[permalink] [raw]
Subject: [PATCH v5 1/3] scripts/faddr2line: Don't filter out non-function symbols from readelf

As Josh points out in 20230724234734.zy67gm674vl3p3wv@treble:

> Problem is, I think the kernel's symbol printing code prints the
> nearest kallsyms symbol, and there are some valid non-FUNC code
> symbols. For example, syscall_return_via_sysret.

so we shouldn't be considering only 'FUNC'-type symbols in the output
from readelf.

Drop the function symbol type filtering from the faddr2line outer loop.

Cc: John Stultz <[email protected]>
Suggested-by: Josh Poimboeuf <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Link: https://lore.kernel.org/r/20230724234734.zy67gm674vl3p3wv@treble
Signed-off-by: Will Deacon <[email protected]>
---
scripts/faddr2line | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/faddr2line b/scripts/faddr2line
index 0e73aca4f908..a35a420d0f26 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -260,7 +260,7 @@ __faddr2line() {

DONE=1

- done < <(${READELF} --symbols --wide $objfile | sed 's/\[.*\]//' | ${AWK} -v fn=$sym_name '$4 == "FUNC" && $8 == fn')
+ done < <(${READELF} --symbols --wide $objfile | sed 's/\[.*\]//' | ${AWK} -v fn=$sym_name '$8 == fn')
}

[[ $# -lt 2 ]] && usage
--
2.42.0.582.g8ccd20d70d-goog

2023-10-02 17:12:58

by Will Deacon

[permalink] [raw]
Subject: [PATCH v5 3/3] scripts/faddr2line: Skip over mapping symbols in output from readelf

Mapping symbols emitted in the readelf output can confuse the
'faddr2line' symbol size calculation, resulting in the erroneous
rejection of valid offsets. This is especially prevalent when building
an arm64 kernel with CONFIG_CFI_CLANG=y, where most functions are
prefixed with a 32-bit data value in a '$d.n' section. For example:

447538: ffff800080014b80 548 FUNC GLOBAL DEFAULT 2 do_one_initcall
104: ffff800080014c74 0 NOTYPE LOCAL DEFAULT 2 $x.73
106: ffff800080014d30 0 NOTYPE LOCAL DEFAULT 2 $x.75
111: ffff800080014da4 0 NOTYPE LOCAL DEFAULT 2 $d.78
112: ffff800080014da8 0 NOTYPE LOCAL DEFAULT 2 $x.79
36: ffff800080014de0 200 FUNC LOCAL DEFAULT 2 run_init_process

Adding a warning to do_one_initcall() results in:

| WARNING: CPU: 0 PID: 1 at init/main.c:1236 do_one_initcall+0xf4/0x260

Which 'faddr2line' refuses to accept:

$ ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
skipping do_one_initcall address at 0xffff800080014c74 due to size mismatch (0x260 != 0x224)
no match for do_one_initcall+0xf4/0x260

Filter out these entries from readelf using a shell reimplementation of
is_mapping_symbol(), so that the size of a symbol is calculated as a
delta to the next symbol present in ksymtab.

Cc: Josh Poimboeuf <[email protected]>
Cc: John Stultz <[email protected]>
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
---
scripts/faddr2line | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/scripts/faddr2line b/scripts/faddr2line
index 6b8206802157..587415a52b6f 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -179,6 +179,11 @@ __faddr2line() {
local cur_sym_elf_size=${fields[2]}
local cur_sym_name=${fields[7]:-}

+ # is_mapping_symbol(cur_sym_name)
+ if [[ ${cur_sym_name} =~ ^(\.L|L0|\$) ]]; then
+ continue
+ fi
+
if [[ $cur_sym_addr = $sym_addr ]] &&
[[ $cur_sym_elf_size = $sym_elf_size ]] &&
[[ $cur_sym_name = $sym_name ]]; then
--
2.42.0.582.g8ccd20d70d-goog

2023-10-02 17:14:19

by Will Deacon

[permalink] [raw]
Subject: [PATCH v5 2/3] scripts/faddr2line: Use LLVM addr2line and readelf if LLVM=1

GNU utilities cannot necessarily parse objects built by LLVM, which can
result in confusing errors when using 'faddr2line':

$ CROSS_COMPILE=aarch64-linux-gnu- ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn'
aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25
do_one_initcall+0xf4/0x260:
aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn'
aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25
$x.73 at main.c:?

Although this can be worked around by setting CROSS_COMPILE to "llvm=-",
it's cleaner to follow the same syntax as the top-level Makefile and
accept LLVM= as an indication to use the llvm- tools, optionally
specifying their location or specific version number.

Cc: Josh Poimboeuf <[email protected]>
Cc: John Stultz <[email protected]>
Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
---
scripts/faddr2line | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/faddr2line b/scripts/faddr2line
index a35a420d0f26..6b8206802157 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -58,8 +58,21 @@ die() {
exit 1
}

-READELF="${CROSS_COMPILE:-}readelf"
-ADDR2LINE="${CROSS_COMPILE:-}addr2line"
+UTIL_SUFFIX=""
+if [[ "${LLVM:-}" == "" ]]; then
+ UTIL_PREFIX=${CROSS_COMPILE:-}
+else
+ UTIL_PREFIX=llvm-
+
+ if [[ "${LLVM}" == *"/" ]]; then
+ UTIL_PREFIX=${LLVM}${UTIL_PREFIX}
+ elif [[ "${LLVM}" == "-"* ]]; then
+ UTIL_SUFFIX=${LLVM}
+ fi
+fi
+
+READELF="${UTIL_PREFIX}readelf${UTIL_SUFFIX}"
+ADDR2LINE="${UTIL_PREFIX}addr2line${UTIL_SUFFIX}"
AWK="awk"
GREP="grep"

--
2.42.0.582.g8ccd20d70d-goog

2023-10-02 21:06:00

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v5 2/3] scripts/faddr2line: Use LLVM addr2line and readelf if LLVM=1

On Mon, Oct 2, 2023 at 9:58 AM Will Deacon <[email protected]> wrote:
>
> GNU utilities cannot necessarily parse objects built by LLVM, which can
> result in confusing errors when using 'faddr2line':
>
> $ CROSS_COMPILE=aarch64-linux-gnu- ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
> aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn'
> aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25
> do_one_initcall+0xf4/0x260:
> aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn'
> aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25
> $x.73 at main.c:?
>
> Although this can be worked around by setting CROSS_COMPILE to "llvm=-",
> it's cleaner to follow the same syntax as the top-level Makefile and
> accept LLVM= as an indication to use the llvm- tools, optionally
> specifying their location or specific version number.
>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: John Stultz <[email protected]>
> Suggested-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Will Deacon <[email protected]>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>

> ---
> scripts/faddr2line | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index a35a420d0f26..6b8206802157 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -58,8 +58,21 @@ die() {
> exit 1
> }
>
> -READELF="${CROSS_COMPILE:-}readelf"
> -ADDR2LINE="${CROSS_COMPILE:-}addr2line"
> +UTIL_SUFFIX=""
> +if [[ "${LLVM:-}" == "" ]]; then
> + UTIL_PREFIX=${CROSS_COMPILE:-}
> +else
> + UTIL_PREFIX=llvm-
> +
> + if [[ "${LLVM}" == *"/" ]]; then
> + UTIL_PREFIX=${LLVM}${UTIL_PREFIX}
> + elif [[ "${LLVM}" == "-"* ]]; then
> + UTIL_SUFFIX=${LLVM}
> + fi
> +fi
> +
> +READELF="${UTIL_PREFIX}readelf${UTIL_SUFFIX}"
> +ADDR2LINE="${UTIL_PREFIX}addr2line${UTIL_SUFFIX}"
> AWK="awk"
> GREP="grep"
>
> --
> 2.42.0.582.g8ccd20d70d-goog
>


--
Thanks,
~Nick Desaulniers

2023-10-02 22:21:27

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v5 3/3] scripts/faddr2line: Skip over mapping symbols in output from readelf

On Mon, Oct 2, 2023 at 9:58 AM Will Deacon <[email protected]> wrote:
>
> Mapping symbols emitted in the readelf output can confuse the
> 'faddr2line' symbol size calculation, resulting in the erroneous
> rejection of valid offsets. This is especially prevalent when building
> an arm64 kernel with CONFIG_CFI_CLANG=y, where most functions are
> prefixed with a 32-bit data value in a '$d.n' section. For example:
>
> 447538: ffff800080014b80 548 FUNC GLOBAL DEFAULT 2 do_one_initcall
> 104: ffff800080014c74 0 NOTYPE LOCAL DEFAULT 2 $x.73
> 106: ffff800080014d30 0 NOTYPE LOCAL DEFAULT 2 $x.75
> 111: ffff800080014da4 0 NOTYPE LOCAL DEFAULT 2 $d.78
> 112: ffff800080014da8 0 NOTYPE LOCAL DEFAULT 2 $x.79
> 36: ffff800080014de0 200 FUNC LOCAL DEFAULT 2 run_init_process
>
> Adding a warning to do_one_initcall() results in:
>
> | WARNING: CPU: 0 PID: 1 at init/main.c:1236 do_one_initcall+0xf4/0x260
>
> Which 'faddr2line' refuses to accept:
>
> $ ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
> skipping do_one_initcall address at 0xffff800080014c74 due to size mismatch (0x260 != 0x224)
> no match for do_one_initcall+0xf4/0x260
>
> Filter out these entries from readelf using a shell reimplementation of
> is_mapping_symbol(), so that the size of a symbol is calculated as a
> delta to the next symbol present in ksymtab.
>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: John Stultz <[email protected]>
> Suggested-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Will Deacon <[email protected]>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>

> ---
> scripts/faddr2line | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 6b8206802157..587415a52b6f 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -179,6 +179,11 @@ __faddr2line() {
> local cur_sym_elf_size=${fields[2]}
> local cur_sym_name=${fields[7]:-}
>
> + # is_mapping_symbol(cur_sym_name)
> + if [[ ${cur_sym_name} =~ ^(\.L|L0|\$) ]]; then
> + continue
> + fi
> +
> if [[ $cur_sym_addr = $sym_addr ]] &&
> [[ $cur_sym_elf_size = $sym_elf_size ]] &&
> [[ $cur_sym_name = $sym_name ]]; then
> --
> 2.42.0.582.g8ccd20d70d-goog
>


--
Thanks,
~Nick Desaulniers

2023-10-23 13:42:44

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] Fix 'faddr2line' for LLVM arm64 builds

Josh,

On Mon, Oct 02, 2023 at 05:57:46PM +0100, Will Deacon wrote:
> Hello again.
>
> This is version five of the fadd2line fixes that I previously posted at:
>
> v1: https://lore.kernel.org/r/[email protected]
> v2: https://lore.kernel.org/r/[email protected]
> v3: https://lore.kernel.org/r/[email protected]
> v4: https://lore.kernel.org/r/[email protected]
>
> Changes since v4 include:
> * Simplify the is_mapping_symbol() regex to reflect the latest version
> of the C code.
> * Add Suggested-by and Reviewed-by tags to patch 1.

Please can you pick this up for 6.7?

Will

2023-10-24 00:18:05

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v5 3/3] scripts/faddr2line: Skip over mapping symbols in output from readelf

On Tue, Oct 3, 2023 at 1:58 AM Will Deacon <[email protected]> wrote:
>
> Mapping symbols emitted in the readelf output can confuse the
> 'faddr2line' symbol size calculation, resulting in the erroneous
> rejection of valid offsets. This is especially prevalent when building
> an arm64 kernel with CONFIG_CFI_CLANG=y, where most functions are
> prefixed with a 32-bit data value in a '$d.n' section. For example:
>
> 447538: ffff800080014b80 548 FUNC GLOBAL DEFAULT 2 do_one_initcall
> 104: ffff800080014c74 0 NOTYPE LOCAL DEFAULT 2 $x.73
> 106: ffff800080014d30 0 NOTYPE LOCAL DEFAULT 2 $x.75
> 111: ffff800080014da4 0 NOTYPE LOCAL DEFAULT 2 $d.78
> 112: ffff800080014da8 0 NOTYPE LOCAL DEFAULT 2 $x.79
> 36: ffff800080014de0 200 FUNC LOCAL DEFAULT 2 run_init_process
>
> Adding a warning to do_one_initcall() results in:
>
> | WARNING: CPU: 0 PID: 1 at init/main.c:1236 do_one_initcall+0xf4/0x260
>
> Which 'faddr2line' refuses to accept:
>
> $ ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
> skipping do_one_initcall address at 0xffff800080014c74 due to size mismatch (0x260 != 0x224)
> no match for do_one_initcall+0xf4/0x260
>
> Filter out these entries from readelf using a shell reimplementation of
> is_mapping_symbol(), so that the size of a symbol is calculated as a
> delta to the next symbol present in ksymtab.
>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: John Stultz <[email protected]>
> Suggested-by: Masahiro Yamada <[email protected]>
> Signed-off-by: Will Deacon <[email protected]>
> ---
> scripts/faddr2line | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/scripts/faddr2line b/scripts/faddr2line
> index 6b8206802157..587415a52b6f 100755
> --- a/scripts/faddr2line
> +++ b/scripts/faddr2line
> @@ -179,6 +179,11 @@ __faddr2line() {
> local cur_sym_elf_size=${fields[2]}
> local cur_sym_name=${fields[7]:-}
>
> + # is_mapping_symbol(cur_sym_name)
> + if [[ ${cur_sym_name} =~ ^(\.L|L0|\$) ]]; then
> + continue
> + fi
> +
> if [[ $cur_sym_addr = $sym_addr ]] &&
> [[ $cur_sym_elf_size = $sym_elf_size ]] &&
> [[ $cur_sym_name = $sym_name ]]; then
> --
> 2.42.0.582.g8ccd20d70d-goog
>


Reviewed-by: Masahiro Yamada <[email protected]>


--
Best Regards
Masahiro Yamada

Subject: [tip: objtool/core] scripts/faddr2line: Skip over mapping symbols in output from readelf

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: 60fd39af33d3f63c4c94bd06784ebdf0d883f5c9
Gitweb: https://git.kernel.org/tip/60fd39af33d3f63c4c94bd06784ebdf0d883f5c9
Author: Will Deacon <[email protected]>
AuthorDate: Mon, 02 Oct 2023 17:57:49 +01:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Mon, 23 Oct 2023 08:36:46 -07:00

scripts/faddr2line: Skip over mapping symbols in output from readelf

Mapping symbols emitted in the readelf output can confuse the
'faddr2line' symbol size calculation, resulting in the erroneous
rejection of valid offsets. This is especially prevalent when building
an arm64 kernel with CONFIG_CFI_CLANG=y, where most functions are
prefixed with a 32-bit data value in a '$d.n' section. For example:

447538: ffff800080014b80 548 FUNC GLOBAL DEFAULT 2 do_one_initcall
104: ffff800080014c74 0 NOTYPE LOCAL DEFAULT 2 $x.73
106: ffff800080014d30 0 NOTYPE LOCAL DEFAULT 2 $x.75
111: ffff800080014da4 0 NOTYPE LOCAL DEFAULT 2 $d.78
112: ffff800080014da8 0 NOTYPE LOCAL DEFAULT 2 $x.79
36: ffff800080014de0 200 FUNC LOCAL DEFAULT 2 run_init_process

Adding a warning to do_one_initcall() results in:

| WARNING: CPU: 0 PID: 1 at init/main.c:1236 do_one_initcall+0xf4/0x260

Which 'faddr2line' refuses to accept:

$ ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
skipping do_one_initcall address at 0xffff800080014c74 due to size mismatch (0x260 != 0x224)
no match for do_one_initcall+0xf4/0x260

Filter out these entries from readelf using a shell reimplementation of
is_mapping_symbol(), so that the size of a symbol is calculated as a
delta to the next symbol present in ksymtab.

Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Josh Poimboeuf <[email protected]>
---
scripts/faddr2line | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/scripts/faddr2line b/scripts/faddr2line
index 6b82068..587415a 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -179,6 +179,11 @@ __faddr2line() {
local cur_sym_elf_size=${fields[2]}
local cur_sym_name=${fields[7]:-}

+ # is_mapping_symbol(cur_sym_name)
+ if [[ ${cur_sym_name} =~ ^(\.L|L0|\$) ]]; then
+ continue
+ fi
+
if [[ $cur_sym_addr = $sym_addr ]] &&
[[ $cur_sym_elf_size = $sym_elf_size ]] &&
[[ $cur_sym_name = $sym_name ]]; then

Subject: [tip: objtool/core] scripts/faddr2line: Use LLVM addr2line and readelf if LLVM=1

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: 86bf86e19d308a1dba41e5f1f7e8cc105a5efa49
Gitweb: https://git.kernel.org/tip/86bf86e19d308a1dba41e5f1f7e8cc105a5efa49
Author: Will Deacon <[email protected]>
AuthorDate: Mon, 02 Oct 2023 17:57:48 +01:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Mon, 23 Oct 2023 08:36:33 -07:00

scripts/faddr2line: Use LLVM addr2line and readelf if LLVM=1

GNU utilities cannot necessarily parse objects built by LLVM, which can
result in confusing errors when using 'faddr2line':

$ CROSS_COMPILE=aarch64-linux-gnu- ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn'
aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25
do_one_initcall+0xf4/0x260:
aarch64-linux-gnu-addr2line: vmlinux: unknown type [0x13] section `.relr.dyn'
aarch64-linux-gnu-addr2line: DWARF error: invalid or unhandled FORM value: 0x25
$x.73 at main.c:?

Although this can be worked around by setting CROSS_COMPILE to "llvm=-",
it's cleaner to follow the same syntax as the top-level Makefile and
accept LLVM= as an indication to use the llvm- tools, optionally
specifying their location or specific version number.

Suggested-by: Masahiro Yamada <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Josh Poimboeuf <[email protected]>
---
scripts/faddr2line | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/faddr2line b/scripts/faddr2line
index a35a420..6b82068 100755
--- a/scripts/faddr2line
+++ b/scripts/faddr2line
@@ -58,8 +58,21 @@ die() {
exit 1
}

-READELF="${CROSS_COMPILE:-}readelf"
-ADDR2LINE="${CROSS_COMPILE:-}addr2line"
+UTIL_SUFFIX=""
+if [[ "${LLVM:-}" == "" ]]; then
+ UTIL_PREFIX=${CROSS_COMPILE:-}
+else
+ UTIL_PREFIX=llvm-
+
+ if [[ "${LLVM}" == *"/" ]]; then
+ UTIL_PREFIX=${LLVM}${UTIL_PREFIX}
+ elif [[ "${LLVM}" == "-"* ]]; then
+ UTIL_SUFFIX=${LLVM}
+ fi
+fi
+
+READELF="${UTIL_PREFIX}readelf${UTIL_SUFFIX}"
+ADDR2LINE="${UTIL_PREFIX}addr2line${UTIL_SUFFIX}"
AWK="awk"
GREP="grep"