Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

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

wants to merge 3 commits into from

Conversation

JF002
Copy link
Collaborator

@JF002 JF002 commented Jan 4, 2023

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.

Copy link
Member

@FintasticMan FintasticMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some references to the specific toolchain version in some of the docs, and those might want updating/generalising. As far as I know, those are: .vscode/launch.json, doc/MemoryAnalysis.md, doc/buildAndProgram.md and doc/buildWithVScode.md.

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?

docker/build.sh Outdated Show resolved Hide resolved
@JF002
Copy link
Collaborator Author

JF002 commented Jan 4, 2023

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
Copy link
Member

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
Copy link
Collaborator Author

JF002 commented Jan 4, 2023

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
Copy link
Collaborator Author

JF002 commented Jan 5, 2023

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 :

  • With those stubs : 432428B
  • Without those stubs : 432476B (current state of the project)

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
Copy link
Contributor

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
Copy link
Member

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

@mark9064
Copy link
Member

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

@lmamane
Copy link

lmamane commented Nov 3, 2024

FYI, I built version 1.14.1 with GCC 13.3.1, "make pinetime-a[[" ends with:

arm-none-eabi/bin/ld: region RAM overflowed with stack

I had to shrink the heap by 620 bytes (in src/FreeRTSConfig.h, reduce the #define configTOTAL_HEAP_SIZE).

@mark9064 mark9064 added the maintenance Background work label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Background work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants