From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12793 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Some questions Date: Tue, 1 May 2018 13:35:35 -0400 Message-ID: <20180501173535.GT1392@brightrain.aerifal.cx> References: <20180430031653.GI1392@brightrain.aerifal.cx> <20180430153112.GL1392@brightrain.aerifal.cx> <20180501155233.GS1392@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="xo44VMWPx7vlQ2+2" X-Trace: blaine.gmane.org 1525196025 10408 195.159.176.226 (1 May 2018 17:33:45 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 1 May 2018 17:33:45 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) Cc: musl@lists.openwall.com To: Patrick Oppenlander Original-X-From: musl-return-12809-gllmg-musl=m.gmane.org@lists.openwall.com Tue May 01 19:33:41 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1fDZ9p-0002d3-4Q for gllmg-musl@m.gmane.org; Tue, 01 May 2018 19:33:41 +0200 Original-Received: (qmail 11742 invoked by uid 550); 1 May 2018 17:35:49 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 11721 invoked from network); 1 May 2018 17:35:48 -0000 Content-Disposition: inline In-Reply-To: <20180501155233.GS1392@brightrain.aerifal.cx> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12793 Archived-At: --xo44VMWPx7vlQ2+2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, May 01, 2018 at 11:52:33AM -0400, Rich Felker wrote: > On Tue, May 01, 2018 at 12:34:13PM +1000, Patrick Oppenlander wrote: > > On Tue, May 1, 2018 at 1:31 AM, Rich Felker wrote: > > > On Mon, Apr 30, 2018 at 03:29:39PM +1000, Patrick Oppenlander wrote: > > >> Actually, my biggest issue with getcwd is that it allocates a PATH_MAX > > >> sized buffer on the stack. That's painful on deeply embedded stuff. > > > > > > That's unrelated, and could/should be fixed by the attached patch I > > > think. > > > > Unfortunately that fails to build on arm with: > > > > src/unistd/getcwd.c: In function 'getcwd': > > src/unistd/getcwd.c:25:1: error: r7 cannot be used in asm here > > Then that's a bug we need to fix or work around elsewhere. > Non-arch-specific source files can't be constrained not to use > perfectly valid C constructs because gcc breaks on them for particular > archs. > > I believe it's related to the thumb+framepointer issue that was raised > a while back, but I forget how that ended and if we ever solved it. > > > I was also having a go at resolving the stack & the buffer size issue > > and came up with the attached (untested) patch. > > > > Patrick > > > diff --git a/src/unistd/getcwd.c b/src/unistd/getcwd.c > > index 103fbbb5..306dbc4f 100644 > > --- a/src/unistd/getcwd.c > > +++ b/src/unistd/getcwd.c > > @@ -3,17 +3,10 @@ > > #include > > #include > > #include "syscall.h" > > +#include "libc.h" > > > > -char *getcwd(char *buf, size_t size) > > +static char *do_getcwd(char *buf, size_t size) > > { > > - char tmp[PATH_MAX]; > > - if (!buf) { > > - buf = tmp; > > - size = PATH_MAX; > > - } else if (!size) { > > - errno = EINVAL; > > - return 0; > > - } > > long ret = syscall(SYS_getcwd, buf, size); > > if (ret < 0) > > return 0; > > @@ -21,5 +14,37 @@ char *getcwd(char *buf, size_t size) > > errno = ENOENT; > > return 0; > > } > > - return buf == tmp ? strdup(buf) : buf; > > + return buf; > > +} > > + > > +static char *getcwd_glibc(size_t size) > > +{ > > + char tmp[PATH_MAX]; > > + if (!do_getcwd(tmp, sizeof tmp)) > > + return 0; > > + size_t len = strlen(tmp) + 1; > > + if (!size) > > + size = len; > > + else if (size < len) { > > + errno = ERANGE; > > + return 0; > > + } > > + char *buf = malloc(size); > > + if (!buf) { > > + errno = ENOMEM; > > + return 0; > > + } > > + memcpy(buf, tmp, len); > > + return buf; > > +} > > + > > +char *getcwd(char *buf, size_t size) > > +{ > > + if (!buf) > > + return getcwd_glibc(size); > > + if (!size) { > > + errno = EINVAL; > > + return 0; > > + } > > + return do_getcwd(buf, size); > > } > > This isn't acceptable. It makes the code much larger (at the source > level) and harder to read, and the only reason it works is failure of > gcc to optimize heavily. It could just as easily still end up using > the full PATH_MAX space on the stack, if gcc inlines and hoists stuff, > or if gcc wanted to be really awful it could still end up using a > frame pointer. > > Let's look back at the framepointer mess and see if there's a way to > get gcc not to break. If not we may need to skip inline syscalls and > call out to the extern __syscall when building for thumb, but I'd > really rather not have to do that. Looking back, it seems where we left it is just that you need to make sure frame pointer is disabled if building as thumb. But that's not reliable because gcc forcibly re-enables frame pointer (including frame pointer ABI constraints, which it doesn't need to) if you use a VLA or alloca. I'm considering applying the attached patch, which would make it so VLAs don't break thumb syscalls and eliminate the need to force frame pointer off when building as thumb. This is all a workaround for gcc being wrong about not letting you use r7, but it seems reasonable and non-invasive. It just omits r7 from the constraints and uses a temp register to save/restore it. Rich --xo44VMWPx7vlQ2+2 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="thumb-syscall.diff" diff --git a/arch/arm/syscall_arch.h b/arch/arm/syscall_arch.h index 6023303..30863eb 100644 --- a/arch/arm/syscall_arch.h +++ b/arch/arm/syscall_arch.h @@ -3,74 +3,89 @@ ((union { long long ll; long l[2]; }){ .ll = x }).l[1] #define __SYSCALL_LL_O(x) 0, __SYSCALL_LL_E((x)) +#ifdef __thumb__ + +#define __ASM____R7__ +#define R7_OPERAND "ri"(r7) +#define __asm_syscall(...) do { \ + __asm__ __volatile__ ( "mov %1,r7 ; mov r7,%2 ; svc 0 ; mov r7,%1" \ + : "=r"(r0), "=&r"((int){0}) : __VA_ARGS__ : "memory"); \ + return r0; \ + } while (0) + +#else + +#define __ASM____R7__ __asm__("r7") +#define R7_OPERAND "r"(r7) #define __asm_syscall(...) do { \ __asm__ __volatile__ ( "svc 0" \ : "=r"(r0) : __VA_ARGS__ : "memory"); \ return r0; \ } while (0) +#endif static inline long __syscall0(long n) { - register long r7 __asm__("r7") = n; + register long r7 __ASM____R7__ = n; register long r0 __asm__("r0"); - __asm_syscall("r"(r7)); + __asm_syscall(R7_OPERAND); } static inline long __syscall1(long n, long a) { - register long r7 __asm__("r7") = n; + register long r7 __ASM____R7__ = n; register long r0 __asm__("r0") = a; - __asm_syscall("r"(r7), "0"(r0)); + __asm_syscall(R7_OPERAND, "0"(r0)); } static inline long __syscall2(long n, long a, long b) { - register long r7 __asm__("r7") = n; + register long r7 __ASM____R7__ = n; register long r0 __asm__("r0") = a; register long r1 __asm__("r1") = b; - __asm_syscall("r"(r7), "0"(r0), "r"(r1)); + __asm_syscall(R7_OPERAND, "0"(r0), "r"(r1)); } static inline long __syscall3(long n, long a, long b, long c) { - register long r7 __asm__("r7") = n; + register long r7 __ASM____R7__ = n; register long r0 __asm__("r0") = a; register long r1 __asm__("r1") = b; register long r2 __asm__("r2") = c; - __asm_syscall("r"(r7), "0"(r0), "r"(r1), "r"(r2)); + __asm_syscall(R7_OPERAND, "0"(r0), "r"(r1), "r"(r2)); } static inline long __syscall4(long n, long a, long b, long c, long d) { - register long r7 __asm__("r7") = n; + register long r7 __ASM____R7__ = n; register long r0 __asm__("r0") = a; register long r1 __asm__("r1") = b; register long r2 __asm__("r2") = c; register long r3 __asm__("r3") = d; - __asm_syscall("r"(r7), "0"(r0), "r"(r1), "r"(r2), "r"(r3)); + __asm_syscall(R7_OPERAND, "0"(r0), "r"(r1), "r"(r2), "r"(r3)); } static inline long __syscall5(long n, long a, long b, long c, long d, long e) { - register long r7 __asm__("r7") = n; + register long r7 __ASM____R7__ = n; register long r0 __asm__("r0") = a; register long r1 __asm__("r1") = b; register long r2 __asm__("r2") = c; register long r3 __asm__("r3") = d; register long r4 __asm__("r4") = e; - __asm_syscall("r"(r7), "0"(r0), "r"(r1), "r"(r2), "r"(r3), "r"(r4)); + __asm_syscall(R7_OPERAND, "0"(r0), "r"(r1), "r"(r2), "r"(r3), "r"(r4)); } static inline long __syscall6(long n, long a, long b, long c, long d, long e, long f) { - register long r7 __asm__("r7") = n; + register long r7 __ASM____R7__ = n; register long r0 __asm__("r0") = a; register long r1 __asm__("r1") = b; register long r2 __asm__("r2") = c; register long r3 __asm__("r3") = d; register long r4 __asm__("r4") = e; register long r5 __asm__("r5") = f; - __asm_syscall("r"(r7), "0"(r0), "r"(r1), "r"(r2), "r"(r3), "r"(r4), "r"(r5)); + __asm_syscall(R7_OPERAND, "0"(r0), "r"(r1), "r"(r2), "r"(r3), "r"(r4), "r"(r5)); } #define VDSO_USEFUL --xo44VMWPx7vlQ2+2--