Field analyser: resolve fields for array accesses

We did not resolve structs for array accesses on pointer types in FieldAnalyser which causes failures when applying the array access operator on pointers to structs: # bpftrace -e 'kfunc:vfs_read { print(args.file[0].f_inode); }' stdin:1:18-37: ERROR: Struct/union of type 'struct file' does not contain a field named 'f_inode' kfunc:vfs_read { print(args.file[0].f_inode); } ~~~~~~~~~~~~~~~~~~~ This fixes the issue by adding the field resolution.
bpftrace · May 20, 2024 · 6977329 · 6977329
1 parent 6d260ed
commit 6977329
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -59,6 +59,8 @@ and this project adheres to
   - [#3095](https://github.com/bpftrace/bpftrace/pull/3095)
 - Fix storing strings of differing lengths in a variable
   - [#3178](https://github.com/bpftrace/bpftrace/pull/3178)
+- Field analyser: resolve fields for array accesses
+  - [#3024](https://github.com/bpftrace/bpftrace/pull/3024)
 #### Docs
 #### Tools
 

diff --git a/src/ast/passes/field_analyser.cpp b/src/ast/passes/field_analyser.cpp
@@ -113,6 +113,16 @@ void FieldAnalyser::visit(FieldAccess &acc)
   }
 }
 
+void FieldAnalyser::visit(ArrayAccess &arr)
+{
+  Visit(*arr.indexpr);
+  Visit(*arr.expr);
+  if (sized_type_.IsPtrTy()) {
+    sized_type_ = *sized_type_.GetPointeeTy();
+    resolve_fields(sized_type_);
+  }
+}
+
 void FieldAnalyser::visit(Cast &cast)
 {
   Visit(*cast.expr);

diff --git a/src/ast/passes/field_analyser.h b/src/ast/passes/field_analyser.h
@@ -31,6 +31,7 @@ class FieldAnalyser : public Visitor {
   void visit(Map &map) override;
   void visit(Variable &var) override;
   void visit(FieldAccess &acc) override;
+  void visit(ArrayAccess &arr) override;
   void visit(Cast &cast) override;
   void visit(Sizeof &szof) override;
   void visit(Offsetof &ofof) override;

diff --git a/tests/codegen/llvm/map_args.ll b/tests/codegen/llvm/map_args.ll
@@ -6,7 +6,7 @@ target triple = "bpf-pc-linux"
 %"struct map_t" = type { i8*, i8*, i8*, i8* }
 %"struct map_t.0" = type { i8*, i8* }
 %"struct map_t.1" = type { i8*, i8*, i8*, i8* }
-%"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args" = type { i32, i64, i64 }
+%"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args" = type { i32, i64, i64, i64 }
 
 @LICENSE = global [4 x i8] c"GPL\00", section "license"
 @AT_ = dso_local global %"struct map_t" zeroinitializer, section ".maps", !dbg !0
@@ -38,12 +38,17 @@ entry:
   %arg2 = load volatile i64, i64* %10, align 8
   %11 = getelementptr %"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args", %"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args"* %args, i64 0, i32 2
   store i64 %arg2, i64* %11, align 8
-  %12 = bitcast i64* %"@_key" to i8*
-  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %12)
+  %12 = bitcast i8* %0 to i64*
+  %13 = getelementptr i64, i64* %12, i64 11
+  %arg3 = load volatile i64, i64* %13, align 8
+  %14 = getelementptr %"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args", %"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args"* %args, i64 0, i32 3
+  store i64 %arg3, i64* %14, align 8
+  %15 = bitcast i64* %"@_key" to i8*
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %15)
   store i64 0, i64* %"@_key", align 8
   %update_elem = call i64 inttoptr (i64 2 to i64 (%"struct map_t"*, i64*, %"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args"*, i64)*)(%"struct map_t"* @AT_, i64* %"@_key", %"uprobe:/tmp/bpftrace-test-dwarf-data:func_1_args"* %args, i64 0)
-  %13 = bitcast i64* %"@_key" to i8*
-  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %13)
+  %16 = bitcast i64* %"@_key" to i8*
+  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %16)
   ret i64 0
 }
 
@@ -80,10 +85,10 @@ attributes #1 = { argmemonly nofree nosync nounwind willreturn }
 !18 = !DIBasicType(name: "int64", size: 64, encoding: DW_ATE_signed)
 !19 = !DIDerivedType(tag: DW_TAG_member, name: "value", scope: !2, file: !2, baseType: !20, size: 64, offset: 192)
 !20 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !21, size: 64)
-!21 = !DICompositeType(tag: DW_TAG_array_type, baseType: !22, size: 160, elements: !23)
+!21 = !DICompositeType(tag: DW_TAG_array_type, baseType: !22, size: 224, elements: !23)
 !22 = !DIBasicType(name: "int8", size: 8, encoding: DW_ATE_signed)
 !23 = !{!24}
-!24 = !DISubrange(count: 20, lowerBound: 0)
+!24 = !DISubrange(count: 28, lowerBound: 0)
 !25 = !DIGlobalVariableExpression(var: !26, expr: !DIExpression())
 !26 = distinct !DIGlobalVariable(name: "ringbuf", linkageName: "global", scope: !2, file: !2, type: !27, isLocal: false, isDefinition: true)
 !27 = !DICompositeType(tag: DW_TAG_structure_type, scope: !2, file: !2, size: 128, elements: !28)

diff --git a/tests/data/data_source.c b/tests/data/data_source.c
@@ -21,7 +21,10 @@ struct Foo3 {
 
 struct Foo3 foo3;
 
-struct Foo3 *func_1(int a, struct Foo1 *foo1, struct Foo2 *foo2)
+struct Foo3 *func_1(int a,
+                    struct Foo1 *foo1,
+                    struct Foo2 *foo2,
+                    struct Foo3 *foo3)
 {
   return 0;
 }
@@ -31,7 +34,8 @@ struct Foo3 *func_2(int a, int *b, struct Foo1 *foo1)
   return 0;
 }
 
-struct Foo3 *func_3(int a, int *b, struct Foo1 *foo1)
+// __attribute__((noinline)) is needed due to a LLDB/GCC compatibility bug
+struct Foo3 *__attribute__((noinline)) func_3(int a, int *b, struct Foo1 *foo1)
 {
   return 0;
 }
@@ -115,7 +119,7 @@ int main(void)
   struct bpf_iter__task_file iter_task_file;
   struct bpf_iter__task_vma iter_task_vma;
 
-  func_1(0, 0, 0);
+  func_1(0, 0, 0, 0);
 
   bpf_iter_task();
   bpf_iter_task_file();

diff --git a/tests/field_analyser.cpp b/tests/field_analyser.cpp
@@ -248,6 +248,31 @@ TEST_F(field_analyser_btf, btf_types_struct_ptr)
   ASSERT_EQ(foo3->fields.size(), 2U); // fields are resolved
 }
 
+TEST_F(field_analyser_btf, btf_types_arr_access)
+{
+  BPFtrace bpftrace;
+  bpftrace.parse_btf({});
+  test(bpftrace,
+       "kfunc:func_1 {\n"
+       "  @foo2 = args.foo3[0].foo2;\n"
+       "}",
+       0);
+
+  // args.foo3[0].foo2 should do 2 things:
+  // - add struct Foo2 (without resolving its fields)
+  // - resolve fields of struct Foo3
+
+  ASSERT_TRUE(bpftrace.structs.Has("struct Foo2"));
+  ASSERT_TRUE(bpftrace.structs.Has("struct Foo3"));
+  auto foo2 = bpftrace.structs.Lookup("struct Foo2").lock();
+  auto foo3 = bpftrace.structs.Lookup("struct Foo3").lock();
+
+  EXPECT_EQ(foo2->size, 24);
+  ASSERT_EQ(foo2->fields.size(), 0U); // fields are not resolved
+  EXPECT_EQ(foo3->size, 16);
+  ASSERT_EQ(foo3->fields.size(), 2U); // fields are resolved
+}
+
 TEST_F(field_analyser_btf, btf_types_bitfields)
 {
   BPFtrace bpftrace;