[PATCH] D148243: BPF: Fix skipping of types in map-in-map inner map def

[Dave Marchevsky via Phabricator via llvm-commits]

davemarchevsky created this revision.
davemarchevsky added a reviewer: yonghong-song.
Herald added a subscriber: hiraditya.
Herald added a project: All.
davemarchevsky requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Consider the following BPF program:

  struct keyval {
    u64 key;
    char whatever[16];
  };
  
  struct {
    __uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
    __uint(max_entries, 1000);
    __type(key, int);
    __type(value, int);
    __array(values, struct {
      __uint(type, BPF_MAP_TYPE_HASH);
      __uint(max_entries, 1000);
      __type(key, struct keyval);
      __type(value, int);
    });
  } array_of_hash_maps SEC(".maps");

This program defines an array of hash maps, with `__array(values`
defining the inner map. `struct keyval` is only used as the key of the
inner hash map.

Currently, when BTFDebug walks DWARF debuginfo and generates BTF, it
will not emit a full type for `struct keyval`, instead emitting a
forward declaration. Without full information emitted, the kernel will
fail to load such a map since the size of `struct keyval` isn't known.

Even if size were known, without BTF information about the type's fields
which the verifier might give special treatment - e.g. `bpf_spin_lock` -
programs wouldn't be able to correctly use the map.

This patch fixes the issue described above by modifying
`BTFDebug::visitMapDefType` so that full BTF info is emitted for `struct
keyval`. Specifically, when `visitMapDefType` sees a "values" array - the
only way inner maps are defined currently - it will treat the array
element type like it's a map definition, recursively calling
`visitMapDefType` on it.

Implementation notes:

- A single-parameter `BTFDebug::visitMapDefType` variant is added, similar to the single-param `BTFDebug::visitTypeEntry` in that it uses a scratch uint32_t for the outparam.
- A new "visit" function is added: `BTFDebug::maybeVisitInnerMapDefType`. The function's purpose is to hop `BaseType`s and cast things around until the inner map type is reached, then call `visitMapDefType`. I don't expect it to be called outside of `visitMapDefType`, but separating this logic out keeps things comprehensible.
- The added test uses the example program above to validate that the fixed logic results in full type BTF type info emitted for `struct keyval`.

Signed-off-by: Dave Marchevsky <davemarchevsky at fb.com>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148243

Files:
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/lib/Target/BPF/BTFDebug.h
  llvm/test/CodeGen/BPF/BTF/inner-map-def.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148243.513265.patch
Type: text/x-patch
Size: 11648 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230413/e6eb16ff/attachment.bin>