Re: [PATCH 1/2] RFC enhance tracing utilities

[Luca Dariz]

Il 09/03/24 16:24, Etienne Brateau ha scritto:
> Le sam. 9 mars 2024 à 15:03, Luca Dariz <luca@orpolo.org 
>     diff --git a/i386/i386/debug.h b/i386/i386/debug.h
>     index 84397ba8..eff330c6 100644
>     --- a/i386/i386/debug.h
>     +++ b/i386/i386/debug.h
>     @@ -54,6 +54,7 @@ void debug_trace_dump(void);
>
>       #else  /* __ASSEMBLER__ */
>
>     +#ifndef __x86_64__
>       #define DEBUG_TRACE                            \
>              pushl   $__LINE__                       ;\
>              pushl   $9f                             ;\
>     @@ -62,10 +63,21 @@ void debug_trace_dump(void);
>              .data                                   ;\
>       9:     .ascii  __FILE__"\0"                    ;\
>              .text
>     -
>     +#else  /* __x86_64__ */
>     +#define DEBUG_TRACE                            \
>     +       pushq   %rdi                            ;\
>     +       pushq   %rs1                            ;\
>
> Is it really %rs1 here and not %rsi ? you are pushing rs1 but popping rsi

it's probably %rsi, this part is still to be cleaned up (see the list in 
the descritption)

>     diff --git a/i386/i386/locore.S b/i386/i386/locore.S
>     index 9d0513a1..26c5843c 100644
>     --- a/i386/i386/locore.S
>     +++ b/i386/i386/locore.S
>     @@ -634,6 +634,15 @@ ENTRY(thread_bootstrap_return)
>        */
>
>       ENTRY(thread_syscall_return)
>     +#if KERNEL_TRACE
>     +       testb   $0xff,EXT(syscall_trace)
>     +       jz      1f
>     +       movl    S_ARG0,%eax                     /* get return value */
>     +       pushl   %eax
>     +       call    syscall_trace_return
>     +       movl    %eax,S_ARG0                     /* restore return
>     value */
>     +1:
>     +#endif
>              movl    S_ARG0,%eax                     /* get return value */
>              movl    %esp,%ecx                       /* get kernel stack */
>              or      $(KERNEL_STACK_SIZE-1),%ecx
>     @@ -1174,18 +1183,26 @@ syscall_native:
>
>       mach_call_call:
>
>     -#ifdef DEBUG
>     +#if KERNEL_TRACE
>              testb   $0xff,EXT(syscall_trace)
>              jz      0f
>     -       pushl   %eax
>     -       call    EXT(syscall_trace_print)
>     -       /* will return with syscallofs still (or again) in eax */
>     -       addl    $4,%esp
>     +       pushl   %eax                     /* add syscall num to args
>     array  */
>     +       pushl   %esp                     /* args array is the first
>     argument*/
>     +       call    EXT(syscall_trace_enter)
>     +       popl    %eax
>     +       popl    %eax
>
> you are popping eax twice here, shouldn’t the second one be esp?

The first popl just discards the %esp value, the second one restores the 
correct %eax, so the real syscall can be called as if nothing had 
happened. For i386 all function parameters are on the stack, so they are 
not touched by the tracing call.


Luca