make: use pre-compiled newlib and libc++ #353

bradjc · 2023-11-01T20:47:58Z

Follow up to #343.

This restructures the newlib and libc++ handling to create .zip files which can be stored remotely and fetched when users want to build libtock-c apps. This removes the need to store the compiled libraries in the repo.

This also updates the versions of the two libraries.

Since we now ship our own newlib we no longer need the user's system to include one. This makes compiling riscv much easier and we enable that by default now.

New features compared to current approach

We don't use the user's installed newlib, for libraries or headers. This means we can work even if there isn't an easy way for a user to get newlib on their platform. It also means we aren't using whatever headers they have on their system with our compiled libraries.
We are compiling the risc-v newlib libraries now (not just cortex-m). This means if we ever get pic support we are in a position to provide the pic compiled libraries.
We now support different versions of the newlib and libc++ libraries depending on the version of the compiler the user has. This is important because different versions of libc++ and newlib can only be compiled with different versions of the compiler. For example, if you are on gcc 10 you cannot use newlib 4.3, and if you are on gcc 13 you cannot use libc++ 10 (because of headers not compiling).
The makefile now chooses which version of the libraries to use (for arm and riscv independently) depending on which compiler version the user is using.
We use docker to build the libraries and document what is happening much better. This should make the provided libraries more reproducible. We also use something closer to the newlib/gcc build/install process, so we are doing less of our own hacking for packaging.
RISC-V now builds by default because we don't require systems to provide a riscv newlib.

Some lingering questions

Including <assert.> now gives a warning due to redefined things. This is because assert gets pulled in twice (once in the reent.h checks and once in the main.c file). I have no idea why this is an issue now and wasn't before, as nothing in this area of newlib seems to have changed. It seems like it should have been an issue before as well.
There are warnings created by c++ apps, but they don't stop the build/CI process.
The created .zip files are pretty big (100-250 MB).
Different users will now be using different dependencies even with the same libtock-c commit. This is annoying, but other than getting all linux distros to update their packages, I'm not sure what to do. Newer versions of newlib and gcc just don't work with older gcc versions, and newer versions of gcc don't work with older gcc headers. So, we can't win. The best we could do is stick with an older version of newlib, and give up on not requiring system headers. But the toolchain ecosystem on every platform is so bad that we end up very constriained (e.g. the new homebrew arm gcc recipe doesn't include headers).

Testing

Please try this branch out. It should work as a user (libtock-c developer).

I haven't done a FULL end-to-end test of the newlib and libc++ generation (it takes forever and is toolchain dependent). I want to do that, but others can look at this in the meantime.

alistair23 · 2023-11-02T00:18:18Z

lib/fetch-libc++.sh

+MIRRORS=(\
+  "https://www.cs.virginia.edu/~bjc8c/archive/tock"\
+  "https://alpha.mirror.svc.schuermann.io/files"\
+)


I don't think we should be automatically downloading binaries from custom places.

What happens if these are replaced with something malicious?
What happens if this is updated or changed, it isn't tracked in git?

The sha256 hash is. If someone can replace the zip with something malicious and the same hash, maybe we accept defeat?

What if we bump the newlib version and then delete the old one? That will break users who don't update as it's not checked into git.

It just seems risky and clunky to have these outside the repo. What is the benefit?

Hopefully we wouldn't delete from every mirror? I don't know why we would delete them.

The benefit is not having to not having to add ~150MB of zip files to the repo on every version update.

Hopefully we wouldn't delete from every mirror?

Hopefully not, but nothing stops that from happening.

The benefit is not having to not having to add ~150MB of zip files to the repo on every version update.

With git-lfs is that really an issue?

This all comes from a discussion we had where I thought the conclusion was pulling in a zip (like we've done with the riscv toolchain) was fine. https://github.com/tock/tock/blob/master/doc/wg/core/notes/core-notes-2023-09-22.md#libtock-c-newlib-updates

@lschuermann @ppannuto

After another discussion I don't think we are eager to pay for github lfs when we have a decent (free, ready to go) solution. https://docs.github.com/en/billing/managing-billing-for-git-large-file-storage/about-billing-for-git-large-file-storage

Happy to continue hosting this on my personal mirror. I can also add a mirror on our cluster web server at Princeton. I believe that, as long as we have a system that checks these URLs and complains if they're down, we should be able to maintain a good number of toolchain versions reliably before bandwidth / storage becomes an issue for us.

FWIW here's a crude example of how we can use GitHub actions to periodically check whether our mirrors are still alive and the links still work: https://github.com/lschuermann/tock-mirrortest. We can add a step to the workflow that automatically opens an issue and tags the admins of the failing mirror, and potentially also a script that collects these links from the various Tock repos. Just need to make sure we don't over-engineer this too much. What do you think @bradjc @alevy?

After another discussion I don't think we are eager to pay for github lfs when we have a decent (free, ready to go) solution. https://docs.github.com/en/billing/managing-billing-for-git-large-file-storage/about-billing-for-git-large-file-storage

Ah, I didn't realise GitHub charges for LFS

lschuermann · 2023-11-08T16:19:00Z

FWIW, GitHub actions supports executing Dockerfiles. We can create a manually started action that devs can use to generate these artifacts (and then copy them over to mirrors), without having to go through the process of getting Docker running on their system: https://docs.github.com/en/actions/creating-actions/creating-a-docker-container-action

alistair23 · 2023-11-10T00:20:51Z

.github/workflows/ci.yml

@@ -12,7 +12,7 @@ jobs:
  ci-format:
    strategy:
      matrix:
-        os: [ubuntu-20.04]
+        os: [ubuntu-22.04]


Should we split this PR up more? A few of these changes could just go right in which would reduce the diff and make reverting any regressions easier

alistair23 · 2023-11-10T00:21:22Z

Configuration.mk

+# Library versions.
+NEWLIB_VERSION ?= 4.2.0.20211231
+LIBCPP_VERSION ?= 12.2.0


Should we split this bump out as well

I think we might be able to split it up. We may also not want to do more work and review more PRs. I'm still trying to get it to work at all.

alistair23 · 2023-11-10T00:23:05Z

newlib/newlib-4.2.0.20211231.patch

+--- a/libtock-newlib-4.2.0.20211231/headers/ssp/string.h
+++ b/libtock-newlib-4.2.0.20211231/headers/ssp/string.h
+@@ -36,7 +36,7 @@
+
+ __BEGIN_DECLS
+ void *__memcpy_chk(void *, const void *, size_t, size_t);
+-void *__memmove_chk(void *, void *, size_t, size_t);
+void *__memmove_chk(void *, const void *, size_t, size_t);
+ void *__mempcpy_chk(void *, const void *, size_t, size_t);
+ void *__memset_chk(void *, int, size_t, size_t);
+ char *__stpcpy_chk(char *, const char *, size_t);
+


Why do we need to patch newlib? What's the status of this, have you submitted it upstream?

We get warnings without those changes.

Would newlib really not notice those warnings after multiple years? I assumed it's something we're doing differently or something they don't want to fix.

That seems like an issue on our side then.

I suspect they don't see the warnings. If we want to keep this we should send a fix upstream. Otherwise this is how nothing ever gets fixed

If they use github, easy enough, but I'm not going to learn how newlib does things via mailing lists, unfortunately.

Not wanting to learn a new tool doesn't seem like a good excuse here.

Newlib is a maintained project so we should be sending all fixes and patches upstream. Otherwise we are stuck patching newlib for the foreseeable future. We also don't even know if this is the correct fix, by sending the change upstream we can get feedback from the maintainers about the fix. We shouldn't just punt on the right way to do things because it might be a little harder

@bradjc FWIW I can volunteer to relay patches upstream, if you don't want to use mailing lists. Would need to dig into this issue first, though.

Please do!

Warning we get:

CC ../../libtock/udp.c In file included from ../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/string.h:180, from ../../libtock/udp.c:1: ../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/ssp/string.h:39:7: warning: mismatch in argument 2 type of built-in function '__memmove_chk'; expected 'const void *' [-Wbuiltin-declaration-mismatch] 39 | void *__memmove_chk(void *, void *, size_t, size_t); | ^~~~~~~~~~~~~

Based on -Wbuiltin-declaration-mismatch, the glibc definition:

https://github.com/bminor/glibc/blob/2e0c0ff95ca0e3122eb5b906ee26a31f284ce5ab/include/string.h#L189

extern void *__memmove_chk (void *__dest, const void *__src, size_t __len, size_t __destlen) __THROW;

versus newlib:

https://github.com/bminor/newlib/blob/d5dcb484c705a214b30826c82b9fd8bf83772093/newlib/libc/include/ssp/string.h#L39

void *__memmove_chk(void *, void *, size_t, size_t);

And I suppose we haven't seen this before because we've been using the headers provided by the user's compiler.

bradjc · 2023-11-21T21:30:29Z

Ok next status update.

libc++

Doesn't work:
- libc++ version 11.2 compiled with gcc 10.3 (on ubuntu)
- compiling libtock-c c++ app with gcc 10.3
Doesn't work:
- libc++ version 11.2 compiled with gcc 10.3 (on ubuntu)
- compiling libtock-c c++ app with gcc 13
Does work:
- libc++ version 11.2 compiled with gcc 10.3 (on ubuntu)
- compiling libtock-c c++ app with gcc 11.2 (at least for arm)

In both error cases the errors seem to propagate from the headers (not our compiled libraries).

It's possible that gcc 11.2 is just not the version to use. I think I will try with a newer gcc source next.

bradjc · 2023-11-22T15:56:10Z

Next finding: building libc++ 13.2 with 10.3 compiler on riscv doesn't work:

  /libtock-c/libc++/gcc-riscv-13.2.0-out/./gcc/xgcc -B/libtock-c/libc++/gcc-riscv-13.2.0-out/./gcc/ -B/libtock-c/libc++/gcc-riscv-13.2.0-install/riscv64-unknown-elf/bin/ -B/libtock-c/libc++/gcc-riscv-13.2.0-install/riscv64-unknown-elf/lib/ -isystem /libtock-c/libc++/gcc-riscv-13.2.0-install/riscv64-unknown-elf/include -isystem /libtock-c/libc++/gcc-riscv-13.2.0-install/riscv64-unknown-elf/sys-include    -g -Os -ffunction-sections -fdata-sections -isystem ../newlib-4.2.0.20211231/newlib/libc/include -march=rv32i -mabi=ilp32 -O2  -g -Os -ffunction-sections -fdata-sections -isystem ../newlib-4.2.0.20211231/newlib/libc/include -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include   -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector   -I. -I. -I../../../.././gcc -I../../../../../gcc-13.2.0/libgcc -I../../../../../gcc-13.2.0/libgcc/. -I../../../../../gcc-13.2.0/libgcc/../gcc -I../../../../../gcc-13.2.0/libgcc/../include  -DHAVE_CC_TLS   -fvisibility=hidden -DHIDE_EXPORTS -c eh_dummy.c        \
     -o eh_dummy.o;                             \
  objects=eh_dummy.o;                           \
fi;                                                     \
riscv64-unknown-elf-ar  rc libgcov.a $objects
riscv64-unknown-elf-ranlib libgcov.a
./../../../../../gcc-13.2.0/libgcc/config/riscv/value-unwind.h: Assembler messages:
./../../../../../gcc-13.2.0/libgcc/config/riscv/value-unwind.h:32: Error: unrecognized opcode `0xc2202573'
make[5]: *** [../../../../../gcc-13.2.0/libgcc/static-object.mk:17: unwind-dw2.o] Error 1
make[5]: Leaving directory '/libtock-c/libc++/gcc-riscv-13.2.0-out/riscv64-unknown-elf/rv32i/ilp32/libgcc'
make[4]: *** [Makefile:1214: multi-do] Error 1
make[4]: Leaving directory '/libtock-c/libc++/gcc-riscv-13.2.0-out/riscv64-unknown-elf/libgcc'
make[3]: *** [Makefile:127: all-multi] Error 2
make[3]: Leaving directory '/libtock-c/libc++/gcc-riscv-13.2.0-out/riscv64-unknown-elf/libgcc'
make[2]: *** [Makefile:13961: all-target-libgcc] Error 2
make[2]: Leaving directory '/libtock-c/libc++/gcc-riscv-13.2.0-out'
make[1]: *** [Makefile:1036: all] Error 2
make[1]: Leaving directory '/libtock-c/libc++/gcc-riscv-13.2.0-out'
make: *** [Makefile:56: rebuild-gcc] Error 2
root@b0c18da1605d:/libtock-c/libc++#

bradjc · 2023-11-22T16:42:55Z

With libc++ 12.3, c++ apps compile with gcc 13.2, but with gcc 10.3 we get the same(-ish) header errors as with libc++ 11.2.

Last chance for a one-size-fits-all option i think is libc++ 10.5.

bradjc · 2023-11-24T19:04:12Z

Ok I think things are back to a working state. The make file now chooses which versions of newlib and libc++ to use based on the versions of the toolchains in use (both arm and riscv independently).

Also the packaging for the built archives is cleaner without so much of our own manual file choosing, now we just use make install and copy the output.

Next up I need to figure out dockerfiles for different versions of libc++ and see if I can get a gcc 13 version. Although I can't find any linux distros that have riscv64-unknown-elf-gcc v13.

bradjc · 2023-11-27T14:34:28Z

examples/tests/udp/udp_virt_app_tests/app1/main.c

@@ -1,7 +1,6 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
-#include <assert.h>


I don't understand this change. Why can we no longer include assert.h? I can't figure out anything that changed from previous versions of newlib.

Without this change we get:

CC main.c In file included from main.c:4: ../../../../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/assert.h:39:6: warning: redundant redeclaration of '__assert' [-Wredundant-decls] 39 | void __assert (const char *, int, const char *) | ^~~~~~~~ In file included from ../../../../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/sys/reent.h:458, from ../../../../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/stdio.h:60, from main.c:2: ../../../../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/assert.h:39:6: note: previous declaration of '__assert' with type 'void(const char *, int, const char *)' 39 | void __assert (const char *, int, const char *) | ^~~~~~~~ ../../../../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/assert.h:41:6: warning: redundant redeclaration of '__assert_func' [-Wredundant-decls] 41 | void __assert_func (const char *, int, const char *, const char *) | ^~~~~~~~~~~~~ ../../../../../lib/libtock-newlib-4.3.0.20230120/arm/arm-none-eabi/include/assert.h:41:6: note: previous declaration of '__assert_func' with type 'void(const char *, int, const char *, const char *)' 41 | void __assert_func (const char *, int, const char *, const char *) | ^~~~~~~~~~~~~ LD build/cortex-m0/cortex-m0.elf

assert.h specifically doesn't have an include guard.

bradjc · 2023-12-20T20:36:04Z

Ok this is ready.

It would be great to make a decision one way or another on this.

lschuermann

Changes look good, and this seems to work on NixOS for ARM and RISC-V. I'll follow this up with some changes to the shell.nix file, such that we no longer build a custom compiler and libc there.

support any version, package with `make install`

Causes conflicts with already included items

new folder where we store precompiled libraries. includes fetch scripts to pull from remote

no newlib riscv by default

lschuermann · 2024-01-17T13:21:28Z

I fixed the CI failures of this PR in 22f8a73 (change LEGACY_LIBS to LINK_LIBS in the LoRa examples).

Both the RadioLib include and newlib were generating a bunch of warnings in CI, most of which seemed to be code-style related. I changed both of these to system includes (-isystem); if this is not desired, feel free to revert.

bradjc · 2024-01-17T16:48:42Z

Hooray, thanks!

lschuermann

Assuming this is good to merge, as it's been previously added to the merge queue already.

bradjc · 2024-01-18T01:41:01Z

!!! Woo!

make: use pre-compiled newlib and libc++

alistair23 suggested changes Nov 2, 2023

View reviewed changes

alistair23 suggested changes Nov 10, 2023

View reviewed changes

bradjc commented Nov 27, 2023

View reviewed changes

bradjc force-pushed the make-precompiled branch 2 times, most recently from fb6414c to 7794f20 Compare December 20, 2023 17:27

bradjc mentioned this pull request Dec 20, 2023

Support picolib #357

Open

alevy previously approved these changes Dec 21, 2023

View reviewed changes

brghena previously approved these changes Jan 5, 2024

View reviewed changes

lschuermann previously approved these changes Jan 5, 2024

View reviewed changes

bradjc mentioned this pull request Jan 7, 2024

Update newlib and libc++ build scripts and artifacts #346

Closed

11 tasks

alevy force-pushed the make-precompiled branch from 10c0c7e to 5370b36 Compare January 12, 2024 17:14

alevy enabled auto-merge January 12, 2024 17:14

bradjc added 12 commits January 12, 2024 15:47

libraries: remove prebuilt newlib and libc++

48a9ec8

hotp: isalnum needs an int

d255c9b

newlib: update build scripts

5d8343f

support any version, package with `make install`

libc++: new build scripts

19a2e72

usb keyboard: tolower needs an int

ad9f182

tests: udp: no include assert

5ef8b4f

Causes conflicts with already included items

mpu test: avoid warning for infinite recursion

9d0d112

lib: fetch precompiled libraries

b124de1

new folder where we store precompiled libraries. includes fetch scripts to pull from remote

newlib/libc++: add docker scripts

c0389df

make: use new precompiled libaries and headers

4ed1fd6

make: compile risc-v by default

42eb709

readme: update instructions

0101be9

no newlib riscv by default

bradjc added 3 commits January 12, 2024 15:47

CI: use more standard install

c08213a

update commit hashes in dockerfiles

4e8f8d9

lib: update library sha256 sums

df5b9b8

alevy force-pushed the make-precompiled branch from 5370b36 to df5b9b8 Compare January 12, 2024 20:47

lib/fetch-{libc++,newlib}: change alpha.mirror.schuermann.io URL

24668f2

lschuermann disabled auto-merge January 17, 2024 13:05

lschuermann dismissed stale reviews from brghena, alevy, and themself via aa882f7 January 17, 2024 13:15

lschuermann added 2 commits January 17, 2024 08:17

examples/lora: fix RadioLib link args and treat as system include

22f8a73

Configuration.mk: treat newlib and libcpp as system includes

27b5991

lschuermann force-pushed the make-precompiled branch from aa882f7 to 27b5991 Compare January 17, 2024 13:19

lschuermann added a commit to tock/mirrorcheck that referenced this pull request Jan 17, 2024

Add precompiled newlib and libc++ from tock/libtock-c#353

9d08a75

lschuermann added a commit to tock/mirrorcheck that referenced this pull request Jan 17, 2024

Add precompiled newlib and libc++ from tock/libtock-c#353

2d46ddd

lschuermann approved these changes Jan 17, 2024

View reviewed changes

lschuermann enabled auto-merge January 17, 2024 23:01

lschuermann disabled auto-merge January 17, 2024 23:02

lschuermann merged commit d6230f8 into master Jan 17, 2024
2 checks passed

lschuermann deleted the make-precompiled branch January 17, 2024 23:02

bradjc mentioned this pull request Jan 18, 2024

newlib: Bump to 4.3.0.20230120 #343

Closed

lschuermann mentioned this pull request Jan 25, 2024

Remove custom RISC-V toolchain from Nix shell, remove Segger J-Link, update Tockloader to v1.11.0 #365

Merged

This was referenced Feb 9, 2024

Build for RISC-V by Default #264

Closed

newlib/libcpp: update commit hash in dockerfile #369

Merged

tyler-potyondy pushed a commit to tyler-potyondy/libtock-c that referenced this pull request Mar 13, 2024

Merge pull request tock#353 from tock/make-precompiled

2a5b3d6

make: use pre-compiled newlib and libc++

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make: use pre-compiled newlib and libc++ #353

make: use pre-compiled newlib and libc++ #353

bradjc commented Nov 1, 2023 •

edited

Loading

alistair23 Nov 2, 2023

bradjc Nov 2, 2023

alistair23 Nov 3, 2023

bradjc Nov 3, 2023

alistair23 Nov 3, 2023

bradjc Nov 3, 2023

bradjc Nov 3, 2023

lschuermann Nov 3, 2023

lschuermann Nov 3, 2023 •

edited

Loading

alistair23 Nov 9, 2023

lschuermann commented Nov 8, 2023

alistair23 Nov 10, 2023

alistair23 Nov 10, 2023

bradjc Nov 10, 2023

alistair23 Nov 10, 2023

bradjc Nov 10, 2023

alistair23 Nov 20, 2023

bradjc Nov 21, 2023

alistair23 Nov 23, 2023

lschuermann Nov 24, 2023

bradjc Nov 27, 2023

bradjc commented Nov 21, 2023

bradjc commented Nov 22, 2023

bradjc commented Nov 22, 2023

bradjc commented Nov 24, 2023

bradjc Nov 27, 2023

bradjc commented Dec 20, 2023

lschuermann left a comment

lschuermann commented Jan 17, 2024

bradjc commented Jan 17, 2024

lschuermann left a comment

bradjc commented Jan 18, 2024

make: use pre-compiled newlib and libc++ #353

make: use pre-compiled newlib and libc++ #353

Conversation

bradjc commented Nov 1, 2023 • edited Loading

New features compared to current approach

Some lingering questions

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lschuermann Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lschuermann commented Nov 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradjc commented Nov 21, 2023

libc++

bradjc commented Nov 22, 2023

bradjc commented Nov 22, 2023

bradjc commented Nov 24, 2023

Choose a reason for hiding this comment

bradjc commented Dec 20, 2023

lschuermann left a comment

Choose a reason for hiding this comment

lschuermann commented Jan 17, 2024

bradjc commented Jan 17, 2024

lschuermann left a comment

Choose a reason for hiding this comment

bradjc commented Jan 18, 2024

bradjc commented Nov 1, 2023 •

edited

Loading

lschuermann Nov 3, 2023 •

edited

Loading