[PATCH] D83242: [clang][BPF] support type exist/size and enum exist/value relocations

[Yonghong Song via Phabricator via llvm-commits]

yonghong-song added a comment.

In D83242#2193989 <https://reviews.llvm.org/D83242#2193989>, @anakryiko wrote:

>> This is different than testing whether a type exists or not where we want to precisely test whether that type exists or not, so we want to preserve `typedef` type vs. `struct t` type.
>
> `typedef t` and `struct t` are also different for field relocations (those t is in different namespaces, actually), so it might be a good idea to disambiguate those for field relocations as well?

Just to be clear. The type recorded in relocation will be always valid since we have type_id there. For field exists, due to we may use nullptr as the object, if CSE happens, it is possible the root type id may be the "struct t" instead of "typedef t" (or the other ordering) assuming the definition "typedef struct t t;". But from root type_id, libbpf will be able to correctly trace down to the field and do the relocation properly.

On the other hand, I agree that this is not ideal as we may use a different root type than what user specified in the source. But everything is hidden and invisible to user so I won't have user visible impact.

To fix this in clang is actually tricky, we do not want to add seq number to every preserve_field_info* IR builtin since this is not really necessary for non-existence relocations. Also preserve_field_info* although used by bpf only so far is llvm generic builtin, I would hesitate to change it with new seq_num fields. The best will be have another bpf specific builtin
for field existence, but given the current mechanism also works, I probably will keep it this way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83242/new/

https://reviews.llvm.org/D83242