Skip to content

Commit 839b9ee

Browse files
umanwizardfabled
umanwizard
and
fabled
authored
Fix FP+RA handling on aarch64 (open-telemetry#1048)
Co-authored-by: Timo Teräs <timo.teras@iki.fi>
1 parent 1adfd51 commit 839b9ee

23 files changed

Lines changed: 67 additions & 122 deletions

‎nativeunwind/elfunwindinfo/elfehframe_aarch64.go‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,19 +144,11 @@ func (regs *vmRegs) getUnwindInfoARM() sdtypes.UnwindInfo {
144144
info.FPOpcode = support.UnwindOpcodeBaseLR
145145
info.FPParam = 0
146146
case regCFA:
147-
if regs.cfa.off != 0 {
148-
// In ARM64, nothing can be assumed regarding RA location, it is
149-
// simply somewhere on the stack, its detailed location needs to
150-
// be extracted from FDE record.
151-
// In our approach, RA offset part of stack delta always points
152-
// to RA location no matter whether CFA is evaluated with respect
153-
// to SP or FP.
154-
// Use same opcode as for CFA:
155-
info.FPOpcode = info.Opcode
156-
// Convert CFA base to SP / FP base in order to keep
157-
// offset to RA from frame bottom (FP based heuristic).
158-
// CFA offset needs to be added to the one denoting RA location.
159-
info.FPParam = int32(regs.cfa.off) + int32(regs.ra.off)
147+
info.FPParam = int32(regs.ra.off)
148+
if regs.fp.reg == regCFA && regs.fp.off+8 == regs.ra.off {
149+
info.FPOpcode = support.UnwindOpcodeBaseCFAFrame
150+
} else {
151+
info.FPOpcode = support.UnwindOpcodeBaseCFA
160152
}
161153
}
162154

‎support/ebpf/native_stack_trace.ebpf.c‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,9 @@ static EBPF_INLINE u64 unwind_register_address(UnwindState *state, u64 cfa, u8 o
300300

301301
// Resolve the 'BASE' register, and fetch the CFA/FP/SP value.
302302
switch (opcode & ~UNWIND_OPCODEF_DEREF) {
303+
#if defined(__aarch64__)
304+
case UNWIND_OPCODE_BASE_CFA_FRAME:
305+
#endif
303306
case UNWIND_OPCODE_BASE_CFA: addr = cfa; break;
304307
case UNWIND_OPCODE_BASE_FP: addr = state->fp; break;
305308
case UNWIND_OPCODE_BASE_SP: addr = state->sp; break;
@@ -342,6 +345,9 @@ static EBPF_INLINE u64 unwind_register_address(UnwindState *state, u64 cfa, u8 o
342345
#ifdef OPTI_DEBUG
343346
switch (opcode) {
344347
case UNWIND_OPCODE_BASE_CFA: DEBUG_PRINT("unwind: cfa+%d", preDeref); break;
348+
#if defined(__aarch64__)
349+
case UNWIND_OPCODE_BASE_CFA_FRAME: DEBUG_PRINT("unwind (fp+ra): cfa+%d", preDeref - 8); break;
350+
#endif
345351
case UNWIND_OPCODE_BASE_FP: DEBUG_PRINT("unwind: fp+%d", preDeref); break;
346352
case UNWIND_OPCODE_BASE_SP: DEBUG_PRINT("unwind: sp+%d", preDeref); break;
347353
case UNWIND_OPCODE_BASE_CFA | UNWIND_OPCODEF_DEREF:
@@ -569,7 +575,18 @@ static EBPF_INLINE ErrorCode unwind_one_frame(struct UnwindState *state, bool *s
569575
DEBUG_PRINT("RA: %016llX", (u64)ra);
570576

571577
// read the value of RA from stack
572-
if (bpf_probe_read_user(&state->pc, sizeof(state->pc), (void *)ra)) {
578+
int err;
579+
u64 fpra[2];
580+
fpra[0] = state->fp;
581+
if (info->fpOpcode == UNWIND_OPCODE_BASE_CFA_FRAME) {
582+
err = bpf_probe_read_user(fpra, sizeof(fpra), (void *)(ra - 8));
583+
} else {
584+
err = bpf_probe_read_user(&fpra[1], sizeof(fpra[0]), (void *)ra);
585+
}
586+
if (!err) {
587+
state->fp = fpra[0];
588+
state->pc = fpra[1];
589+
} else {
573590
// error reading memory, mark RA as invalid
574591
ra = 0;
575592
}
@@ -586,22 +603,6 @@ static EBPF_INLINE ErrorCode unwind_one_frame(struct UnwindState *state, bool *s
586603
return ERR_NATIVE_PC_READ;
587604
}
588605

589-
// Try to resolve frame pointer
590-
// simple heuristic for FP based frames
591-
// the GCC compiler usually generates stack frame records in such a way,
592-
// so that FP/RA pair is at the bottom of a stack frame (stack frame
593-
// record at lower addresses is followed by stack vars at higher ones)
594-
// this implies that if no other changes are applied to the stack such
595-
// as alloca(), following the prolog SP/FP points to the frame record
596-
// itself, in such a case FP offset will be equal to 8
597-
if (info->fpParam == 8) {
598-
// we can assume the presence of frame pointers
599-
if (info->fpOpcode != UNWIND_OPCODE_BASE_LR) {
600-
// FP precedes the RA on the stack (Aarch64 ABI requirement)
601-
bpf_probe_read_user(&state->fp, sizeof(state->fp), (void *)(ra - 8));
602-
}
603-
}
604-
605606
state->sp = cfa;
606607
unwinder_mark_nonleaf_frame(state);
607608
frame_ok:

‎support/ebpf/stackdeltatypes.h‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,22 @@
22
#define OPTI_STACKDELTATYPES_H
33

44
// Command without arguments, the argument is instead an UNWIND_COMMAND_* value
5-
#define UNWIND_OPCODE_COMMAND 0x00
5+
#define UNWIND_OPCODE_COMMAND 0x00
66
// Expression with base value being the Canonical Frame Address (CFA)
7-
#define UNWIND_OPCODE_BASE_CFA 0x01
7+
#define UNWIND_OPCODE_BASE_CFA 0x01
88
// Expression with base value being the Stack Pointer
9-
#define UNWIND_OPCODE_BASE_SP 0x02
9+
#define UNWIND_OPCODE_BASE_SP 0x02
1010
// Expression with base value being the Frame Pointer
11-
#define UNWIND_OPCODE_BASE_FP 0x03
11+
#define UNWIND_OPCODE_BASE_FP 0x03
1212
// Expression with base value being the Link Register (ARM64)
13-
#define UNWIND_OPCODE_BASE_LR 0x04
13+
#define UNWIND_OPCODE_BASE_LR 0x04
1414
// Expression with base value being a Generic Register
15-
#define UNWIND_OPCODE_BASE_REG 0x05
15+
#define UNWIND_OPCODE_BASE_REG 0x05
16+
// Expression for RA with base value being the CFA, and
17+
// also indicating that the FP immediately precedes the RA (ARM64).
18+
#define UNWIND_OPCODE_BASE_CFA_FRAME 0x06
1619
// An opcode flag to indicate that the value should be dereferenced
17-
#define UNWIND_OPCODEF_DEREF 0x80
20+
#define UNWIND_OPCODEF_DEREF 0x80
1821

1922
// Unsupported or no value for the register
2023
#define UNWIND_COMMAND_INVALID 0

‎support/ebpf/tracer.ebpf.amd64‎

0 Bytes
Binary file not shown.

‎support/ebpf/tracer.ebpf.arm64‎

1.41 KB
Binary file not shown.

‎support/types.go‎

Lines changed: 8 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎support/types_def.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,14 @@ const (
146146

147147
const (
148148
// UnwindOpcodes from the C header file
149-
UnwindOpcodeCommand uint8 = C.UNWIND_OPCODE_COMMAND
150-
UnwindOpcodeBaseCFA uint8 = C.UNWIND_OPCODE_BASE_CFA
151-
UnwindOpcodeBaseSP uint8 = C.UNWIND_OPCODE_BASE_SP
152-
UnwindOpcodeBaseFP uint8 = C.UNWIND_OPCODE_BASE_FP
153-
UnwindOpcodeBaseLR uint8 = C.UNWIND_OPCODE_BASE_LR
154-
UnwindOpcodeBaseReg uint8 = C.UNWIND_OPCODE_BASE_REG
155-
UnwindOpcodeFlagDeref uint8 = C.UNWIND_OPCODEF_DEREF
149+
UnwindOpcodeCommand uint8 = C.UNWIND_OPCODE_COMMAND
150+
UnwindOpcodeBaseCFA uint8 = C.UNWIND_OPCODE_BASE_CFA
151+
UnwindOpcodeBaseSP uint8 = C.UNWIND_OPCODE_BASE_SP
152+
UnwindOpcodeBaseFP uint8 = C.UNWIND_OPCODE_BASE_FP
153+
UnwindOpcodeBaseLR uint8 = C.UNWIND_OPCODE_BASE_LR
154+
UnwindOpcodeBaseReg uint8 = C.UNWIND_OPCODE_BASE_REG
155+
UnwindOpcodeBaseCFAFrame uint8 = C.UNWIND_OPCODE_BASE_CFA_FRAME
156+
UnwindOpcodeFlagDeref uint8 = C.UNWIND_OPCODEF_DEREF
156157

157158
// UnwindCommands from the C header file
158159
UnwindCommandInvalid int32 = C.UNWIND_COMMAND_INVALID

‎tools/coredump/testdata/arm64/node-hidden-internal-symbols.json‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
"node+0x17f07fb",
5959
"node+0x90848b",
6060
"libc.so.6+0x8202f",
61-
"libc.so.6+0x8202f",
6261
"libc.so.6+0xebf1b"
6362
]
6463
},
@@ -70,7 +69,6 @@
7069
"node+0x17f07fb",
7170
"node+0x90848b",
7271
"libc.so.6+0x8202f",
73-
"libc.so.6+0x8202f",
7472
"libc.so.6+0xebf1b"
7573
]
7674
},
@@ -82,7 +80,6 @@
8280
"node+0x17f07fb",
8381
"node+0x90848b",
8482
"libc.so.6+0x8202f",
85-
"libc.so.6+0x8202f",
8683
"libc.so.6+0xebf1b"
8784
]
8885
},
@@ -94,7 +91,6 @@
9491
"node+0x17f07fb",
9592
"node+0x90848b",
9693
"libc.so.6+0x8202f",
97-
"libc.so.6+0x8202f",
9894
"libc.so.6+0xebf1b"
9995
]
10096
},
@@ -106,7 +102,6 @@
106102
"node+0x17e1abb",
107103
"node+0x90d2ff",
108104
"libc.so.6+0x8202f",
109-
"libc.so.6+0x8202f",
110105
"libc.so.6+0xebf1b"
111106
]
112107
}

‎tools/coredump/testdata/arm64/node1600-inlining.json‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@
7272
"node+0x14efc2f",
7373
"node+0xb6add7",
7474
"libc.so.6+0x7d5c7",
75-
"libc.so.6+0x7d5c7",
7675
"libc.so.6+0xe5d9b"
7776
]
7877
},
@@ -84,7 +83,6 @@
8483
"node+0x14fd073",
8584
"node+0xb66667",
8685
"libc.so.6+0x7d5c7",
87-
"libc.so.6+0x7d5c7",
8886
"libc.so.6+0xe5d9b"
8987
]
9088
},
@@ -96,7 +94,6 @@
9694
"node+0x14fd073",
9795
"node+0xb66667",
9896
"libc.so.6+0x7d5c7",
99-
"libc.so.6+0x7d5c7",
10097
"libc.so.6+0xe5d9b"
10198
]
10299
},
@@ -108,7 +105,6 @@
108105
"node+0x14fd073",
109106
"node+0xb66667",
110107
"libc.so.6+0x7d5c7",
111-
"libc.so.6+0x7d5c7",
112108
"libc.so.6+0xe5d9b"
113109
]
114110
},
@@ -120,7 +116,6 @@
120116
"node+0x14fd073",
121117
"node+0xb66667",
122118
"libc.so.6+0x7d5c7",
123-
"libc.so.6+0x7d5c7",
124119
"libc.so.6+0xe5d9b"
125120
]
126121
},

‎tools/coredump/testdata/arm64/node16110-inlining.json‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
"node+0x14bb6bf",
7171
"node+0xb5a0b7",
7272
"libc.so.6+0x7d5c7",
73-
"libc.so.6+0x7d5c7",
7473
"libc.so.6+0xe5d9b"
7574
]
7675
},
@@ -82,7 +81,6 @@
8281
"node+0x14c8bfb",
8382
"node+0xb55947",
8483
"libc.so.6+0x7d5c7",
85-
"libc.so.6+0x7d5c7",
8684
"libc.so.6+0xe5d9b"
8785
]
8886
},
@@ -94,7 +92,6 @@
9492
"node+0x14c8bfb",
9593
"node+0xb55947",
9694
"libc.so.6+0x7d5c7",
97-
"libc.so.6+0x7d5c7",
9895
"libc.so.6+0xe5d9b"
9996
]
10097
},
@@ -106,7 +103,6 @@
106103
"node+0x14c8bfb",
107104
"node+0xb55947",
108105
"libc.so.6+0x7d5c7",
109-
"libc.so.6+0x7d5c7",
110106
"libc.so.6+0xe5d9b"
111107
]
112108
},
@@ -118,7 +114,6 @@
118114
"node+0x14c8bfb",
119115
"node+0xb55947",
120116
"libc.so.6+0x7d5c7",
121-
"libc.so.6+0x7d5c7",
122117
"libc.so.6+0xe5d9b"
123118
]
124119
},

0 commit comments

Comments
 (0)