Re: [PATCH v2 02/24] objtool: Introduce CFI hash

Peter Zijlstra
2021-08-25T10:16:58Z

On Fri, Aug 20, 2021 at 03:27:55PM -0700, Josh Poimboeuf wrote:
> > +static struct cfi_state *cfi_alloc(void)
> > +{
> > + struct cfi_state *cfi = calloc(sizeof(struct cfi_state), 1);
> > + if (!cfi) {
> > + WARN("calloc failed");
> > + exit(1);
> > + }
> > + nr_cfi++;
> > + init_cfi_state(cfi);
> > + return cfi;
> > +}
>
> I'm thinking this should also add it to the hash. i.e. I don't think
> there's a scenario where you'd alloc a cfi and not want to add it to the
> hash. The more sharing the better.

Right, changed it like below.

> > +
> > +struct cfi_state *insn_get_cfi(struct instruction *insn)
> > +{
> > + if (!insn->cfip)
> > + insn->cfip = cfi_alloc();
> > +
> > + return insn->cfip;
> > +}
>
> Call it something like insn_get_or_alloc_cfi()?
>
> Also, the function can be static.

It's gone now.

> > +static struct cfi_state *cfi_hash_find(struct cfi_state *cfi)
> > +{
> > + struct hlist_head *head = &cfi_hash[hash_min(cfi_key(cfi), cfi_bits)];
> > + struct cfi_state *obj;
> > +
> > + hlist_for_each_entry(obj, head, hash) {
> > + if (!cficmp(cfi, obj)) {
> > + nr_cfi_cache++;
> > + return obj;
> > + }
> > + }
> > +
> > + obj = cfi_alloc();
> > + *obj = *cfi;
> > + hlist_add_head(&obj->hash, head);
> > +
> > + return obj;
>
> cfi_hash_find_or_alloc_cfi()?

Made that cfi_hash_find_or_add()

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -30,6 +30,7 @@ struct alternative {
static unsigned long nr_cfi, nr_cfi_reused, nr_cfi_cache;

static struct cfi_init_state initial_func_cfi;
+static struct cfi_state init_cfi;
static struct cfi_state func_cfi;

struct instruction *find_insn(struct objtool_file *file,
@@ -278,7 +279,6 @@ static struct cfi_state *cfi_alloc(void)
exit(1);
}
nr_cfi++;
- init_cfi_state(cfi);
return cfi;
}

@@ -298,7 +298,7 @@ static inline u32 cfi_key(struct cfi_sta
sizeof(*cfi) - sizeof(cfi->hash), 0);
}

-static struct cfi_state *cfi_hash_find(struct cfi_state *cfi)
+static struct cfi_state *cfi_hash_find_or_add(struct cfi_state *cfi)
{
struct hlist_head *head = &cfi_hash[hash_min(cfi_key(cfi), cfi_bits)];
struct cfi_state *obj;
@@ -1620,10 +1620,10 @@ static void set_func_state(struct cfi_st

static int read_unwind_hints(struct objtool_file *file)
{
+ struct cfi_state cfi = init_cfi;
struct section *sec, *relocsec;
struct unwind_hint *hint;
struct instruction *insn;
- struct cfi_state *cfi;
struct reloc *reloc;
int i;

@@ -1666,9 +1666,8 @@ static int read_unwind_hints(struct objt
continue;
}

- cfi = insn->cfip;
- if (!cfi)
- cfi = insn->cfip = cfi_alloc();
+ if (insn->cfip)
+ cfi = *(insn->cfip);

if (arch_decode_hint_reg(insn, hint->sp_reg)) {
WARN_FUNC("unsupported unwind_hint sp base reg %d",
@@ -1679,6 +1678,8 @@ static int read_unwind_hints(struct objt
cfi->cfa.offset = bswap_if_needed(hint->sp_offset);
cfi->type = hint->type;
cfi->end = hint->end;
+
+ insn->cfip = cfi_hash_find_or_add(&cfi);
}

return 0;
@@ -2831,7 +2832,7 @@ static int validate_branch(struct objtoo
insn->cfip = prev_insn->cfip;
nr_cfi_reused++;
} else {
- insn->cfip = cfi_hash_find(&state.cfi);
+ insn->cfip = cfi_hash_find_or_add(&state.cfi);
}
}

@@ -3239,6 +3240,7 @@ int check(struct objtool_file *file)
int ret, warnings = 0;

arch_initial_func_cfi_state(&initial_func_cfi);
+ init_cfi_state(&init_cfi);
init_cfi_state(&func_cfi);
set_func_state(&func_cfi);

@@ -3250,6 +3252,7 @@ int check(struct objtool_file *file)
if (!cfi_hash_alloc(1UL << (file->elf->symbol_bits - 3)))
goto out;

+ cfi_hash_add(&init_cfi);
cfi_hash_add(&func_cfi);

if (list_empty(&file->insn_list))