Hi, things are getting too complicated and we need some advice how to deal
with this frame_pointer issue.

On Mon, Dec 10, 2018 at 10:26:50PM +0800, kbuild test robot wrote:
> Hi Elvira,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.20-rc6]
> [cannot apply to next-20181207]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:    
> https://github.com/0day-ci/linux/commits/Dmitry-V-Levin/ptrace-add-PTRACE_GET_SYSCALL_INFO-request/20181210-174745
> config: mips-malta_kvm_defconfig (attached as .config)
> compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>         wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.2.0 make.cross ARCH=mips 
> 
> All errors (new ones prefixed by >>):
> 
>    kernel/ptrace.c: In function 'ptrace_get_syscall_info':
> >> kernel/ptrace.c:942:20: error: implicit declaration of function 
> >> 'frame_pointer'; did you mean 'trace_printk'? 
> >> [-Werror=implicit-function-declaration]
>       .frame_pointer = frame_pointer(regs)
>                        ^~~~~~~~~~~~~
>                        trace_printk
>    cc1: some warnings being treated as errors
> 
> vim +942 kernel/ptrace.c
> 
>    931        
>    932        static int
>    933        ptrace_get_syscall_info(struct task_struct *child, unsigned 
> long user_size,
>    934                                void __user *datavp)
>    935        {
>    936                struct pt_regs *regs = task_pt_regs(child);
>    937                struct ptrace_syscall_info info = {
>    938                        .op = PTRACE_SYSCALL_INFO_NONE,
>    939                        .arch = syscall_get_arch(child),
>    940                        .instruction_pointer = 
> instruction_pointer(regs),
>    941                        .stack_pointer = user_stack_pointer(regs),
>  > 942                        .frame_pointer = frame_pointer(regs)
>    943                };
>    944                unsigned long actual_size = offsetof(struct 
> ptrace_syscall_info, entry);
>    945                unsigned long write_size;
>    946        
>    947                /*
>    948                 * This does not need lock_task_sighand() to access
>    949                 * child->last_siginfo because ptrace_freeze_traced()
>    950                 * called earlier by ptrace_check_attach() ensures that
>    951                 * the tracee cannot go away and clear its last_siginfo.
>    952                 */
>    953                switch (child->last_siginfo ? 
> child->last_siginfo->si_code : 0) {
>    954                case SIGTRAP | 0x80:
>    955                        switch (child->ptrace_message) {
>    956                        case PTRACE_EVENTMSG_SYSCALL_ENTRY:
>    957                                actual_size = 
> ptrace_get_syscall_info_entry(child, regs,
>    958                                                                        
>     &info);
>    959                                break;
>    960                        case PTRACE_EVENTMSG_SYSCALL_EXIT:
>    961                                actual_size = 
> ptrace_get_syscall_info_exit(child, regs,
>    962                                                                        
>    &info);
>    963                                break;
>    964                        }
>    965                        break;
>    966                case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
>    967                        actual_size = 
> ptrace_get_syscall_info_seccomp(child, regs,
>    968                                                                      
> &info);
>    969                        break;
>    970                }
>    971        
>    972                write_size = min(actual_size, user_size);
>    973                return copy_to_user(datavp, &info, write_size) ? 
> -EFAULT : actual_size;
>    974        }
>    975        

We decided to add .frame_pointer to struct ptrace_syscall_info just for
consistency with .instruction_pointer and .stack_pointer; I must have been
misled by comments in asm-generic/ptrace.h into thinking that
frame_pointer() is universally available across architectures.

Unlike .instruction_pointer and .stack_pointer that are actually needed
in strace, .frame_pointer is not used, so from strace PoV we don't really
need it.

So the question is, does anybody need a
struct ptrace_syscall_info.frame_pointer?

If yes, how can frame_pointer() be defined on MIPS?
Or should we just forget about making sense of frame_pointer() and remove
struct ptrace_syscall_info.frame_pointer from the proposed API?


-- 
ldv

Attachment: signature.asc
Description: PGP signature

Reply via email to