rxi / microui

A tiny immediate-mode UI library
MIT License
3.29k stars 239 forks source link

Align command size to fix undefined behavior #67

Open gaultier opened 8 months ago

gaultier commented 8 months ago

Compiling with -fsanitize=address,memory (e.g.: clang -fsanitize=address,undefined src/microui.c demo/main.c demo/renderer.c -I src/ -lSDL2 -lGL) shows many instances of the same issue (excerpt):

src/microui.c:445:20: runtime error: member access within misaligned address 0x7fa3639908fb for type 'mu_Command', which requires 8 byte alignment
0x7fa3639908fb: note: pointer points here
 73  73 73 ff 01 00 00 00 10  00 00 00 1b 09 99 63 a3  7f 00 00 01 00 00 00 10  00 00 00 27 0c 99 63
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/microui.c:445:20 in
src/microui.c:445:20: runtime error: member access within misaligned address 0x7fa3639908fb for type 'mu_JumpCommand', which requires 8 byte alignment
0x7fa3639908fb: note: pointer points here
 73  73 73 ff 01 00 00 00 10  00 00 00 1b 09 99 63 a3  7f 00 00 01 00 00 00 10  00 00 00 27 0c 99 63
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/microui.c:445:20 in
src/microui.c:445:25: runtime error: load of misaligned address 0x7fa363990903 for type 'void *', which requires 8 byte alignment
0x7fa363990903: note: pointer points here
 10  00 00 00 1b 09 99 63 a3  7f 00 00 01 00 00 00 10  00 00 00 27 0c 99 63 a3  7f 00 00 03 00 00 00
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/microui.c:445:25 in

That's on x86_64 but other architectures will very likely show something similar.

The issue is that some commands such as TextCommand uses a variable size which is not in general a multiple of the pointer size (8 on 64 bits, or 4 on 32 bits platforms).

However this is required and not doing so leads to undefined behavior when dereferencing pointers to commands coming from the command list.

The easiest fix is to align the size to the next multiple of 8. If 32 bits platforms matter it could be tweaked with a ifdef to align to the next multiple of 4 for these.

There is open PR for 3 years that's similar, but I'd like to revive the topic once more since it matters to applications using sanitizers (which should be the majority or totality of modern applications), and it's a small non-intrusive fix.

See https://nullprogram.com/blog/2023/09/27/ and https://en.cppreference.com/w/c/language/object , 'Alignment' section, for a lengthier explanation.