cortexm_common/ldscript: simplify generation of firmwares in section of the ROM

cpu/cortexm_common/Makefile.include

@@ -2,13 +2,24 @@
 INCLUDES += -I$(RIOTCPU)/cortexm_common/include
 INCLUDES += -I$(RIOTCPU)/cortexm_common/include/vendor
 
-ifneq (,$(ROM_OFFSET))
-  LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_rom_offset=$(ROM_OFFSET)
-endif
-
+# All variables must be defined in the CPU configuration when using the common
+# `ldscripts/cortexm.ld`
 ifneq (,$(ROM_START_ADDR)$(RAM_START_ADDR)$(ROM_LEN)$(RAM_LEN))
+  $(if $(ROM_START_ADDR),,$(error ROM_START_ADDR is not defined))
+  $(if $(RAM_START_ADDR),,$(error RAM_START_ADDR is not defined))
+  $(if $(ROM_LEN),,$(error ROM_LEN is not defined))
+  $(if $(RAM_LEN),,$(error RAM_LEN is not defined))
   LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_rom_start_addr=$(ROM_START_ADDR)
   LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_ram_start_addr=$(RAM_START_ADDR)
   LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_rom_length=$(ROM_LEN)
   LINKFLAGS += $(LINKFLAGPREFIX)--defsym=_ram_length=$(RAM_LEN)
 endif
+
+
+# Only define the linker symbol if the variable is set
+# The variable can be set using target specific variable thanks to lazy evaluation
+
+# ROM_OFFSET: offset in rom to start linking, allows supporting a bootloader
+LINKFLAGS += $(if $(ROM_OFFSET),$(LINKFLAGPREFIX)--defsym=_rom_offset=$(ROM_OFFSET))
+# FW_ROM_LEN: rom length to use for firmware linking. Allows linking only in a section of the rom.
+LINKFLAGS += $(if $(FW_ROM_LEN),$(LINKFLAGPREFIX)--defsym=_fw_rom_length=$(FW_ROM_LEN))

cpu/cortexm_common/ldscripts/cortexm.ld

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2017 Inria
+ *               2018 Freie Universität Berlin
  *
  * This file is subject to the terms and conditions of the GNU Lesser
  * General Public License v2.1. See the file LICENSE in the top level
@@ -14,16 +15,20 @@
  * @brief           Memory definitions for the Cortex-M family
  *
  * @author          Francisco Acosta <[email protected]>
+ *                  Gaëtan Harter <[email protected]>
  *
  * @}
  */
 
-_boot_offset = DEFINED( _rom_offset ) ? _rom_offset : 0x0 ;
+_rom_offset = DEFINED( _rom_offset ) ? _rom_offset : 0x0;
+_fw_rom_length = DEFINED( _fw_rom_length ) ? _fw_rom_length : _rom_length - _rom_offset;
+
+ASSERT((_fw_rom_length <= _rom_length - _rom_offset), "Specified firmware size does not fit in ROM");
 
 MEMORY
 {
-    rom (rx)    : ORIGIN = _rom_start_addr + _boot_offset, LENGTH = _rom_length - _boot_offset
-    ram (w!rx)  : ORIGIN = _ram_start_addr,                LENGTH = _ram_length
+    rom (rx)    : ORIGIN = _rom_start_addr + _rom_offset, LENGTH = _fw_rom_length
+    ram (w!rx)  : ORIGIN = _ram_start_addr,               LENGTH = _ram_length
 }
 
 INCLUDE cortexm_base.ld

tests/cortexm_common_ldscript/Makefile

@@ -0,0 +1,144 @@
+BOARD ?= samr21-xpro
+include ../Makefile.tests_common
+
+
+# Normally all boards using `cortexm_common/ldscripts/cortexm.ld` linkerscript
+# Only tested on these ones for the moment
+BOARD_WHITELIST += iotlab-a8-m3
+BOARD_WHITELIST += iotlab-m3
+BOARD_WHITELIST += samr21-xpro
+# Boards using a bootloader and ROM_OFFSET by default
+BOARD_WHITELIST += arduino-mkr1000
+BOARD_WHITELIST += arduino-mkrfox1200
+BOARD_WHITELIST += arduino-mkrzero
+BOARD_WHITELIST += bluepill
+BOARD_WHITELIST += feather-m0
+BOARD_WHITELIST += opencm904
+BOARD_WHITELIST += spark-core
+BOARD_WHITELIST += stm32mindev
+
+include $(RIOTBASE)/Makefile.include
+
+
+# # # # # # # # # # # # # # # # # # # # # # # # # # # #
+# Compile time tests for ROM_OFFSET and FW_ROM_LENGTH #
+# # # # # # # # # # # # # # # # # # # # # # # # # # # #
+
+COMPILE_TESTS  = test-elffile-overflow test-elffile-fw_rom_length
+COMPILE_TESTS += tests-offsets tests-fw_rom_len tests-rom-overflow
+
+all: compile-tests
+
+compile-tests: $(COMPILE_TESTS)
+.PHONY: compile-tests $(COMPILE_TESTS)
+
+
+# iotlab-m3 defines ROM_LEN as 512K which is not handled by bash math operations
+ROM_LEN_BYTES = $(shell printf "0x%x\n" $$(($(ROM_LEN:%K=%*1024))))
+
+
+# test-elffile-overflow depends on $(BINFILE) to prevent:
+# * ROM_LEN to be passed to $(ELFFILE) generation
+# * dummy error message of wc not finding the .bin file (ELFFILE is not enough)
+test-elffile-overflow: $(BINFILE) _test-elffile-overflow-runtest
+
+.PHONY: _test-elffile-overflow-runtest
+_test-elffile-overflow-runtest: ROM_LEN=$(firstword $(shell wc -c $(BINFILE)))-1+$(if $(ROM_OFFSET),$(ROM_OFFSET),0)
+_test-elffile-overflow-runtest: $(BINFILE)
+	$(Q)echo -n "Test rom offset 1 byte overflow detection: "
+	$(Q)\
+	{ $(_LINK) -o /dev/null  2>&1 | grep -q "region \`rom' overflowed by 1 byte" ; } \
+	  && echo [OK] || { echo [ERROR] Compilation should have failed >&2; exit 1; }
+
+
+# Test `ROM_OFFSET` is removed from firmware rom length if the board defines it
+test-elffile-fw_rom_length: $(ELFFILE)
+	$(Q)echo -n "Test rom offset substracted from rom length in elffile: "
+	$(Q)\
+	if test -n "$(ROM_OFFSET)"; then \
+	  TEST_FW_LEN=$$($(PREFIX)readelf --symbols $^ 2>/dev/null | awk '/_fw_rom_length/{printf "0x%s\n", $$2}'); \
+	  EXPECT_FW_LEN=$$(printf "0x%08x" $$(( $(ROM_LEN_BYTES) - $(ROM_OFFSET) ))); \
+	  if test $${TEST_FW_LEN} != $${EXPECT_FW_LEN}; then \
+	    echo "[ERROR] Rom offset not taken into account for firmware length $${TEST_FW_LEN} != $${EXPECT_FW_LEN}" >&2; \
+	    exit 1;\
+	  fi ;\
+	  echo [OK] ; \
+	else \
+	  echo "[SKIP](Reason: board does not have a ROM_OFFSET configured)" ;\
+	fi
+
+
+# Test elffiles must not have $(ELFFILE) prerequisite as target specific
+# variables are used for configuration and they also apply to prerequisites.
+#
+# https://www.gnu.org/software/make/manual/make.html#Target_002dspecific
+
+ELFFILES_DEPS = $(BASELIBS) FORCE
+
+
+# Compile elf files with different ROM_OFFSET
+# and verify the offset is taken into account
+
+OFFSETS_TESTS = 0x1000 0x2000
+tests-offsets: $(OFFSETS_TESTS:%=test-offset_%)
+
+.PHONY: test-offset_%
+test-offset_%: $(BINDIR)/$(APPLICATION)_offset_%.elf
+	$(Q)echo -n "Test compilation with offset $*: "
+	$(Q)\
+	TEST_START_ADDR=$$($(PREFIX)readelf --section-headers $^ 2>/dev/null | awk '/.text/{printf "0x%s\n", $$5}'); \
+	EXPECT_START_ADDR=$$(printf "0x%08x" $$(( $(ROM_START_ADDR) + $* ))); \
+	if test $${TEST_START_ADDR} != $${EXPECT_START_ADDR}; then \
+	  echo "[ERROR] Linker offset not used $${TEST_START_ADDR} != $${EXPECT_START_ADDR}" >&2; \
+	  exit 1;\
+	fi
+	$(Q)echo [OK]
+
+$(BINDIR)/$(APPLICATION)_offset_%.elf: ROM_OFFSET=$*
+$(BINDIR)/$(APPLICATION)_offset_%.elf: $(ELFFILES_DEPS)
+	$(Q)$(_LINK) -o $@
+.PRECIOUS: $(BINDIR)/$(APPLICATION)_offset_%.elf
+
+
+# Compile elf files with FW_ROM_LEN and verify the length is taken into account
+#   I arbitrarily do 'half' size because I needed to take a value and
+#   that it is similar to what is required for doing dual firmware in rom ota
+
+tests-fw_rom_len: test-fw_len_half_rom
+
+.PHONY: test-fw_len_half_rom
+test-fw_len_half_rom: $(BINDIR)/$(APPLICATION)_fw_len_half_rom.elf
+	$(Q)echo -n "Test compilation with half ROM length: "
+	$(Q)\
+	TEST_FW_LEN=$$($(PREFIX)readelf --symbols $^ 2>/dev/null | awk '/_fw_rom_length/{printf "0x%s\n", $$2}'); \
+	EXPECT_FW_LEN=$$(printf "0x%08x" $$(( $(ROM_LEN_BYTES) / 2 ))); \
+	if test $${TEST_FW_LEN} != $${EXPECT_FW_LEN}; then \
+	  echo "[ERROR] Linker firmware length not used $${TEST_FW_LEN} != $${EXPECT_FW_LEN}" >&2; \
+	  exit 1;\
+	fi
+	$(Q)echo [OK]
+$(BINDIR)/$(APPLICATION)_fw_len_half_rom.elf: FW_ROM_LEN=$$(($(ROM_LEN_BYTES)/2))
+$(BINDIR)/$(APPLICATION)_fw_len_half_rom.elf: $(ELFFILES_DEPS)
+	$(Q)$(_LINK) -o $@
+.PRECIOUS: $(BINDIR)/$(APPLICATION)_fw_len_half_rom.elf
+
+
+# Test FW_ROM_LEN overflow detection
+
+OVERFLOW_TESTS = too_big_for_rom offset_and_romlen
+tests-rom-overflow: $(OVERFLOW_TESTS:%=test-assert_overflow_%)
+
+
+# Simple FW_ROM_LEN overflow
+test-assert_overflow_too_big_for_rom: FW_ROM_LEN=$$(($(ROM_LEN_BYTES) + 1))
+
+# ROM_OFFSET and FW_ROM_LEN set ROM_LEN
+test-assert_overflow_offset_and_romlen: ROM_OFFSET=0x1000
+test-assert_overflow_offset_and_romlen: FW_ROM_LEN=$(ROM_LEN_BYTES)
+
+.PHONY: test-assert_overflow_%
+test-assert_overflow_%: $(ELFFILES_DEPS)
+	$(Q) echo -n "Test ROM overflow detection ($*): "
+	$(Q)\
+	{ $(_LINK) -o /dev/null 2>&1 | grep -q 'Specified firmware size does not fit in ROM' ; } \
+	  && echo [OK] || { echo [ERROR] Compilation should have failed >&2; exit 1; }

tests/cortexm_common_ldscript/README.md

@@ -0,0 +1,45 @@
+Cortexm-common ldscript
+=======================
+
+This test checks the support for building firmwares with a rom offset and
+specific sized firmwares using `cortexm-common` ldscript.
+
+When building `all`, the `compile-tests` target is also build and executes build
+time compilation tests checking different firmwares configurations verified by
+inspecting the result elf files.
+
+
+Test output
+-----------
+
+For a board that does not have a `ROM_OFFSET` variable configured by default
+(== a board without bootloader installed), it produces the following output
+after the normal compilation:
+
+```
+make all BOARD=iotlab-m3
+...
+Test rom offset 1 byte overflow detection: [OK]
+Test rom offset substracted from rom length in elffile: [SKIP](Reason: board does not have a ROM_OFFSET configured)
+Test compilation with offset 0x1000: [OK]
+Test compilation with offset 0x2000: [OK]
+Test compilation with half ROM length: [OK]
+Test ROM overflow detection (too_big_for_rom): [OK]
+Test ROM overflow detection (offset_and_romlen): [OK]
+```
+
+For a bord that have a `ROM_OFFSET` variable configured by default (== a board
+with bootloader installed), it produces the following output after the normal
+compilation:
+
+```
+make BOARD=bluepill  PROGRAMMER=dfu-util
+...
+Test rom offset 1 byte overflow detection: [OK]
+Test rom offset substracted from rom length in elffile: [OK]
+Test compilation with offset 0x1000: [OK]
+Test compilation with offset 0x2000: [OK]
+Test compilation with half ROM length: [OK]
+Test ROM overflow detection (too_big_for_rom): [OK]
+Test ROM overflow detection (offset_and_romlen): [OK]
+```

tests/cortexm_common_ldscript/main.c

@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2018 Freie Universität Berlin
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser General
+ * Public License v2.1. See the file LICENSE in the top level directory for more
+ * details.
+ */
+
+/**
+ * @ingroup     tests
+ * @{
+ *
+ * @file
+ * @brief       Empty main file
+ *
+ * @author      Gaëtan Harter <[email protected]>
+ *
+ * @}
+ */
+
+int main(void)
+{
+    /* The important rules are in the Makefile */
+    return 0;
+}