qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.28k stars 39.38k forks source link

GCC version installed on Fedora causes build errors #7006

Closed cyky closed 2 years ago

cyky commented 5 years ago

Fedora 30 and above use GCC 9.2.x. This seems to cause errors at least when targeting ergodox_infinity (Similar to ones mentioned in https://github.com/qmk/qmk_firmware/issues/6719).

./lib/chibios/os/common/ext/CMSIS/include/core_cm4.h:93:28: error: listing the stack pointer register 'sp' in a clobber list is deprecated [-Werror=deprecated]
   93 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler */
      |                            ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h:164:3: note: in expansion of macro '__ASM'
  164 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/core_cm4.h:93:28: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
   93 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler */
      |                            ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h:164:3: note: in expansion of macro '__ASM'
  164 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h: In function '_port_irq_epilogue':
./lib/chibios/os/common/ext/CMSIS/include/core_cm4.h:93:28: error: listing the stack pointer register 'sp' in a clobber list is deprecated [-Werror=deprecated]
   93 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler */
      |                            ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h:164:3: note: in expansion of macro '__ASM'
  164 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/core_cm4.h:93:28: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
   93 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler */
      |                            ^~~~~
./lib/chibios/os/common/ext/CMSIS/include/cmsis_gcc.h:164:3: note: in expansion of macro '__ASM'
  164 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
cc1: all warnings being treated as errors
 [ERRORS]
 | 
 | 
 | 
make[1]: *** [tmk_core/rules.mk:380: .build/obj_ergodox_infinity/lib/chibios/os/common/ports/ARMCMx/chcore_v7m.o] Error 1
Make finished with errors

Using CFLAGS = -Wno-error=deprecated makes it possible to bypass these errors and build the binary successfully, but this probably isn't the correct way to fix the building for Fedora based systems. Also downgrading the GCC doesn't seem to be that straightforward as no base repositories seem to provide the 8.x versions.

I'd like to start discussion regarding how we should handle similar situations and what should be done with the current linux_install.sh as it installs the wrong version on newer Fedora versions.

At least with ergodox infinity I cannot reproduce the size issue mentioned in https://github.com/qmk/qmk_firmware/issues/6719 as the binary built with docker is actually larger than the one built with the newer GCC.

Should the install script have some kind of comment regarding this or should the documentation somehow describe this issue?

yanfali commented 5 years ago

@cyky we don't support gcc 9.2 sorry, it's bugged. Please try 8.3, 7.3 or 6.3. We don't attempt to handle issues with your distro. You can try looking for an RPM or installing from source. There are simply far too many linux distros to handle this in a sane way.

As you've noticed we have a docker container. That is probably the easiest way to deal with your rolling distro issues if you can install docker.

We could also just print out a message after installation just explaining if you have newer version, but I think that's about the best we can do. We have very little control over upstream GCC.

cyky commented 5 years ago

Yeah probably modifying the install script to want about possible issues would be the way to go and maybe the documentation should also reflect that?

Erovia commented 5 years ago

Could you please try this patch?

1) Copy&paste the content of the code block into a file:

diff --git a/os/common/ext/CMSIS/include/cmsis_gcc.h b/os/common/ext/CMSIS/include/cmsis_gcc.h
index d868f2e64..bc8138a8c 100644
--- a/os/common/ext/CMSIS/include/cmsis_gcc.h
+++ b/os/common/ext/CMSIS/include/cmsis_gcc.h
@@ -161,7 +161,7 @@ __attribute__( ( always_inline ) ) __STATIC_INLINE uint32_t __get_PSP(void)
  */
 __attribute__( ( always_inline ) ) __STATIC_INLINE void __set_PSP(uint32_t topOfProcStack)
 {
-  __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
+  __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : );
 }

2) Run this in your QMK directory: git apply --directory lib/chibios/ /path/to/the/patch

cyky commented 5 years ago

@Erovia that seems to fix the issue

Erovia commented 5 years ago

Great. Can you confirm the functionality of the built firmware?

cyky commented 5 years ago

Well I haven't seen any issues within last 30 minutes of use.

joshuarubin commented 4 years ago
diff --git a/os/common/ext/CMSIS/include/cmsis_gcc.h b/os/common/ext/CMSIS/include/cmsis_gcc.h
index d868f2e64..8b72e1492 100644
--- a/os/common/ext/CMSIS/include/cmsis_gcc.h
+++ b/os/common/ext/CMSIS/include/cmsis_gcc.h
@@ -161,7 +161,7 @@ __attribute__( ( always_inline ) ) __STATIC_INLINE uint32_t __get_PSP(void)
  */
 __attribute__( ( always_inline ) ) __STATIC_INLINE void __set_PSP(uint32_t topOfProcStack)
 {
-  __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
+  __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : );
 }

@@ -187,7 +187,7 @@ __attribute__( ( always_inline ) ) __STATIC_INLINE uint32_t __get_MSP(void)
  */
 __attribute__( ( always_inline ) ) __STATIC_INLINE void __set_MSP(uint32_t topOfMainStack)
 {
-  __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : "sp");
+  __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : );
 }

I've been having to use this patch for quite some time now. It would be nice if we could get it merged.

Erovia commented 4 years ago

There are some on-going efforts to upgrade the ChibiOS that QMK uses.