> On Dec 10, 2018, at 8:09 AM, Dmitry V. Levin <ldv@xxxxxxxxxxxx> wrote: > > 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? > I would suggest getting rid of frame_pointer. Anyone who needs that degree of debugging can use existing ptrace APIs for it. > > -- > ldv