[BUG] hex_long breaks on 8 bit cpus

[ellensp]

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

Since #26715 any 8 bit boards that compile in Marlin/src/libs/hex_print.cpp generate this warning

Marlin/src/libs/hex_print.cpp: In function 'char* hex_long(uintptr_t)':
Marlin/src/libs/hex_print.cpp:57:31: warning: right shift count >= width of type [-Wshift-count-overflow]
     _hex[2] = hex_nybble(l >> 28);
                               ^~
Marlin/src/libs/hex_print.cpp:58:31: warning: right shift count >= width of type [-Wshift-count-overflow]
     _hex[3] = hex_nybble(l >> 24);
                               ^~
Marlin/src/libs/hex_print.cpp:59:31: warning: right shift count >= width of type [-Wshift-count-overflow]
     _hex[4] = hex_nybble(l >> 20);
                               ^~
Marlin/src/libs/hex_print.cpp:60:31: warning: right shift count >= width of type [-Wshift-count-overflow]
     _hex[5] = hex_nybble(l >> 16);
                               ^~

The cause is this function

  char* hex_long(const uintptr_t l) {
    _hex[2] = hex_nybble(l >> 28);
    _hex[3] = hex_nybble(l >> 24);
    _hex[4] = hex_nybble(l >> 20);
    _hex[5] = hex_nybble(l >> 16);
    _hex_word((uint16_t)(l & 0xFFFF));
    return &_hex[2];
  }

This used to be in a #ifdef CPU_32_BIT block so 8 bit boards would not look at this function.
But currently the #ifdef block was removed and this function is included for all boards, but uintptr_t is not a independent specific size.
On 8 bit boards typedef uint16_t uintptr_t;
on 32 bit boards typedef unsigned int uintptr_t;
on 64 bit simulator typedef unsigned long int uintptr_t

The function expects 32bits
so on 8 bit boards it is passing a 16bit number and the high 16bits are lost and the warning are generated

Bug Timeline

since #26715

Expected behavior

No warning, no truncation.

Actual behavior

Warnings and data truncation

Steps to Reproduce

1. download current bugfx
2. add the line "#define NEED_HEX_PRINT 1" to the Configuration.h to force PIO to compile Marlin/src/libs/hex_print.cpp
3. see the warnings

Version of Marlin Firmware

Bugfix-2.1.x

Electronics

RAMPS

POSSABLE fixes

a) put the #if block back around the entire function

b) update uintptr_t to uint32_t
At present this would conflict with the variable size of _hex

#ifdef CPU_32_BIT
  constexpr int byte_start = 4;
  static char _hex[] = "0x00000000";
#else
  constexpr int byte_start = 0;
  static char _hex[] = "0x0000";
#endif

c) add a #if CPU_32_BIT around the top 4 bytes so the code becomes

char* _hex_long(const uintptr_t l) {
  #ifdef CPU_32_BIT
    _hex[2] = hex_nybble(l >> 28);
    _hex[3] = hex_nybble(l >> 24);
    _hex[4] = hex_nybble(l >> 20);
    _hex[5] = hex_nybble(l >> 16);
  #endif
  __hex_word((uint16_t)(l & 0xFFFF));
  return &_hex[2];
}

d) (The 'thinkyhead' method) refactor all of marlin to use yet to be created

hex_8bit
hex_16bit
hex_32bit

and forget about the somewhat ambiguous "byte", "word" and "long"

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.