Byte by byte access to 32bit peripheral registers causes issues

Looks like the compiler is accessing NVM_SPI->CTRL one byte at a time. There must be something in the declaration of NVM_SPI->CTRL and maybe in the compiler command line options that make it do it.

Could be. I'm switching back and forth between 4.9 and 5.4, using the same options. The thing is, why the newer versions try to access a 32bit variable byte by byte on a 32bit CPU? Let's say they have their own causes. So why is the functionality broken when accessing one byte at a time?

Changes in major compiler version number are known to change the way some pragmas, attributes and so on are interpreted. You should examine your code and see if anything in the changelog from gcc 4 to gcc 5 affects it.

Couldn't find something definite. Just have seen this one:

****************************

2015-12-17 Thomas Preud'homme <email address hidden>

  * config/arm/arm.h (TARGET_USE_MOVT): Check MOVT/MOVW availability
  with TARGET_HAVE_MOVT.
  (TARGET_HAVE_MOVT): Define.
  * config/arm/arm.c (const_ok_for_op): Check MOVT/MOVW
  availability with TARGET_HAVE_MOVT.
  * config/arm/arm.md (arm_movt): Use TARGET_HAVE_MOVT to check movt
  availability.
  (addsi splitter): Use TARGET_USE_MOVT to check whether to use
  movt + movw.
  (symbol_refs movsi splitter): Remove TARGET_32BIT check.
  (arm_movtas_ze): Use TARGET_HAVE_MOVT to check movt availability.
  * config/arm/constraints.md (define_constraint "j"): Use
  TARGET_HAVE_MOVT to check movt availability.
***********************

Maybe removing TARGET_32BIT could have an effect on this issue. It's a long shot, but I don't have further ideas based on this document:

https://gcc.gnu.org/svn/gcc/branches/ARM/embedded-5-branch/gcc/ChangeLog.arm

Maybe you should show us the declarations of the structure and the variable in your code. Compiler options too.

Sure,

This is the most obvious failing line:

    NVM_SPI->CTRL = USART_CTRL_SYNC | USART_CTRL_MSBF;

NVM_SPI is defined in a header file like this:

    #define NVM_SPI USART1

USART1 is defined in a vendor library like this:

#define USART1 ((USART_TypeDef *) USART1_BASE) /**< USART1 base pointer */

USART1_BASE is defined in another file like this:

#define USART1_BASE (0x4000C400UL) /**< USART1 base address */

Here's the struct definition for USART_TypeDef:

typedef struct
{
  __IOM uint32_t CTRL; /**< Control Register */
  __IOM uint32_t FRAME; /**< USART Frame Format Register */
  __IOM uint32_t TRIGCTRL; /**< USART Trigger Control register */
  __IOM uint32_t CMD; /**< Command Register */
  __IM uint32_t STATUS; /**< USART Status Register */
  __IOM uint32_t CLKDIV; /**< Clock Control Register */
  __IM uint32_t RXDATAX; /**< RX Buffer Data Extended Register */
  __IM uint32_t RXDATA; /**< RX Buffer Data Register */
  __IM uint32_t RXDOUBLEX; /**< RX Buffer Double Data Extended Register */
  __IM uint32_t RXDOUBLE; /**< RX FIFO Double Data Register */
  __IM uint32_t RXDATAXP; /**< RX Buffer Data Extended Peek Register */
  __IM uint32_t RXDOUBLEXP; /**< RX Buffer Double Data Extended Peek Register */
  __IOM uint32_t TXDATAX; /**< TX Buffer Data Extended Register */
  __IOM uint32_t TXDATA; /**< TX Buffer Data Register */
  __IOM uint32_t TXDOUBLEX; /**< TX Buffer Double Data Extended Register */
  __IOM uint32_t TXDOUBLE; /**< TX Buffer Double Data Register */
  __IM uint32_t IF; /**< Interrupt Flag Register */
  __IOM uint32_t IFS; /**< Interrupt Flag Set Register */
  __IOM uint32_t IFC; /**< Interrupt Flag Clear Register */
  __IOM uint32_t IEN; /**< Interrupt Enable Register */
  __IOM uint32_t IRCTRL; /**< IrDA Control Register */
  __IOM uint32_t ROUTE; /**< I/O Routing Register */
  __IOM uint32_t INPUT; /**< USART Input Register */
  __IOM uint32_t I2SCTRL; /**< I2S Control Register */
} USART_TypeDef; /** @} */

And __IOM is defined as:

#define __IOM volatile

The C++ compiler options are:

-g3 -gdwarf-2 -mcpu=cortex-m3 -mthumb -std=c++1y '-DDEBUG=1' -O0 -pedantic -Wall -Wextra -c -fmessage-length=0 -fno-rtti -fno-exceptions -mno-sched-prolog -fno-builtin -fpack-struct -fshort-enums -ffunction-sections -fdata-sections

Note:I've removed folder inclusions and some simple preprocessor definitions to hide some sensitive data about our project. I'm sure they are not related.

Do you need anything else?

-fpack-struct must be responsible for this. Why would you want this option anyway? If you really insist, you might want to have a look at the option "-munaligned-access" as well. The manual says " If unaligned access is not enabled then words in packed data structures are accessed a byte at a time."

John,

You're right. -fpack-struct option definitely changes the behavior for 6.3.1.

Here's enabled and disabled assembly output (got with 6 branch):

Enabled:
 ldr r3, .L11+4
 ldrb r2, [r3]
 movs r2, #0
 orr r2, r2, #1
 strb r2, [r3]
 ldrb r2, [r3, #1]
 movs r2, #0
 orr r2, r2, #4
 strb r2, [r3, #1]
 ldrb r2, [r3, #2]
 movs r2, #0
 strb r2, [r3, #2]
 ldrb r2, [r3, #3]
 movs r2, #0
 strb r2, [r3, #3]

Disabled:

 ldr r3, .L11+4
 movw r2, #1025
 str r2, [r3]

Looks like unaligned access is enabled for v7-m architecture by default.

One thing remains. OK, it has to access byte by byte to the packed struct members, but why with wrong values? This smells like a bug, isn't it?

[ After some short time ]

By the way, I've looked at the code again and looks like it has to set the register with correct values even with -fpacked-struct option. So why I'm seeing different values in the debug session? Let's say GDB reports wrong values, why the peripheral is not working as expected.

Note: I need packed structs for serializing communication and storage.

And additionally, why is still accessing words one byte at a time even unaligned access is enabled? This behavior is correct on 4.9 but not correct on 5 and 6 branch.

New tools can reveal old problems, in this case the use of -fpacked-struct on the entire project.
> Note: I need packed structs for serializing communication and storage.
We all do. Use #pragma pack(push, 1) and #pragma pack(pop) in the affected header files.

Hi Leo, yes I was going to revise the project and put pragma's for the necessary structs. But this still does not clarify what bothers me, byte by byte method is used even with enabled unaligned access. What's worse is, this creates issues on the real hardware, with an unknown cause, not just making the final image longer.

Clearly there's a regression on 5+ but I don't know the internals enough to find the root cause/commit.

I think you should try to construct a minimal example demonstrating the problem. Who knows, maybe you'll find something in your setup that causes this.

I guess the problem lies under the -fpack-struct option. No matter unaligned access state is, the compiler switches one byte at a time mode with seeing -fpack-struct option.

I tried this on a small file with the options I said before. Therefore my setup should not be the cause. I will try to build a really small reproducible example, tomorrow.

I've found the holy grail. Problem is happening if you are using a pointer to a struct. Normally, while accessing to that struct members, nothing is wrong, it accesses word by word. But if you declare a pointer to that struct and do your business with it, problem arises and GCC starts using byte by byte method even while unaligned access is enabled.

Here's worse part: It doesn't matter whether you use -fpack-struct flag or #pragma pack(push, 1). You'll get the same result.

Please find the simplest example as attachment. And use this line (don't forget to add your toolchains path in the beginning)

arm-none-eabi-g++ -gdwarf-2 -mcpu=cortex-m3 -mthumb -std=c++1y '-DDEBUG=1' -O0 -pedantic -Wall -Wextra -c -fmessage-length=0 -fno-rtti -fno-exceptions -mno-sched-prolog -fno-builtin -fshort-enums -ffunction-sections -fdata-sections -MMD -MP -MF"main.d" -MT"main.o" -S -o "main.s" "main.cpp"

Here's the output of the important part:

.LCFI2:
 .cfi_def_cfa_register 7
 .loc 1 17 0
 ldr r2, .L3
 mov r3, r7
 str r3, [r2]
 .loc 1 19 0
 ldr r3, .L3
 ldr r3, [r3]
 ldrb r2, [r3]
 movs r2, #0
 orr r2, r2, #1
 strb r2, [r3]
 ldrb r2, [r3, #1]
 movs r2, #0
 orr r2, r2, #4
 strb r2, [r3, #1]
 ldrb r2, [r3, #2]
 movs r2, #0
 strb r2, [r3, #2]
 ldrb r2, [r3, #3]
 movs r2, #0
 strb r2, [r3, #3]

I would be glad if anyone can pinpoint the root cause. Things should be gone bad between 4.9 and 5.4.

Regards,

Update: Problem still exists on the fresh release, 7-2017-q4-major.

Update: The problem also exists on the first official 5 branch release, 5.2 (5-2015-q4-major). Now I'm trying to isolate the change from 4.9 to 5.2, with my non-existing GCC infrastructure knowledge :) I feel like I'm entering adark cave with full of bats :)

Just tested with the latest release. GCC: (GNU Tools for Arm Embedded Processors 7-2017-q4-major) 7.2.1 20170904 (release) [ARM/embedded-7-branch revision 255204

gcc -mcpu=cortex-m3 -mthumb -O0 -munaligned-access -S test.c -o test.s

test.c:

#pragma pack(1)
struct s { short i; } volatile *p;
void f(void) { p->i = 0; }

output:

ldr r3, .L2
ldr r3, [r3]
ldrb r2, [r3]
movs r2, #0
strb r2, [r3]
ldrb r2, [r3, #1]
movs r2, #0
strb r2, [r3, #1]

There two problems:
1) Byte-wise access. One would expect "-munaligned-access" to help avoid this.
2) The variable is read before it is written for no apparent reason. This is in violation of the volatile semantics, as far as I can see. And this problem goes away when I remove "volatile" from source.

Hi John,

Thanks for your interest and effort. I appreciate it. By the way, I've checked the output with 5.2 and the result is the same.

And here's the 4.9 output:

 ldr r3, .L2
 ldr r3, [r3]
 ldrh r2, [r3] @ movhi
 movs r2, #0
 strh r2, [r3] @ movhi
 mov sp, r7

Another boring thing, there is a redundant nop instruction before popping sp, in 5,6 and 7 versions. I don't know the necessity.

For me, the most suspicious thing is the addition of ARM-v8 architec. This is the major change for the 5. Even though the change is announced in 4.7, I've seen significant changes in 5.2 comparing to 4.9. Maybe mixing 64bit logic with 32bit logic could have an effect on this issue. Who knows? (Definitely not me :)

Regards,

My guess is, the nop is there to make sure the literal pool is word-aligned.
As for the bug, our hope is that the guys from GNU ARM Embedded pick it up and report upstream. I don't think there is a point for you to try and pinpoint the exact commit that introduced it.

Today, I really tried, but fell short. Meld is still open for the source directories, 4.9 and 5.2.

I guess the time is not right. Maybe in the new year. Will be waiting for the team.

Thanks again.

By the way, could you click on "affects me" link to heat the issue up? This way the team may give it a better priority?

Hi,

I've opened a bug record on GCC Bugzilla (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87373) and was able to trace what is going on under the hood.

You can find all the details on that bug record.

I'm not sure about the GCC teams willingness to fix this but there is a issue with packed structs anyway. Maybe that could be fixed by the Linaro team via reverting back to 4.9 style.

Hi Murat,

I've read the thread and I agree with the final conclusion from Richard Earnshaw: doing byte by byte is the safest option from GCC's perspective when doing a volatile access as you cannot guarantee that unaligned access do. I think the source code should be changed to explicitely mark the first field as being aligned, or to exclude the volatile field from being packed.

Best regards.

Hi Thomas,

I've applied the workaround and it is working as designed. But the library developers were unaware from this situation and I've informed them to track this internally.

My conclusion is; never use project wide pack struct option, because you'd never know when it would hit you.

Regards,