[BPF] Support external globals

[yonghong-song]

Summary

[BPF] Support external globals

Previously, only types for external variables and globals without
section names are omitted.
This patch emitted types for all non-common-linkage globals.
From https://llvm.org/docs/LangRef.html,

common
  “common” linkage is most similar to “weak” linkage,
  but they are used for tentative definitions in C,
  such as “int X;” at global scope. ...

It is not clear what are "common"/"weak" semantics for bpf programs,
so omit them for now.

For non-extern global variables without section names,

• a BTF_KIND_VAR will be generated.
• the BTF_KIND_VAR will be put into .bss/.data DataSec based on whether they are zero initialized or not.

For extern globals:

• A BTF_KIND_VAR will be generated.
• if with section name, a DataSec with that section name will be created containing that global variable.
• external globals do not have types in BTF_KIND_VAR since clang does not generate debug info type for external variables.
• the in-section offset for extern variables will be all 0 since these variables are not allocated in current ELF.

[anakryiko]

llvm/lib/Target/BPF/BTF.h

183–184

what's the difference between EXTERNAL and TENTATIVE?

llvm/lib/Target/BPF/BTFDebug.cpp

1064

I remember there were problems figuring out datasec size when emitting BTF. Is this still the case? Does the same problem prevent readonly flag in DATASEC or you are trying to not put redundant information into BTF that can be derived from ELF?

1112

we'll just ignore externs with GlobalValue::CommonLinkage, right? Should we just assign a special section name to them instead? feels bad to only have a partial list of externs.

[yonghong-song]

llvm/lib/Target/BPF/BTF.h

183–184

TENTATIVE is for .common symbols. Just do "int g;" and compile, you will find out.

llvm/lib/Target/BPF/BTFDebug.cpp

1064

Yes this is still the case. The debugging info is emitted earlier and we do not know the data section until very end of compilation which is too late.

The section readonly thing is marked by ELF object writer. The BPF backend can only detect readonly variables through type inspection, so it MAY mark the container DataSec as readonly section. Let us do it now with ELF checking since the ELF section inspection is required to get the readonly data.

1112

No, all externs will emitted with their special section name.
CommonLinkage is not an extern.

[yonghong-song]

do not recognize section names for extern variables in llvm and leave it to bpf loader
upgrade and add test cases

[anakryiko]

if there is no type information available for extern variables, there is little benefit in having them as DATASEC+VAR. No size, no type. We can get the same information from ELF itself without having to teach verifier and libbpf how to handle these new types. Let's drop this for now? The only potential benefit is having a section name associated with extern, but we can either add that later, or even add it in a way that will require no verifier changes - BTF.ext.

[yonghong-song]

Yes, we can drop it now. There is no rush if there is no real need. The extern variables are not used yet. Let us revisit this once we have some concrete use cases.

[yonghong-song]

Actually, there is 'size' but no 'type'. And 'size' can be used to do cross checking. But anyway, we can delay this until there is a concrete thing we need to implement.

[yonghong-song]

Also, another thing. we COULD emit debug info type for externs for BPF in clang. We just have to think whether this is really necessary or not.

[anakryiko]

hm... size is useful, actually. Ok, let me play with it, it's hard to get a good idea of BTF from LLVM tests. Verifier will definitely reject such variables, though, so we'll need to do sanitization for sure.

[anakryiko]

In D68822#1717603, @yonghong-song wrote:

Also, another thing. we COULD emit debug info type for externs for BPF in clang. We just have to think whether this is really necessary or not.

I think it's extremely useful. Think about exposing some internal kernel stuff. This is going to be a requirement for BPF verifier, I think. I'll let @ast confirm, though.

[yonghong-song]

This patch is not needed any more.

llvm/lib/Target/BPF/BTF.h

struct BTFParam {
   uint32_t NameOff;
   uint32_t Type;
 };
 
 /// Variable scoping information.
 enum : uint8_t {
   VAR_STATIC = 0,           ///< Linkage: InternalLinkage
   VAR_GLOBAL_ALLOCATED = 1, ///< Linkage: ExternalLinkage
-  VAR_GLOBAL_TENTATIVE = 2, ///< Linkage: CommonLinkage
-  VAR_GLOBAL_EXTERNAL = 3,  ///< Linkage: ExternalLinkage
+  VAR_GLOBAL_EXTERNAL = 2,  ///< Linkage: ExternalLinkage
+  VAR_GLOBAL_TENTATIVE = 3, ///< Linkage: CommonLinkage
 };
 
 /// BTF_KIND_DATASEC are followed by multiple "struct BTFDataSecVar".
 /// The exist number of BTFDataSec is stored in the vlen (of the info
 /// in "struct CommonType").
 struct BTFDataSec {
   uint32_t Type;   ///< A BTF_KIND_VAR type
   uint32_t Offset; ///< In-section offset

llvm/lib/Target/BPF/BTFDebug.cpp

void BTFDebug::beginInstruction(const MachineInstr *MI) {
   LineInfoGenerated = true;
   PrevInstLoc = DL;
 }
 
 void BTFDebug::processGlobals(bool ProcessingMapDef) {
   // Collect all types referenced by globals.
   const Module *M = MMI->getModule();
   for (const GlobalVariable &Global : M->globals()) {
-    // Ignore external globals for now.
-    if (!Global.hasInitializer() && Global.hasExternalLinkage())
-      continue;
+    // Support all globals:
+    //   . static: VAR_STATIC, in DataSec.
+    //   . initialized global (0 or non-0): VAR_GLOBAL_ALLOCATED, in DataSec.
+    //   . external global: VAR_GLOBAL_EXTERNAL, not in DataSec.
+    //   . common global: VAR_GLOBAL_TENTATIVE, not in DataSec.
+    // Whether DataSec is readonly or not can be found from the
+    // corresponding ELF section flags.
 
     // Decide the section name.
+    // CommonLinkage variables do not have section names, and we
+    // don't want to put them into .bss/.data/.rodata sections.
     StringRef SecName;
+    uint32_t GVarInfo;
+    auto Linkage = Global.getLinkage();
+    if (Global.hasInitializer() && Linkage != GlobalValue::CommonLinkage) {
       if (Global.hasSection()) {
         SecName = Global.getSection();
       } else {
         // data, bss, or readonly sections
         if (Global.isConstant())
           SecName = ".rodata";
         else
           SecName = Global.getInitializer()->isZeroValue() ? ".bss" : ".data";
       }
+      if (Linkage == GlobalValue::InternalLinkage)
+        GVarInfo = BTF::VAR_STATIC;
+      else
+        GVarInfo = BTF::VAR_GLOBAL_ALLOCATED;
+    } else if (Linkage == GlobalValue::CommonLinkage) {
+      GVarInfo = BTF::VAR_GLOBAL_TENTATIVE;
+    } else {
+      // External variables.
+      GVarInfo = BTF::VAR_GLOBAL_EXTERNAL;
+      SecName = Global.getSection();
+    }
 
     if (ProcessingMapDef != SecName.startswith(".maps"))
       continue;
 
     SmallVector<DIGlobalVariableExpression *, 1> GVs;
     Global.getDebugInfo(GVs);
     uint32_t GVTypeId = 0;
     for (auto *GVE : GVs) {
       if (SecName.startswith(".maps"))
         visitMapDefType(GVE->getVariable()->getType(), GVTypeId);
       else
         visitTypeEntry(GVE->getVariable()->getType(), GVTypeId, false, false);
       break;
     }
 
-    // Only support the following globals:
-    //  . static variables
-    //  . non-static global variables with section attributes
-    // Essentially means:
-    //  . .bcc/.data/.rodata DataSec entities only contain static data
-    //  . Other DataSec entities contain static or initialized global data.
-    //    Initialized global data are mostly used for finding map key/value type
-    //    id's. Whether DataSec is readonly or not can be found from
-    //    corresponding ELF section flags.
-    auto Linkage = Global.getLinkage();
-    if (Linkage != GlobalValue::InternalLinkage &&
-        (Linkage != GlobalValue::ExternalLinkage || !Global.hasSection()))
-      continue;
-
-    uint32_t GVarInfo = Linkage == GlobalValue::ExternalLinkage
-                            ? BTF::VAR_GLOBAL_ALLOCATED
-                            : BTF::VAR_STATIC;
     auto VarEntry =
         std::make_unique<BTFKindVar>(Global.getName(), GVTypeId, GVarInfo);
     uint32_t VarId = addType(std::move(VarEntry));
 
+    if (SecName.empty())
+      continue;
+
     // Find or create a DataSec
     if (DataSecEntries.find(SecName) == DataSecEntries.end()) {
       DataSecEntries[SecName] = std::make_unique<BTFKindDataSec>(Asm, SecName);
     }
 
     // Calculate symbol size
     const DataLayout &DL = Global.getParent()->getDataLayout();
     uint32_t Size = DL.getTypeAllocSize(Global.getType()->getElementType());