From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 4919 invoked from network); 19 May 2021 02:50:45 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 19 May 2021 02:50:45 -0000 Received: (qmail 24305 invoked by uid 550); 19 May 2021 02:50:43 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 24275 invoked from network); 19 May 2021 02:50:42 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:subject:to:cc:references:in-reply-to:mime-version :message-id:content-transfer-encoding; bh=cWQFzxhZycEzz2TPKI8LNP/whPnYVsPv2PnECv9BXUc=; b=cZow6f+1zRszSqH/3ePyeyl1icp8IwEGnsZRWPYmD0xy+6zHngx87MPKP/+vbU1qvP Tx8p3qqB0SeFb/b5nyx2/NyDayGcMY5w/ScjbKk4z/iFNK6QvVouB6vpA5qDeRdOMw+i KbXJ5BGsizsaJs0KCzwbjFCAz8X7CmJiFmCdPwPsmQ2IqmstQuPYZDAxJuTXdfEMkyNI 3gNMLCzcXj5RyqG2hXAX9oMdpyx27dehDqLvHHLS+2Os9GRvReK2FEY5e03/ZylauuSA ECaY3cgOpZnWIyygQQ+DuBgO0jOVDWwc/L/uWfamvFETOSt5VhXEQvme7jN27PRwIG13 hdWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:subject:to:cc:references:in-reply-to :mime-version:message-id:content-transfer-encoding; bh=cWQFzxhZycEzz2TPKI8LNP/whPnYVsPv2PnECv9BXUc=; b=oEgEpO8SnsYrPj1ENu7rihxdNHCsd7itVJudi7MV9UqOqUV+czmuuJwMgbJ0tt6ozT 3Pgawwnbr+Rj1+NXT5tTgzblZgb3eJMmt3LfmIxIrhknm+94oOYBTQ48m8LGq6SV+A90 BI+Hcf7/NSD2UCG/3ro0T1vNJaeiHUmUX8qQhLQDw/Zn5Y2wHWWTEDXdAWxF3nTBrHTC 5XmLeDfcCIGMUYx4qlvVD0R4O6SMtliek3/6Ia2GfDVmti6u1PDu6yRXkCVd+G+RpKj6 ULIIoAmZAmRXq4msTJqH5da4gKF7gzw8vcZeN2jltk08D4op1A4Qz+ZAAiFLKOfHHhPW 1fjw== X-Gm-Message-State: AOAM5331kjLk/+oLqIUWwJWSudMCvoZeH60utKdUzC2EYAIRzX2OATlR Qc89Ig3zRDT+29l4LFeJi0Q= X-Google-Smtp-Source: ABdhPJwDcXEMRseuSruQGmCaMPdDStr4laYhCXRJ1PsxjkPgfdindWrV3H9h/r7BFaOztH7tFGiGYg== X-Received: by 2002:a17:90a:df8d:: with SMTP id p13mr8470070pjv.67.1621392629847; Tue, 18 May 2021 19:50:29 -0700 (PDT) Date: Wed, 19 May 2021 12:50:24 +1000 From: Nicholas Piggin To: "Dmitry V. Levin" , Michael Ellerman Cc: libc-dev@lists.llvm.org, linux-api@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, musl@lists.openwall.com, Matheus Castanho , libc-alpha@sourceware.org References: <20200611081203.995112-1-npiggin@gmail.com> <20210518231331.GA8464@altlinux.org> In-Reply-To: <20210518231331.GA8464@altlinux.org> MIME-Version: 1.0 Message-Id: <1621385544.nttlk5qugb.astroid@bobo.none> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [musl] Re: Linux powerpc new system call instruction and ABI Excerpts from Dmitry V. Levin's message of May 19, 2021 9:13 am: > Hi, >=20 > On Thu, Jun 11, 2020 at 06:12:01PM +1000, Nicholas Piggin wrote: > [...] >> - Error handling: The consensus among kernel, glibc, and musl is to move= to >> using negative return values in r3 rather than CR0[SO]=3D1 to indicate= error, >> which matches most other architectures, and is closer to a function ca= ll. >=20 > Apparently, the patchset merged by commit v5.9-rc1~100^2~164 was > incomplete: all functions defined in arch/powerpc/include/asm/ptrace.h an= d > arch/powerpc/include/asm/syscall.h that use ccr are broken when scv is us= ed. > This includes syscall_get_error() and all its users including > PTRACE_GET_SYSCALL_INFO API, which in turn makes strace unusable > when scv is used. >=20 > See also https://bugzilla.redhat.com/1929836 I see, thanks. Using latest strace from github.com, the attached kernel patch makes strace -k check results a lot greener. Some of the remaining failing tests look like this (I didn't look at all of them yet): signal(SIGUSR1, 0xfacefeeddeadbeef) =3D 0 (SIG_DFL) write(1, "signal(SIGUSR1, 0xfacefeeddeadbe"..., 50signal(SIGUSR1, 0xfacefee= ddeadbeef) =3D 0 (SIG_DFL) ) =3D 50 signal(SIGUSR1, SIG_IGN) =3D 0xfacefeeddeadbeef write(2, "errno2name.c:461: unknown errno "..., 41errno2name.c:461: unknown= errno 559038737) =3D 41 write(2, ": Unknown error 559038737\n", 26: Unknown error 559038737 ) =3D 26 exit_group(1) =3D ? I think the problem is glibc testing for -ve, but it should be comparing against -4095 (+cc Matheus) #define RET_SCV \ cmpdi r3,0; \ bgelr+; \ neg r3,r3; With this patch, I think the ptrace ABI should mostly be fixed. I think=20 a problem remains with applications that look at system call return=20 registers directly and have powerpc specific error cases. Those probably will just need to be updated unfortunately. Michael thought it might be possible to return an indication via ptrace somehow that the syscall is using a new ABI, so such apps can be updated to test for it. I don't=20 know how that would be done. Thanks, Nick -- diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/p= trace.h index 9c9ab2746168..b476a685f066 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h @@ -19,6 +19,7 @@ #ifndef _ASM_POWERPC_PTRACE_H #define _ASM_POWERPC_PTRACE_H =20 +#include #include #include =20 @@ -152,25 +153,6 @@ extern unsigned long profile_pc(struct pt_regs *regs); long do_syscall_trace_enter(struct pt_regs *regs); void do_syscall_trace_leave(struct pt_regs *regs); =20 -#define kernel_stack_pointer(regs) ((regs)->gpr[1]) -static inline int is_syscall_success(struct pt_regs *regs) -{ - return !(regs->ccr & 0x10000000); -} - -static inline long regs_return_value(struct pt_regs *regs) -{ - if (is_syscall_success(regs)) - return regs->gpr[3]; - else - return -regs->gpr[3]; -} - -static inline void regs_set_return_value(struct pt_regs *regs, unsigned lo= ng rc) -{ - regs->gpr[3] =3D rc; -} - #ifdef __powerpc64__ #define user_mode(regs) ((((regs)->msr) >> MSR_PR_LG) & 0x1) #else @@ -235,6 +217,31 @@ static __always_inline void set_trap_norestart(struct = pt_regs *regs) regs->trap |=3D 0x1; } =20 +#define kernel_stack_pointer(regs) ((regs)->gpr[1]) +static inline int is_syscall_success(struct pt_regs *regs) +{ + if (trap_is_scv(regs)) + return !IS_ERR_VALUE((unsigned long)regs->gpr[3]); + else + return !(regs->ccr & 0x10000000); +} + +static inline long regs_return_value(struct pt_regs *regs) +{ + if (trap_is_scv(regs)) + return regs->gpr[3]; + + if (is_syscall_success(regs)) + return regs->gpr[3]; + else + return -regs->gpr[3]; +} + +static inline void regs_set_return_value(struct pt_regs *regs, unsigned lo= ng rc) +{ + regs->gpr[3] =3D rc; +} + #define arch_has_single_step() (1) #define arch_has_block_step() (true) #define ARCH_HAS_USER_SINGLE_STEP_REPORT diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/= syscall.h index fd1b518eed17..e8b40149bf7e 100644 --- a/arch/powerpc/include/asm/syscall.h +++ b/arch/powerpc/include/asm/syscall.h @@ -41,11 +41,20 @@ static inline void syscall_rollback(struct task_struct = *task, static inline long syscall_get_error(struct task_struct *task, struct pt_regs *regs) { - /* - * If the system call failed, - * regs->gpr[3] contains a positive ERRORCODE. - */ - return (regs->ccr & 0x10000000UL) ? -regs->gpr[3] : 0; + if (trap_is_scv(regs)) { + unsigned long error =3D regs->gpr[3]; + + if (task_is_32bit(task)) + error =3D (long)(int)error; + + return IS_ERR_VALUE(error) ? error : 0; + } else { + /* + * If the system call failed, + * regs->gpr[3] contains a positive ERRORCODE. + */ + return (regs->ccr & 0x10000000UL) ? -regs->gpr[3] : 0; + } } =20 static inline long syscall_get_return_value(struct task_struct *task, @@ -58,18 +67,22 @@ static inline void syscall_set_return_value(struct task= _struct *task, struct pt_regs *regs, int error, long val) { - /* - * In the general case it's not obvious that we must deal with CCR - * here, as the syscall exit path will also do that for us. However - * there are some places, eg. the signal code, which check ccr to - * decide if the value in r3 is actually an error. - */ - if (error) { - regs->ccr |=3D 0x10000000L; - regs->gpr[3] =3D error; + if (trap_is_scv(regs)) { + regs->gpr[3] =3D (long) error ?: val; } else { - regs->ccr &=3D ~0x10000000L; - regs->gpr[3] =3D val; + /* + * In the general case it's not obvious that we must deal with + * CCR here, as the syscall exit path will also do that for us. + * However there are some places, eg. the signal code, which + * check ccr to decide if the value in r3 is actually an error. + */ + if (error) { + regs->ccr |=3D 0x10000000L; + regs->gpr[3] =3D error; + } else { + regs->ccr &=3D ~0x10000000L; + regs->gpr[3] =3D val; + } } } =20 diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/= asm/thread_info.h index b4ec6c7dd72e..c4bb9f305eaf 100644 --- a/arch/powerpc/include/asm/thread_info.h +++ b/arch/powerpc/include/asm/thread_info.h @@ -165,8 +165,10 @@ static inline bool test_thread_local_flags(unsigned in= t flags) =20 #ifdef CONFIG_COMPAT #define is_32bit_task() (test_thread_flag(TIF_32BIT)) +#define task_is_32bit(task) (test_tsk_thread_flag(task, TIF_32BIT)) #else #define is_32bit_task() (IS_ENABLED(CONFIG_PPC32)) +#define task_is_32bit(task) (IS_ENABLED(CONFIG_PPC32)) #endif =20 #if defined(CONFIG_PPC64)