10

I posted the same question in the STM32 community forum as well, but didn't receive an answer.

I am using stm32 HAL library in a project with C++14 enabled. It issues me the following warning which I can't get rid of.

../platform/stm32/l4/STM32L4xx_HAL_Driver/Inc/stm32l4xx_hal_rcc.h:735:57:

warning: conversion to void will not access object of type 'volatile uint32_t {aka volatile long unsigned int}' UNUSED(tmpreg); \

This happens, when a call to __GPIOX_CLK_ENABLE() or __HAL_RCC_GPIOX_CLK_ENABLE is called.

Has anyone been able to get rid of the above warning leaving the HAL source code intact.

Or any ideas as what is possible to be done.

The current warning level is -Wall.

I've experienced the above issue with both l4 & f4 series code.

An Example code:

int main(void)
{
    HAL_Init();

    __GPIOB_CLK_ENABLE();
    GPIO_InitTypeDef GPIO_InitStructure;

    GPIO_InitStructure.Pin = GPIO_PIN_7;

    GPIO_InitStructure.Mode = GPIO_MODE_OUTPUT_PP;
    GPIO_InitStructure.Speed = GPIO_SPEED_HIGH;
    GPIO_InitStructure.Pull = GPIO_NOPULL;
    HAL_GPIO_Init(GPIOB, &GPIO_InitStructure);

    for (;;)
    {
        HAL_GPIO_WritePin(GPIOB, GPIO_PIN_7, GPIO_PIN_SET);
        HAL_Delay(500);
        HAL_GPIO_WritePin(GPIOB, GPIO_PIN_7, GPIO_PIN_RESET);
        HAL_Delay(500);
    }
}

The culprit is __GPIOB_CLK_ENABLE(), which gets expanded to the following(in ST drivers).

#define __HAL_RCC_GPIOB_CLK_ENABLE()           do { \
                                                 __IO uint32_t tmpreg; \
                                                 SET_BIT(RCC->AHB2ENR, RCC_AHB2ENR_GPIOBEN); \
                                                 /* Delay after an RCC peripheral clock enabling */ \
                                                 tmpreg = READ_BIT(RCC->AHB2ENR, RCC_AHB2ENR_GPIOBEN); \
                                                 UNUSED(tmpreg); \
                                               } while(0)

My original question is intended to find out a solution, leaving the underlying ST driver intact. One possible solution would be to use the direct register access without going through the library provided convenient macro.

Thank you in advance.

aep
  • 1,583
  • 10
  • 18
  • I did some research and the reason for this warning is that the `UNUSED` macro, which is part of the mentioned macros, cast a volatile reference to `void`. It is not related to C++14 nor -Wall, but all g++ versions give the same diagnostic. The reason why can be found in the linked duplicate. The solution would be not to use volatile references, which is fishy practice when writing hardware-related code - use volatile pointers instead. Perhaps you are using a reference by accident? – Lundin Mar 02 '18 at 10:06
  • The warning is not issued in C++11. I can successfully compile the same code with C++11 without getting any warning with `-Wall`. It is definitely not `all g++` compiler versions. That is the reason behind this question. – aep Mar 02 '18 at 10:12
  • It is definitely not a `duplicate`. I would urge you to download the STM32 CubeMX HAL source code and compile it both in C++11 & C++14. The warning becomes evident in C++14 but never in C++11. – aep Mar 02 '18 at 10:15
  • I was able to reproduce it down to C++03 by simply casting any volatile reference to void. So this has nothing to do with the compiler version. There must be something in your caller code that behaves differently in C++14. Please edit your question with a [MCVE](https://stackoverflow.com/help/mcve) that contains the caller code giving the warning. – Lundin Mar 02 '18 at 11:48
  • I'll re-open the question for now, but I don't believe it can be answered without an example. It may very well be that the problem lies in the ST drivers, though as I understand it these are written in pure C? Where does the reference come from? – Lundin Mar 02 '18 at 11:51
  • @Lundin I edited the question and provided an example as well. It's clear that the issue stems from ST drivers. But I would like to know a way to suppress the warning leaving the ST code intact. – aep Mar 02 '18 at 12:45
  • @Lundin noone mentioned a reference but you, and the question you've linked. **It's a different warning.** – followed Monica to Codidact Mar 02 '18 at 13:06
  • @berendi Fair enough, I was wrong. But the question is much better now with the edit. Still, as far as I can tell there is nothing in C++14 specifically which would change the behavior of the code. Whether `(void)voltatile_expression;` should yield an access or not has been a debated topic in both C and C++ for quite some time. – Lundin Mar 02 '18 at 15:10
  • It is btw obvious that the purpose of the UNUSED macro is to silence the incorrect gcc warning "x has been assigned a value but is never used", which doesn't make any sense for volatile variables. In this case the whole purpose of the variable is to trigger a read - nobody cares about the result. This gcc defect is present in all versions of the compiler. So I wouldn't be so eager to point the finger at ST here. – Lundin Mar 02 '18 at 15:17
  • Now what ST should have done is to get rid of the UNUSED macro and instead use `#pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-but-set-variable"` ... `#pragma GCC diagnostic pop`, since the problem is related to the GCC compiler specifically. – Lundin Mar 02 '18 at 15:25

3 Answers3

10

The problem is -std=c++14 changing the semantics of a volatile expression cast to (void), and introducing an apparently* unconditional warning for it, and a coder at ST trying to make "triple sure" that a register read would take place.

The definition of the UNUSED() macro is

#define UNUSED(x) ((void)(x))

and __IO is defined as

#define     __IO    volatile

Then the expansion of __HAL_RCC_GPIOB_CLK_ENABLE() would be

do {
    volatile uint32_t tmpreg;
    RCC->AHB2ENR |= RCC_AHB2ENR_GPIOBEN;
    /* Delay after an RCC peripheral clock enabling */
    tmpreg = RCC->AHB2ENR & RCC_AHB2ENR_GPIOBEN;
    ((void)(tmpreg));
} while(0)

The delay and read-back of the register is recommended by various STM32 errata saying

A delay between an RCC peripheral clock enable and the effective peripheral enabling should be taken into account in order to manage the peripheral read/write to registers.

[...]

insert a dummy read operation from the corresponding register just after enabling the peripheral clock.

As all peripheral registers are of course declared as volatile, a simple expression containing just the register in question would force a readback with the necessary wait states via the same peripheral bus, so this would suffice:

do {
    RCC->AHB2ENR |= RCC_AHB2ENR_GPIOBEN;
    /* Delay after an RCC peripheral clock enabling */
    RCC->AHB2ENR;
} while(0)

the rest is presumably an overengineered workaround for some buggy compilers, but I'm yet to see one so broken that an expression with a volatile type would be optimized out.

There is that edge case however, with a volatile variable cast to (void), where the semantics have apparently changed in C++14.

Let's take this simple example

void x() {
    volatile int t;
    t=1;
    ((void)(t));
}

Arm gcc 7.2.1 invoked with -O3 -mcpu=cortex-m4 -mthumb -Wall -x c++ -std=c++11 would compile it to

x():
  sub sp, sp, #8
  movs r3, #1
  str r3, [sp, #4]
  ldr r3, [sp, #4]
  add sp, sp, #8
  bx lr

and the same code compiled with -std=c++14 is

x():
  sub sp, sp, #8
  movs r3, #1
  str r3, [sp, #4]
  add sp, sp, #8
  bx lr

... and a warning:

<source>: In function 'void x()':
<source>:5:13: warning: conversion to void will not access object of type 'volatile int'
     ((void)(t));
            ~^~

Also notice the missing ldr instruction in the second case. The variable is not accessed after the write with C++14.

My original question is intended to find out a solution, leaving the underlying ST driver intact. One possible solution would be to use the direct register access without going through the library provided convenient macro.

I'd suggest go ahead and avoid the library, IMHO HAL is better treated as a collection of examples or implementation suggestions.

*I couldn't find a way to disable it. That doesn't mean there is none.

followed Monica to Codidact
  • 7,779
  • 1
  • 23
  • 39
  • 1
    Good work! Surely however the (pragmatic) way to disable it is exactly `-std=c++11`? The OP has to ask whether he needs C++14 more than the HAL and warning free builds. I imagine that if you are inclined to use the HAL, it is of far greater utility to you than C++14 features. I agree that the HAL itself has a lot to criticise, but if you are using it in order to utilise the STM32Cube ecosystem of higher-level middleware that depends upon it (USB stack, filesystem, RTOS etc.), then it is the path of least resistance for many. – Clifford Mar 02 '18 at 17:09
  • Thank you @Clifford for the detailed explanation. I suppose, at this stage I would use register access to enable the clock as you've mentioned and still keep using STM32 HAL for the rest of the work. Thanks. – aep Mar 02 '18 at 22:07
  • @aep : I think you have addressed your comment to me in error - this was not my answer. If you found one issue with the HAL's C++14 comparability, I am willing to bet you will encounter it again; using some kind of hybrid approach is unlikely to avoid that. – Clifford Mar 02 '18 at 23:44
  • @Clifford Yep. It was my bad. I should have addressed it to berendi. BTW, I too reckon that I would bump into similar cases down the line. Will see how it goes. – aep Mar 02 '18 at 23:58
  • @berendi: Thanks for the detailed explanation. For the time being I would use register access to enable the clocks, but it's quite likely that I would bump into similar cases down the line(just as I mentioned to Clifford). This is why many recommend to stick to the data sheet and implement your own driver. Thanks once again for the feedback. – aep Mar 03 '18 at 00:01
  • 2
    This is not a C++14 feature but a GCC bug! I filled a bug, gcc does not evaluate parenthesized discarded value expression while [\[expr\]/11.1](https://timsong-cpp.github.io/cppwp/n4140/expr#11.1) explicitly specifies that the expression shall be evaluated. – Oliv Mar 03 '18 at 08:42
4

There is code you can commit to your own repository to work around the issue and still compile the code with c++14.

/* Workaround for the broken UNUSED macro */
#include "stm32f3xx_hal_def.h"
#undef UNUSED
#define UNUSED(x) ((void)((uint32_t)(x)))

This needs to be added before any of the HAL headers are included. For me it was convenient to place into the stm32f3xx_hal_conf.h file right after the module enable macros (i.e. #define HAL_WWDG_MODULE_ENABLED line) but before the actual HAL headers are included. I updated all my sources to #include "stm32f3xx_hal_conf.h" instead of individual HAL headers.

This works because based on @berendi's excellent research the warning comes form the volatile designation. By casting the value to something that's not volatile first, the new clause in the C++14 standard is eluded.

user2501488
  • 141
  • 3
  • I believe this macro would be friendlier to the type of x: `#define UNUSED(x) ((void)(__typeof__(x))(x))` – Glenn Oct 10 '18 at 18:50
  • @Glenn I don't see how that removes a volatile qualification. – user2501488 Jan 12 '20 at 12:34
  • It does not. The use of `__typeof__` in my macro merely respects the type of the expression `x` passed to `UNUSED`. The macro you propose forces a cast to `uint32_t`. The compiler may choke on this cast, depending upon your compiler flags (e.g., those that warn on certain type conversions). – Glenn Jan 20 '20 at 20:58
  • @Glenn if you don't remove the volatile qualification you will always get the compiler warning shown in the OP. So your suggested macro does not solve the problem asked. – user2501488 Jul 25 '20 at 09:57
  • My understanding is that the C-style cast to `void` removes the volatile qualification. Perhaps I've stumbled into UB territory? We don't need to use something like `const_cast`. I use the macro I describe in my code base in the STM HAL with C++14 without issue. – Glenn Jul 30 '20 at 04:47
0

As @oliv mentioned in reply to @berendi's answer, the root cause appears to be a bug in GCC, which has been fixed in more recent versions. The warning went away for me when I upgraded to the "Version 9-2019-q4-major" toolchain (GCC 9.2).

Also, for the STM32G0 series, ST changed the definition of UNUSED to:

#define UNUSED(X) (void)X      /* To avoid gcc/g++ warnings */

which makes the warning go away for earlier versions of the compiler.

Dave Mackersie
  • 1,033
  • 10
  • 14