This is an archive of the discontinued LLVM Phabricator instance.

[MSAN] Allow inserting array checks
ClosedPublic

Authored by guiand on Jul 23 2020, 9:57 AM.

Details

Summary

Flattens arrays by ORing together all their elements.

Diff Detail

Event Timeline

guiand created this revision.Jul 23 2020, 9:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2020, 9:57 AM

Do you want to use this in eager checks when passing an array as a function argument (does that even happen in C++?) ?

llvm/test/Instrumentation/MemorySanitizer/check-array.ll
10

just pass a [2xi24]* through the function arguments (and call it something other than main)

vitalybuka added inline comments.Jul 23 2020, 12:38 PM
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
1160

Please reformat

I don't believe this happens in C++ but wanted to add it for completeness's sake (or if some other LLVM based language might use it).

I don't believe this happens in C++ but wanted to add it for completeness's sake (or if some other LLVM based language might use it).

Right, but I don't see you doing it in this patch.

guiand marked an inline comment as done.Jul 23 2020, 1:37 PM
guiand added inline comments.
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
1784

@eugenis With eager checks enabled, any noundef array arguments or return values will attempt to call this code. The behavior without this patch is for this assertion to fire. The patch enables these checks to work correctly.

If you'd like, I can make this clear in the test I added by using -msan-eager-checks.

eugenis accepted this revision.Jul 23 2020, 1:49 PM

LGTM

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
1784

Got it, MSan is simply broken without this patch - it crashes on valid IR.

This revision is now accepted and ready to land.Jul 23 2020, 1:49 PM
guiand marked an inline comment as done.Jul 23 2020, 2:03 PM
guiand added inline comments.
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
1784

Yep, that's why I made the test case use checking the main return value -- it's independent of eager-checks

vitalybuka accepted this revision.Jul 23 2020, 2:28 PM

LGTM with clang-format

This revision was automatically updated to reflect the committed changes.