diff mbox series

perf symbol: correction while adjusting symbol

Message ID 1669198696-50547-1-git-send-email-akaher@vmware.com
State New
Headers
Series perf symbol: correction while adjusting symbol |

Commit Message

Ajay Kaher Nov. 23, 2022, 10:18 a.m. UTC 
  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@vmware.com>
---
 tools/perf/util/symbol-elf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Leo Yan Nov. 24, 2022, 9:50 a.m. UTC | #1 
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@vmware.com>


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@linaro.org>
Ajay Kaher Nov. 24, 2022, 10:20 a.m. UTC | #2 
> On 24/11/22, 3:20 PM, "Leo Yan" <leo.yan@linaro.org> wrote:
>>
>> Fixes: 2d86612aacb7 ("perf symbol: Correct address for bss symbols")
>> Signed-off-by: Ajay Kaher <akaher@vmware.com>
>
> 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?

While creating debuginfo rpm we can have extra stripping.

> Though I have above question, this patch itself looks good to me,
> thanks for the fixing!
>
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

Thanks.
- Ajay
Arnaldo Carvalho de Melo Nov. 24, 2022, 12:55 p.m. UTC | #3 
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@vmware.com>
> 
> 
> 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@linaro.org>

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@google.com>	2022-07-31 09:49:23 -0700
committer	Arnaldo Carvalho de Melo <acme@redhat.com>	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);
diff mbox series

Patch

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 5fba57c..8dde436 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1129,7 +1129,7 @@  int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 			   (!used_opd && syms_ss->adjust_symbols)) {
 			GElf_Phdr phdr;
 
-			if (elf_read_program_header(syms_ss->elf,
+			if (elf_read_program_header(runtime_ss->elf,
 						    (u64)sym.st_value, &phdr)) {
 				pr_warning("%s: failed to find program header for "
 					   "symbol: %s st_value: %#" PRIx64 "\n",