bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Maguire <alan.maguire@oracle.com>
To: acme@kernel.org
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
	eddyz87@gmail.com, haoluo@google.com, jolsa@kernel.org,
	john.fastabend@gmail.com, kpsingh@chromium.org,
	sinquersw@gmail.com, martin.lau@kernel.org,
	songliubraving@fb.com, sdf@google.com, timo@incline.eu,
	yhs@fb.com, bpf@vger.kernel.org,
	Alan Maguire <alan.maguire@oracle.com>
Subject: [PATCH dwarves 3/3] btf_encoder: compare functions via prototypes not parameter names
Date: Fri, 10 Mar 2023 14:50:50 +0000	[thread overview]
Message-ID: <1678459850-16140-4-git-send-email-alan.maguire@oracle.com> (raw)
In-Reply-To: <1678459850-16140-1-git-send-email-alan.maguire@oracle.com>

Parameter names are a brittle choice for comparing function prototypes.
As noted by Jiri [1], a function can have a weak definition in one
CU with differing names from another definition, and because we require
name-based matches, we can omit functions unnecessarily.  Using a
conf_fprintf that omits parameter names, generate function prototypes
for string comparison instead.

[1] https://lore.kernel.org/bpf/ZAsBYpsBV0wvkhh0@krava/

Reported-by: Jira Olsa <olsajiri@gmail.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 btf_encoder.c | 67 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 07a9dc5..65f6e71 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -33,13 +33,13 @@
 #include <search.h> /* for tsearch(), tfind() and tdestroy() */
 #include <pthread.h>
 
-#define BTF_ENCODER_MAX_PARAMETERS	12
+#define BTF_ENCODER_MAX_PROTO	512
 
 /* state used to do later encoding of saved functions */
 struct btf_encoder_state {
 	uint32_t type_id_off;
-	bool got_parameter_names;
-	const char *parameter_names[BTF_ENCODER_MAX_PARAMETERS];
+	bool got_proto;
+	char proto[BTF_ENCODER_MAX_PROTO];
 };
 
 struct elf_function {
@@ -798,25 +798,25 @@ static int32_t btf_encoder__add_decl_tag(struct btf_encoder *encoder, const char
 	return id;
 }
 
-static void parameter_names__get(struct ftype *ftype, size_t nr_parameters,
-				 const char **parameter_names)
+static bool proto__get(struct function *func, char *proto, size_t len)
 {
-	struct parameter *parameter;
-	int i = 0;
-
-	ftype__for_each_parameter(ftype, parameter) {
-		if (i >= nr_parameters)
-			return;
-		parameter_names[i++] = parameter__name(parameter);
-	}
+	const struct conf_fprintf conf = {
+						.name_spacing = 23,
+						.type_spacing = 26,
+						.emit_stats = 0,
+						.no_parm_names = 1,
+						.skip_emitting_errors = 1,
+						.skip_emitting_modifier = 1,
+					};
+
+	return function__prototype_conf(func, func->priv, &conf, proto, len) != NULL;
 }
 
 static bool funcs__match(struct btf_encoder *encoder, struct elf_function *func, struct function *f2)
 {
-	const char *parameter_names[BTF_ENCODER_MAX_PARAMETERS];
+	char proto[BTF_ENCODER_MAX_PROTO];
 	struct function *f1 = func->function;
 	const char *name;
-	int i;
 
 	if (!f1)
 		return false;
@@ -833,33 +833,27 @@ static bool funcs__match(struct btf_encoder *encoder, struct elf_function *func,
 	if (f1->proto.nr_parms == 0)
 		return true;
 
-	if (!func->state.got_parameter_names) {
-		parameter_names__get(&f1->proto, BTF_ENCODER_MAX_PARAMETERS,
-				     func->state.parameter_names);
-		func->state.got_parameter_names = true;
-	}
-	parameter_names__get(&f2->proto, BTF_ENCODER_MAX_PARAMETERS, parameter_names);
-	for (i = 0; i < f1->proto.nr_parms && i < BTF_ENCODER_MAX_PARAMETERS; i++) {
-		if (!func->state.parameter_names[i]) {
-			if (!parameter_names[i])
-				continue;
-		} else if (parameter_names[i]) {
-			if (strcmp(func->state.parameter_names[i], parameter_names[i]) == 0)
-				continue;
-		}
-		if (encoder->verbose) {
-			printf("function mismatch for '%s'(%s): parameter #%d '%s' != '%s'\n",
-			       name, f1->alias ?: name, i,
-			       func->state.parameter_names[i] ?: "<null>",
-			       parameter_names[i] ?: "<null>");
+	if (f1->proto.tag.type == f2->proto.tag.type)
+		return true;
+
+	if (!func->state.got_proto)
+		func->state.got_proto = proto__get(f1, func->state.proto, sizeof(func->state.proto));
+
+	if (proto__get(f2, proto, sizeof(proto))) {
+		if (strcmp(func->state.proto, proto) != 0) {
+			if (encoder->verbose)
+				printf("function mismatch for '%s'('%s'): '%s' != '%s'\n",
+				       name, f1->alias ?: name,
+				       func->state.proto, proto);
+			return false;
 		}
-		return false;
 	}
 	return true;
 }
 
 static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)
 {
+	fn->priv = encoder->cu;
 	if (func->function) {
 		struct function *existing = func->function;
 
@@ -1022,7 +1016,8 @@ static int btf_encoder__collect_function(struct btf_encoder *encoder, GElf_Sym *
 	}
 	encoder->functions.entries[encoder->functions.cnt].generated = false;
 	encoder->functions.entries[encoder->functions.cnt].function = NULL;
-	encoder->functions.entries[encoder->functions.cnt].state.got_parameter_names = false;
+	encoder->functions.entries[encoder->functions.cnt].state.got_proto = false;
+	encoder->functions.entries[encoder->functions.cnt].state.proto[0] = '\0';
 	encoder->functions.entries[encoder->functions.cnt].state.type_id_off = 0;
 	encoder->functions.cnt++;
 	return 0;
-- 
1.8.3.1


  parent reply	other threads:[~2023-03-10 15:43 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 14:50 [PATCH dwarves 0/3] dwarves: improve BTF encoder comparison method Alan Maguire
2023-03-10 14:50 ` [PATCH dwarves 1/3] dwarves_fprintf: generalize function prototype print to support passing conf Alan Maguire
2023-03-10 14:50 ` [PATCH dwarves 2/3] dwarves_fprintf: support skipping modifier Alan Maguire
2023-03-13 12:20   ` Arnaldo Carvalho de Melo
2023-03-13 12:29     ` Arnaldo Carvalho de Melo
2023-03-13 13:16       ` Alan Maguire
2023-03-13 13:50   ` Eduard Zingerman
2023-03-13 16:37     ` Alan Maguire
2023-03-13 17:12       ` Eduard Zingerman
2023-03-13 18:28         ` Arnaldo Carvalho de Melo
2023-03-13 14:45   ` Eduard Zingerman
2023-03-13 17:18     ` Alan Maguire
2023-03-13 18:26       ` Arnaldo Carvalho de Melo
2023-03-10 14:50 ` Alan Maguire [this message]
2023-03-10 15:18 ` [PATCH dwarves 0/3] dwarves: improve BTF encoder comparison method Arnaldo Carvalho de Melo
2023-03-13  9:40 ` Jiri Olsa
2023-03-13 12:33   ` Arnaldo Carvalho de Melo

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=1678459850-16140-4-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=jolsa@kernel.org \
    --cc=kpsingh@chromium.org \
    --cc=martin.lau@kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).