Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Undefined behavior in `btf_trim_retval()` when evaluating `GENMASK()` for zero-sized types like empty structs. - [Medium] Zero-extension of signed return values breaks display formatting for negative integers. --
commit 9a068f6032f17c7055ec2012793c0a9a00b1e9a0 Author: pengdonglin <[email protected]> tracing: Update funcgraph-retval documentation The existing documentation for funcgraph-retval is outdated and partially incorrect, as it describes limitations that have now been resolved. Recent changes (e.g., using BTF to obtain function return types) have addressed key issues. > diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst > index 7261f25f8b4ba..696bfbede5905 100644 > --- a/Documentation/trace/ftrace.rst > +++ b/Documentation/trace/ftrace.rst [ ... ] > +- Under certain procedure-call standards (e.g., arm64's AAPCS64), when the > return > + type is smaller than a general-purpose register (GPR), the caller is > responsible > + for narrowing the value; the upper bits of the register may contain > undefined data. > + For instance, when a u8 is returned in 64-bit GPR, bits [63:8] can hold > arbitrary > + values, especially when larger types are truncated (explicitly or > implicitly). It > + is therefore advisable to inspect the code in such cases. If > CONFIG_DEBUG_INFO_BTF > + is enabled (y), the return value is automatically trimmed to the width of > the return > + type. [Severity: High] Does the trimming logic in btf_trim_retval() handle empty structs correctly? Looking at btf_trim_retval() in kernel/trace/trace_btf.c: msb = BITS_PER_BYTE * t->size - 1; *retval &= GENMASK(msb, 0); If a function returns a zero-sized type (like an empty struct), t->size is 0. This causes msb to underflow to 0xFFFFFFFF, which means GENMASK(0xFFFFFFFF, 0) attempts a shift by 64. Can this trigger undefined behavior and potentially a UBSAN panic if CONFIG_UBSAN_SHIFT is enabled? [Severity: Medium] Also, does this zero-extension break display formatting for negative integers? When a function returns a negative value, btf_trim_retval() zeroes out the upper bits. Later in kernel/trace/trace_functions_graph.c:print_graph_retval(): if (!IS_ERR_VALUE(err_code)) err_code = 0; // ... trace_seq_printf(s, " ret=%ld", err_code ?: retval); For non-error negative values (like -5000) or small signed types (like s8), IS_ERR_VALUE() evaluates to false, resetting err_code to 0. The code then falls back to printing the zero-extended retval as a positive number using %ld. Won't this cause these negative returns to be printed as large positive numbers instead? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
