InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.69k stars 921 forks source link

Update GCC (in doc and docker) to GCC12 #1512

Open JF002 opened 1 year ago

JF002 commented 1 year ago

Update the documentation and Dockerfile to use GCC12. A newer version of GCC is needed to use features from C++20 like concepts. Needed by #1387.

JF002 commented 1 year ago

Thanks for your prompt review, @FintasticMan ! I updated the mentions to the toolchain in .vscode/launch.json, doc/buildAndProgram.md and doc/buildWithVScode.md. I did not edit doc/MemoryAnalysis.md since this analysis was done in the older version of the toolchain.

I also remember there being some new warnings in the compiler output using 11.3.Rel1 and 12.2.MPACBTI-Bet1. Have these been fixed in the 12.2.Rel1 release?

Yes, the compiler issues a few new warnings. Since most of them come from 3rd party code, they should be fix with #1501.

FintasticMan commented 1 year ago

With regards to the warnings, I was specifically talking about these ones, and they seem to still be there with this new version. They occur during linking.

image

JF002 commented 1 year ago

Oh... It's strange, those error are not displayed when I build in CLion, but I get them when building with docker. The good news is that we certainly do not use those functions, but that would be better if the compiler did not complain about them... I'll have a look at this!

JF002 commented 1 year ago

My understanding is that we link with newlib. It provides many function from the standard library. If we disable it (add -nostdlib in LINK_FLAGS), it yields many error about missing references to operator new, std::string related functions,... It means that we currently need newlib.

My searches show that many people noticed that these warning appeared as of GCC 11.3 (I tried with gcc11.2 : no warning). Relevant link (with no actual response, unfortunately) : https://community.arm.com/support-forums/f/compilers-and-libraries-forum/53519/arm-gcc-11-3-rel1-newlib-nano-linker-warning-_read-is-not-implemented-and-will-always-fail

I checked the .map files from GCC10 and GCC12 and I couldn't find relevant differences regarding the symbols _close(), _close_r(),... mentioned in those warning. My guess is that previous versions of GCC would simply ignore those warnings and that more recent versions issue them.

However, I don't know how to prove this.

I could silence those warnings by adding this in main.cpp :

extern "C" {
__attribute__((weak)) int _isatty(int fd) {
  errno = EBADF;
  return 0;
}

__attribute__((weak)) int _close(int fd) {
  errno = EBADF;
  return -1;
}

__attribute__((weak)) int _lseek(int fd, int ptr, int dir) {
  (void) fd;
  (void) ptr;
  (void) dir;

  errno = EBADF;
  return -1;
}

__attribute__((weak)) int _fstat(int fd, struct stat* st) {
  errno = EBADF;
  return 0;
}

__attribute__((weak)) int _read(int file, char* ptr, int len) {
  (void) file;

  return 0;
}

__attribute__((unused)) __attribute__((weak)) int _write(int file, char* ptr, int len) {
  (void) file;

  return 0;
}

__attribute__((unused)) __attribute__((weak)) int _getpid(void) {
  return -1;
}

__attribute__((weak)) void _kill(int pid, int sig) {
  return;
}
}

It simply provide an empty implementation for those functions required by newlib. I checked the size of the bin file :

Does that mean that the "default" implementation is 48B bigger than mine ? I don't know...

So the question is : do we need to worry about those warning, or can we simply silence them by implementing the missing functions needed by newlib?

Riksu9000 commented 1 year ago

Neither seems like a good idea to me. Hopefully this issue gets resolved before we need to update. Just noticed that there's a new comment in the linked thread, which says changing --specs=nosys.specs to --specs=nano.specs fixed the issue.

FintasticMan commented 1 year ago

I saw that too, but it seems we are already using --specs=nano.specs.

mark9064 commented 2 months ago

With the new 13.3 toolchain, the message has changed!

/home/desktop/src_builds/pinetime/arm-gnu-toolchain-13.3.rel1-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/13.3.1/../../../../arm-none-eabi/bin/ld: /home/desktop/src_builds/pinetime/arm-gnu-toolchain-13.3.rel1-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/13.3.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-closer.o): in function `_close_r':
closer.c:(.text._close_r+0xc): warning: _close is not implemented and will always fail
/home/desktop/src_builds/pinetime/arm-gnu-toolchain-13.3.rel1-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/13.3.1/../../../../arm-none-eabi/bin/ld: /home/desktop/src_builds/pinetime/arm-gnu-toolchain-13.3.rel1-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/13.3.1/../../../../arm-none-eabi/lib/thumb/v7e-m+fp/hard/libc_nano.a(libc_a-closer.o): note: the message above does not take linker garbage collection into account

Specifically: note: the message above does not take linker garbage collection into account So we can safely ignore them (provided the linker does GC), but they are noisy