Re: [PATCH] perf symbol: correction while adjusting symbol

[Arnaldo Carvalho de Melo]

Em Thu, Nov 24, 2022 at 05:50:23PM +0800, Leo Yan escreveu:
> Hi Ajay,
> 
> On Wed, Nov 23, 2022 at 03:48:16PM +0530, Ajay Kaher wrote:
> > perf doesn't provide proper symbol information for specially crafted
> > .debug files.
> > 
> > Sometimes .debug file may not have similar program header as runtime
> > ELF file. For example if we generate .debug file using objcopy
> > --only-keep-debug resulting file will not contain .text, .data and
> > other runtime sections. That means corresponding program headers will
> > have zero FileSiz and modified Offset.
> > 
> > Example: program header of text section of libxxx.so:
> > 
> > Type           Offset             VirtAddr           PhysAddr
> >                FileSiz            MemSiz              Flags  Align
> > LOAD        0x00000000003d3000 0x00000000003d3000 0x00000000003d3000
> >             0x000000000055ae80 0x000000000055ae80  R E    0x1000
> > 
> > Same program header after executing:
> > objcopy --only-keep-debug libxxx.so libxxx.so.debug
> > 
> > LOAD        0x0000000000001000 0x00000000003d3000 0x00000000003d3000
> >             0x0000000000000000 0x000000000055ae80  R E    0x1000
> > 
> > Offset and FileSiz have been changed. 
> > 
> > Following formula will not provide correct value, if program header
> > taken from .debug file (syms_ss):
> > 
> >     sym.st_value -= phdr.p_vaddr - phdr.p_offset;
> > 
> > Correct program header information is located inside runtime ELF
> > file (runtime_ss).
> > 
> > Fixes: 2d86612aacb7 ("perf symbol: Correct address for bss symbols")
> > Signed-off-by: Ajay Kaher <akaher@xxxxxxxxxx>
> 
> 
> Just curious how you can produce this issue?  IIUC, the runtime symbol
> files are copied into .debug folder and they can be found by perf tool
> by matching build ID.  Seems to me, you manully use
> "objcopy --only-keep-debug" command to strip runtime info from elf files
> under .debug folder.  Do I understand correctly?
> 
> Though I have above question, this patch itself looks good to me,
> thanks for the fixing!
> 
> Reviewed-by: Leo Yan <leo.yan@xxxxxxxxxx>

Had to apply it manually, as it was done on a codebase older than this:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6d518ac7be6223811ab947897273b1bbef846180

author	Ian Rogers <irogers@xxxxxxxxxx>	2022-07-31 09:49:23 -0700
committer	Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>	2022-08-01 09:30:36 -0300

@@ -1305,16 +1305,29 @@ 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);