| 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
| ||
|
Date: Fri, 29 Jul 2022 22:13:04 -0700
From: Ian Rogers <irogers@...gle.com>
To: Leo Yan <leo.yan@...aro.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Fangrui Song <maskray@...gle.com>,
Chang Rui <changruinj@...il.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] perf symbol: Correct address for bss symbols
On Sat, Jul 23, 2022 at 11:00 PM Leo Yan <leo.yan@...aro.org> wrote:
>
> When using 'perf mem' and 'perf c2c', an issue is observed that tool
> reports the wrong offset for global data symbols. This is a common
> issue on both x86 and Arm64 platforms.
>
> Let's see an example, for a test program, below is the disassembly for
> its .bss section which is dumped with objdump:
>
> ...
>
> Disassembly of section .bss:
>
> 0000000000004040 <completed.0>:
> ...
>
> 0000000000004080 <buf1>:
> ...
>
> 00000000000040c0 <buf2>:
> ...
>
> 0000000000004100 <thread>:
> ...
>
> First we used 'perf mem record' to run the test program and then used
> 'perf --debug verbose=4 mem report' to observe what's the symbol info
> for 'buf1' and 'buf2' structures.
>
> # ./perf mem record -e ldlat-loads,ldlat-stores -- false_sharing.exe 8
> # ./perf --debug verbose=4 mem report
> ...
> dso__load_sym_internal: adjusting symbol: st_value: 0x40c0 sh_addr: 0x4040 sh_offset: 0x3028
> symbol__new: buf2 0x30a8-0x30e8
> ...
> dso__load_sym_internal: adjusting symbol: st_value: 0x4080 sh_addr: 0x4040 sh_offset: 0x3028
> symbol__new: buf1 0x3068-0x30a8
> ...
>
> Perf tool relies on libelf to parse symbols, in executable and shared
> object files, 'st_value' holds a virtual address; 'sh_addr' is the
> address at which section's first byte should reside in memory, and
> 'sh_offset' is the byte offset from the beginning of the file to the
> first byte in the section. Perf tool uses below formula to convert
> symbol's memory address to file address:
>
> file_address = st_value - sh_addr + sh_offset
> ^
> ` Memory address
>
> We can see the final adjusted address ranges for buf1 and buf2 are
> [0x30a8-0x30e8) and [0x3068-0x30a8) respectively, apparently this is
> incorrect, in the code, the structure for 'buf1' and 'buf2' specifies
> compiler attribute with 64-byte alignment.
>
> The problem happens for 'sh_offset', libelf returns it as 0x3028 which
> is not 64-byte aligned, combining with disassembly, it's likely libelf
> doesn't respect the alignment for .bss section, therefore, it doesn't
> return the aligned value for 'sh_offset'.
>
> Suggested by Fangrui Song, elf file contains program header which
> contains PT_LOAD segments, the fields p_vaddr and p_offset in PT_LOAD
> segments contain the execution info. A better choice for converting
> memory address to file address is using the formula:
>
> file_address = st_value - p_vaddr + p_offset
>
> This patch introduces function elf_read_program_header() which returns
> out the program header based on the passed 'st_value', then it uses
> above formula to calculate the symbol file address; and the debugging
> log is updated respectively.
>
> After applying the change:
>
> # ./perf --debug verbose=4 mem report
> ...
> dso__load_sym_internal: adjusting symbol: st_value: 0x40c0 p_vaddr: 0x3d28 p_offset: 0x2d28
> symbol__new: buf2 0x30c0-0x3100
> ...
> dso__load_sym_internal: adjusting symbol: st_value: 0x4080 p_vaddr: 0x3d28 p_offset: 0x2d28
> symbol__new: buf1 0x3080-0x30c0
> ...
>
> Fixes: f17e04afaff8 ("perf report: Fix ELF symbol parsing")
> Reported-by: Chang Rui <changruinj@...il.com>
> Suggested-by: Fangrui Song <maskray@...gle.com>
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
I am seeing a problem with this patch with jvmti. To repro:
1) download a Java workload dacapo-9.12-MR1-bach.jar from
https://sourceforge.net/projects/dacapobench/
2) build perf such as "make -C tools/perf O=/tmp/perf NO_LIBBFD=1" it
should detect Java and create /tmp/perf/libperf-jvmti.so
3) run perf with the jvmti agent:
/tmp/perf/perf record -k 1 java -agentpath:/tmp/perf/libperf-jvmti.so
-jar dacapo-9.12-MR1-bach.jar -n 10 fop
4) run perf inject:
/tmp/perf/perf inject -i perf.data -o perf-injected.data -j
5) run perf report
/tmp/perf/perf report -i perf-injected.data | grep org.apache.fop
With this patch reverted I see lots of symbols like:
0.00% java jitted-388040-4656.so [.]
org.apache.fop.fo.FObj.bind(org.apache.fop.fo.PropertyList)
With the patch I see lots of:
dso__load_sym_internal: failed to find program header for symbol:
Lorg/apache/fop/fo/FObj;bind(Lorg/apache/fop/fo/PropertyList;)V
st_value: 0x40
> ---
> tools/perf/util/symbol-elf.c | 45 ++++++++++++++++++++++++++++++++----
> 1 file changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index ecd377938eea..ef6ced5c5746 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -233,6 +233,33 @@ Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
> return NULL;
> }
>
> +static int elf_read_program_header(Elf *elf, u64 vaddr, GElf_Phdr *phdr)
> +{
> + size_t i, phdrnum;
> + u64 sz;
> +
> + if (elf_getphdrnum(elf, &phdrnum))
> + return -1;
> +
> + for (i = 0; i < phdrnum; i++) {
> + if (gelf_getphdr(elf, i, phdr) == NULL)
> + return -1;
> +
> + if (phdr->p_type != PT_LOAD)
> + continue;
> +
> + sz = max(phdr->p_memsz, phdr->p_filesz);
> + if (!sz)
> + continue;
> +
> + if (vaddr >= phdr->p_vaddr && (vaddr < phdr->p_vaddr + sz))
> + return 0;
> + }
> +
> + /* Not found any valid program header */
> + return -1;
> +}
> +
> static bool want_demangle(bool is_kernel_sym)
> {
> return is_kernel_sym ? symbol_conf.demangle_kernel : symbol_conf.demangle;
> @@ -1209,6 +1236,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
> sym.st_value);
> used_opd = true;
> }
> +
> /*
> * When loading symbols in a data mapping, ABS symbols (which
> * has a value of SHN_ABS in its st_shndx) failed at
> @@ -1262,11 +1290,20 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
> goto out_elf_end;
> } else if ((used_opd && runtime_ss->adjust_symbols) ||
> (!used_opd && syms_ss->adjust_symbols)) {
> + GElf_Phdr phdr;
> +
> + if (elf_read_program_header(syms_ss->elf,
> + (u64)sym.st_value, &phdr)) {
> + pr_warning("%s: failed to find program header for "
> + "symbol: %s st_value: %#" PRIx64 "\n",
> + __func__, elf_name, (u64)sym.st_value);
> + continue;
> + }
> pr_debug4("%s: adjusting symbol: st_value: %#" PRIx64 " "
> - "sh_addr: %#" PRIx64 " sh_offset: %#" PRIx64 "\n", __func__,
> - (u64)sym.st_value, (u64)shdr.sh_addr,
> - (u64)shdr.sh_offset);
> - sym.st_value -= shdr.sh_addr - shdr.sh_offset;
> + "p_vaddr: %#" PRIx64 " p_offset: %#" PRIx64 "\n",
> + __func__, (u64)sym.st_value, (u64)phdr.p_vaddr,
> + (u64)phdr.p_offset);
> + sym.st_value -= phdr.p_vaddr - phdr.p_offset;
> }
Combining the old and new behaviors fixes the issue for me, wdyt?
```
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1305,16 +1305,21 @@ dso__load_sym_internal(struct dso *dso, struct
map *map, struct symsrc *syms
_ss,
if (elf_read_program_header(syms_ss->elf,
(u64)sym.st_value, &phdr)) {
- pr_warning("%s: failed to find program
header for "
+ pr_debug4("%s: failed to find program
header for "
"symbol: %s st_value: %#" PRIx64 "\n",
__func__, elf_name,
(u64)sym.st_value);
- continue;
+ pr_debug4("%s: adjusting symbol:
st_value: %#" PRIx64 " "
+ "sh_addr: %#" PRIx64 "
sh_offset: %#" PRIx64 "\n",
+ __func__, (u64)sym.st_value,
(u64)shdr.sh_addr,
+ (u64)shdr.sh_offset);
+ sym.st_value -= shdr.sh_addr - shdr.sh_offset;
+ } else {
+ pr_debug4("%s: adjusting symbol:
st_value: %#" PRIx64 " "
+ "p_vaddr: %#" PRIx64 "
p_offset: %#" PRIx64 "\n",
+ __func__, (u64)sym.st_value,
(u64)phdr.p_vaddr,
+ (u64)phdr.p_offset);
+ sym.st_value -= phdr.p_vaddr - phdr.p_offset;
}
- pr_debug4("%s: adjusting symbol: st_value: %#"
PRIx64 " "
- "p_vaddr: %#" PRIx64 " p_offset: %#"
PRIx64 "\n",
- __func__, (u64)sym.st_value,
(u64)phdr.p_vaddr,
- (u64)phdr.p_offset);
- sym.st_value -= phdr.p_vaddr - phdr.p_offset;
}
demangled = demangle_sym(dso, kmodule, elf_name);
```
Thanks,
Ian
>
> demangled = demangle_sym(dso, kmodule, elf_name);
> --
> 2.25.1
>
Powered by blists - more mailing lists