Replies: 2 comments
-
Nice writeup. I'm indeed neglected to fix unaligned memory access in mold because it haven't caused any issue so far. It is very unfortunate that even though most ELF data members are naturally aligned, we can't assume that they are aligned in an mmap'ed memory, because once they are in an I think I'm happy to merge your change to fix all unaligned accesses. Can you send a pull request? |
Beta Was this translation helpful? Give feedback.
-
Thanks for quick feedback! Yes, I'm gonna rebase yurai007@b23078d and make sure it works fine on all platforms (including Mach-O which, I realised, wasn't covered yet). I think that appropriate pull request should be ready this week. |
Beta Was this translation helpful? Give feedback.
-
Hi,
TL;DR
In Mold's Makefile there is already recommendation about how to use ASan but no information about UBSan usage.
In this thread I'm proposing adding UBSan to Makefile and that can be done in (at least) 2 ways depending what we want to achieve.
Full story
We already have information in Makefile about how to use ASan. For now there is no analogous UBsan usage recommendation.
In general UBSan can detect different types of issues unnoticed by ASan. Also some Mold third party packages like Mimalloc and xxHash already support UBSan options in their build systems. Finally linkers like LLD support UBsan options in their build systems. Putting it all together - having explicit information about UBSan in Mold's Makefile for tests purpose sounds like reasonable idea.
For now I can see 2 possible ways of updating Mold with UBSan support.
Option 1 - Enabling partial support of UBSan
It requires almost no change except adding short information to Makefile about passing proper flags to make.
Unfortunately for now running tests on UBSan instrumented Mold binary results in errors flood caused by -fsanitize=alignment check making UBSan rather impractical from user perspective. However adding -fsanitize=undefined -fno-sanitize=alignment to CXXFLAGS and LDFLAGS would enable UBSan with alignment check disabled. It's not perfect but probably still something worth consideration.
Option 2 - Enabling full support of UBSan
It's considered just as second option because misaligned pointer usage is in general controversial topic. In fact many CPU architectures such X86 or modern ARMs are tolerant to misaligned pointer usage. From the other hand unaligned access is undefined behavior according C/C++ standards. In context of linkers for example one of 2B aligned data sources can be archive files.
There is a way to bring full UBSan support to Mold but it requires some effort to fix alignment errors flood.
One of approach could be just following some of LLVM's ideas coming from ELFTypes: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Object/ELFTypes.h#L86 I prepared demo of change: yurai007@b23078d. For now in the sake of simplicity it completely ignores endianess topic but still patch seem to fix all UBSan errors.
There are alternative approaches of doing this like marking structs with [[gnu::packed]]. It would be simpler change but perhaps less safe due to how pointers can loose packed awareness in some cases. And last but not least since GCC and Clang do pretty good job in lowering small compile time size memcpy into movs, "ELFTypes-like" patch doesn't cause any performance regression.
Summary
It would be great to hear what do you think about such ideas. Also I'd like to emphasize that I don't have strong opinion about any of above proposals, but still I can see some value in both options. Anyway, if you are interested I can prepare appropriate pull request for review.
Cheers
Beta Was this translation helpful? Give feedback.
All reactions