On Wed, May 08, 2024 at 02:26:03PM -0700, Andrii Nakryiko wrote:
> +static void fixup_uretprobe_trampoline_entries(struct perf_callchain_entry *entry,
> + int start_entry_idx)
> +{
> +#ifdef CONFIG_UPROBES
> + struct uprobe_task *utask = current->utask;
> + struct return_instance *ri;
> + __u64 *cur_ip, *last_ip, tramp_addr;
> +
> + if (likely(!utask || !utask->return_instances))
> + return;
> +
> + cur_ip = &entry->ip[start_entry_idx];
> + last_ip = &entry->ip[entry->nr - 1];
> + ri = utask->return_instances;
> + tramp_addr = uprobe_get_trampoline_vaddr();
> +
> + /* If there are pending uretprobes for current thread, they are
Comment style fail. Also 'for *the* current thread'.
> + * recorded in a list inside utask->return_instances; each such
> + * pending uretprobe replaces traced user function's return address on
> + * the stack, so when stack trace is captured, instead of seeing
> + * actual function's return address, we'll have one or many uretprobe
> + * trampoline addresses in the stack trace, which are not helpful and
> + * misleading to users.
I would beg to differ, what if the uprobe is causing the performance
issue?
While I do think it makes sense to fix the unwind in the sense that we
should be able to continue the unwind, I don't think it makes sense to
completely hide the presence of uprobes.
> + * So here we go over the pending list of uretprobes, and each
> + * encountered trampoline address is replaced with actual return
> + * address.
> + */
> + while (ri && cur_ip <= last_ip) {
> + if (*cur_ip == tramp_addr) {
> + *cur_ip = ri->orig_ret_vaddr;
> + ri = ri->next;
> + }
> + cur_ip++;
> + }
> +#endif
> +}
