tracepoint: Support __data_loc fields in tracepoints

[danobi]

This PR teaches bpftrace to read __data_loc arguments from tracepoints.

Note that this PR causes a minor compatibility break where fields previously
named args->data_loc_name are now named args->name.

This closes #385 . This supercedes #770 .

Checklist

• Language changes are updated in docs/reference_guide.md
• User-visible and non-trivial changes updated in CHANGELOG.md
• The new behaviour is covered by tests

[danobi]

There's still one thing I'm really confused about. So the typical case works ok:

$ sudo ./build/src/bpftrace -e 't:irq:irq_handler_entry { print(str(args->name)) }' | head -10
Attaching 1 probe...
nvidia
nvidia
nvidia
nvidia
nvidia
nvidia
nvidia
nvidia

But printing the address fails:

$ sudo ./build/src/bpftrace -e 't:irq:irq_handler_entry { print(args->name) }' -v
BTF: using data from /sys/kernel/btf/vmlinux
Attaching 1 probe...

Error log:
0: (bf) r6 = r1
1: (61) r1 = *(u32 *)(r6 +12)
2: (b7) r2 = 30007
3: (7b) *(u64 *)(r10 -24) = r2
4: (57) r1 &= 65535
5: (0f) r1 += r6
last_idx 5 first_idx 0
regs=2 stack=0 before 4: (57) r1 &= 65535
regs=2 stack=0 before 3: (7b) *(u64 *)(r10 -24) = r2
regs=2 stack=0 before 2: (b7) r2 = 30007
regs=2 stack=0 before 1: (61) r1 = *(u32 *)(r6 +12)
6: (7b) *(u64 *)(r10 -8) = r1
7: (b7) r1 = 0
8: (7b) *(u64 *)(r10 -16) = r1
last_idx 8 first_idx 0
regs=2 stack=0 before 7: (b7) r1 = 0
9: (18) r7 = 0xffff89de0a4b1600
11: (85) call bpf_get_smp_processor_id#8
12: (bf) r4 = r10
13: (07) r4 += -24
14: (bf) r1 = r6
15: (bf) r2 = r7
16: (bf) r3 = r0
17: (b7) r5 = 24
18: (85) call bpf_perf_event_output#25
invalid indirect read from stack off -24+16 size 24
processed 18 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

ERROR: Error loading program: tracepoint:irq:irq_handler_entry

As far as I can tell, the "-24+16 size 24" error is bogus. Instructions 3, 6 and 8 completely initialize the 24 byte region starting at -24 + 16 = -8.

If anyone has ideas I would be ecstatic to hear them.

[fbs]

Maybe its unhappy about adding ctx into the mix? The ir and output look valid in both cases, but this seems to fix it:

diff --git a/src/ast/codegen_llvm.cpp b/src/ast/codegen_llvm.cpp
index 7c72ce3e..a32e9af1 100644
--- a/src/ast/codegen_llvm.cpp
+++ b/src/ast/codegen_llvm.cpp
@@ -1552,7 +1552,7 @@ void CodegenLLVM::visit(FieldAccess &acc)
                             b_.CreateGEP(ctx_, b_.getInt64(field.offset)));
       expr_ = b_.CreateIntCast(expr_, b_.getInt64Ty(), false);
       expr_ = b_.CreateAnd(expr_, b_.getInt64(0xFFFF));
-      expr_ = b_.CreateAdd(expr_, b_.CreatePtrToInt(ctx_, b_.getInt64Ty()));
+      // expr_ = b_.CreateAdd(expr_, b_.CreatePtrToInt(ctx_, b_.getInt64Ty()));
     }

And this works too

diff --git a/src/ast/codegen_llvm.cpp b/src/ast/codegen_llvm.cpp
index 7c72ce3e..b2d3d6b3 100644
--- a/src/ast/codegen_llvm.cpp
+++ b/src/ast/codegen_llvm.cpp
@@ -1552,7 +1552,8 @@ void CodegenLLVM::visit(FieldAccess &acc)
                             b_.CreateGEP(ctx_, b_.getInt64(field.offset)));
       expr_ = b_.CreateIntCast(expr_, b_.getInt64Ty(), false);
       expr_ = b_.CreateAnd(expr_, b_.getInt64(0xFFFF));
-      expr_ = b_.CreateAdd(expr_, b_.CreatePtrToInt(ctx_, b_.getInt64Ty()));
+      // expr_ = b_.CreateAdd(expr_, b_.CreatePtrToInt(ctx_, b_.getInt64Ty()));
+      expr_ = b_.CreateAdd(expr_, b_.getInt64(1234));
     }

But here it does an i16 load to avoid the masking:

Bytecode:
0: (bf) r6 = r1
1: (69) r1 = *(u16 *)(r6 +12)
2: (b7) r2 = 30007
3: (7b) *(u64 *)(r10 -24) = r2
4: (07) r1 += 1234
5: (7b) *(u64 *)(r10 -8) = r1
6: (b7) r1 = 0
7: (7b) *(u64 *)(r10 -16) = r1
last_idx 7 first_idx 0

[danobi]

Maybe its unhappy about adding ctx into the mix? The ir and output look valid in both cases, but this seems to fix it:

Commenting out the ctx + offset fixed it for me. Not sure there's anything we can do from our side. It seems like a kernel issue to me.

[danobi]

Chatted w/ Alexei. When we add an offset to ctx and store the result to the stack, the verifier thinks it's a register spill and doesn't let anyone read from the location on stack. I'm not sure that's what the correct behavior should be but it's the current limitation.

We'll probably have to live with the unprintable data_loc integers. Shouldn't be too bad functionally b/c it's not like userspace could do anything with that pointer.

Edit: there may be a kernel fix coming sometime.

[fbs]

Just landed the new tuple code, so this might need a rebase

[danobi]

Rebased.

Also moved the type rewrite from semantic analyser into clang parser. Probably better to reduce the spread of the logic. Also made it easier to write a better clang parser test.