Add support for Picolibc and building RISC-V applications by default

[alistair23]

This PR adds support for using picolibc as the libc. This means that most major distros now support RISC-V bulids just using the package manager. So let's also enable RISC-V builds by default, allowing users to disable the build if they wish.

Fixes: #304
Fixes: #264

[bradjc]

Can you group the instructions by host platform and not by target architecture?

[alistair23]

Any more comments?

[bradjc]

We vendor libc for arm so we can compile with pic support. So we probably don't want to suggest sudo apt install libnewlib-arm-none-eabi then?

I don't like the separation of compiler from std lib. Most users just want to copy and paste and get things working as fast as possible, they don't want to read all the steps or understand how the compilation infrastructure works. If we are ready for risc-v by default, then this needs to be a lot simpler for ubuntu and mac.

I also don't like instruction number 4. What is using picolib conditional on? How should a user know if that step applies to them?

[alistair23]

We vendor libc for arm so we can compile with pic support. So we probably don't want to suggest sudo apt install libnewlib-arm-none-eabi then?

Dropped

I don't like the separation of compiler from std lib.

I'm not sure I follow. They are separate things, there isn't much we can do about that. Pretending that they are not separate just seems more confusing.

Most users just want to copy and paste and get things working as fast as possible, they don't want to read all the steps or understand how the compilation infrastructure works. If we are ready for risc-v by default, then this needs to be a lot simpler for ubuntu and mac.

Yep! This PR significantly reduces the amount of steps in the README, it removes more text then it adds. Hopefully making it easier for others to follow. All of the steps are pretty easy to copy and paste, just with some information and context provided for people who are curious or want to dig into it more.

I also don't like instruction number 4. What is using picolib conditional on? How should a user know if that step applies to them?

I have updated the information here to provide more details.

[alistair23]

This also probably fixes #309 as well

[alistair23]

Any more comments on this?

[lschuermann]

I'm generally in favor of these changes, as the instructions in the README right now are broken and I'm entirely unable to get things built with picolibc on a recent-ish Ubuntu (22.04). The instructions already suggest using Picolibc, but it is not at all integrated.

However, I see a few problems with these changes:

• Picolibc includes its own sbrk implementation (picosbrk.c). This implementation assumes that the application is running on bare-metal and does not share certain memory sections with e.g., a Tock kernel. AFAIK the pre-compiled newlib we ship does also define the sbrk symbol, but then calls into our custom _sbrk implementation which invokes a memop.

  For reference, here's the generated assembly for ARM (with the precompiled newlib) and RISC-V (with the Picolibc, can't actually get it to compile with newlib):

  • ARM Cortex-M4 sbrk

    800008c0 <sbrk>:
    800008c0:       4b03            ldr     r3, [pc, #12]   ; (800008d0 <sbrk+0x10>)
    800008c2:       4601            mov     r1, r0
    800008c4:       f859 3003       ldr.w   r3, [r9, r3]
    800008c8:       6818            ldr     r0, [r3, #0]
    800008ca:       f000 bea5       b.w     80001618 <_sbrk_r>
    800008ce:       bf00            nop
    800008d0:       00000094        .word   0x00000094
    
    80001618 <_sbrk_r>:
    80001618:       b538            push    {r3, r4, r5, lr}
    8000161a:       4b07            ldr     r3, [pc, #28]   ; (80001638 <_sbrk_r+0x20>)
    8000161c:       4604            mov     r4, r0
    8000161e:       f859 5003       ldr.w   r5, [r9, r3]
    80001622:       2300            movs    r3, #0
    80001624:       4608            mov     r0, r1
    80001626:       602b            str     r3, [r5, #0]
    80001628:       f000 f8e9       bl      800017fe <_sbrk>
    8000162c:       1c43            adds    r3, r0, #1
    8000162e:       d102            bne.n   80001636 <_sbrk_r+0x1e>
    80001630:       682b            ldr     r3, [r5, #0]
    80001632:       b103            cbz     r3, 80001636 <_sbrk_r+0x1e>
    80001634:       6023            str     r3, [r4, #0]
    80001636:       bd38            pop     {r3, r4, r5, pc}
    80001638:       00000068        .word   0x00000068
    
    800017fe <_sbrk>:
    caddr_t _sbrk(int incr)
    {
    800017fe:       b507            push    {r0, r1, r2, lr}
    80001800:       4602            mov     r2, r0
      memop_return_t ret;
      ret = memop(1, incr);
    80001802:       2101            movs    r1, #1
    80001804:       4668            mov     r0, sp
    80001806:       f7fe fef4       bl      800005f2 <memop>
      if (ret.status != TOCK_STATUSCODE_SUCCESS) {
    8000180a:       f89d 3000       ldrb.w  r3, [sp]
    8000180e:       b143            cbz     r3, 80001822 <_sbrk+0x24>
        errno = ENOMEM;
    80001810:       f000 f8a4       bl      8000195c <__errno>
    80001814:       230c            movs    r3, #12
    80001816:       6003            str     r3, [r0, #0]
        return (caddr_t) -1;
    80001818:       f04f 30ff       mov.w   r0, #4294967295 ; 0xffffffff
      }
      return (caddr_t) ret.data;
    }
    8000181c:       b003            add     sp, #12
    8000181e:       f85d fb04       ldr.w   pc, [sp], #4
      return (caddr_t) ret.data;
    80001822:       9801            ldr     r0, [sp, #4]
    80001824:       e7fa            b.n     8000181c <_sbrk+0x1e>
    
    80001826 <putnstr_async>:
      free(data);
    
      return ret;
    }
    

  • RV32IMAC Picolibc sbrk

    20040946 <sbrk>:
    20040946:       80003737                lui     a4,0x80003
    2004094a:       87aa                    mv      a5,a0
    2004094c:       86ba                    mv      a3,a4
    2004094e:       02c72503                lw      a0,44(a4) # 8000302c <__heap_end+0xfffffa98>
    20040952:       0007de63                bgez    a5,2004096e <sbrk+0x28>
    20040956:       80003737                lui     a4,0x80003
    2004095a:       19470713                addi    a4,a4,404 # 80003194 <__heap_end+0xfffffc00>
    2004095e:       40e50733                sub     a4,a0,a4
    20040962:       40f00633                neg     a2,a5
    20040966:       00c75b63                bge     a4,a2,2004097c <sbrk+0x36>
    2004096a:       557d                    li      a0,-1
    2004096c:       8082                    ret
    2004096e:       80003737                lui     a4,0x80003
    20040972:       59470713                addi    a4,a4,1428 # 80003594 <__heap_end+0x0>
    20040976:       8f09                    sub     a4,a4,a0
    20040978:       fef749e3                blt     a4,a5,2004096a <sbrk+0x24>
    2004097c:       97aa                    add     a5,a5,a0
    2004097e:       02f6a623                sw      a5,44(a3)
    20040982:       8082                    ret
    

• Somehow the picolibc headers aren't picked up correctly. This is evident when compiling anything which relies on the tinystdio's stdio.h, e.g. the mpu_walk_region test app:

  libtock-c/examples/tests/mpu_walk_region$ make PICOLIBC=1
    LD        build/rv32imac/rv32imac.0x20040060.0x80002800.elf
  /usr/lib/riscv64-unknown-elf/bin/ld: build/rv32imac/main.o: in function `main':
  /home/leons/dev/libtock-c/examples/tests/mpu_walk_region/main.c:77: undefined reference to `stdout'/usr/lib/riscv64-unknown-elf/bin/ld: /home/leons/dev/libtock-c/examples/tests/mpu_walk_region/main.c:77: undefined reference to `stdout'
  /usr/lib/riscv64-unknown-elf/bin/ld: /usr/lib/picolibc/riscv64-unknown-elf/lib/rv32imac/ilp32/libc.a(libc_tinystdio_printf.c.o): in function `printf':
  ./riscv64-unknown-elf/../../../newlib/libc/tinystdio/printf.c:42: undefined reference to `stdout'
  /usr/lib/riscv64-unknown-elf/bin/ld: ./riscv64-unknown-elf/../../../newlib/libc/tinystdio/printf.c:42: undefined reference to `stdout'
  /usr/lib/riscv64-unknown-elf/bin/ld: /usr/lib/picolibc/riscv64-unknown-elf/lib/rv32imac/ilp32/libc.a(libc_tinystdio_puts.c.o): in function `puts':
  ./riscv64-unknown-elf/../../../newlib/libc/tinystdio/puts.c:41: undefined reference to `stdout'
  /usr/lib/riscv64-unknown-elf/bin/ld: /usr/lib/picolibc/riscv64-unknown-elf/lib/rv32imac/ilp32/libc.a(libc_tinystdio_puts.c.o):./riscv64-unknown-elf/../../../newlib/libc/tinystdio/puts.c:41: more undefined references to `stdout' follow
  collect2: error: ld returned 1 exit status
  make: *** [../../../AppMakefile.mk:307: build/rv32imac/rv32imac.0x20040060.0x80002800.elf] Error 1
  

  Virtually the only app I managed to compile with this is c_hello.

• The added macro definitions in console.h may conflict with the definitions from tinystdio's stdio.h. We probably want to import that in console.h instead of redefining things locally, but that makes it easy to accidentally depend on other (more expensive) functions from tinystdio.

I don't really know how to elegantly solve any of these issues, especially the sbrk implementation. Can we selectively exclude select object files from a pre-compiled & -packaged library?

[ppannuto]

Subsumed by #357.