* [PATCH v4 dwarves 1/2] btf_encoder: fix strncpy issues, other issues identified in code review
2024-09-30 8:17 [PATCH v4 dwarves 0/2] fixes for memory optimized BTF Alan Maguire
@ 2024-09-30 8:17 ` Alan Maguire
2024-09-30 8:17 ` [PATCH v4 dwarves 2/2] tests: improve btf_functions.sh by reducing greps/pipes Alan Maguire
2024-09-30 11:20 ` [PATCH v4 dwarves 0/2] fixes for memory optimized BTF Jiri Olsa
2 siblings, 0 replies; 5+ messages in thread
From: Alan Maguire @ 2024-09-30 8:17 UTC (permalink / raw)
To: acme
Cc: dwarves, eddyz87, olsajiri, shung-hsi.yu, jirislaby, sedat.dilek,
w, Alan Maguire
- fixed checks of func->state since it is always non-NULL and removed
now-unneeded fields from dwarves.h; added assertions that either
ELF or DWARF data is present for func encoding (Jiri)
- fixed strncpy() issues, improved equality comparions between types
(Eduard)
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
btf_encoder.c | 68 ++++++++++++++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 22 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index d535b90..92dc5c4 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -28,6 +28,7 @@
#include <unistd.h>
+#include <assert.h>
#include <errno.h>
#include <stdint.h>
#include <search.h> /* for tsearch(), tfind() and tdestroy() */
@@ -692,7 +693,8 @@ static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t tag_t
return encoder->type_id_off + tag_type;
}
-static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype, struct elf_function *func)
+static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype,
+ struct elf_function *func)
{
struct btf *btf = encoder->btf;
const struct btf_type *t;
@@ -701,16 +703,22 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
int32_t id, type_id;
char tmp_name[KSYM_NAME_LEN];
const char *name;
- struct btf_encoder_func_state *state = &func->state;
+ struct btf_encoder_func_state *state;
+
+ assert(ftype != NULL || func != NULL);
/* add btf_type for func_proto */
if (ftype) {
nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
- } else {
+ } else if (func) {
+ state = &func->state;
nr_params = state->nr_parms;
type_id = state->ret_type_id;
+ } else {
+ return 0;
}
+
id = btf__add_func_proto(btf, type_id);
if (id > 0) {
t = btf__type_by_id(btf, id);
@@ -730,25 +738,29 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
++param_idx;
- if (btf_encoder__add_func_param(encoder, name, type_id, param_idx == nr_params))
+ if (btf_encoder__add_func_param(encoder, name, type_id,
+ param_idx == nr_params))
return -1;
}
++param_idx;
if (ftype->unspec_parms)
- if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params))
+ if (btf_encoder__add_func_param(encoder, NULL, 0,
+ param_idx == nr_params))
return -1;
} else {
for (param_idx = 0; param_idx < nr_params; param_idx++) {
struct btf_encoder_func_parm *p = &state->parms[param_idx];
+
name = btf__name_by_offset(btf, p->name_off);
/* adding BTF data may result in a move of the
* name string memory, so make a temporary copy.
*/
- strncpy(tmp_name, name, sizeof(tmp_name));
+ strncpy(tmp_name, name, sizeof(tmp_name) - 1);
- if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id, param_idx == nr_params))
+ if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id,
+ param_idx == nr_params))
return -1;
}
}
@@ -950,10 +962,17 @@ static bool types__match(struct btf_encoder *encoder,
return false;
}
- switch (btf_kind(t1)) {
+ switch (k1) {
case BTF_KIND_INT:
+ if (t1->size != t2->size)
+ return false;
+ if (*(__u32 *)(t1 + 1) != *(__u32 *)(t2 + 1))
+ return false;
+ return names__match(btf1, t1, btf2, t2);
case BTF_KIND_FLOAT:
- case BTF_KIND_FWD:
+ if (t1->size != t2->size)
+ return false;
+ return names__match(btf1, t1, btf2, t2);
case BTF_KIND_TYPEDEF:
case BTF_KIND_STRUCT:
case BTF_KIND_UNION:
@@ -1009,9 +1028,6 @@ static bool funcs__match(struct btf_encoder *encoder, struct elf_function *func,
{
uint8_t i;
- if (!s1->initialized || !s2->initialized)
- return true;
-
if (s1->nr_parms != s2->nr_parms) {
btf_encoder__log_func_skip(encoder, func,
"param count mismatch; %d params != %d params\n",
@@ -1081,7 +1097,8 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
goto out;
}
state.parms[param_idx].name_off = str_off;
- state.parms[param_idx].type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
+ state.parms[param_idx].type_id = param->tag.type == 0 ? 0 :
+ encoder->type_id_off + param->tag.type;
param_idx++;
}
if (ftype->unspec_parms)
@@ -1139,7 +1156,8 @@ out:
return err;
}
-static int32_t btf_encoder__add_func(struct btf_encoder *encoder, struct function *fn, struct elf_function *func)
+static int32_t btf_encoder__add_func(struct btf_encoder *encoder, struct function *fn,
+ struct elf_function *func)
{
int btf_fnproto_id, btf_fn_id, tag_type_id = 0;
int16_t component_idx = -1;
@@ -1147,19 +1165,23 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, struct functio
const char *value;
char tmp_value[KSYM_NAME_LEN];
+ assert(fn != NULL || func != NULL);
+
btf_fnproto_id = btf_encoder__add_func_proto(encoder, fn ? &fn->proto : NULL, func);
name = func->alias ?: func->name;
if (btf_fnproto_id >= 0)
- btf_fn_id = btf_encoder__add_ref_type(encoder, BTF_KIND_FUNC, btf_fnproto_id, name, false);
+ btf_fn_id = btf_encoder__add_ref_type(encoder, BTF_KIND_FUNC, btf_fnproto_id,
+ name, false);
if (btf_fnproto_id < 0 || btf_fn_id < 0) {
- printf("error: failed to encode function '%s': invalid %s\n", name, btf_fnproto_id < 0 ? "proto" : "func");
+ printf("error: failed to encode function '%s': invalid %s\n",
+ name, btf_fnproto_id < 0 ? "proto" : "func");
return -1;
}
if (!fn) {
struct btf_encoder_func_state *state = &func->state;
uint16_t idx;
- if (!state || state->nr_annots == 0)
+ if (state->nr_annots == 0)
return 0;
for (idx = 0; idx < state->nr_annots; idx++) {
@@ -1169,10 +1191,11 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, struct functio
/* adding BTF data may result in a mode of the
* value string memory, so make a temporary copy.
*/
- strncpy(tmp_value, value, sizeof(tmp_value));
+ strncpy(tmp_value, value, sizeof(tmp_value) - 1);
component_idx = a->component_idx;
- tag_type_id = btf_encoder__add_decl_tag(encoder, tmp_value, btf_fn_id, component_idx);
+ tag_type_id = btf_encoder__add_decl_tag(encoder, tmp_value,
+ btf_fn_id, component_idx);
if (tag_type_id < 0)
break;
}
@@ -1190,7 +1213,8 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, struct functio
}
}
if (tag_type_id < 0) {
- fprintf(stderr, "error: failed to encode tag '%s' to func %s with component_idx %d\n",
+ fprintf(stderr,
+ "error: failed to encode tag '%s' to func %s with component_idx %d\n",
value, name, component_idx);
return -1;
}
@@ -1207,7 +1231,7 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder)
struct btf_encoder_func_state *state = &func->state;
struct btf_encoder *other_encoder = NULL;
- if (!state || !state->initialized || state->processed)
+ if (!state->initialized || state->processed)
continue;
/* merge optimized-out status across encoders; since each
* encoder has the same elf symbol table we can use the
@@ -1223,7 +1247,7 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder)
other_func = &other_encoder->functions.entries[i];
other_state = &other_func->state;
- if (!other_state)
+ if (!other_state->initialized)
continue;
optimized = state->optimized_parms | other_state->optimized_parms;
unexpected = state->unexpected_reg | other_state->unexpected_reg;
--
2.43.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v4 dwarves 2/2] tests: improve btf_functions.sh by reducing greps/pipes
2024-09-30 8:17 [PATCH v4 dwarves 0/2] fixes for memory optimized BTF Alan Maguire
2024-09-30 8:17 ` [PATCH v4 dwarves 1/2] btf_encoder: fix strncpy issues, other issues identified in code review Alan Maguire
@ 2024-09-30 8:17 ` Alan Maguire
2024-09-30 11:20 ` [PATCH v4 dwarves 0/2] fixes for memory optimized BTF Jiri Olsa
2 siblings, 0 replies; 5+ messages in thread
From: Alan Maguire @ 2024-09-30 8:17 UTC (permalink / raw)
To: acme
Cc: dwarves, eddyz87, olsajiri, shung-hsi.yu, jirislaby, sedat.dilek,
w, Alan Maguire
Reduce number of greps/pipes to minimize overhead of test; in my
measurements this shaves ~1min off test run time to be approximately
4 minutes.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tests/btf_functions.sh | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
index 4a60a18..b8645cc 100755
--- a/tests/btf_functions.sh
+++ b/tests/btf_functions.sh
@@ -47,7 +47,8 @@ trap cleanup EXIT
test -n "$VERBOSE" && printf "Encoding..."
-pahole --btf_features=default --btf_encode_detached=$outdir/vmlinux.btf --verbose $vmlinux |grep "skipping BTF encoding of function" > ${outdir}/skipped_fns
+pahole --btf_features=default --btf_encode_detached=$outdir/vmlinux.btf --verbose $vmlinux |\
+ grep "skipping BTF encoding of function" > ${outdir}/skipped_fns
test -n "$VERBOSE" && printf "done.\n"
@@ -57,7 +58,7 @@ funcs=$(pfunct --format_path=btf $outdir/vmlinux.btf |sort)
# all functions from DWARF; some inline functions are not inlined so include them too
pfunct --all --no_parm_names --format_path=dwarf $vmlinux | \
- awk '{ print $0}'|sort|uniq > $outdir/dwarf.funcs
+ sort|uniq > $outdir/dwarf.funcs
# all functions from BTF (removing bpf_kfunc prefix where found)
pfunct --all --no_parm_names --format_path=btf $outdir/vmlinux.btf |\
awk '{ gsub("^bpf_kfunc ",""); print $0}'|sort|uniq > $outdir/btf.funcs
@@ -66,14 +67,12 @@ exact=0
inline=0
const_insensitive=0
-for f in $funcs ; do
- btf=$(grep " $f(" $outdir/btf.funcs)
+while IFS= read -r btf ; do
# look for non-inline match first
- dwarf=$(grep " $f(" $outdir/dwarf.funcs | grep -Ev "^inline ")
+ dwarf=$(grep -F "$btf" $outdir/dwarf.funcs)
if [[ "$btf" != "$dwarf" ]]; then
# function might be declared inline in DWARF.
- inline_dwarf=$(grep " $f(" $outdir/dwarf.funcs |grep "^inline ")
- if [[ "inline $btf" != "$inline_dwarf" ]]; then
+ if [[ "inline $btf" != "$dwarf" ]]; then
# some functions have multiple instances in DWARF where one has
# const param(s) and another does not (see errpos()). We do not
# mark these functions inconsistent as though they technically
@@ -83,7 +82,7 @@ for f in $funcs ; do
if [[ "$dwarf_noconst" =~ "$btf_noconst" ]]; then
const_insensitive=$((const_insensitive+1))
else
- echo "ERROR: mismatch for '$f()' : BTF '$btf' not found; DWARF '$dwarf'"
+ echo "ERROR: mismatch : BTF '$btf' not found; DWARF '$dwarf'"
fail
fi
else
@@ -92,7 +91,7 @@ for f in $funcs ; do
else
exact=$((exact+1))
fi
-done
+done < $outdir/btf.funcs
echo "Matched $exact functions exactly."
echo "Matched $inline functions with inlines."
@@ -133,8 +132,6 @@ for r in $return_mismatches ; do
grep " $r(" $outdir/dwarf.funcs | \
awk -v FN=$r '{i = index($0, FN); if (i>0) print substr($0, 0, i-1) }' \
| uniq > ${outdir}/retvals.$r
- test -n "$VERBOSE" && echo "'${r}()' has return values:"
- test -n "$VERBOSE" && cat ${outdir}/retvals.$r
cnt=$(wc -l ${outdir}/retvals.$r | awk '{ print $1 }')
if [[ $cnt -lt 2 ]]; then
echo "ERROR: '${r}()' has only one return value; it should not be reported as having incompatible return values"
@@ -159,14 +156,11 @@ for p in $param_mismatches ; do
skipmsg=$(awk -v FN=$p '{ if ($1 == FN) print $0 }' $outdir/skipped_fns)
altname=$(echo $skipmsg | awk '{ i=index($2,")"); print substr($2,2,i-2); }')
if [[ "$altname" != "$p" ]]; then
- test -n "$VERBOSE" && echo "skipping optimized function $p ($altname); pfunct may not reflect late optimizations."
optimized=$((optimized+1))
continue
fi
# Ensure there are multiple instances with incompatible params
grep " $p(" $outdir/dwarf.funcs | uniq > ${outdir}/protos.$p
- test -n "$VERBOSE" && echo "'${p}()' has prototypes:"
- test -n "$VERBOSE" && cat ${outdir}/protos.$p
cnt=$(wc -l ${outdir}/protos.$p | awk '{ print $1 }')
if [[ $cnt -lt 2 ]]; then
# function may be inlined in multiple sites with different protos
--
2.43.5
^ permalink raw reply related [flat|nested] 5+ messages in thread