All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Maguire <alan.maguire@oracle.com>
To: acme@kernel.org, olsajiri@gmail.com, ast@kernel.org
Cc: daniel@iogearbox.net, andrii@kernel.org, yhs@fb.com,
	eddyz87@gmail.com, sinquersw@gmail.com, timo@incline.eu,
	songliubraving@fb.com, john.fastabend@gmail.com,
	kpsingh@chromium.org, sdf@google.com, haoluo@google.com,
	martin.lau@kernel.org, bpf@vger.kernel.org,
	Alan Maguire <alan.maguire@oracle.com>
Subject: [RFC dwarves 2/4] btf_encoder: exclude functions with unexpected param register use not optimizations
Date: Fri, 17 Feb 2023 23:10:31 +0000	[thread overview]
Message-ID: <1676675433-10583-3-git-send-email-alan.maguire@oracle.com> (raw)
In-Reply-To: <1676675433-10583-1-git-send-email-alan.maguire@oracle.com>

A key problem with the existing approach of excluding functions
with optimized-out parameters is that it does not distinguish
between cases where a parameter was passed but not used versus
cases where a parameter is not passed or used.  The latter
is the only problematic case as for the former case, the
expected register states at function call time are as
they would be absent optimization.  Ideally call-site analysis
could tell us what actually happens when a function is called,
but in practice we use a different method to exclude functions
from BTF representation.  If a function clearly violates the
calling-convention expectations, where such a violation
means "parameter X does not use the expected register
as specified in calling conventions", it is excluded.

Note that we continue to mark functions which have optimized-out
parameters as this is good to know, but only actively exclude
functions with unexpected register usage for parameters or
multiple inconsistent function prototypes.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 btf_encoder.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index ea5b47b..da776f4 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -871,14 +871,16 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
 
 		/* If saving and we find an existing entry, we want to merge
 		 * observations across both functions, checking that the
-		 * "seen optimized parameters" and "inconsistent prototype"
-		 * status is reflected in the func entry.
+		 * "seen optimized parameters", "inconsistent prototype"
+		 * and "unexpected register" status is reflected in the
+		 * the func entry.
 		 * If the entry is new, record encoder state required
 		 * to add the local function later (encoder + type_id_off)
 		 * such that we can add the function later.
 		 */
 		existing->proto.optimized_parms |= fn->proto.optimized_parms;
-		if (!existing->proto.optimized_parms && !existing->proto.inconsistent_proto &&
+		existing->proto.unexpected_reg |= fn->proto.unexpected_reg;
+		if (!existing->proto.unexpected_reg && !existing->proto.inconsistent_proto &&
 		     !funcs__match(encoder, func, fn))
 			existing->proto.inconsistent_proto = 1;
 	} else {
@@ -940,20 +942,26 @@ static void btf_encoder__add_saved_funcs(struct btf_encoder *encoder)
 			if (!other_fn)
 				continue;
 			fn->proto.optimized_parms |= other_fn->proto.optimized_parms;
+			fn->proto.unexpected_reg |= other_fn->proto.unexpected_reg;
 			if (other_fn->proto.inconsistent_proto)
 				fn->proto.inconsistent_proto = 1;
-			if (!fn->proto.optimized_parms && !fn->proto.inconsistent_proto &&
+			if (!fn->proto.unexpected_reg && !fn->proto.inconsistent_proto &&
 			    !funcs__match(encoder, func, other_fn))
 				fn->proto.inconsistent_proto = 1;
 			other_fn->proto.processed = 1;
 		}
-		if (fn->proto.optimized_parms || fn->proto.inconsistent_proto) {
+		/* do not exclude functions with optimized-out parameters; they
+		 * may still be _called_ with the right parameter values, they
+		 * just do not _use_ them.  Only exclude functions with
+		 * unexpected register use or multiple inconsistent prototypes.
+		 */
+		if (fn->proto.unexpected_reg || fn->proto.inconsistent_proto) {
 			if (encoder->verbose) {
 				const char *name = function__name(fn);
 
 				printf("skipping addition of '%s'(%s) due to %s\n",
 				       name, fn->alias ?: name,
-				       fn->proto.optimized_parms ? "optimized-out parameters" :
+				       fn->proto.unexpected_reg ? "unexpected register used for parameter" :
 								   "multiple inconsistent function prototypes");
 			}
 		} else {
@@ -1856,7 +1864,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 						printf("matched function '%s' with '%s'%s\n",
 						       name, func->name,
 						       fn->proto.optimized_parms ?
-						       ", has optimized-out parameters" : "");
+						       ", has optimized-out parameters" :
+						       fn->proto.unexpected_reg ? ", has unexpected register use by params" :
+						       "");
 					fn->alias = func->name;
 				}
 			}
-- 
2.31.1


  parent reply	other threads:[~2023-02-18  0:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-17 23:10 [RFC dwarves 0/4] dwarves: change BTF encoding skip logic for functions Alan Maguire
2023-02-17 23:10 ` [RFC dwarves 1/4] dwarf_loader: mark functions that do not use expected registers for params Alan Maguire
2023-02-20 19:03   ` Alexei Starovoitov
2023-02-20 19:42     ` Alan Maguire
2023-02-20 22:30       ` Alexei Starovoitov
2023-02-21 15:52         ` Alan Maguire
2023-02-17 23:10 ` Alan Maguire [this message]
2023-02-17 23:10 ` [RFC dwarves 3/4] pahole: update descriptions for btf_gen_optimized, skip_encoding_btf_inconsistent_proto Alan Maguire
2023-02-17 23:10 ` [RFC dwarves 4/4] pahole: update man page for options also Alan Maguire
2023-02-18 14:12 ` [RFC dwarves 0/4] dwarves: change BTF encoding skip logic for functions Arnaldo Carvalho de Melo
2023-02-18 14:37   ` Arnaldo Carvalho de Melo
2023-02-19 22:23 ` Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1676675433-10583-3-git-send-email-alan.maguire@oracle.com \
    --to=alan.maguire@oracle.com \
    --cc=acme@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kpsingh@chromium.org \
    --cc=martin.lau@kernel.org \
    --cc=olsajiri@gmail.com \
    --cc=sdf@google.com \
    --cc=sinquersw@gmail.com \
    --cc=songliubraving@fb.com \
    --cc=timo@incline.eu \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.