[DTrace-devel] [PATCH 09/12] Add support for the ustack() action

[Eugene Loh]

I posted a v2 of the patch, but...

On 6/9/21 5:50 PM, Kris Van Hees wrote:

> On Fri, May 28, 2021 at 02:35:13PM -0400, eugene.loh at oracle.com wrote:
>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>> @@ -1503,37 +1503,82 @@ dt_cg_act_trunc(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>>   static void
>>   dt_cg_act_ustack(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
>>   {
>> 	[...]
>> +
>> +	/* Now call bpf_get_stack(ctx, buf, size, flags). */
>> +	emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX));
>> +	emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_1, DCTX_CTX));
>> +	emit(dlp,  BPF_MOV_REG(BPF_REG_2, BPF_REG_9));
>> +	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, off + 8));
>> +	emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 8 * nframes));
>> +	emit(dlp,  BPF_MOV_IMM(BPF_REG_4, (skip & BPF_F_SKIP_FIELD_MASK) | BPF_F_USER_STACK));
>> +	emit(dlp,  BPF_CALL_HELPER(BPF_FUNC_get_stack));
>> +	dt_regset_free_args(drp);
>> +	emit(dlp,  BPF_BRANCH_IMM(BPF_JSGE, BPF_REG_0, 0, lbl_valid));
>> +	/* FIXME: what should the illval be? */
> I believe illval is undefined in the case of a BADSTACK fault, so I would go
> with 0 indeed.

I sure am confused about this one.

I looked at the docs;  e.g.:
https://docs.oracle.com/en/operating-systems/oracle-linux/dtrace-guide/dt_prov.html#dt_error_prov

It says that DTRACEFLT_BADSTACK means "Invalid user process stack" and 
arg5/illval is "Address of invalid stack pointer".  Does this mean that 
illval=0 is fine for kernel stack() errors, but for ustack() errors we 
have to report the SP?

>> +	dt_cg_probe_error(pcb, -1, DTRACEFLT_BADSTACK, 0);
>> +	emitl(dlp, lbl_valid,
>> +		   BPF_NOP());
>> +	dt_regset_free(drp, BPF_REG_0);
>>   }