Fix alignment of byte arrays inside tuples

When using types internally represented by byte arrays inside tuples
(e.g. usym, inet, etc.), LLVM aligns them to 1 byte inside the tuple. We
must do the same in our internal type system so that the size of our
type matches the size of the type generated by LLVM.

This also renames the `SizedType::GetAlignment` method to
`SizedType::GetInTupleAlignment` as it's only used to get alignment of a
type when it is used inside a tuple and the old name was confusing.

Added runtime and codegen tests for a case where bpftrace previously
crashed.

CHANGELOG.md

@@ -19,6 +19,8 @@ and this project adheres to
   - [#2631](https://github.com/iovisor/bpftrace/pull/2631)
 - Fix crashes when maps are concurrently modified
   - [#2623](https://github.com/iovisor/bpftrace/pull/2623)
+- Fix alignment of byte arrays inside tuples
+  - [#2625](https://github.com/iovisor/bpftrace/pull/2625)
 
 
 ## [0.18.0] 2023-05-15

src/struct.cpp

@@ -65,7 +65,7 @@ std::unique_ptr<Struct> Struct::CreateTuple(std::vector<SizedType> fields)
 
   for (auto &field : fields)
   {
-    auto align = field.GetAlignment();
+    auto align = field.GetInTupleAlignment();
     struct_align = std::max(align, struct_align);
     auto size = field.GetSize();
 

src/types.cpp

@@ -516,9 +516,9 @@ void SizedType::DumpStructure(std::ostream &os)
   return inner_struct_.lock()->Dump(os);
 }
 
-ssize_t SizedType::GetAlignment() const
+ssize_t SizedType::GetInTupleAlignment() const
 {
-  if (IsStringTy())
+  if (IsByteArray())
     return 1;
 
   if (IsTupleTy() || IsRecordTy())
@@ -527,8 +527,8 @@ ssize_t SizedType::GetAlignment() const
   if (GetSize() <= 2)
     return GetSize();
   else if (IsArrayTy())
-    return element_type_->GetAlignment();
-  else if (IsByteArray() || GetSize() <= 4)
+    return element_type_->GetInTupleAlignment();
+  else if (GetSize() <= 4)
     return 4;
   else
     return 8;

src/types.h

@@ -167,9 +167,9 @@ class SizedType
   std::weak_ptr<const Struct> GetStruct() const;
 
   /**
-     Required alignment for this type
+     Required alignment for this type when used inside a tuple
    */
-  ssize_t GetAlignment() const;
+  ssize_t GetInTupleAlignment() const;
 
   /**
      Dump the underlying structure for debug purposes

tests/codegen/llvm/tuple_bytearray.ll

@@ -0,0 +1,66 @@
+; ModuleID = 'bpftrace'
+source_filename = "bpftrace"
+target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+target triple = "bpf-pc-linux"
+
+%usym_t = type { i64, i64 }
+%"unsigned int8_usym_int64__tuple_t" = type { i8, [16 x i8], i64 }
+
+; Function Attrs: nounwind
+declare i64 @llvm.bpf.pseudo(i64 %0, i64 %1) #0
+
+define i64 @"kprobe:f"(i8* %0) section "s_kprobe:f_1" {
+entry:
+  %"@t_key" = alloca i64, align 8
+  %usym = alloca %usym_t, align 8
+  %tuple = alloca %"unsigned int8_usym_int64__tuple_t", align 8
+  %1 = bitcast %"unsigned int8_usym_int64__tuple_t"* %tuple to i8*
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %1)
+  %2 = bitcast %"unsigned int8_usym_int64__tuple_t"* %tuple to i8*
+  call void @llvm.memset.p0i8.i64(i8* align 1 %2, i8 0, i64 32, i1 false)
+  %3 = getelementptr %"unsigned int8_usym_int64__tuple_t", %"unsigned int8_usym_int64__tuple_t"* %tuple, i32 0, i32 0
+  store i8 1, i8* %3, align 1
+  %4 = bitcast i8* %0 to i64*
+  %5 = getelementptr i64, i64* %4, i64 16
+  %reg_ip = load volatile i64, i64* %5, align 8
+  %6 = bitcast %usym_t* %usym to i8*
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %6)
+  %get_pid_tgid = call i64 inttoptr (i64 14 to i64 ()*)()
+  %7 = lshr i64 %get_pid_tgid, 32
+  %8 = getelementptr %usym_t, %usym_t* %usym, i64 0, i32 0
+  %9 = getelementptr %usym_t, %usym_t* %usym, i64 0, i32 1
+  store i64 %reg_ip, i64* %8, align 8
+  store i64 %7, i64* %9, align 8
+  %10 = getelementptr %"unsigned int8_usym_int64__tuple_t", %"unsigned int8_usym_int64__tuple_t"* %tuple, i32 0, i32 1
+  %11 = bitcast [16 x i8]* %10 to i8*
+  %12 = bitcast %usym_t* %usym to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %11, i8* align 1 %12, i64 16, i1 false)
+  %13 = getelementptr %"unsigned int8_usym_int64__tuple_t", %"unsigned int8_usym_int64__tuple_t"* %tuple, i32 0, i32 2
+  store i64 10, i64* %13, align 8
+  %14 = bitcast i64* %"@t_key" to i8*
+  call void @llvm.lifetime.start.p0i8(i64 -1, i8* %14)
+  store i64 0, i64* %"@t_key", align 8
+  %pseudo = call i64 @llvm.bpf.pseudo(i64 1, i64 0)
+  %update_elem = call i64 inttoptr (i64 2 to i64 (i64, i64*, %"unsigned int8_usym_int64__tuple_t"*, i64)*)(i64 %pseudo, i64* %"@t_key", %"unsigned int8_usym_int64__tuple_t"* %tuple, i64 0)
+  %15 = bitcast i64* %"@t_key" to i8*
+  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %15)
+  %16 = bitcast %"unsigned int8_usym_int64__tuple_t"* %tuple to i8*
+  call void @llvm.lifetime.end.p0i8(i64 -1, i8* %16)
+  ret i64 0
+}
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg %0, i8* nocapture %1) #1
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly %0, i8 %1, i64 %2, i1 immarg %3) #2
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly %0, i8* noalias nocapture readonly %1, i64 %2, i1 immarg %3) #1
+
+; Function Attrs: argmemonly nofree nosync nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg %0, i8* nocapture %1) #1
+
+attributes #0 = { nounwind }
+attributes #1 = { argmemonly nofree nosync nounwind willreturn }
+attributes #2 = { argmemonly nofree nosync nounwind willreturn writeonly }

tests/codegen/tuple_bytearray.cpp

@@ -0,0 +1,16 @@
+#include "common.h"
+
+namespace bpftrace {
+namespace test {
+namespace codegen {
+
+TEST(codegen, tuple_bytearray)
+{
+  test(R"_(k:f { @t = ((uint8)1, usym(reg("ip")), 10); })_",
+
+       NAME);
+}
+
+} // namespace codegen
+} // namespace test
+} // namespace bpftrace

tests/runtime/tuples

@@ -88,3 +88,9 @@ NAME tuple strftime type is packed
 PROG BEGIN { @ = (nsecs, strftime("%M:%S", nsecs)); exit(); }
 EXPECT ^@: \(\d+, \d+:\d+\)$
 TIMEOUT 5
+
+NAME bytearray in tuple
+PROG uprobe:./testprogs/uprobe_test:function1 { @ = ((int8)1, usym(reg("ip")), 10); exit(); }
+EXPECT ^@: \(1, 0x[0-9a-f]+, 10\)$
+TIMEOUT 5
+AFTER ./testprogs/uprobe_test