[PATCH] perf docs: Correct JIT_CODE_UNWINDING_INFO specification

[PATCH] perf docs: Correct JIT_CODE_UNWINDING_INFO specification

[Letian Jiang]

The JIT_CODE_UNWINDING_INFO description places the EH Frame Header before
the EH Frame. However, jit_add_eh_frame_info() uses the beginning of the
unwinding data for .eh_frame and the trailing eh_frame_hdr_size bytes for
.eh_frame_hdr.

Correct the unwinding_data description and the eh_frame_hdr_size field
description to match the layout consumed by perf.

Also document that an unwinding record precedes and applies to the next
JIT_CODE_LOAD record. perf caches the unwinding data and uses it when
processing that load record.

Keep the first sentence of the mapped_size note unchanged and remove the
second sentence, which claims that only the EH Frame Header is read when
the unwinding data is not mapped and associates this with FP-based
unwinding. perf copies the entire unwinding payload regardless of
mapped_size; the field affects the length of the synthesized MMAP event,
not which unwinding data is read.

Signed-off-by: Letian Jiang <letian.jiang@outlook.com>
---
 tools/perf/Documentation/jitdump-specification.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/jitdump-specification.txt b/tools/perf/Documentation/jitdump-specification.txt
index 7993635..c813c6e 100644
--- a/tools/perf/Documentation/jitdump-specification.txt
+++ b/tools/perf/Documentation/jitdump-specification.txt
@@ -153,12 +153,15 @@ The record type is optional.
 
 The record is used to describe the unwinding information for a jitted function.
 
+The record must precede the corresponding JIT_CODE_LOAD record. It applies to
+the next JIT_CODE_LOAD record in the file.
+
 The record has the following fields following the fixed-size record header in order:
 
 uint64_t unwind_data_size   : the size in bytes of the unwinding data table at the end of the record
-uint64_t eh_frame_hdr_size  : the size in bytes of the DWARF EH Frame Header at the start of the unwinding data table at the end of the record
+uint64_t eh_frame_hdr_size  : the size in bytes of the DWARF EH Frame Header at the end of the unwinding data table
 uint64_t mapped_size        : the size of the unwinding data mapped in memory
-const char unwinding_data[n]: an array of unwinding data, consisting of the EH Frame Header, followed by the actual EH Frame
+const char unwinding_data[n]: an array of unwinding data, consisting of the EH Frame, followed by the EH Frame Header
 
 
 The EH Frame header follows the Linux Standard Base (LSB) specification as described in the document at https://refspecs.linuxfoundation.org/LSB_1.3.0/gLSB/gLSB/ehframehdr.html
@@ -167,4 +170,4 @@ The EH Frame header follows the Linux Standard Base (LSB) specification as descr
 The EH Frame follows the LSB specification as described in the document at https://refspecs.linuxbase.org/LSB_3.0.0/LSB-PDA/LSB-PDA/ehframechpt.html
 
 
-NOTE: The mapped_size is generally either the same as unwind_data_size (if the unwinding data was mapped in memory by the running process) or zero (if the unwinding data is not mapped by the process). If the unwinding data was not mapped, then only the EH Frame Header will be read, which can be used to specify FP based unwinding for a function which does not have unwinding information.
+NOTE: The mapped_size is generally either the same as unwind_data_size (if the unwinding data was mapped in memory by the running process) or zero (if the unwinding data is not mapped by the process).

Re: [PATCH] perf docs: Correct JIT_CODE_UNWINDING_INFO specification

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf docs: jitdump spec wrongly drops mapped_size=0 unwinding limitation
--

commit fce68361b6cd7f448891e65b30a518cba3f31f05
Author: Letian Jiang <letian.jiang@outlook.com>

perf docs: Correct JIT_CODE_UNWINDING_INFO specification

This commit updates the JIT_CODE_UNWINDING_INFO documentation in
tools/perf/Documentation/jitdump-specification.txt to fix the ordering of the
EH Frame and EH Frame Header, document record ordering relative to
JIT_CODE_LOAD, and remove the second sentence of the mapped_size note.

> diff --git a/tools/perf/Documentation/jitdump-specification.txt b/tools/perf/Documentation/jitdump-specification.txt
> index 79936355d819a..c813c6ebbcf2d 100644
> --- a/tools/perf/Documentation/jitdump-specification.txt
> +++ b/tools/perf/Documentation/jitdump-specification.txt
> @@ -153,12 +153,15 @@ The record type is optional.

[ ... ]

> @@ -167,4 +170,4 @@ The EH Frame header follows the Linux Standard Base (LSB) specification as descr
>  The EH Frame follows the LSB specification as described in the document at https://refspecs.linuxbase.org/LSB_3.0.0/LSB-PDA/LSB-PDA/ehframechpt.html
>  
>  
> -NOTE: The mapped_size is generally either the same as unwind_data_size (if the unwinding data was mapped in memory by the running process) or zero (if the unwinding data is not mapped by the process). If the unwinding data was not mapped, then only the EH Frame Header will be read, which can be used to specify FP based unwinding for a function which does not have unwinding information.
> +NOTE: The mapped_size is generally either the same as unwind_data_size (if the unwinding data was mapped in memory by the running process) or zero (if the unwinding data is not mapped by the process).

[Severity: Medium]
Is it accurate to drop the second sentence of this note on the basis that
mapped_size does not affect which unwinding data is read?

While jit_repipe_unwinding_info() and jit_write_elf() copy the full unwinding
payload into the synthesized ELF file, jit_repipe_code_load() in
tools/perf/util/jitdump.c uses mapped_size (usize) to set the length of the
synthesized PERF_RECORD_MMAP2 event:

    event->mmap2.pgoff = GEN_ELF_TEXT_OFFSET;
    event->mmap2.start = addr;
    event->mmap2.len   = usize ? ALIGN_8(csize) + usize : csize;

During DWARF unwinding in tools/perf/util/unwind-libunwind.c,
read_unwind_spec_eh_frame() -> unwind_spec_ehframe() reads the fixed header
of .eh_frame_hdr (up to fde_count) directly by file offset:

    r = dso__data_read_offset(dso, machine, offset, (u8 *) &hdr, sizeof(hdr));
    if (r != sizeof(hdr))
        return -EINVAL;

However, libunwind reads the .eh_frame_hdr binary search table (table_data)
and the .eh_frame FDE/CIE entries by target virtual address via
__libunwind__access_mem() -> access_dso_mem():

    map = find_map(addr, ui);
    if (!map) {
        pr_debug("unwind: no map for %lx\n", (unsigned long)addr);
        return -1;
    }

When mapped_size is zero, the MMAP2 event only covers [addr, addr + csize)
and excludes the .eh_frame and .eh_frame_hdr sections placed at
addr + ALIGN_8(csize), so find_map() returns NULL for any table_data or
.eh_frame access.

As described in commit 0284fecd13b6 ("perf jit: Add unwinding support"), when
mapped_size is zero, only the .eh_frame_hdr header (with fde_count == 0 and
an empty lookup table) can be read to trigger frame-pointer unwinding
fallback, whereas reading .eh_frame FDEs requires a non-zero mapped_size.
Should this limitation remain documented in the specification?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CYXPR84MB347620A9EC3D829B4279F138826D2@CYXPR84MB3476.NAMPRD84.PROD.OUTLOOK.COM?part=1