This is an archive of the discontinued LLVM Phabricator instance.

[BPF] fix a bug for BTF pointee type pruning
ClosedPublic

Authored by yonghong-song on Jun 17 2020, 12:14 PM.

Details

Summary

In BTF, pointee type pruning is used to reduce cluttering
too many unused types into prog BTF. For example,

struct task_struct {
   ... 
   struct mm_struct *mm;
   ... 
}

If bpf program does not access members of "struct mm_struct",
there is no need to bring types for "struct mm_struct" to BTF.

This patch fixed a bug where an incorrect pruning happened.
The test case like below:

struct t;
typedef struct t _t; 
struct s1 { _t *c; };
int test1(struct s1 *arg) { ... }

struct t { int a; int b; };
struct s2 { _t c; }
int test2(struct s2 *arg) { ... }

After processing test1(), among others, BPF backend generates BTF types for

"struct s1", "_t" and a placeholder for "struct t".

Note that "struct t" is not really generated. If later a direct access
to "struct t" member happened, "struct t" BTF type will be generated
properly.

During processing test2(), when processing member type "_t c",
BPF backend sees type "_t" already generated, so returned.
This caused the problem that "struct t" BTF type is never generated and
eventually causing incorrect type definition for "struct s2".

To fix the issue, during DebugInfo type traversal, even if a
typedef/const/volatile/restrict derived type has been recorded in BTF,
if it is not a type pruning candidate, type traversal of its base type continues.

Diff Detail

Event Timeline

yonghong-song created this revision.Jun 17 2020, 12:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2020, 12:14 PM
anakryiko added inline comments.Jun 17 2020, 12:26 PM
llvm/lib/Target/BPF/BTFDebug.cpp
606

if it is the !Ty case, would it crash immediately one line below? Was it supposed to be if (Ty && DIToIdMap.find(Ty) != DIToIdMap.end())?

yonghong-song marked an inline comment as done.Jun 17 2020, 12:59 PM
yonghong-song added inline comments.
llvm/lib/Target/BPF/BTFDebug.cpp
606

It should not crash. The intention is if Ty is null, then DIToIdMap[Ty] will return TypeId 0.
Note that DIToIdMap is a map, the key is a pointer, so it won't crash even if the pointer
value is null. We do have cases where e.g., Ty->getBaseType() is null. For example, const void where the type pointed by const is null type and its type id is 0.

ast accepted this revision.Jun 17 2020, 1:16 PM
This revision is now accepted and ready to land.Jun 17 2020, 1:16 PM
This revision was automatically updated to reflect the committed changes.