openwch/ch32v307

Undefined behavior in GPIO_PinRemapConfig

alexisvl opened this issue · 5 comments

Does this project accept bug reports on the actual peripheral library code itself? I'm not sure how official this is.

There is a bug in GPIO_PinRemapConfig(), where undefined behavior is invoked during the calculation of AFIO->PCFRx:

    if(NewState != DISABLE)
    {
        tmpreg |= (tmp << ((GPIO_Remap >> 0x15) * 0x10));
    }

This results in a left shift by a very large number. In the case of GPIO_PartialRemap_USART4, it's tmp << 0x4010. Per C99 (6.5.7 paragraph 3), this is undefined:

If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined.

When compiling with GCC 12.2.0, it produces unpredictable results. Some pin remaps work, but I just lost a lot of time tracking down why USART4 couldn't remap properly because the calculation breaks for that one. The result of the multiplication needs to be masked before performing the shift:

    if(NewState != DISABLE)
    {
        tmpreg |= (tmp << (((GPIO_Remap >> 0x15) * 0x10) & 0x1F));
    }

Thank you for your report .which chip are you using? CH32V30x only has UART4. I remap UART4 normally.
emobile_2023-02-09_11-18-26
emobile_2023-02-09_11-18-37
emobile_2023-02-09_11-19-12

I'm using CH32V307.

This error will not occur in every situation. I am using GCC 12.2.0 with link-time optimization which is more likely to cause trouble with undefined behavior (as the compiler is performing build-time constant folding into the function body). It may work for you depending on your compiler and build settings, but it is undefined behavior and does not work reliably for me.

CH32V30x only has UART4

Yes, but the GPIO_*Remap_* constants erroneously call it USART4.

Thank you very much for your bug reports.Based on your findings,We will modify GPIO_PinRemapConfig in the next EVT update.
emobile_2023-02-13_14-26-29

Looks like it should work. I'm not sure why you don't just mask off the specific field you're trying to access in GPIO_Remap, but if it works, it works. 🤷

I am very glad to receive your reply. Does the problem still exist? I can't understand what "mask off the specific field you're trying to access in GPIO_Remap" means?Which specific field?