DiSlord / NanoVNA-D

Firmware for NanoVNA, NanoVNA-H, NanoVNA-H4. Support SD Card, external Serial connection, fast measure, fast exchange vs CPU
290 stars 64 forks source link

Buggy firmware when compiling with GCC-ARM version 12 - ok with GCC-ARM version 8 #65

Closed Ho-Ro closed 3 weeks ago

Ho-Ro commented 1 year ago

Compiling the tinySA source code with debian stable gcc version 12.2.1 20221205 (15:12.2.rel1-1) gives a buggy firmware - latest tinySA version as well as older versions.

Compiling the NanoVNA-H firmware gives comparable warnings caused by ChibiOS. (I did not download this version to my NanoVNA-H because I assume that it also leads to a faulty device).

After downgrade to the debian oldstable version gcc version 8.3.1 20190703 (release) [gcc-8-branch revision 273027] (15:8-2019-q3-1+b1) it compiles fine.

Warnings when compiling with GCC-ARM version 12:

Compiling crt1.c
Compiling vectors.c
Compiling chsys.c
Compiling chdebug.c
Compiling chtrace.c
Compiling chvt.c
Compiling chschd.c
Compiling chthreads.c
Compiling chcore.c
Compiling chcore_v6m.c
In file included from ChibiOS/os/common/ext/CMSIS/ST/STM32F0xx/stm32f072xb.h:129,
                 from ChibiOS/os/common/ext/CMSIS/ST/STM32F0xx/stm32f0xx.h:157,
                 from ChibiOS/os/common/startup/ARMCMx/devices/STM32F0xx/cmparams.h:72,
                 from ChibiOS/os/common/ports/ARMCMx/chcore.h:70,
                 from ChibiOS/os/rt/include/ch.h:81,
                 from ChibiOS/os/common/ports/ARMCMx/chcore_v6m.c:28:
In function '__set_PSP',
    inlined from 'NMI_Handler' at ChibiOS/os/common/ports/ARMCMx/chcore_v6m.c:72:3:
ChibiOS/os/common/ext/CMSIS/include/core_cm0.h:85:28: warning: listing the stack pointer register 'sp' in a clobber list is deprecated [-Wdeprecated]
   85 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler          */
      |                            ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cmFunc.h:443:3: note: in expansion of macro '__ASM'
  443 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cm0.h:85:28: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
   85 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler          */
      |                            ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cmFunc.h:443:3: note: in expansion of macro '__ASM'
  443 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
In function '__set_PSP',
    inlined from '_port_irq_epilogue' at ChibiOS/os/common/ports/ARMCMx/chcore_v6m.c:125:5:
ChibiOS/os/common/ext/CMSIS/include/core_cm0.h:85:28: warning: listing the stack pointer register 'sp' in a clobber list is deprecated [-Wdeprecated]
   85 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler          */
      |                            ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cmFunc.h:443:3: note: in expansion of macro '__ASM'
  443 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cm0.h:85:28: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
   85 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler          */
      |                            ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cmFunc.h:443:3: note: in expansion of macro '__ASM'
  443 |   __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");
      |   ^~~~~
Compiling osal.c
Compiling hal.c
Compiling hal_st.c
Compiling hal_buffers.c
Compiling hal_queues.c
Compiling hal_mmcsd.c
Compiling hal_ext.c
Compiling hal_gpt.c
Compiling hal_i2c.c
Compiling hal_i2s.c
Compiling hal_pal.c
Compiling hal_serial.c
Compiling hal_serial_usb.c
Compiling hal_spi.c
Compiling hal_usb.c
ChibiOS/os/hal/src/hal_usb.c: In function '_usb_ep0in':
ChibiOS/os/hal/src/hal_usb.c:842:8: warning: this statement may fall through [-Wimplicit-fallthrough=]
  842 |     if ((usbp->ep0n < max) &&
      |        ^
ChibiOS/os/hal/src/hal_usb.c:851:3: note: here
  851 |   case USB_EP0_WAITING_TX0:
      |   ^~~~
In file included from ChibiOS/os/rt/include/ch.h:82,
                 from ChibiOS/os/hal/osal/rt/osal.h:32,
                 from ChibiOS/os/hal/include/hal.h:28,
                 from ChibiOS/os/hal/src/hal_usb.c:27:
ChibiOS/os/rt/include/chdebug.h:129:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
  129 |   if (CH_DBG_ENABLE_ASSERTS != FALSE) {                                     \
      |      ^
ChibiOS/os/hal/osal/rt/osal.h:241:34: note: in expansion of macro 'chDbgAssert'
  241 | #define osalDbgAssert(c, remark) chDbgAssert(c, remark)
      |                                  ^~~~~~~~~~~
ChibiOS/os/hal/src/hal_usb.c:873:5: note: in expansion of macro 'osalDbgAssert'
  873 |     osalDbgAssert(false, "EP0 state machine error");
      |     ^~~~~~~~~~~~~
ChibiOS/os/hal/src/hal_usb.c:875:3: note: here
  875 |   case USB_EP0_ERROR:
      |   ^~~~
ChibiOS/os/hal/src/hal_usb.c: In function '_usb_ep0out':
ChibiOS/os/rt/include/chdebug.h:129:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
  129 |   if (CH_DBG_ENABLE_ASSERTS != FALSE) {                                     \
      |      ^
ChibiOS/os/hal/osal/rt/osal.h:241:34: note: in expansion of macro 'chDbgAssert'
  241 | #define osalDbgAssert(c, remark) chDbgAssert(c, remark)
      |                                  ^~~~~~~~~~~
ChibiOS/os/hal/src/hal_usb.c:932:5: note: in expansion of macro 'osalDbgAssert'
  932 |     osalDbgAssert(false, "EP0 state machine error");
      |     ^~~~~~~~~~~~~
ChibiOS/os/hal/src/hal_usb.c:934:3: note: here
  934 |   case USB_EP0_ERROR:
      |   ^~~~
Compiling nvic.c
Compiling hal_lld.c
Compiling stm32_dma.c
Compiling hal_st_lld.c
Compiling hal_ext_lld.c
Compiling hal_ext_lld_isr.c
Compiling hal_pal_lld.c
Compiling hal_i2c_lld.c
Compiling hal_i2s_lld.c
ChibiOS/os/hal/ports/STM32/LLD/SPIv2/hal_i2s_lld.c: In function 'i2s_lld_start':
ChibiOS/os/hal/ports/STM32/LLD/SPIv2/hal_i2s_lld.c:461:19: warning: assignment to 'uint32_t' {aka 'long unsigned int'} from 'volatile uint32_t *' {aka 'volatile long unsigned int *'} makes integer from pointer without a cast [-Wint-conversion]
  461 |       i2sp->rx_dr = &i2sp->spi->DR;
      |                   ^
Compiling hal_spi_lld.c
Compiling hal_gpt_lld.c
Compiling hal_serial_lld.c
Compiling hal_usb_lld.c
In file included from ChibiOS/os/rt/include/ch.h:82,
                 from ChibiOS/os/hal/osal/rt/osal.h:32,
                 from ChibiOS/os/hal/include/hal.h:28,
                 from ChibiOS/os/hal/ports/STM32/LLD/USBv1/hal_usb_lld.c:27:
ChibiOS/os/hal/ports/STM32/LLD/USBv1/hal_usb_lld.c: In function 'usb_lld_init_endpoint':
ChibiOS/os/rt/include/chdebug.h:129:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
  129 |   if (CH_DBG_ENABLE_ASSERTS != FALSE) {                                     \
      |      ^
ChibiOS/os/hal/osal/rt/osal.h:241:34: note: in expansion of macro 'chDbgAssert'
  241 | #define osalDbgAssert(c, remark) chDbgAssert(c, remark)
      |                                  ^~~~~~~~~~~
ChibiOS/os/hal/ports/STM32/LLD/USBv1/hal_usb_lld.c:588:5: note: in expansion of macro 'osalDbgAssert'
  588 |     osalDbgAssert(false, "isochronous support disabled");
      |     ^~~~~~~~~~~~~
ChibiOS/os/hal/ports/STM32/LLD/USBv1/hal_usb_lld.c:590:3: note: here
  590 |   case USB_EP_MODE_TYPE_BULK:
      |   ^~~~
Compiling board.c
In file included from ChibiOS/os/common/ext/CMSIS/ST/STM32F0xx/stm32f072xb.h:129,
                 from ChibiOS/os/common/ext/CMSIS/ST/STM32F0xx/stm32f0xx.h:157,
                 from ChibiOS/os/common/startup/ARMCMx/devices/STM32F0xx/cmparams.h:72,
                 from ChibiOS/os/common/ports/ARMCMx/chcore.h:70,
                 from ChibiOS/os/rt/include/ch.h:81,
                 from ChibiOS/os/hal/osal/rt/osal.h:32,
                 from ChibiOS/os/hal/include/hal.h:28,
                 from ./NANOVNA_STM32_F072/board.c:17:
In function '__set_MSP',
    inlined from '__early_init' at ./NANOVNA_STM32_F072/board.c:82:5:
ChibiOS/os/common/ext/CMSIS/include/core_cm0.h:85:28: warning: listing the stack pointer register 'sp' in a clobber list is deprecated [-Wdeprecated]
   85 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler          */
      |                            ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cmFunc.h:470:3: note: in expansion of macro '__ASM'
  470 |   __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : "sp");
      |   ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cm0.h:85:28: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
   85 |   #define __ASM            __asm                                      /*!< asm keyword for GNU Compiler          */
      |                            ^~~~~
ChibiOS/os/common/ext/CMSIS/include/core_cmFunc.h:470:3: note: in expansion of macro '__ASM'
  470 |   __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : "sp");
      |   ^~~~~
Compiling chprintf.c
Compiling memstreams.c
Compiling nullstreams.c
Compiling ff.c
Compiling ffunicode.c
Compiling usbcfg.c
Compiling main.c
Compiling si5351.c
Compiling tlv320aic3204.c
Compiling dsp.c
dsp.c:222:23: warning: argument 1 of type 'float[2]' with mismatched bound [-Warray-parameter=]
  222 | calculate_gamma(float gamma[2])
      |                 ~~~~~~^~~~~~~~
In file included from dsp.c:23:
nanovna.h:291:29: note: previously declared as 'float *'
  291 | void calculate_gamma(float *gamma);
      |                      ~~~~~~~^~~~~
dsp.c:253:23: warning: argument 1 of type 'float[2]' with mismatched bound [-Warray-parameter=]
  253 | fetch_amplitude(float gamma[2])
      |                 ~~~~~~^~~~~~~~
nanovna.h:292:29: note: previously declared as 'float *'
  292 | void fetch_amplitude(float *gamma);
      |                      ~~~~~~~^~~~~
dsp.c:260:27: warning: argument 1 of type 'float[2]' with mismatched bound [-Warray-parameter=]
  260 | fetch_amplitude_ref(float gamma[2])
      |                     ~~~~~~^~~~~~~~
nanovna.h:293:33: note: previously declared as 'float *'
  293 | void fetch_amplitude_ref(float *gamma);
      |                          ~~~~~~~^~~~~
Compiling plot.c
plot.c:903:1: warning: 'cell_drawstring' defined but not used [-Wunused-function]
  903 | cell_drawstring(char *str, int x, int y)
      | ^~~~~~~~~~~~~~~
Compiling ui.c
Compiling ili9341.c
Compiling numfont20x22.c
Compiling Font5x7.c
Compiling Font7x13b.c
Compiling Font10x14.c
Compiling flash.c
Compiling adc.c
Compiling rtc.c
Linking build/ch.elf
Creating build/ch.hex
Creating build/ch.bin
Creating build/ch.dmp

   text    data     bss     dec     hex filename
  79816     528   16048   96392   17888 build/ch.elf
Creating build/ch.list

Done
DiSlord commented 1 year ago

Most of warnings not critical and related to ChibiOS

I use gcc v9.3.1 for compile, this version provide more compact code: text data bss dec hex filename 86668 648 40728 128044 1f42c build/H4.elf

If use v12.2.1 result: text data bss dec hex filename 87216 648 40728 128592 1f650 build/H4.elf

PS I use Eclipse xPack, this allow easy select compiler version PSS upload firmware builded by v12.2.1 on my H4 device, and it works well.

Remove warnings in tis commit https://github.com/DiSlord/NanoVNA-D/commit/ff96b9cc595f43e61d0d73acd0f1844fe3947024

Ho-Ro commented 1 year ago

Thx, I will try, but until tinySA is fixed I'll stay at the debian oldstable version 8.

Ho-Ro commented 1 year ago

I installed v8..v12 in parallel and can select one of these versions for my builds. Starting with version 9 these "ChibiOS" warnings appear, would an update of ChibiOS be required for the newer compiler versions?

Ho-Ro commented 2 months ago

To fix the ChibiOS warnings change this line

  __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) : "sp");

to

  __ASM volatile ("MSR psp, %0\n" : : "r" (topOfProcStack) :);

and this line

  __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) : "sp");

to

  __ASM volatile ("MSR msp, %0\n" : : "r" (topOfMainStack) :);

Reason Starting from gcc v9 it is deprecated that the SP is part of the clobber list, even if the SP is changed after the ASM code is called (because that is the purpose of the command "set SP"). Newer ChibiOS versions just omit the clobber list in both commands.