Re: bpf_core_type_id_kernel is not consistent with bpf_core_type_id_local

[Yonghong Song <yhs@meta.com>]

On 7/4/23 6:30 AM, Lorenz Bauer wrote:
> Hi,
> 
> I think that CO-RE has inconsistent behaviour wrt. BPF_TYPE_ID_LOCAL
> and BPF_TYPE_ID_TARGET when dealing with qualifiers (modifiers?) Given
> the following C:
> 
> enum bpf_type_id_kind {
>      BPF_TYPE_ID_LOCAL = 0,        /* BTF type ID in local program */
>      BPF_TYPE_ID_TARGET = 1,        /* BTF type ID in target kernel */
> };
> 
> int foo(void) {
>      return __builtin_btf_type_id(*(const int *)0, BPF_TYPE_ID_TARGET)
> != __builtin_btf_type_id(*(const int *)0, BPF_TYPE_ID_LOCAL);
> }
> 
> That line with __builtin_btf_type_id is just the expansion of
> bpf_core_type_id_kernel, etc. clang generates the following BPF:
> 
> foo:
>   18 01 00 00 02 00 00 00 00 00 00 00 00 00 00 00    r1 = 0x2 ll
>   79 11 00 00 00 00 00 00    r1 = *(u64 *)(r1 + 0x0)
>   18 02 00 00 04 00 00 00 00 00 00 00 00 00 00 00    r2 = 0x4 ll
>   79 22 00 00 00 00 00 00    r2 = *(u64 *)(r2 + 0x0)
>   b7 03 00 00 00 00 00 00    r3 = 0x0
>   7b 3a f0 ff 00 00 00 00    *(u64 *)(r10 - 0x10) = r3
>   b7 03 00 00 01 00 00 00    r3 = 0x1
>   7b 3a f8 ff 00 00 00 00    *(u64 *)(r10 - 0x8) = r3
>   5d 21 02 00 00 00 00 00    if r1 != r2 goto +0x2 <LBB0_2>
>   79 a1 f0 ff 00 00 00 00    r1 = *(u64 *)(r10 - 0x10)
>   7b 1a f8 ff 00 00 00 00    *(u64 *)(r10 - 0x8) = r1
> LBB0_2:
>   79 a0 f8 ff 00 00 00 00    r0 = *(u64 *)(r10 - 0x8)
>   95 00 00 00 00 00 00 00    exit
> 
> Link to godbolt: https://godbolt.org/z/jr63hKz9E  (contains version info)
> 
> Note that the first two ldimm64 have distinct type IDs. I added some
> debug logging to cilium/ebpf and found that the compiler indeed also
> emits distinct CO-RE relocations:
> 
> foo {InsnOff:0 TypeID:2 AccessStrOff:69 Kind:local_type_id}
> foo {InsnOff:2 TypeID:4 AccessStrOff:69 Kind:target_type_id}
> 
> It seems that for BPF_TYPE_ID_TARGET the outer const is peeled, while
> this doesn't happen for the local variant.
> 
> CORERelocation(local_type_id, Const[0], local_id=4) local_type_id=4->4
> CORERelocation(target_type_id, Int:"int"[0], local_id=2) target_type_id=2->2
> 
> Similar behaviour exists for BPF_TYPE_EXISTS, probably others.
> 
> The behaviour goes away if I drop the pointer casting magic:
> 
> __builtin_btf_type_id((const int)0, BPF_TYPE_ID_TARGET) !=
> __builtin_btf_type_id((const int)0, BPF_TYPE_ID_LOCAL)
> 
> Intuitively I'd say that the root cause is that dereferencing the
> pointer drops the constness of the type. Why does TARGET behave
> differently than LOCAL though?

Thanks for reporting. The difference of type w.r.t. 'const' modifier
is a deliberate choice in llvm:

See 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/BPF/BPFPreserveDIType.cpp#L84-L103

     if (FlagValue == BPFCoreSharedInfo::BTF_TYPE_ID_LOCAL_RELOC) {
       Reloc = BPFCoreSharedInfo::BTF_TYPE_ID_LOCAL;
     } else {
       Reloc = BPFCoreSharedInfo::BTF_TYPE_ID_REMOTE;
       DIType *Ty = cast<DIType>(MD);
       while (auto *DTy = dyn_cast<DIDerivedType>(Ty)) {
         unsigned Tag = DTy->getTag();
         if (Tag != dwarf::DW_TAG_const_type &&
             Tag != dwarf::DW_TAG_volatile_type)
           break;
         Ty = DTy->getBaseType();
       }

       if (Ty->getName().empty()) {
         if (isa<DISubroutineType>(Ty))
           report_fatal_error(
               "SubroutineType not supported for BTF_TYPE_ID_REMOTE reloc");
         else
           report_fatal_error("Empty type name for BTF_TYPE_ID_REMOTE 
reloc");
       }
       MD = Ty;
     }

Basically, the BTF_TYPE_ID_REMOTE (the kernel term BPF_TYPE_ID_TARGET)
needs further checking to prevent some invalid cases.
Also for kernel type matching, it would be good to eliminate modifiers
otherwise, there could be many instances of 'const' which makes
kernel matching is more complicated.

But I see your point. Maybe we should preserve the original type
for BTF_TYPE_ID_TARGET as well. Will check what libbpf/kernel
will handle 'const int *' case and get back to this thread later.

> 
> Cheers
> Lorenz