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
next prev 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).