All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 dwarves 0/5] dwarves: support encoding of optimized-out parameters, removal of inconsistent static functions
@ 2023-01-30 14:29 Alan Maguire
  2023-01-30 14:29 ` [PATCH v2 dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters Alan Maguire
                   ` (4 more replies)
  0 siblings, 5 replies; 40+ messages in thread
From: Alan Maguire @ 2023-01-30 14:29 UTC (permalink / raw)
  To: acme, yhs, ast, olsajiri, eddyz87, sinquersw, timo
  Cc: daniel, andrii, songliubraving, john.fastabend, kpsingh, sdf,
	haoluo, martin.lau, bpf, Alan Maguire

At optimization level -O2 or higher in gcc, static functions may be
optimized such that they have suffixes like .isra.0, .constprop.0 etc.
These represent 
    
- constant propagation (.constprop.0);
- interprocedural scalar replacement of aggregates, removal of
  unused parameters and replacement of parameters passed by
  reference by parameters passed by value (.isra.0)
  
See [1] for details. 
    
Currently BTF encoding does not handle such optimized functions
that get renamed with a "." suffix such as ".isra.0", ".constprop.0".
This is safer because such suffixes can often indicate parameters have
been optimized out.  This series addresses this by matching a
function to a suffixed version ("foo" matching "foo.isra.0") while
ensuring that the function signature does not contain optimized-out
parameters.  Note that if the function is found ("foo") it will
be preferred, only falling back to "foo.isra.0" if lookup of the
function fails.  Addition to BTF is skipped if the function has
optimized-out parameters, since the expected function signature
will not match. BTF encoding does not include the "."-suffix to
be consistent with DWARF. In addition, the kernel currently does
not allow a "." suffix in a BTF function name.

A problem with this approach however is that BTF carries out the
encoding process in parallel across multiple CUs, and sometimes
a function has optimized-out parameters in one CU but not others;
we see this for NF_HOOK.constprop.0 for example.  So in order to
determine if the function has optimized-out parameters in any
CU, its addition is not carried out until we have processed all
CUs and are about to merge BTF.  At this point we know if any
such optimizations have occurred.  Patches 1-4 handle the
optimized-out parameter identification and matching "."-suffixed
functions with the original function to facilitate BTF
encoding.

Patch 5 addresses a related problem - it is entirely possible
for a static function of the same name to exist in different
CUs with different function signatures.  Because BTF does not
currently encode any information that would help disambiguate
which BTF function specification matches which static function
(in the case of multiple different function signatures), it is
best to eliminate such functions from BTF for now.  The same
mechanism that is used to compare static "."-suffixed functions
is re-used for the static function comparison.  A superficial
comparison of number of parameters/parameter names is done to
see if such representations are consistent, and if inconsistent
prototypes are observed, the function is flagged for exclusion
from BTF.

When these methods are combined - the additive encoding of
"."-suffixed functions and the subtractive elimination of
functions with inconsistent parameters - we see an overall
drop in the number of functions in vmlinux BTF, from
51150 to 49871.

Changes since v1 [2]

- Eduard noted that a DW_AT_const_value attribute can signal
  an optimized-out parameter, and that the lack of a location
  attribute signals optimization; ensure we handle those cases
  also (Eduard, patch 1).
- Jiri noted we can have inconsistencies between a static
  and non-static function; apply the comparison process to
  all functions (Jiri, patch 5)
- segmentation fault was observed when handling functions with
  > 10 parameters; needed parameter comparison loop to exit
  at BTF_ENCODER_MAX_PARAMETERS (patch 5)
- Kui-Feng Lee pointed out that having a global shared function
  tree would lead to a lot of contention; here a per-encoder 
  tree is used, and once the threads are collected the trees
  are merged. Performance numbers are provided in patch 5 
  (Kui-Feng Lee, patches 4/5)

Alan Maguire (5):
  dwarves: help dwarf loader spot functions with optimized-out
    parameters
  btf_encoder: refactor function addition into dedicated
    btf_encoder__add_func
  btf_encoder: rework btf_encoders__*() API to allow traversal of
    encoders
  btf_encoder: represent "."-suffixed functions (".isra.0") in BTF
  btf_encoder: delay function addition to check for function prototype
    inconsistencies

 btf_encoder.c  | 392 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 btf_encoder.h  |   6 -
 dwarf_loader.c | 125 ++++++++++++++++--
 dwarves.h      |   8 +-
 pahole.c       |  14 +--
 5 files changed, 464 insertions(+), 81 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2023-02-03  1:10 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30 14:29 [PATCH v2 dwarves 0/5] dwarves: support encoding of optimized-out parameters, removal of inconsistent static functions Alan Maguire
2023-01-30 14:29 ` [PATCH v2 dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters Alan Maguire
2023-01-30 18:36   ` Arnaldo Carvalho de Melo
2023-01-30 20:10     ` Arnaldo Carvalho de Melo
2023-01-30 20:23       ` Arnaldo Carvalho de Melo
2023-01-30 22:37         ` Alan Maguire
2023-01-31  0:25           ` Arnaldo Carvalho de Melo
2023-01-31  1:04             ` Arnaldo Carvalho de Melo
2023-01-31 12:14               ` Alan Maguire
2023-01-31 12:33                 ` Arnaldo Carvalho de Melo
2023-01-31 13:35                   ` Jiri Olsa
2023-01-31 17:43                 ` Alexei Starovoitov
2023-01-31 18:16                   ` Alexei Starovoitov
2023-01-31 23:45                     ` Alan Maguire
2023-01-31 23:58                       ` David Vernet
2023-02-01  0:14                         ` Alexei Starovoitov
2023-02-01  3:02                           ` David Vernet
2023-02-01 13:59                             ` Alan Maguire
2023-02-01 15:02                               ` Arnaldo Carvalho de Melo
2023-02-01 15:13                                 ` Alan Maguire
2023-02-01 15:19                                 ` David Vernet
2023-02-01 16:49                                   ` Alexei Starovoitov
2023-02-01 17:01                                     ` Arnaldo Carvalho de Melo
2023-02-01 17:18                                       ` Alan Maguire
2023-02-01 18:54                                         ` Arnaldo Carvalho de Melo
2023-02-01 22:33                                         ` Alan Maguire
2023-02-01 22:32                                       ` Arnaldo Carvalho de Melo
2023-02-02  1:09                                         ` Arnaldo Carvalho de Melo
2023-02-03  1:09                             ` Yonghong Song
2023-01-30 14:29 ` [PATCH v2 dwarves 2/5] btf_encoder: refactor function addition into dedicated btf_encoder__add_func Alan Maguire
2023-02-01 17:19   ` Arnaldo Carvalho de Melo
2023-02-01 17:50     ` Alan Maguire
2023-02-01 18:59       ` Arnaldo Carvalho de Melo
2023-01-30 14:29 ` [PATCH v2 dwarves 3/5] btf_encoder: rework btf_encoders__*() API to allow traversal of encoders Alan Maguire
2023-01-30 22:04   ` Jiri Olsa
2023-01-31  0:24     ` Arnaldo Carvalho de Melo
2023-01-30 14:29 ` [PATCH v2 dwarves 4/5] btf_encoder: represent "."-suffixed functions (".isra.0") in BTF Alan Maguire
2023-01-30 14:29 ` [PATCH v2 dwarves 5/5] btf_encoder: delay function addition to check for function prototype inconsistencies Alan Maguire
2023-01-30 17:20   ` Alexei Starovoitov
2023-01-30 18:08     ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.