A client recently asked me to help them get a legacy HAL driver under test. They want to use TDD to maintain and extend the driver. We ran into a couple of problems, because the tests run on a 64-bit computer, whereas the production code runs on a 32-bit microcontroller. I’ll explain the problems and how to solve them.
Motivation
One of my clients manufactures solid-state drives (SSDs). They implemented a HAL driver for the SSDs. They hired me to bring the HAL driver under test so that they can use TDD to refactor and extend the HAL driver. We used James Grenning’s crash-to-pass algorithm to get their code under test. The tests run on a computer with a 64-bit microprocessor (e.g., Intel, AMD, Apple Silicon), whereas the production code runs on a 32-bit ARM Cortex-M microcontroller.
The microcontroller uses memory-mapped I/O to write to and read from the SSD. The registers and memory of the SSD are mapped into the RAM of the microcontroller at fixed 32-bit addresses. These addresses (e.g., 0x40003C00UL for the control register CR1 of an SPI driver) make sense to the 32-bit microcontroller and the peripheral device (e.g., the SSD). They don’t make any sense on the computer, on which we run the tests. Accessing these 32-bit addresses on our development computer inevitably crashes the test executable. Even if we compiled the tests as a 32-bit executable, the executable would simply crash. See section Accessing Hard-Coded 32-Bit Addresses for a fix.
The compiler spewed out a couple of these warnings
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]They come from converting 64-bit pointers to 32-bit unsigned integers. These conversions cut off the upper 32 bits of 64-bit pointers on our 64-bit development computers. They will eventually lead to crashes. We could avoid these crashes by compiling the tests as 32-bit executables. It’s a quick-and-dirty fix. The splendid fix would be to change the code such that it works both with 32-bit pointers and with 64-bit pointers. See section Converting between 64-Bit Pointers and 32-Bit Unsigned Integers for both fixes.
For confidentiality reasons, I cannot use my client’s HAL driver for SSDs. So, I did the next best thing and searched for an example application exhibiting the same problems. Such examples are not hard to find. We can find them in the firmware packages of nearly every microcontroller family.
I settled for the STM32CubeF2 MCU Firmware Package and the example application in the directory ./Projects/STM322xG_EVAL/Examples/SPI/SPI_FullDuplex_ComDMA. The example shows how to use the HAL driver for full-duplex SPI communication with a peripheral. I copied the example code and the files needed from the firmware package to the repository Add-Ons for Using TDD on Legacy Code. I removed a lot of code from the source and header files to focus on the above problems. The example code might not work properly.
You can follow along by loading the project examples/hal-spi/tests/CMakeLists.txt into QtCreator or any other IDE and by building and running the test case as usual. The starting point is at the commit bed383d5.
Accessing Hard-Coded 32-Bit Addresses
The test case contains a single test function TestHalSpi.init. When we run the test case, it crashes. Running it in the debugger reveals the problematic line (line 290 of file stm32f2xx_hal_spi.c). The two subsequent WRITE_REG commands (lines 295 and 306) would also cause segmentation faults.
HAL_StatusTypeDef HAL_SPI_Init(SPI_HandleTypeDef *hspi)
{
...
__HAL_SPI_DISABLE(hspi); // CLEAR_BIT(hspi->Instance->CR1, SPI_CR1_SPE)
WRITE_REG(hspi->Instance->CR1, ...);
WRITE_REG(hspi->Instance->CR2, ...);The definitions of these three calls resolve to
hspi->Instance == handle.Instance == SPIx == SPI3
== ((SPI_TypeDef *) SPI3_BASE)
== ((SPI_TypeDef *) 0x40003C00UL)
Hence:
((SPI_TypeDef *) (0x40003C00UL))->CR1 &= ~0x0040UL;
((SPI_TypeDef *) (0x40003C00UL))->CR1 = 0x031dUL;
((SPI_TypeDef *) (0x40003C00UL))->CR2 = 0x000UL;SPI_TypeDef is a structure with nine fields of type uint32_t. The fields are the memory-mapped registers for the SPI communication between microcontroller and peripheral. CR1 and CR2, for example, are the two control registers. The memory region from SPI3 = 0x40003C00UL to SPI3 + 9 * sizeof(uint32_t) (excluded) is reserved for SPI. The instance hspi->instance of SPI_TypeDef starts at the 32-bit address SPI3 = 0x40003C00UL.
The address SPI3 doesn’t make sense on the computer running the test. Hence, the test crashes with a segmentation fault. We fix the crash by making the macro SPI3 point to an address on our computer. We double the header Drivers/CMSIS/Device/ST/STM32F2xx/Include/stm32f205xx.h as doubles/stm32f205xx.h. We replace the definition of SPI3 in the header double
#define SPI3 ((SPI_TypeDef *) SPI3_BASE)by the lines
inline static SPI_TypeDef *spi3_base()
{
static SPI_TypeDef spi3 = {0};
return &spi3;
}
#define SPI3 (spi3_base());The test runs without crashing. We can tighten the test a bit and check whether reading the control registers works as well (see the last two checks).
TEST_F(TestHalSpi, init)
{
SPI_HandleTypeDef handle;
handle.Instance = SPIx; // SPIx == SPI3
...
EXPECT_EQ(HAL_SPI_Init(&handle), HAL_OK);
EXPECT_EQ(READ_REG(handle.Instance->CR1), 0x033e);
EXPECT_EQ(READ_REG(handle.Instance->CR2), 0x0000);
}Commit ecc888c1 provides the code changes.
Converting between 64-Bit Pointers and 32-Bit Unsigned Integers
The warnings about dodgy conversion between 32-bit pointers and 32-bit unsigned integers come from the function HAL_DMA_Start_IT, which is called by the function HAL_SPI_TransmitReceive_DMA. I added the test TestHalSpi.transmitReceive (in tst_hal_spi.cpp) that calls HAL_SPI_TransmitReceive_DMA. As the function HAL_DMA_Start_IT isn’t part of the module under test, stm32f2xx_hal_spi.[hc], I provide a fake implementation with the original signature in fakes-spi.c. This fake implementation does nothing. The commit ca7ae867 contains the changes.
Calling HAL_DMA_Start_IT in line 429 of stm32f2xx_hal_spi.c causes the warnings.
HAL_DMA_Start_IT(NULL, (uint32_t)&hspi->Instance->DR,
(uint32_t)hspi->pRxBuffPtr, hspi->RxXferCount)In the original code, &hspi->Instance->DR is the address of the data register DR of the structure SPI_TypeDef, which also contains the control registers CR1 and CR2. Hence, this address is a hard-coded 32-bit address. In our test code, &hspi->Instance->DR is a 64-bit address. The cast converts a 64-bit address into 32-bit unsigned integer and cuts off the upper half of the 64-bit address. That’s what the warning tells us.
stm32f2xx_hal_spi.c:429:40: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]The compiler emits the same warning for the 3rd argument (uint32_t)hspi->pRxBuffPtr of the function call – for the same reason. The source address (2nd argument) and the destination address (3rd) are converted into uint32_t values, because the HAL_DMA stores the values in the address registers PAR and M0AR in the memory-mapped structure DMA_Stream_TypeDef (in doubles/stm32f205xx.h).
typedef struct
{
__IO uint32_t CR; /*!< DMA stream x configuration register */
__IO uint32_t NDTR; /*!< DMA stream x number of data register */
__IO uint32_t PAR; /*!< DMA stream x peripheral address register */
__IO uint32_t M0AR; /*!< DMA stream x memory 0 address register */
...
} DMA_Stream_TypeDef;Defining a “Universal” Address Value Type
In the original file stm32f205xx.h, we define the type address_uint_t before the definitions of the peripheral registers structures, which include DMA_Stream_TypeDef.
#define address_uint_t uint32_tIn the double stm32f205xx.h, we define the type address_uint_t as
#define address_uint_t uint64_tIn both header files, we change the types of address registers to address_uint_t. For example:
typedef struct
{
__IO uint32_t CR; /*!< DMA stream x configuration register */
__IO uint32_t NDTR; /*!< DMA stream x number of data register */
__IO address_uint_t PAR; /*!< DMA stream x peripheral address register */
__IO address_uint_t M0AR; /*!< DMA stream x memory 0 address register */
...
} DMA_Stream_TypeDef;The compiler warnings tell us where to change the type in client code. In our example, we change the calls to HAL_DMA_Start_IT similar to this:
HAL_DMA_Start_IT(NULL, (address_uint_t)&hspi->Instance->DR,
(address_uint_t)hspi->pRxBuffPtr, hspi->RxXferCount)Commit 4bab3516 provides these changes. My client would only have to do the changes for the HAL SSD driver and the peripheral registers structures used by this driver. When they write the tests for their driver, the compiler will diligently warn about conversions from 64-bit pointers to 32-bit unsigned integers. Companies like my client may do these changes anyway, because they also want to run their drivers on a 64-bit SoC.
For the complete STM32CubeF2 firmware, we would have to replace hundreds of such conversions. That’s not feasible for the clients of the SoC makers. As microcontrollers will move from 32 to 64 bits – in the same inevitable way they moved from 16 to 32 bits some years ago, the SoC makers should make their firmware work on 32-bit and 64-bit architectures.
Void Pointers
The function HAL_DMA_Start_IT puts the onus of the pointer-to-value conversion on its clients. Instead of forcing many clients to do the conversion many times, the function does the conversion once in its implementation. We change the signature of HAL_DMA_Start_IT (in stm32f2xx_hal_dma.h) from
HAL_StatusTypeDef HAL_DMA_Start_IT(DMA_HandleTypeDef *hdma, uint32_t SrcAddress,
uint32_t DstAddress, uint32_t DataLength);to
HAL_StatusTypeDef HAL_DMA_Start_IT(DMA_HandleTypeDef *hdma, volatile void *SrcPointer,
volatile void *DstPointer, uint32_t DataLength);The volatile qualifier is required, because registers like DR are typed as volatile uint32_t. We can cast a non-volatile pointer to a volatile pointer but not vice versa. The implementation of HAL_DMA_Start_IT converts the pointers into unsigned integers and continues with the old implementation.
HAL_StatusTypeDef HAL_DMA_Start_IT(DMA_HandleTypeDef *hdma, volatile void *SrcPointer,
volatile void *DstPointer, uint32_t DataLength)
{
address_uint_t SrcAddress = (address_uint_t)SrcPointer;
address_uint_t DstAddress = (address_uint_t)DstPointer;
// Old implementation ...
return HAL_OK;
}The compiler diligently warns about all the places where HAL_DMA_Start_IT is called.
warning: passing argument 2 of ‘HAL_DMA_Start_IT’ makes pointer from integer without a cast [-Wint-conversion]
warning: passing argument 3 of ‘HAL_DMA_Start_IT’ makes pointer from integer without a cast [-Wint-conversion]We remove the casts and just pass the pointers:
HAL_DMA_Start_IT(NULL, &hspi->Instance->DR, hspi->pRxBuffPtr, hspi->RxXferCount))All warnings are gone and the tests pass. Commit f209ec76 contains the changes. The two methods Universal Address Value Types and Void Pointers are not alternatives but complements.
Compiling Tests into 32-Bit Executables
Producing 32-bit test executables on our 64-bit development computers is the easy way out. It glosses over the bad code, which only works on microcontrollers with a 32-bit architecture. Short-term, if we must catch a deadline, for example, this approach is OK. Midterm, it is not. Eventually we’ll have to face the music and make our code work both on 32-bit and 64-bit architectures.
I created the branch hal-driver-32 for 32-bit compilation of the test. I also undid the changes from the sections Defining a “Universal” Address Value Type and Void Pointers. I kept the changes from the section Accessing Hard-Coded 32-Bit Addresses, because the hard-coded addresses from the micocontroller don’t make sense on an Intel processor even if compiled for 32 bits. Commit 2937dc1f is the starting point.
We know that the 32-bit compilation is working, once the compiler stops issuing the warnings of this type:
stm32f2xx_hal_spi.c:429:40: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]Multiarch support allows us to install packages of different architectures (e.g., i386 and armhf) on a computer with x86_64 architecture. We enable multiarch support for the i386 architecture, update the package list and install the most important 32-bit packages with the following commands.
$ sudo dpkg --add-architecture i386
$ sudo apt-get update
$ sudo apt-get install libc6-dev-i386 lib32gcc-s1 lib32gcc-9-dev \
lib32stdc++6 lib32stdc++-9-devI am not sure whether the above commands are enough. Please let me know if not.
We create a CMake toolchain file for 32-bit compilation (see section 22.6.2 GCC With 32-bit Target on 64-bit Host from the book Professional CMake: A Practical Guide by Craig Scott).
// File: examples/hal-spi/tests/Gcc32BitToolchain.cmake
set(CMAKE_SYSTEM_NAME Linux)
set(CMAKE_SYSTEM_PROCESSOR i686)
set(CMAKE_C_COMPILER gcc)
set(CMAKE_CXX_COMPILER g++)
set(CMAKE_C_FLAGS_INIT -m32)
set(CMAKE_CXX_FLAGS_INIT -m32)
set(CMAKE_EXE_LINKER_FLAGS_INIT -m32)
set(CMAKE_SHARED_LINKER_FLAGS_INIT -m32)
set(CMAKE_MODULE_LINKER_FLAGS_INIT -m32)We add the option
-DCMAKE_TOOLCHAIN_FILE=/public/Projects/tdd-training-add-ons/examples/hal-spi/tests/Gcc32BitToolchain.cmaketo the command-line call to cmake. Alternatively, we add the toolchain file to QtCreator.
Then, we run CMake and perform a rebuild. The compiler warnings about casting a 64-bit pointer to a 32-bit unsigned integer are gone. Note that QtCreator’s syntax checker still shows them, because it still thinks that we are targeting a 64-bit architecture. The test creates a valid address for SPI3 and runs without crashing. Commit 5c7b97c4 on branch hal-driver-32 contains the changes for 32-bit compilation.