Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 11 Jul 2022 09:09:37 -0700 | Subject | Re: [RFC PATCH v1] perf symbol: Correct address for bss symbols |
| |
On Sat, Jul 9, 2022 at 6:22 PM Leo Yan <leo.yan@linaro.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 .data: > > 0000000000004000 <__data_start>: > ... > > 0000000000004008 <__dso_handle>: > 4008: 08 40 00 or %al,0x0(%rax) > 400b: 00 00 add %al,(%rax) > 400d: 00 00 add %al,(%rax) > ... > > 0000000000004010 <wait_to_begin>: > 4010: 01 00 add %eax,(%rax) > 4012: 00 00 add %al,(%rax) > 4014: 00 00 add %al,(%rax) > ... > > 0000000000004018 <lock_thd_name>: > 4018: 08 20 or %ah,(%rax) > 401a: 00 00 add %al,(%rax) > 401c: 00 00 add %al,(%rax) > ... > > 0000000000004020 <reader_thd_name>: > 4020: 10 20 adc %ah,(%rax) > 4022: 00 00 add %al,(%rax) > 4024: 00 00 add %al,(%rax) > ... > > 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, here 'st_value' is the > address from executable file, 'sh_addr' is the belonged section's linked > start address, and 'sh_offset' is the dynamic loaded address for this > section, then perf tool uses below formula to adjust symbol address: > > adjusted_address = st_value - sh_addr + sh_offset > > So 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, on the other hand, we can see both 'st_value' > and 'sh_addr' are 64-byte aligned. Combining with disassembly, it's > likely libelf uses the .data section end address as .bss section > start address, therefore, it doesn't respect the alignment attribute for > structures in .bss section. > > Since .data and .bss sections are in the continuous virtual address > space, and .data section info returned by libelf is reliable, to fix > this issue, if detects it's a bss symbol, it rolls back to use .data > section info to adjust symbol's virtual address. > > Essentially, we need to fix libelf to return correct offsets for > sections, on the other hand, we live commonly with existed versions of > libelf. So we also need this change in perf tool. > > Fixes: f17e04afaff8 ("perf report: Fix ELF symbol parsing") > Reported-by: Chang Rui <changruinj@gmail.com> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
This looks good to me, I'm happy to add my: Acked-by: Ian Rogers <irogers@google.com> I've added Fangrui Song who is more knowledge-able on ELF, libelf, etc. than me and may have additional thoughts.
Thanks, Ian
> --- > tools/perf/util/dso.h | 1 + > tools/perf/util/symbol-elf.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 27 insertions(+) > > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > index 3a9fd4d389b5..00f57f4ac6bc 100644 > --- a/tools/perf/util/dso.h > +++ b/tools/perf/util/dso.h > @@ -180,6 +180,7 @@ struct dso { > u8 rel; > struct build_id bid; > u64 text_offset; > + int data_sec_index; > const char *short_name; > const char *long_name; > u16 long_name_len; > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index ecd377938eea..ed65dd26d58e 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -1095,6 +1095,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss, > Elf *elf; > int nr = 0; > bool remap_kernel = false, adjust_kernel_syms = false; > + size_t sec_index; > > if (kmap && !kmaps) > return -1; > @@ -1113,6 +1114,10 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss, > ".text", NULL)) > dso->text_offset = tshdr.sh_addr - tshdr.sh_offset; > > + if (elf_section_by_name(runtime_ss->elf, &runtime_ss->ehdr, &tshdr, > + ".data", &sec_index)) > + dso->data_sec_index = sec_index; > + > if (runtime_ss->opdsec) > opddata = elf_rawdata(runtime_ss->opdsec, NULL); > > @@ -1227,6 +1232,27 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss, > > gelf_getshdr(sec, &shdr); > > + /* > + * When the first data structure in .bss section is attributed > + * with alignment (e.g. 64-byte aligned), libelf doesn't reflect > + * the alignment in the 'shdr.sh_offset' field, at the end the > + * field is filled with the end loading address of a prior > + * section rather than the aligned address of .bss section. > + * This leads to mess for later parsing .bss symbols. > + * > + * Since .data and .bss sections are in the continuous virtual > + * address space, and .data section's info is reliable. So if > + * detects it's a bss symbol, we retrieve .data section info > + * for adjusting address. > + */ > + if (!strcmp(elf_sec__name(&shdr, secstrs_sym), ".bss")) { > + sec = elf_getscn(syms_ss->elf, dso->data_sec_index); > + if (!sec) > + goto out_elf_end; > + > + gelf_getshdr(sec, &shdr); > + } > + > secstrs = secstrs_sym; > > /* > -- > 2.25.1 >
| |