search

erikkaashoek / tinySA

tiny Spectrum Analyzer
225 stars 50 forks source link

Buggy firmware when compiling with GCC-ARM version 12 #70

Open Ho-Ro opened 1 year ago

Ho-Ro commented 1 year ago

Compiling the 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: buggy_version error_4 error_all

This is what I see with the input terminated with 50 Ohm and with the 30 MHz calibration signal: error_in_none error_in_30MHz

During compile it gives a lot of warnings:

make 
Makefile:305: warning: overriding recipe for target 'clean'
ChibiOS/os/common/startup/ARMCMx/compilers/GCC/rules.mk:301: warning: ignoring old recipe for target 'clean'
Compiling crt0_v6m.s
Compiling chcoreasm_v6m.s
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 chregistry.c
Compiling chcore.c
Compiling chcore_v6m.c
In file included from ChibiOS/os/common/ext/CMSIS/ST/STM32F0xx/stm32f072xb.h:132,
                 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_adc.c
Compiling hal_ext.c
Compiling hal_gpt.c
Compiling hal_pal.c
Compiling hal_pwm.c
Compiling hal_serial.c
Compiling hal_serial_usb.c
Compiling hal_usb.c
Compiling nvic.c
Compiling hal_lld.c
Compiling stm32_dma.c
Compiling hal_st_lld.c
Compiling hal_adc_lld.c
Compiling hal_ext_lld.c
Compiling hal_ext_lld_isr.c
Compiling hal_pal_lld.c
Compiling hal_gpt_lld.c
Compiling hal_pwm_lld.c
Compiling hal_serial_lld.c
Compiling hal_usb_lld.c
Compiling board.c
In file included from ChibiOS/os/common/ext/CMSIS/ST/STM32F0xx/stm32f072xb.h:132,
                 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:101: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 usbcfg.c
Compiling adc.c
Compiling main.c
In file included from main.c:1132:
sa_core.c: In function 'interpolate_maximum':
sa_core.c:2855:35: warning: using floating-point absolute value function 'fabs' when argument is of integer type 'int32_t' {aka 'long int'} [-Wabsolute-value]
 2855 |     const INTER_TYPE d          = fabs(delta_Hz) * 0.5 * (y1 - y3) / ((y1 - (2 * y2) + y3) + 1e-12);
      |                                   ^~~~
Compiling plot.c
Compiling ui.c
ui.c:1539:26: warning: array 'menu_settings' assumed to have one element
 1539 | static const menuitem_t  menu_settings[];
      |                          ^~~~~~~~~~~~~
ui.c:1541:26: warning: array 'menu_lowoutput_settings' assumed to have one element
 1541 | static const menuitem_t  menu_lowoutput_settings[];
      |                          ^~~~~~~~~~~~~~~~~~~~~~~
ui.c:1541:26: warning: 'menu_lowoutput_settings' defined but not used [-Wunused-const-variable=]
ui.c:1539:26: warning: 'menu_settings' defined but not used [-Wunused-const-variable=]
 1539 | static const menuitem_t  menu_settings[];
      |                          ^~~~~~~~~~~~~
ui.c:1160:24: warning: 'keypads_plusmin_freq' defined but not used [-Wunused-const-variable=]
 1160 | static const keypads_t keypads_plusmin_freq[] = {
      |                        ^~~~~~~~~~~~~~~~~~~~
Compiling ili9341.c
Compiling numfont20x22.c
Compiling Font5x7.c
Compiling Font10x14.c
Compiling flash.c
Compiling si4432.c
Compiling Font7x13b.c
Linking build/tinySA.elf
Memory region         Used Size  Region Size  %age Used
          flash0:      111700 B       116 KB     94.04%
          flash1:          0 GB         0 GB
          flash2:          0 GB         0 GB
          flash3:          0 GB         0 GB
          flash4:          0 GB         0 GB
          flash5:          0 GB         0 GB
          flash6:          0 GB         0 GB
          flash7:          0 GB        12 KB      0.00%
            ram0:         16 KB        16 KB    100.00%
            ram1:          0 GB         0 GB
            ram2:          0 GB         0 GB
            ram3:          0 GB         0 GB
            ram4:          0 GB         0 GB
            ram5:          0 GB         0 GB
            ram6:          0 GB         0 GB
            ram7:          0 GB         0 GB
Creating build/tinySA.hex
Creating build/tinySA.bin
Creating build/tinySA.dmp

   text    data     bss     dec     hex filename
 110852     848   15728  127428   1f1c4 build/tinySA.elf
Creating build/tinySA.list

Done

(I see comparable warnings when compiling the NanoVNA firmware.)

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 with only one warning (I must again remove the comments in nanovna.h to compile the tinySA3 version):

...
Compiling ui.c
ui.c:1160:24: warning: 'keypads_plusmin_freq' defined but not used [-Wunused-const-variable=]
 static const keypads_t keypads_plusmin_freq[] = {
...

ok_version selftest_ok

This is what I see with the input terminated with 50 Ohm and with the 30 MHz calibration signal: ok_in_none ok_in_30MHz

DiSlord commented 1 year ago

I think this related to gcc speed optimisation, TinySA code critical to execute timings and can depend from how compiler optimise code Most of warnings related to ChibiOS low level code

Ho-Ro commented 1 year ago

Erik uses gcc version 7.2.1 (see below), I have gcc version 8.3.1. Is version 9 the latest "good" version for tinySA? I see that 12.x works well for NanoVNA.

Version: tinySA_v1.4-96-gc1fcd94
Build Time: May 11 2023 - 08:41:39
Kernel: 4.0.0
Compiler: GCC 7.2.1 20170904 (release) [ARM/embedded-7-branch revision 255204]
DiSlord commented 1 year ago

Difficult to answer, as I test code generation for NanoVNA, gcc last v9 provide more compact code. After code size bigger but not chage size, up to v12

Also as I see v7 (code build by Eric) more bigger then my builded in v9. And need test how gcc v9 code work on Tiny (several wait delays hardcoded and depend from how fast code)