Skip to content

Commit ffb33d9

Browse files
authored
dyninst/*: report why @duration is missing more clearly (#44370)
### What does this PR do? This commit does plumbing to make it more explicit why we don't have a value for @duration. ### Motivation It was confusing when before it just said it wasn't available. ### Describe how you validated your changes There's testing ### Additional Notes Fixes [DEBUG-4866](https://datadoghq.atlassian.net/browse/DEBUG-4866) [DEBUG-4866]: https://datadoghq.atlassian.net/browse/DEBUG-4866?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: andrew.werner <andrew.werner@datadoghq.com>
1 parent 29a58bd commit ffb33d9

File tree

150 files changed

+1160
-185
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

150 files changed

+1160
-185
lines changed

pkg/dyninst/compiler/functions.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type ProcessEvent struct {
5050
StringSizeLimit uint32
5151
Frameless bool
5252
HasAssociatedReturn bool
53+
NoReturnReason ir.NoReturnReason
5354
EventKind ir.EventKind
5455
TopPCOffset int8
5556
EventRootType *ir.EventRootType

pkg/dyninst/compiler/generate.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ func (g *generator) addEventHandler(
169169
StringSizeLimit: captureConfig.GetMaxLength(),
170170
Frameless: injectionPoint.Frameless,
171171
HasAssociatedReturn: injectionPoint.HasAssociatedReturn,
172+
NoReturnReason: injectionPoint.NoReturnReason,
172173
TopPCOffset: injectionPoint.TopPCOffset,
173174
ProbeID: probeID,
174175
EventKind: eventKind,

pkg/dyninst/decode/decoder.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ func (s *message) init(
300300
s.Debugger.Snapshot.captures.Lines = &decoder.line
301301
}
302302
var returnHeader *output.EventHeader
303+
var durationMissingReason *string
303304
if event.Return != nil {
304305
if err := decoder._return.init(
305306
event.Return, decoder.program.Types, &s.Debugger.EvaluationErrors,
@@ -331,20 +332,27 @@ func (s *message) init(
331332
reason = "call map capacity exceeded"
332333
case output.EventPairingExpectationCallCountExceeded:
333334
reason = "maximum call count exceeded"
335+
case output.EventPairingExpectationNoneInlined:
336+
reason = "function was inlined"
337+
case output.EventPairingExpectationNoneNoBody:
338+
reason = "function has no body"
334339
}
340+
log.Tracef("no return reason: %v pairing expectation: %v", reason, pairingExpectation)
335341
// The choice to use @duration here is somewhat arbitrary; we want to
336342
// choose something that definitely can't collide with a real variable
337343
// and is evocative of the thing that is missing. Indeed we know in this
338344
// situation we will never @duration, so it seems like a good choice.
339345
const missingReturnReasonExpression = "@duration"
340346
if reason != "" {
347+
message := "not available: " + reason
341348
s.Debugger.EvaluationErrors = append(
342349
s.Debugger.EvaluationErrors,
343350
evaluationError{
344351
Expression: missingReturnReasonExpression,
345-
Message: "no return value available: " + reason,
352+
Message: message,
346353
},
347354
)
355+
durationMissingReason = &message
348356
}
349357
}
350358

@@ -432,6 +440,8 @@ func (s *message) init(
432440
}
433441
if s.Duration != 0 {
434442
s.Message.duration = &s.Duration
443+
} else if durationMissingReason != nil {
444+
s.Message.durationMissingReason = durationMissingReason
435445
}
436446
}
437447

pkg/dyninst/decode/decoder_test.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,28 +1448,42 @@ func TestDecoderMissingReturnEventEvaluationError(t *testing.T) {
14481448
name: "return pairing expected",
14491449
pairingExpectation: output.EventPairingExpectationReturnPairingExpected,
14501450
expectedErrorExpression: "@duration",
1451-
expectedErrorMessage: "no return value available: return event not received",
1451+
expectedErrorMessage: "not available: return event not received",
14521452
shouldHaveError: true,
14531453
},
14541454
{
14551455
name: "buffer full",
14561456
pairingExpectation: output.EventPairingExpectationBufferFull,
14571457
expectedErrorExpression: "@duration",
1458-
expectedErrorMessage: "no return value available: userspace buffer capacity exceeded",
1458+
expectedErrorMessage: "not available: userspace buffer capacity exceeded",
14591459
shouldHaveError: true,
14601460
},
14611461
{
14621462
name: "call map full",
14631463
pairingExpectation: output.EventPairingExpectationCallMapFull,
14641464
expectedErrorExpression: "@duration",
1465-
expectedErrorMessage: "no return value available: call map capacity exceeded",
1465+
expectedErrorMessage: "not available: call map capacity exceeded",
14661466
shouldHaveError: true,
14671467
},
14681468
{
14691469
name: "call count exceeded",
14701470
pairingExpectation: output.EventPairingExpectationCallCountExceeded,
14711471
expectedErrorExpression: "@duration",
1472-
expectedErrorMessage: "no return value available: maximum call count exceeded",
1472+
expectedErrorMessage: "not available: maximum call count exceeded",
1473+
shouldHaveError: true,
1474+
},
1475+
{
1476+
name: "inlined",
1477+
pairingExpectation: output.EventPairingExpectationNoneInlined,
1478+
expectedErrorExpression: "@duration",
1479+
expectedErrorMessage: "not available: function was inlined",
1480+
shouldHaveError: true,
1481+
},
1482+
{
1483+
name: "no body",
1484+
pairingExpectation: output.EventPairingExpectationNoneNoBody,
1485+
expectedErrorExpression: "@duration",
1486+
expectedErrorMessage: "not available: function has no body",
14731487
shouldHaveError: true,
14741488
},
14751489
{

pkg/dyninst/decode/marshal.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ type debuggerData struct {
3838
}
3939

4040
type messageData struct {
41-
duration *uint64
42-
entryOrLine *captureEvent
43-
_return *captureEvent
44-
template *ir.Template
41+
duration *uint64
42+
durationMissingReason *string
43+
entryOrLine *captureEvent
44+
_return *captureEvent
45+
template *ir.Template
4546
}
4647

4748
func (m *messageData) MarshalJSONTo(enc *jsontext.Encoder) error {
@@ -83,7 +84,11 @@ func (m *messageData) MarshalJSONTo(enc *jsontext.Encoder) error {
8384
writeBoundedError(&result, limits, "error", seg.Error)
8485
case *ir.DurationSegment:
8586
if m.duration == nil {
86-
writeBoundedError(&result, limits, "error", "@duration is not available")
87+
if m.durationMissingReason != nil {
88+
writeBoundedError(&result, limits, "error", *m.durationMissingReason)
89+
} else {
90+
writeBoundedError(&result, limits, "error", "@duration is not available")
91+
}
8792
} else {
8893
n, _ := fmt.Fprintf(&result, "%f", time.Duration(*m.duration).Seconds()*1000)
8994
limits.consume(n)

pkg/dyninst/ebpf/event.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,19 @@ probe_run(uint64_t start_ns, const probe_params_t* params, struct pt_regs* regs)
182182
}
183183
}
184184
} else {
185-
header->event_pairing_expectation = EVENT_PAIRING_EXPECTATION_NONE;
185+
switch (params->no_return_reason) {
186+
case NO_RETURN_REASON_INLINED:
187+
LOG(4, "no return reason: inlined for goid %lld stack byte depth %d probe id %d", header->goid, header->stack_byte_depth, params->probe_id);
188+
header->event_pairing_expectation = EVENT_PAIRING_EXPECTATION_NONE_INLINED;
189+
break;
190+
case NO_RETURN_REASON_NO_BODY:
191+
LOG(4, "no return body for goid %lld stack byte depth %d probe id %d", header->goid, header->stack_byte_depth, params->probe_id);
192+
header->event_pairing_expectation = EVENT_PAIRING_EXPECTATION_NONE_NO_BODY;
193+
break;
194+
default:
195+
header->event_pairing_expectation = EVENT_PAIRING_EXPECTATION_NONE;
196+
break;
197+
}
186198
}
187199
__maybe_unused int process_steps = 0;
188200
__maybe_unused int chase_steps = 0;

pkg/dyninst/ebpf/framing.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ typedef enum event_pairing_expectation {
1616
EVENT_PAIRING_RETURN_PAIRING_EXPECTED = 2,
1717
EVENT_PAIRING_EXPECTATION_CALL_COUNT_EXCEEDED = 3,
1818
EVENT_PAIRING_EXPECTATION_CALL_MAP_FULL = 4,
19-
} return_call_ommitted_reason_t;
19+
EVENT_PAIRING_EXPECTATION_BUFFER_FULL = 5, // only used in userspace
20+
EVENT_PAIRING_EXPECTATION_NONE_INLINED = 6,
21+
EVENT_PAIRING_EXPECTATION_NONE_NO_BODY = 7,
22+
} event_pairing_expectation_t;
2023

2124
// The message header used for the event program.
2225
typedef struct di_event_header {

pkg/dyninst/ebpf/types.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ typedef struct probe_params {
1414
bool has_associated_return;
1515
char kind; // actually an event_kind_t
1616
char top_pc_offset;
17-
char __padding[4];
17+
char no_return_reason;
18+
char __padding[3];
1819
} probe_params_t;
1920

2021
typedef struct throttler_params {
@@ -49,6 +50,16 @@ typedef enum event_kind {
4950
EVENT_KIND_RETURN = 2,
5051
} event_kind_t;
5152

53+
// To be kept in sync with the ir.NoReturnReason enum in the ir/program.go file.
54+
typedef enum no_return_reason {
55+
NO_RETURN_REASON_NONE = 0,
56+
NO_RETURN_REASON_RETURNS_DISABLED = 1,
57+
NO_RETURN_REASON_LINE_PROBE = 2,
58+
NO_RETURN_REASON_INLINED = 3,
59+
NO_RETURN_REASON_NO_BODY = 4,
60+
NO_RETURN_REASON_IS_RETURN = 5,
61+
} no_return_reason_t;
62+
5263
typedef enum sm_opcode {
5364
SM_OP_INVALID = 0,
5465
// Execution flow ops.

pkg/dyninst/ir/program.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,22 @@ type InjectionPoint struct {
224224
// HasAssociatedReturn is true if there is going to be a return associated
225225
// with this call.
226226
HasAssociatedReturn bool `json:"-"`
227+
// NoReturnReason is the reason why there is no return associated with this
228+
// call. Only set if HasAssociatedReturn is false.
229+
NoReturnReason NoReturnReason `json:"-"`
227230
// TopPCOffset is the offset of the top PC from the entry PC.
228231
TopPCOffset int8 `json:"-"`
229232
}
233+
234+
// This must be kept in sync with the no_return_reason enum in the ebpf/types.h
235+
// file.
236+
type NoReturnReason uint8
237+
238+
const (
239+
NoReturnReasonNone NoReturnReason = 0
240+
NoReturnReasonReturnsDisabled NoReturnReason = 1
241+
NoReturnReasonLineProbe NoReturnReason = 2
242+
NoReturnReasonInlined NoReturnReason = 3
243+
NoReturnReasonNoBody NoReturnReason = 4
244+
NoReturnReasonIsReturn NoReturnReason = 5
245+
)

pkg/dyninst/irgen/irgen.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3559,6 +3559,7 @@ func pickInjectionPoint(
35593559
PC: ranges[0][0],
35603560
Frameless: frameless,
35613561
HasAssociatedReturn: false,
3562+
NoReturnReason: ir.NoReturnReasonInlined,
35623563
})
35633564
} else {
35643565
call, err := pickCallInjectionPoint(arch, addr, frameless, lines)
@@ -3580,21 +3581,30 @@ func pickInjectionPoint(
35803581
return buf, nil, issue, nil
35813582
}
35823583

3583-
// Add a workaround for the fact that single-instruction functions
3584-
// would have the same entry and exit probes, but the ordering between
3585-
// them would not be well-defined, so in this extremely uncommon case
3586-
// the user doesn't get to see the return probe. It's okay because
3587-
// there literally cannot be a return value.
3588-
hasAssociatedReturn := !skipReturnEvents
3589-
if len(returnLocations) == 1 && returnLocations[0].PC == call.pc {
3584+
var hasAssociatedReturn bool
3585+
var noReturnReason ir.NoReturnReason
3586+
if skipReturnEvents {
35903587
hasAssociatedReturn = false
3588+
noReturnReason = ir.NoReturnReasonReturnsDisabled
3589+
} else if len(returnLocations) == 1 && returnLocations[0].PC == call.pc {
3590+
// Add a workaround for the fact that single-instruction
3591+
// functions would have the same entry and exit probes, but the
3592+
// ordering between them would not be well-defined, so in this
3593+
// extremely uncommon case the user doesn't get to see the
3594+
// return probe. It's okay because there literally cannot be a
3595+
// return value.
3596+
hasAssociatedReturn = false
3597+
noReturnReason = ir.NoReturnReasonNoBody
35913598
returnLocations = returnLocations[:0]
3599+
} else {
3600+
hasAssociatedReturn = true
35923601
}
35933602

35943603
buf = append(buf, ir.InjectionPoint{
35953604
PC: call.pc,
35963605
Frameless: call.frameless,
35973606
HasAssociatedReturn: hasAssociatedReturn,
3607+
NoReturnReason: noReturnReason,
35983608
TopPCOffset: call.topPCOffset,
35993609
})
36003610
if hasAssociatedReturn {
@@ -3635,6 +3645,7 @@ func pickInjectionPoint(
36353645
PC: injectionPC,
36363646
Frameless: frameless,
36373647
HasAssociatedReturn: false,
3648+
NoReturnReason: ir.NoReturnReasonLineProbe,
36383649
})
36393650
}
36403651
return buf, returnEvent, ir.Issue{}, nil
@@ -3826,6 +3837,7 @@ func disassembleAmd64Function(
38263837
PC: epilogueStart,
38273838
Frameless: frameless,
38283839
HasAssociatedReturn: false,
3840+
NoReturnReason: ir.NoReturnReasonIsReturn,
38293841
})
38303842
}
38313843
}
@@ -3834,6 +3846,7 @@ func disassembleAmd64Function(
38343846
PC: addr + uint64(offset),
38353847
Frameless: frameless,
38363848
HasAssociatedReturn: false,
3849+
NoReturnReason: ir.NoReturnReasonIsReturn,
38373850
})
38383851
}
38393852
offset += instruction.Len

0 commit comments

Comments
 (0)