[compiler-rt] [win/asan] Don't intercept memset etc. in ntdll (PR #120397)
Yannis Juglaret via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 18 08:34:21 PST 2024
yjugl wrote:
For Mozilla, `firefox.exe` imports and uses `ntdll`'s `memset`, whereas our DLLs like `mozglue.dll` and `xul.dll` import and use the one from `VCRUNTIME140.dll`. @glandium, would you know why we have this difference and whether it is intended? If yes, do you think our reasons for using `ntdll`'s `memset` here could apply to other projects?
Nonetheless this patch sounds like a reasonable compromise to me. I got progressively convinced that it's probably just best to not have any instrumentation for these two calls, like this patch suggests. As you mention, the uses of `ntdll`'s `memset` should be fairly marginal, and this instrumentation could be considered out-of-scope in the same way that, more generally, ASAN cannot and will not instrument every memory read/write operation performed by `ntdll`.
For the record here is a summary of [my own attempts](https://phabricator.services.mozilla.com/D228165) at fixing this issue:
- force-committing the shadow memory in `memset` instrumentation, which resulted in too much performance overhead;
- detecting `memset` reentrancy, which seemed to have a more reasonable overhead for non-24H2 platforms with Firefox, but could have had a more significant impact on 24H2 performance, where the reentrant calls would actually occur -- often.
#120110 by @zacklj89 is another attempt at force-committing the shadow memory, this time in a smarter way since it only does that for callers that live in `ntdll`. This should perform better than my own attempt -- perhaps similar to my `memset` reentrancy attempt in that regard.
As far as I am concerned, the correct way forward for Mozilla is probably to stop relying on `ntdll`'s `memset` at all here. And if we have a good reason to keep using it, the tradeoff from this patch seems acceptable -- there are not that many actual calls to this import.
https://github.com/llvm/llvm-project/pull/120397
More information about the llvm-commits
mailing list