search

ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.66k stars 2.97k forks source link

Unittests are appending CMAKE_CXX_FLAGS and CMAKE_C_FLAGS over tests #12750

Closed EmbedEngineer closed 2 years ago

EmbedEngineer commented 4 years ago

Description of defect

Hi!

I stumbled upon the following problem: When designing my own unittests I figured out that all the defines set in CMAKE_CXX_FLAGS and CMAKE_C_FLAGS are carried on to my unittest cmake file. So after adding message("${CMAKE_CXX_FLAGS}") to my cmake file the following output was generated:

 -g -O0 --coverage -DUNITTEST -DDEVICE_PWMOUT -DDEVICE_WATCHDOG -DMBED_WDOG_ASSERT=1 -DDEVICE_ANALOGIN -DDEVICE_ANALOGOUT -DDEVICE_CAN -DDEVICE_CRC -DDEVICE_ETHERNET -DDEVICE_FLASH -DDEVICE_I2C -DDEVICE_I2CSLAVE -DDEVICE_I2C_ASYNCH -DDEVICE_INTERRUPTIN -DDEVICE_LPTICKER -DDEVICE_PORTIN -DDEVICE_PORTINOUT -DDEVICE_PORTOUT -DDEVICE_PWMOUT -DDEVICE_QSPI -DDEVICE_SERIAL -DDEVICE_SERIAL_ASYNCH -DDEVICE_SERIAL_FC -DDEVICE_SPI -DDEVICE_SPISLAVE -DDEVICE_SPI_ASYNCH -DDEVICE_FLASH -DCOMPONENT_FLASHIAP -DMBED_CONF_PLATFORM_CTHUNK_COUNT_MAX=10 -DMBED_CONF_DATAFLASH_SPI_FREQ=1 -DMBED_CONF_FLASHIAP_BLOCK_DEVICE_BASE_ADDRESS=0 -DMBED_CONF_FLASHIAP_BLOCK_DEVICE_SIZE=0 -DMBED_CONF_QSPIF_QSPI_FREQ=1 -DMBED_CONF_QSPIF_QSPI_MIN_READ_SIZE=1 -DMBED_CONF_QSPIF_QSPI_MIN_PROG_SIZE=1 -DMBED_LFS_READ_SIZE=64 -DMBED_LFS_PROG_SIZE=64 -DMBED_LFS_BLOCK_SIZE=512 -DMBED_LFS_LOOKAHEAD=512 -DFLASHIAP_APP_ROM_END_ADDR=0x80000 -DMBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_SIZE=1024 -DMBED_CONF_STORAGE_TDB_INTERNAL_INTERNAL_BASE_ADDRESS=0x80000 -DMBED_CONF_STORAGE_STORAGE_TYPE=default -pthread -DEQUEUE_PLATFORM_POSIX -DDEVICE_SERIAL=1 -DDEVICE_INTERRUPTIN=1 -DMBED_CONF_NSAPI_DEFAULT_CELLULAR_APN=NULL -DMBED_CONF_NSAPI_DEFAULT_CELLULAR_USERNAME=NULL -DMBED_CONF_NSAPI_DEFAULT_CELLULAR_PASSWORD=NULL -DMBED_CONF_NSAPI_DEFAULT_CELLULAR_PLMN=NULL -DMBED_CONF_NSAPI_DEFAULT_CELLULAR_SIM_PIN=NULL -DMDMTXD=NC -DMDMRXD=NC -DMBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE=115200 -DMBED_CONF_CELLULAR_USE_SMS=1 -DMBED_CONF_CELLULAR_DEBUG_AT=true -DOS_STACK_SIZE=2048 -DMDMRTS=PTC0 -DMDMCTS=PTC1 -DMDMTXD=NC -DMDMRXD=NC -DMBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE=115200 -DCELLULAR_DEVICE=myCellularDevice -DDEVICE_SERIAL_FC=1 -DMBED_CONF_CELLULAR_CONTROL_PLANE_OPT=0 -DMDMRTS=PTC0 -DMDMCTS=PTC1 -DMDMTXD=NC -DMDMRXD=NC -DMBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE=115200 -DCELLULAR_DEVICE=myCellularDevice -DDEVICE_SERIAL_FC=1 -DMDMRTS=PTC0 -DMDMCTS=PTC1 -DMDMTXD=NC -DMDMRXD=NC -DMBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE=115200 -DCELLULAR_DEVICE=myCellularDevice -DDEVICE_SERIAL_FC=1 -DMBED_CONF_RTOS_PRESENT=1 -DMBED_CONF_LORA_ADR_ON=true -DMBED_CONF_LORA_PUBLIC_NETWORK=true -DMBED_CONF_LORA_NB_TRIALS=2 -DMBED_CONF_LORA_DOWNLINK_PREAMBLE_LENGTH=5 -DMBED_CONF_LORA_DUTY_CYCLE_ON=true -DMBED_CONF_LORA_MAX_SYS_RX_ERROR=10 -DMBED_CONF_LORA_NWKSKEY="{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}" -DMBED_CONF_LORA_APPSKEY="{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}" -DMBED_CONF_LORA_DEVICE_ADDRESS="0x00000000" -DMBED_CONF_LORA_TX_MAX_SIZE=255 -DMBED_CONF_LORA_UPLINK_PREAMBLE_LENGTH=8 -DMBED_CONF_LORA_DUTY_CYCLE_ON_JOIN=true -DMBED_CONF_LORA_WAKEUP_TIME=5 -DMBED_CONF_LORA_FSB_MASK="{0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0x00FF}" -DMBED_CONF_LORA_FSB_MASK_CHINA="{0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0x00FF}" -DMBED_CONF_LORA_FSB_MASK="{0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0x00FF}" -DMBED_CONF_LORA_PHY="EU868" -DMBED_CONF_LORA_OVER_THE_AIR_ACTIVATION=true -DMBED_CONF_LORA_DEVICE_EUI="{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}" -DMBED_CONF_LORA_AUTOMATIC_UPLINK_MESSAGE=true -DMBED_CONF_LORA_APPLICATION_EUI="{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}" -DMBED_CONF_LORA_APPLICATION_KEY="{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}" -DNDEBUG=1 -DMBED_CONF_NSAPI_DNS_RESPONSE_WAIT_TIME=10000 -DMBED_CONF_NSAPI_DNS_RETRIES=1 -DMBED_CONF_NSAPI_DNS_TOTAL_ATTEMPTS=10 -DMBED_CONF_NSAPI_DNS_CACHE_SIZE=5 -DMBED_CONF_FAT_CHAN_FFS_DBG=0 -DMBED_CONF_FAT_CHAN_FF_FS_READONLY=0 -DMBED_CONF_FAT_CHAN_FF_FS_MINIMIZE=0 -DMBED_CONF_FAT_CHAN_FF_USE_STRFUNC=0 -DMBED_CONF_FAT_CHAN_FF_USE_FIND=0 -DMBED_CONF_FAT_CHAN_FF_USE_MKFS=1 -DMBED_CONF_FAT_CHAN_FF_USE_FASTSEEK=0 -DMBED_CONF_FAT_CHAN_FF_USE_EXPAND=0 -DMBED_CONF_FAT_CHAN_FF_USE_CHMOD=0 -DMBED_CONF_FAT_CHAN_FF_USE_LABEL=0 -DMBED_CONF_FAT_CHAN_FF_USE_FORWARD=0 -DMBED_CONF_FAT_CHAN_FF_CODE_PAGE=437 -DMBED_CONF_FAT_CHAN_FF_USE_LFN=3 -DMBED_CONF_FAT_CHAN_FF_MAX_LFN=255 -DMBED_CONF_FAT_CHAN_FF_LFN_UNICODE=0 -DMBED_CONF_FAT_CHAN_FF_LFN_BUF=255 -DMBED_CONF_FAT_CHAN_FF_SFN_BUF=12 -DMBED_CONF_FAT_CHAN_FF_STRF_ENCODE=3 -DMBED_CONF_FAT_CHAN_FF_FS_RPATH=1 -DMBED_CONF_FAT_CHAN_FF_VOLUMES=4 -DMBED_CONF_FAT_CHAN_FF_STR_VOLUME_ID=0 -DMBED_CONF_FAT_CHAN_FF_VOLUME_STRS="RAM","NAND","CF","SD","SD2","USB","USB2","USB3" -DMBED_CONF_FAT_CHAN_FF_MULTI_PARTITION=0 -DMBED_CONF_FAT_CHAN_FF_MIN_SS=512 -DMBED_CONF_FAT_CHAN_FF_MAX_SS=4096 -DMBED_CONF_FAT_CHAN_FF_USE_TRIM=1 -DMBED_CONF_FAT_CHAN_FF_FS_NOFSINFO=0 -DMBED_CONF_FAT_CHAN_FF_FS_TINY=1 -DMBED_CONF_FAT_CHAN_FF_FS_EXFAT=0 -DMBED_CONF_FAT_CHAN_FF_FS_HEAPBUF=1 -DMBED_CONF_FAT_CHAN_FF_FS_NORTC=0 -DMBED_CONF_FAT_CHAN_FF_NORTC_MON=1 -DMBED_CONF_FAT_CHAN_FF_NORTC_MDAY=1 -DMBED_CONF_FAT_CHAN_FF_NORTC_YEAR=2017 -DMBED_CONF_FAT_CHAN_FF_FS_LOCK=0 -DMBED_CONF_FAT_CHAN_FF_FS_REENTRANT=0 -DMBED_CONF_FAT_CHAN_FF_FS_TIMEOUT=1000 -DMBED_CONF_FAT_CHAN_FF_SYNC_t=HANDLE -DMBED_CONF_FAT_CHAN_FLUSH_ON_NEW_CLUSTER=0 -DMBED_CONF_FAT_CHAN_FLUSH_ON_NEW_SECTOR=1 -DNG_NWP_MAX_PACKET_LENGTH=255 -DNG_NWP_MAC_LENGTH=4 -DNG_NWP_PACKET_POOL_SIZE=10 -DNG_NWP_MAX_NUM_NETWORKADDR=5 -DNG_NWP_MAX_NUM_FLUIDS=30 -DNG_NWP_MAX_PACKET_LENGTH=255 -DNG_NWP_MAC_LENGTH=4 -DNG_NWP_PACKET_POOL_SIZE=10 -DNG_NWP_MAX_NUM_NETWORKADDR=5 -DNG_ERRORLOG_PARAM_SIZE=16 -DNG_AISLE_BUS_ADDRESS=2 -DNG_DOOR_BUS_ADDRESS=0x80

This happens altough my cmake file looks like:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNG_NWP_MAX_PACKET_LENGTH=255")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNG_NWP_MAC_LENGTH=4")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNG_NWP_PACKET_POOL_SIZE=10")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNG_NWP_MAX_NUM_NETWORKADDR=5")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNG_ERRORLOG_PARAM_SIZE=16")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNG_AISLE_BUS_ADDRESS=2")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNG_DOOR_BUS_ADDRESS=0x80")

So there should not be the other defines in the list apart from --coverage -DUNITTEST. Trying to delete them also doesn't work. Since I am using the MbedCRC in that specific test the addition of "-DDEVICE_CRC " is breaking the test build. So in my opinion this shouldn't happen.

Target(s) affected by this defect ?

unittests

Toolchain(s) (name and version) displaying this defect ?

make: GNU Make 4.2.1 cmake: 3.13.4

What version of Mbed-os are you using (tag or sha) ?

mbed-os-5.15.1

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed cli: 1.10.2

How is this defect reproduced ?

add message("${CMAKE_CXX_FLAGS}") to any other cmake file or try to to run unittests with -vv flag

0xc0170 commented 4 years ago

cc @ARMmbed/mbed-os-test (or who could help with unittests?)

EmbedEngineer commented 4 years ago

Quote from cmake.org:

AFAIK, CMAKE_CXX_FLAGS et al. are special in as much as the last value they receive within a CMakeLists.txt file is effective for all targets defined in that file, i.e. they undergo a kind of lazy evaluation. So, the answer to your question is: No, you can't specify CMAKE_CXX_FLAGS individually for targets within the same CMakeLists.txt, but you can specify them per directory, i.e. individually in each CMakeLists.txt. Nevertheless, CMAKE_CXX_FLAGS should be considered as project-wide settings, IMO.

So as I see it the whole "set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ...") is just wrong and should be replaced by target_compile_definitions() in all unittest cmake files.

EmbedEngineer commented 4 years ago

After trying to fix that, I figured out that you are relying on that, since some unittests are expecting that all defines from empty_baseline test are present. Well that is in my opinion just a misuse of the CMAKE_CXX_FLAGS and CMAKE_C_FLAGS. The better solution would be to add is permanently as add_compile_definitions()

ciarmcom commented 3 years ago

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers. Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2434

ciarmcom commented 2 years ago

We closed this issue because it has been inactive for quite some time and we believe it to be low priority. If you think that the priority should be higher, then please reopen with your justification for increasing the priority.