Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161208211116.GO1555@brightrain.aerifal.cx>
Date: Thu, 8 Dec 2016 16:11:16 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: cortex-m support?

On Tue, Dec 06, 2016 at 11:52:29PM -0600, Rob Landley wrote:
> Added support for the Cortex-M.
> ------------------------------------------------------------------------
> Index: src/internal/arm/syscall.s
> ===================================================================
> --- src/internal/arm/syscall.s	(revision 4919)
> +++ src/internal/arm/syscall.s	(revision 4920)
> @@ -11,5 +11,6 @@
>  	svc 0
>  	ldmfd sp!,{r4,r5,r6,r7}
>  	tst lr,#1
> +        it eq
>  	moveq pc,lr
>  	bx lr

There's a gas option -Wa,-mimplicit-it=always that will make these
kind of changes unnecessary. I think it's preferable to just have
musl's configure always add that option when targeting arm if it's
accepted by the toolchain. Otherwise new code might get added without
checking that it builds as thumb.

> Index: src/setjmp/arm/setjmp.S
> ===================================================================
> --- src/setjmp/arm/setjmp.S	(revision 0)
> +++ src/setjmp/arm/setjmp.S	(revision 4920)
> @@ -0,0 +1,45 @@
> +.global __setjmp
> +.global _setjmp
> +.global setjmp
> +.type __setjmp,%function
> +.type _setjmp,%function
> +.type setjmp,%function
> +__setjmp:
> +_setjmp:
> +setjmp:
> +	mov ip,r0
> +#if defined(__thumb__)
> +	stmia ip!,{v1,v2,v3,v4,v5,v6,sl,fp,lr}
> +        str sp, [ip], #4
> +#else
> +	stmia ip!,{v1,v2,v3,v4,v5,v6,sl,fp,sp,lr}
> +#endif

My leaning is to just always use the version that works on thumb, as I
don't think the performance difference will be measurable. I think you
did change the order of the storage in jmp_buf; I'm okay with that but
we might want to see if anyone objects because of wacky stuff
expecting a particular order. (Note: I don't think the order should be
treated as public but we probably want to know what if anything is
poking at it so we know if there are third-party things that need
fixing.)

> Index: src/thread/arm/clone.s
> ===================================================================
> --- src/thread/arm/clone.s	(revision 4919)
> +++ src/thread/arm/clone.s	(revision 4920)
> @@ -16,6 +16,7 @@
>  	beq 1f
>  	ldmfd sp!,{r4,r5,r6,r7}
>  	tst lr,#1
> +        it eq
>  	moveq pc,lr
>  	bx lr

Clone will also need a fix something like what I did for sh in commit
234c58467c3709bafdd3ffa6ac73655e1dfd9ddb to be compatible with fdpic,
but that's separate from thumb support.

> Index: Makefile
> ===================================================================
> --- Makefile	(revision 4919)
> +++ Makefile	(revision 4920)
> @@ -106,6 +106,10 @@
>  $(dir $(patsubst %/,%,$(dir $(1))))$(notdir $(1:.s=.o)): $(1)
>  endef
>  $(foreach s,$(wildcard src/*/$(ARCH)*/*.s),$(eval $(call mkasmdep,$(s))))
> +define mkasmdepS
> +$(dir $(patsubst %/,%,$(dir $(1))))$(notdir $(1:.S=.o)): $(1)
> +endef
> +$(foreach s,$(wildcard src/*/$(ARCH)*/*.S),$(eval $(call mkasmdepS,$(s))))
>  
>  %.o: $(ARCH)$(ASMSUBARCH)/%.sub
>  	$(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $(dir $<)$(shell cat $<)
> @@ -113,6 +117,9 @@
>  %.o: $(ARCH)/%.s
>  	$(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $<
>  
> +%.o: $(ARCH)/%.S
> +	$(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $<
> +
>  %.o: %.c $(GENH) $(IMPH)
>  	$(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $<
>  
> @@ -122,6 +129,9 @@
>  %.lo: $(ARCH)/%.s
>  	$(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $<
>  
> +%.lo: $(ARCH)/%.S
> +	$(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $<
> +
>  %.lo: %.c $(GENH) $(IMPH)
>  	$(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $<

This looks like it's against a really old version of musl. The modern
build system with out-of-tree support already accepts .S files.

> Index: arch/arm/src/arm/atomics.s
> ===================================================================
> --- arch/arm/src/arm/atomics.s	(revision 4919)
> +++ arch/arm/src/arm/atomics.s	(revision 4920)
> @@ -6,12 +6,13 @@
>  __a_barrier:
>  	ldr ip,1f
>  	ldr ip,[pc,ip]
> -	add pc,pc,ip
> +	add pc,ip

I don't think this actually works; the arithmetic on pc does different
things in thumb mode. This file just needs to be scrapped and
redesigned since it's not going to be compatible with fdpic anyway (it
relies on PC-relative accesses to global data).

> Index: arch/arm/crt_arch.h
> ===================================================================
> --- arch/arm/crt_arch.h	(revision 4919)
> +++ arch/arm/crt_arch.h	(revision 4920)
> @@ -1,11 +1,17 @@
> -__asm__("\
> -.text \n\
> -.global _start \n\
> -.type _start,%function \n\
> -_start: \n\
> -	mov fp, #0 \n\
> -	mov lr, #0 \n\
> -	mov a1, sp \n\
> -	bic sp, sp, #0xF \n\
> -	bl __cstart \n\
> -");
> +__asm__(
> +".text \n"
> +".global _start \n"
> +".type _start,%function \n"
> +"_start: \n"
> +"	mov fp, #0 \n"
> +"	mov lr, #0 \n"
> +"	mov a1, sp \n"
> +#if defined(__thumb__)
> +"       mov a2, sp \n"
> +"       bic a2, #0xF \n"
> +"       mov sp, a2 \n"
> +#else
> +"	bic sp, sp, #0xF \n"
> +#endif
> +"	bl __cstart \n"
> +);

I think I had a simpler version of this tucked away somewhere; I'll
check and see.

Thanks for working on this and reviving interest in the topic.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.