On Tue, Apr 29, 2025 at 12:30 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes:
>
> [...]
>
> > Ok, so sleeping on this a bit more, I'm hesitant to do this more
> > generic approach, as now we'll be running a risk of potentially
> > looping indefinitely (the max_depth check I added doesn't completely
> > prevent this).
>
> Is that different from what happens already?
yes, it's different, we don't follow pointers. that identical_structs
recurse only for embedded/nested structs, not structs-by-pointer.
idential_arrays also doesn't follow any references
so it could lead to loop only if BTF is broken and we have one struct
embedding another, while that other embeds its parent, which isn't
legal in BTF and C (so whatever)
>
> /* Check if given two types are identical STRUCT/UNION definitions */
> static bool btf_dedup_identical_structs(struct btf_dedup *d, __u32 id1, __u32 id2)
> {
> [...]
> for (i = 0, n = btf_vlen(t1); i < n; i++, m1++, m2++) {
> if (...
> !btf_dedup_identical_structs(d, m1->type, m2->type)) // <-- recursion,
> return false; // afaiu it is unbounded.
> }
> return true;
> }
>
> E.g. adding an array to mark visited types and stop descend should stop
> inifinite recurusion. I agree with Alan, the patch you shared in the
> evening looks interesting. (But it was butchered by gmail,
> fixing patches by hand is a bit annoyiong, maybe just attach such files?).
yeah, I know that gmail butchers diffs, I just wanted to give a rough
idea, didn't expect anyone wanting to apply it (plus I did it on top
of Github-based libbpf inside pahole's libbpf submodule)
But here you go if you want to play with it ([0]).
And yes, "visited" marks are the solution, but I was thinking that if
we implement a pre-processing deduplication step as we discussed
offline, we won't need to do any of this, so didn't want to pursue
this further. But we can talk about this, of course. So far this
generality doesn't buy us anything, I got byte-for-byte identical
bpf_testmod.ko with Alan's and my changes all the same.
[0] https://gist.github.com/anakryiko/fd1c84dcad91141d27d8bd33453521d1
>
> [...]
