Re: duplicate BTF_IDs leading to symbol redefinition errors?

Jiri Olsa
2023-09-08T11:47:48Z

On Thu, Sep 07, 2023 at 10:33:00PM +0200, Jiri Olsa wrote:
> On Thu, Sep 07, 2023 at 12:01:18PM -0700, Nick Desaulniers wrote:
> > So we've got a curious report recently:
> > https://github.com/ClangBuiltLinux/linux/issues/1913
> >
> > ld.lld: error: ld-temp.o <inline asm>:14577:1: symbol
> > '__BTF_ID__struct__cgroup__624' is already defined
> > __BTF_ID__struct__cgroup__624:
> > ^
> >
> > It's been hard to pin down a SHA and .config to reproduce this, but
> > looking at the definition of BTF_ID's usage of __ID's usage of
> > __COUNTER__, and the two statements:
> >
> > kernel/bpf/helpers.c:2460:BTF_ID(struct, cgroup)
> > kernel/bpf/verifier.c:5075:BTF_ID(struct, cgroup)
> >
> > Is it possible that __COUNTER__ could evaluate to the same value
> > across 2 different translation units, leading to a name collision like
> > the above?
>
> hum, that probably the case, I see same counter values at different
> __BTF_ID_ symbols:
>
> ffffffff833fe540 r __BTF_ID__struct__bpf_bloom_filter__380
> ffffffff833fe548 r __BTF_ID__struct__bpf_queue_stack__380
> ffffffff833fe578 r __BTF_ID__struct__cgroup__380
>
> perhaps we were just lucky not to hit that :-\
>
> >
> > looking at another usage of BTF_ID other than struct
> > cgroup;kernel/bpf/helpers.c:2461:BTF_ID(func, bpf_cgroup_release)
> > is only defined in one translation unit
> >
> > Should one of those two `BTF_ID(struct, cgroup)` be removed? Is there
> > some other way we can avoid these collisions in the future?
>
> need to find some way to make the symbol unique, will check

the change below uses object's path as the __BTF_ID_.. symbol suffix to make
it unique

I'm still looking, but can't think of a better way so far, perhaps somebody
will have better idea

jirka

---
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index a3462a9b8e18..564953f9cbc7 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -49,7 +49,7 @@ word \
____BTF_ID(symbol, word)

#define __ID(prefix) \
- __PASTE(prefix, __COUNTER__)
+ __PASTE(__PASTE(prefix, __COUNTER__), BTF_ID_BASE)

/*
* The BTF_ID defines unique symbol for each ID pointing
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 68d0134bdbf9..2ef8b2798be0 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -200,6 +200,10 @@ _c_flags += $(if $(patsubst n%,, \
-D__KCSAN_INSTRUMENT_BARRIERS__)
endif

+ifeq ($(CONFIG_DEBUG_INFO_BTF),y)
+_c_flags += -DBTF_ID_BASE=$(subst =,,$(shell echo -n $(modfile) | base32 -w0))
+endif
+
# $(srctree)/$(src) for including checkin headers from generated source files
# $(objtree)/$(obj) for including generated headers from checkin source files
ifeq ($(KBUILD_EXTMOD),)