Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix alignment of byte arrays inside tuples #2625

Merged
merged 1 commit into from
Jun 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/struct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
8 changes: 4 additions & 4 deletions src/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
66 changes: 66 additions & 0 deletions tests/codegen/llvm/tuple_bytearray.ll
Original file line number Diff line number Diff line change
@@ -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 }
16 changes: 16 additions & 0 deletions tests/codegen/tuple_bytearray.cpp
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions tests/runtime/tuples
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading