provide padded structure for table_{key|leaf}_desc API

This patch intends to fix issue #606.

Currently, the key/value type information is passed from
C++ to Python through a JSON interface. The JSON is
constructed by traversing the struct/field's through clang
AST interface. Once Python gets the JSON, it will
reconstruct the C structure through ctype module.

There are two known issues where Python reconstructed
C structure may not be the same as the original C structure:
  . if user explicitly use "__attribute__ ((align))" to alter
    field alignment and such information is not passed to
    Python.
  . the "__int128" type is a u64[2] array in python.
    So in C, __int128 needs to align on 16 bytes boundary, and
    in Python, it aligns with 8 bytes boundary.

To solve this issue, this patch provided the structure
with added padding fields to Python. For example,
  struct {
    char a;
    __int128 b;
  };
Python will receive
  struct {
    char a;
    char __pad_1[15];
    __int128 b;
  };
Signed-off-by: Yonghong Song <yhs@fb.com>

src/cc/json_map_decl_visitor.cc

@@ -18,6 +18,7 @@
 #include <string>
 
 #include <clang/AST/ASTContext.h>
+#include <clang/AST/RecordLayout.h>
 #include <clang/AST/RecursiveASTVisitor.h>
 #include "common.h"
 #include "table_desc.h"
@@ -43,6 +44,10 @@ class BMapDeclVisitor : public clang::RecursiveASTVisitor<BMapDeclVisitor> {
   bool VisitEnumConstantDecl(clang::EnumConstantDecl *D);
   bool VisitEnumDecl(clang::EnumDecl *D);
 
+ private:
+  bool shouldSkipPadding(const RecordDecl *D);
+  void genJSONForField(FieldDecl *F);
+
  private:
   clang::ASTContext &C;
   std::string &result_;
@@ -50,6 +55,36 @@ class BMapDeclVisitor : public clang::RecursiveASTVisitor<BMapDeclVisitor> {
 
 // Encode the struct layout as a json description
 BMapDeclVisitor::BMapDeclVisitor(ASTContext &C, string &result) : C(C), result_(result) {}
+
+bool BMapDeclVisitor::shouldSkipPadding(const RecordDecl *D) {
+  if (D->isUnion() || D->field_empty())
+    return true;
+  for (auto F : D->getDefinition()->fields()) {
+    if (F->isBitField())
+      return true;
+    QualType Ty = F->getType();
+    if (Ty->isIncompleteArrayType())
+      return true;
+  }
+  return false;
+}
+
+void BMapDeclVisitor::genJSONForField(FieldDecl *F) {
+  if (F->isAnonymousStructOrUnion()) {
+    if (const RecordType *R = dyn_cast<RecordType>(F->getType()))
+      TraverseDecl(R->getDecl());
+    result_ += ", ";
+    return;
+  }
+  result_ += "[";
+  TraverseDecl(F);
+  if (const ConstantArrayType *T = dyn_cast<ConstantArrayType>(F->getType()))
+    result_ += ", [" + T->getSize().toString(10, false) + "]";
+  if (F->isBitField())
+    result_ += ", " + to_string(F->getBitWidthValue(C));
+  result_ += "], ";
+}
+
 bool BMapDeclVisitor::VisitFieldDecl(FieldDecl *D) {
   result_ += "\"";
   result_ += D->getName();
@@ -82,25 +117,36 @@ bool BMapDeclVisitor::TraverseRecordDecl(RecordDecl *D) {
     return false;
   return true;
 }
+
 bool BMapDeclVisitor::VisitRecordDecl(RecordDecl *D) {
   result_ += "[\"";
   result_ += D->getName();
   result_ += "\", [";
+
+  bool SkipPadding = shouldSkipPadding(D);
+  if (SkipPadding) {
     for (auto F : D->getDefinition()->fields()) {
-    if (F->isAnonymousStructOrUnion()) {
-      if (const RecordType *R = dyn_cast<RecordType>(F->getType()))
-        TraverseDecl(R->getDecl());
-      result_ += ", ";
-      continue;
+      genJSONForField(F);
     }
-    result_ += "[";
-    TraverseDecl(F);
-    if (const ConstantArrayType *T = dyn_cast<ConstantArrayType>(F->getType()))
-      result_ += ", [" + T->getSize().toString(10, false) + "]";
-    if (F->isBitField())
-      result_ += ", " + to_string(F->getBitWidthValue(C));
-    result_ += "], ";
+  } else {
+    const ASTRecordLayout &Layout = C.getASTRecordLayout(D);
+    CharUnits Offset = C.toCharUnitsFromBits(Layout.getFieldOffset(0));
+    for (auto F : D->getDefinition()->fields()) {
+      CharUnits FieldSize = C.getTypeSizeInChars(F->getType());
+      auto FieldOffsetBits = Layout.getFieldOffset(F->getFieldIndex());
+      CharUnits FieldOffset = C.toCharUnitsFromBits(FieldOffsetBits);
+
+      uint64_t Padding = (FieldOffset - Offset).getQuantity();
+      if (Padding) {
+        /* Padding before this field with "char __pad_<FieldIndex>[Padding]". */
+        result_ += "[\"__pad_" + to_string(F->getFieldIndex()) + "\",\"char\",["
+                + to_string(Padding) + "]], ";
+      }
+      Offset = FieldOffset + FieldSize;
+      genJSONForField(F);
     }
+  }
+
   if (!D->getDefinition()->field_empty())
     result_.erase(result_.end() - 2);
   result_ += "]";

tests/python/test_clang.py

@@ -675,6 +675,28 @@ BPF_HASH(table1, unsigned __int128, __int128);
                                               struct.pack('LL', k[0], k[1])),
                              "2001:db8::")
 
+    def test_padding_types(self):
+        text = """
+struct key_t {
+  u32 f1_1;               /* offset 0 */
+  struct {
+    char f2_1;            /* offset 16 */
+    __int128 f2_2;        /* offset 32 */
+  };
+  u8 f1_3;                /* offset 48 */
+  unsigned __int128 f1_4; /* offset 64 */
+};
+struct value_t {
+  u8 src[4] __attribute__ ((aligned (8))); /* offset 0 */
+  u8 dst[4] __attribute__ ((aligned (8))); /* offset 8 */
+};
+BPF_HASH(table1, struct key_t, struct value_t);
+"""
+        b = BPF(text=text)
+        table = b['table1']
+        self.assertEqual(ct.sizeof(table.Key), 80)
+        self.assertEqual(ct.sizeof(table.Leaf), 12)
+
 
 if __name__ == "__main__":
     main()

tests/python/test_trace2.py

@@ -11,12 +11,14 @@ from unittest import main, TestCase
 text = """
 #include <linux/ptrace.h>
 struct Ptr { u64 ptr; };
-struct Counters { u64 stat1; };
+struct Counters { char unused; __int128 stat1; };
 BPF_HASH(stats, struct Ptr, struct Counters, 1024);
 
 int count_sched(struct pt_regs *ctx) {
   struct Ptr key = {.ptr=PT_REGS_PARM1(ctx)};
-  struct Counters zleaf = {0};
+  struct Counters zleaf;
+
+  memset(&zleaf, 0, sizeof(zleaf));
   stats.lookup_or_init(&key, &zleaf)->stat1++;
   return 0;
 }
@@ -32,7 +34,7 @@ class TestTracingEvent(TestCase):
         for i in range(0, 100):
             sleep(0.01)
         for key, leaf in self.stats.items():
-            print("ptr %x:" % key.ptr, "stat1 %d" % leaf.stat1)
+            print("ptr %x:" % key.ptr, "stat1 (%d %d)" % (leaf.stat1[1], leaf.stat1[0]))
 
 if __name__ == "__main__":
     main()