Re: [PATCH] Revert "scripts/faddr2line: Check only two symbols when calculating symbol size"

[Will Deacon]

On Mon, Aug 12, 2024 at 11:01:20PM +0000, Carlos Llamas wrote:
> This reverts commit c02904f05ff805d6c0631634d5751ebd338f75ec.
> 
> Such commit assumed that only two symbols are relevant for the symbol
> size calculation. However, this can lead to an incorrect symbol size
> calculation when there are mapping symbols emitted by readelf.
> 
> For instance, when feeding 'update_irq_load_avg+0x1c/0x1c4', faddr2line
> might need to processes the following readelf lines:
> 
>  784284: ffffffc0081cca30   428 FUNC    GLOBAL DEFAULT     2 update_irq_load_avg
>   87319: ffffffc0081ccb0c     0 NOTYPE  LOCAL  DEFAULT     2 $x.62522
>   87321: ffffffc0081ccbdc     0 NOTYPE  LOCAL  DEFAULT     2 $x.62524
>   87323: ffffffc0081ccbe0     0 NOTYPE  LOCAL  DEFAULT     2 $x.62526
>   87325: ffffffc0081ccbe4     0 NOTYPE  LOCAL  DEFAULT     2 $x.62528
>   87327: ffffffc0081ccbe8     0 NOTYPE  LOCAL  DEFAULT     2 $x.62530
>   87329: ffffffc0081ccbec     0 NOTYPE  LOCAL  DEFAULT     2 $x.62532
>   87331: ffffffc0081ccbf0     0 NOTYPE  LOCAL  DEFAULT     2 $x.62534
>   87332: ffffffc0081ccbf4     0 NOTYPE  LOCAL  DEFAULT     2 $x.62535
>  783403: ffffffc0081ccbf4   424 FUNC    GLOBAL DEFAULT     2 sched_pelt_multiplier
> 
> The symbol size of 'update_irq_load_avg' should be calculated with the
> address of 'sched_pelt_multiplier', after skipping the mapping symbols
> seen in between. However, the offending commit cuts the list short and
> faddr2line incorrectly assumes 'update_irq_load_avg' is the last symbol
> in the section, resulting in:
> 
>   $ scripts/faddr2line vmlinux update_irq_load_avg+0x1c/0x1c4
>   skipping update_irq_load_avg address at 0xffffffc0081cca4c due to size mismatch (0x1c4 != 0x3ff9a59988)
>   no match for update_irq_load_avg+0x1c/0x1c4
> 
> After reverting the commit the issue is resolved:
> 
>   $ scripts/faddr2line vmlinux update_irq_load_avg+0x1c/0x1c4
>   update_irq_load_avg+0x1c/0x1c4:
>   cpu_of at kernel/sched/sched.h:1109
>   (inlined by) update_irq_load_avg at kernel/sched/pelt.c:481
> 
> Cc: Brian Johannesmeyer <bjohannesmeyer@xxxxxxxxx>
> Cc: John Stultz <jstultz@xxxxxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Signed-off-by: Carlos Llamas <cmllamas@xxxxxxxxxx>
> ---
>  scripts/faddr2line | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Will Deacon <will@xxxxxxxxxx>

I'm assuming that Josh will pick this up.

Cheers,

Will