From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12796 Path: news.gmane.org!.POSTED!not-for-mail From: Andre McCurdy Newsgroups: gmane.linux.lib.musl.general Subject: Re: Some questions Date: Tue, 1 May 2018 14:49:00 -0700 Message-ID: References: <20180430031653.GI1392@brightrain.aerifal.cx> <20180430153112.GL1392@brightrain.aerifal.cx> <20180501155233.GS1392@brightrain.aerifal.cx> <20180501173535.GT1392@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" X-Trace: blaine.gmane.org 1525211232 5207 195.159.176.226 (1 May 2018 21:47:12 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 1 May 2018 21:47:12 +0000 (UTC) Cc: Patrick Oppenlander To: musl@lists.openwall.com Original-X-From: musl-return-12812-gllmg-musl=m.gmane.org@lists.openwall.com Tue May 01 23:47:08 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 1fDd74-0001EW-E7 for gllmg-musl@m.gmane.org; Tue, 01 May 2018 23:47:06 +0200 Original-Received: (qmail 22138 invoked by uid 550); 1 May 2018 21:49:14 -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 22116 invoked from network); 1 May 2018 21:49:13 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=0veoxs3YxhSpyPtTrNn9ZFx9uLNE8zPs03acAHR7n6E=; b=PTQq8BCQKJxjFZvi+7l9pQQCnrYvAa310KREqQm3wfkWtgnEnTzEXu57w6srLRjq4u dXNHD/P5PmPNFEbfqv+VX0bSzyJ3InzNEWg7gf/BR5eq58JSudlDERZ6VcOJpp8GDBnz IifYXdB+Mkuo9cGCZeFQvsSOExyOin5ZmWZfaFPJLH5YM4CUUoXLAPsD9bZNHqNnU0Rj 9Jv/Cc3OSJrsKBpFuG++ooVJ8gFPNpwUvIZSVDeQv5lyUJ7dZKK41hnz7QxpUX/oHOd6 f4W50b8EHQiEIi/GeKKiKKgnQFB77CFFqUITbX+G9TYweC5IBospQi17goj0T6jRvjya XWrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=0veoxs3YxhSpyPtTrNn9ZFx9uLNE8zPs03acAHR7n6E=; b=OdmFWv0WxVzItKVdLjvx5svnHoYaaBLjuwZuKMSOocO2akwYgEcAFgLf3dkxnFwcpY 36Dux0ofcq8D7fUsGxMljJ72LE8A7ua4hOu9QjNvMC4olW9B7ijRppjeWpVvu6PtqgQx 5zZh/LNGhkRvhBjvZDkawd9hwFVsqw+wA+sF7nLtEjbrbpYsYi41hG25cNmxKdhfAuWm 3XQacfYHh4CbdGOh12Tk+PRMlWx2Xg2DPqaIbQYIf2tXAIeSkVeEmGfhFOoHq/spTZq8 cKHa9E2+av0LQ3PmX/mGLbMFxnOcCGG4BKh+xSr1jDIzCu93CwVKzlG/XI5WJo5V5aeL eMQQ== X-Gm-Message-State: ALQs6tDPMGxMXpazcgxXc06tPMIW0cOJiRkpbygWyBmSia2p4ZRIeqCc D2dLBQXutg6fxNSL8zcs0fnzzmdw7IT+Kr4GimPRhA== X-Google-Smtp-Source: AB8JxZoQ0NpYYz+qycu/VsdXvPd2Bj/9tm3isZo3vk23Mmn2N/VQnfP3z1wc0t4hjextlB/u+B0SEnYFoDxXzxmoZc8= X-Received: by 10.159.40.35 with SMTP id c32mr16715851uac.193.1525211341531; Tue, 01 May 2018 14:49:01 -0700 (PDT) In-Reply-To: <20180501173535.GT1392@brightrain.aerifal.cx> Xref: news.gmane.org gmane.linux.lib.musl.general:12796 Archived-At: On Tue, May 1, 2018 at 10:35 AM, Rich Felker wrote: > 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. This seems to fail when compiling src/thread/arm/__set_thread_area.c: {standard input}: Assembler messages: {standard input}:45: Error: invalid constant (f0005) after fixup make: *** [obj/src/thread/arm/__set_thread_area.o] Error 1 Without the patch, __set_thread_area() effectively compiles to: __set_thread_area: push {r7, lr} ldr r7, .L2 pop {r7, pc} .L2: .word 983045 With the patch: __set_thread_area: push {r7, lr} add r7, sp, #0 mov r3,r7 ; mov r7,#983045 ; svc 0 ; mov r7,r3 pop {r7, pc} ie the immediate value 0xf0005 can't be loaded directly into r7 with a single Thumb2 mov instruction. I tried a quick test to replace the single mov instruction in __asm_syscall() with a movw + movt pair: #define __asm_syscall(...) do { \ __asm__ __volatile__ ( "mov %1,r7 ; movw r7,%2 & 0xffff ; movt r7,%2 >> 16 ; svc 0 ; mov r7,%1" \ : "=r"(r0), "=&r"((int){0}) : __VA_ARGS__ : "memory"); \ return r0; \ } while (0) It fixes __set_thread_area() but causes a new error in syscall() as the syscall number is passed to __asm_syscall() as a variable rather than an immediate: {standard input}: Assembler messages: {standard input}:75: Error: constant expression expected -- `movw r7,r6&0xffff' {standard input}:75: Error: constant expression expected -- `movt r7,r6>>16' make: *** [obj/src/misc/syscall.o] Error 1