This is an archive of the discontinued LLVM Phabricator instance.

[BPF] Handle anon record for CO-RE relocations
ClosedPublic

Authored by yonghong-song on Jul 13 2022, 12:35 AM.

Details

Summary

When doing experiment in kernel, for kernel data structure sockptr_t
in CO-RE operation, I hit an assertion error. The sockptr_t definition
and usage look like below:

#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
typedef struct {
      union {
              void    *kernel;
              void    *user;
      };  
      unsigned is_kernel : 1;
} sockptr_t;
#pragma clang attribute pop 
int test(sockptr_t *arg) {
  return arg->is_kernel;
}

The assertion error looks like

clang: ../lib/Target/BPF/BPFAbstractMemberAccess.cpp:878: llvm::Value*
 {anonymous}::BPFAbstractMemberAccess::computeBaseAndAccessKey(llvm::CallInst*,
 {anonymous}::BPFAbstractMemberAccess::CallInfo&, std::__cxx11::string&,
 llvm::MDNode*&): Assertion `TypeName.size()' failed.

In this particular, the clang frontend attach the debuginfo metadata associated
with anon structure with the preserve_access_info IR intrinsic. But the first
debuginfo type has to be a named type so libbpf can have a sound start to
do CO-RE relocation.

Besides the above approach using pragma to push attribute, the below typedef/struct
definition can have preserve_access_index directly applying to the anon struct.

typedef struct {
      union {
              void    *kernel;
              void    *user;
      };  
      unsigned is_kernel : 1;
} __attribute__((preserve_access_index) sockptr_t;

This patch fixed the issue by preprocessing function argument/return types
and local variable types used by other CO-RE intrinsics. For any

typedef struct/union { ... } typedef_name

an association of <anon struct/union, typedef> is recorded to replace
the IR intrinsic metadata 'anon struct/union' to 'typedef'.
It is possible that two different 'typedef' types may have identical
anon struct/union type. For such a case, the association will be
<anon struct/union, nullptr> to indicate the invalid case.

Diff Detail

Event Timeline

yonghong-song created this revision.Jul 13 2022, 12:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2022, 12:35 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
yonghong-song requested review of this revision.Jul 13 2022, 12:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2022, 12:35 AM
ast accepted this revision.Jul 13 2022, 12:58 PM

The workaround is not pretty, but I couldn't come up with a better way.

This revision is now accepted and ready to land.Jul 13 2022, 12:58 PM
This revision was landed with ongoing or failed builds.Jul 13 2022, 3:17 PM
This revision was automatically updated to reflect the committed changes.