[BPF] Strip map struct names

[vadorovsky]

Linux kernel rejects programs which use named structs as map definitions. BPF programs written in C usually follow that restriction by using anonymous structs. But some LLVM frontend (e.g. Rust) don't support defining anonymous structs. To make sure they work, strip the map struct name unconditionally.

[vadorovsky]

@eddyz87 @tamird It's hopefully my last PR in this topic. After that, we should be able to not do any fixups in bpf-linker.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions cpp,h -- llvm/lib/Target/BPF/BTFDebug.cpp llvm/lib/Target/BPF/BTFDebug.h --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/llvm/lib/Target/BPF/BTFDebug.cpp b/llvm/lib/Target/BPF/BTFDebug.cpp
index 7718e36f1..43ea28b1c 100644
--- a/llvm/lib/Target/BPF/BTFDebug.cpp
+++ b/llvm/lib/Target/BPF/BTFDebug.cpp
@@ -757,9 +757,8 @@ void BTFDebug::visitStructType(const DICompositeType *CTy, bool IsStruct,
     }
   }
 
-  auto TypeEntry =
-      std::make_unique<BTFTypeStruct>(CTy, IsStruct, HasBitField, VLen,
-                                      StripName);
+  auto TypeEntry = std::make_unique<BTFTypeStruct>(CTy, IsStruct, HasBitField,
+                                                   VLen, StripName);
   StructTypes.push_back(TypeEntry.get());
   TypeId = addType(std::move(TypeEntry), CTy);
 
@@ -864,8 +863,8 @@ void BTFDebug::visitFwdDeclType(const DICompositeType *CTy, bool IsUnion,
 }
 
 /// Handle structure, union, array and enumeration types.
-void BTFDebug::visitCompositeType(const DICompositeType *CTy,
-                                  uint32_t &TypeId, bool StripName) {
+void BTFDebug::visitCompositeType(const DICompositeType *CTy, uint32_t &TypeId,
+                                  bool StripName) {
   auto Tag = CTy->getTag();
   switch (Tag) {
   case dwarf::DW_TAG_structure_type:
@@ -875,7 +874,8 @@ void BTFDebug::visitCompositeType(const DICompositeType *CTy,
     if (CTy->isForwardDecl())
       visitFwdDeclType(CTy, Tag == dwarf::DW_TAG_union_type, TypeId);
     else
-      visitStructType(CTy, Tag == dwarf::DW_TAG_structure_type, TypeId, StripName);
+      visitStructType(CTy, Tag == dwarf::DW_TAG_structure_type, TypeId,
+                      StripName);
     break;
   case dwarf::DW_TAG_array_type:
     visitArrayType(CTy, TypeId);
@@ -905,8 +905,8 @@ void BTFDebug::visitDerivedType(const DIDerivedType *DTy, uint32_t &TypeId,
   unsigned Tag = DTy->getTag();
 
   if (Tag == dwarf::DW_TAG_atomic_type)
-    return visitTypeEntry(DTy->getBaseType(), TypeId, CheckPointer,
-                          SeenPointer, false);
+    return visitTypeEntry(DTy->getBaseType(), TypeId, CheckPointer, SeenPointer,
+                          false);
 
   /// Try to avoid chasing pointees, esp. structure pointees which may
   /// unnecessary bring in a lot of types.

[tamird]

is it time to sprinkle some /* argName = */ falsedecorations?

here and everywhere

[tamird]

Better to say why rather than what.

[tamird]

does this (and the diffs in map-def-3.ll and map-def.ll) mean that this snippet would have been rejected by the kernel?

[eddyz87]

Linux kernel rejects programs which use named structs as map definitions. BPF programs written in C usually follow that restriction by using anonymous structs. But some LLVM frontend (e.g. Rust) don't support defining anonymous structs. To make sure they work, strip the map struct name unconditionally.

By map definition structs, you mean these things:

struct {
        __uint(type, BPF_MAP_TYPE_HASH);
        __uint(max_entries, 1);
        __type(key, long long);
        __type(value, long long);
} map_hash_8b SEC(".maps");

right?

These are not processed by kernel, but by libbpf, so relaxing the requirement on the libbpf side is also an option. (See libbpf.c:parse_btf_map_def()).
Don't like adding more flags to visitTypeEntry, need to think a bit.

[eddyz87]

Actually, I don't see where libbpf enforces map type to be nameless. Could you please provide some detail on what fails w/o this change?

[vadorovsky]

I'm going to close this.

We had a fixup for removing the map names in bpf-linker since forever and I made this PR without questioning it. But now I tested with different kernels down to 5.10 and I cannot reproduce the issue anymore. Perhaps the issues we were facing were just about the pointer types present in the map structs and we got too eager with these fixups.

Sorry for the noise!