Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align command size to fix undefined behavior #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gaultier
Copy link

@gaultier gaultier commented Dec 22, 2023

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.

namandixit added a commit to namandixit/microui that referenced this pull request Oct 29, 2024
Merged rxi#67

Merge branch 'master' of github.com:gaultier/microui
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant