llvm: stubbed st_size does matter for copy-relocs/bss

llvm/tools/llc/StubMaker.cpp

Index: llvm/tools/llc/StubMaker.cpp
===================================================================
--- a/llvm/tools/llc/StubMaker.cpp
+++ b/llvm/tools/llc/StubMaker.cpp
@@ -88,11 +88,8 @@
   llvm_unreachable("Unknown visibility in GETELFVisibility");
 }
 
-// Return a value for the symbol table's st_size, which is the number of bytes
-// in a data object.  This may be 0 if the size is unknown.
-static ELF::Elf32_Word GetELFSize(const GlobalValue *GV) {
-  const class PointerType *PT = GV->getType();
-  const Type *ElemType = PT->getElementType();
+static ELF::Elf32_Word GetElfSizeForType(const GlobalValue *GV,
+                                         const Type *ElemType) {
   unsigned bit_size = ElemType->getPrimitiveSizeInBits();
   if (bit_size != 0) {
     return bit_size / 8;
@@ -106,10 +103,51 @@
       }
     }
   }
-  // For other cases, claim that we don't know the size (0).
+  if (const VectorType *VTy = dyn_cast<VectorType>(ElemType)) {
+    unsigned bit_width = VTy->getBitWidth();
+    if (bit_width) {
+      return bit_width / 8;
+    } else {
+      // It's a vector of pointers, and pointers are 32-bit in NaCl
+      return VTy->getNumElements() * 4;
+    }
+  }
+  if (isa<PointerType>(ElemType)) {
+    // Pointers are 32-bit for NaCl.
+    return 4;
+  }
+  if (isa<FunctionType>(ElemType)) {
+    // This is not a data object, so shouldn't be in BSS. Just say unknown (0).
+    return 0;
+  }
+  if (const StructType *STy = dyn_cast<StructType>(ElemType)) {
+    // Alignment padding should have been added to the type in the front-end.
+    unsigned size_so_far = 0;
+    for (unsigned i = 0; i < STy->getNumElements(); ++i) {
+      size_so_far += GetElfSizeForType(GV, STy->getElementType(i));
+    }
+    return size_so_far;
+  }
+  // Unknown type!
+  DEBUG({

[robertm, 2012/04/25 17:42:07]

would it be better to assert for now?
otherwise we might get hard to catch errors.

[jvoung - send to chromium..., 2012/04/25 18:20:51]

Good point. Switched over to an assert and already caught a bug =)

(ArrayType ElemSize computation should have been a recursive call).

Actually, now I'm worried that the ELF -> PSO-stub maker is too loose about
datatype and size (might lose info there), but that's a different issue... 
hopefully only pointers are exported I guess.

[jvoung - send to chromium..., 2012/04/25 18:22:19]

Err... n/m about the PSO stub. The header file should let the pexe know the type
of things and get the declaration.

+      dbgs() << "Unknown GetELFSize for var=";
+      GV->dump();
+      dbgs() << " type= ";
+      ElemType->dump();
+      dbgs() << "\n";
+    });
   return 0;
 }
 
+// Return a value for the symbol table's st_size, which is the number of bytes
+// in a data object.  This is important for symbols that may sit in BSS

[robertm, 2012/04/25 17:42:07]

why wouldn't it also be important for non bss symbols?
Maybe add some more info while it is still fresh  in your mind.

[jvoung - send to chromium..., 2012/04/25 18:20:51]

Well, I'm not sure, but I don't think it matters for functions.  Rephrased.

+// with copy relocations.  This may be 0 if the size is unknown.
+static ELF::Elf32_Word GetELFSize(const GlobalValue *GV) {
+  const class PointerType *PT = GV->getType();
+  const Type *ElemType = PT->getElementType();
+  return GetElfSizeForType(GV, ElemType);
+}
+
 static unsigned char GetELFType(const GlobalValue *GV) {
   if (const GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV)) {
     return GVar->isThreadLocal() ? ELF::STT_TLS : ELF::STT_OBJECT;