This is an archive of the discontinued LLVM Phabricator instance.

[msan] Copy origin of byval arguments
ClosedPublic

Authored by vitalybuka on Jan 13 2022, 11:56 PM.

Diff Detail

Event Timeline

vitalybuka created this revision.Jan 13 2022, 11:56 PM
vitalybuka requested review of this revision.Jan 13 2022, 11:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2022, 11:56 PM

fix condition

eugenis accepted this revision.Jan 14 2022, 12:40 PM

LGTM

A compiler-rt test for origin propagation would be great.

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

This can get weird if the byval pointer is less than 4 byte aligned, but the alternatives are not pretty, either.

Also, you can bump the alignment std::max(CopyAlign, kMinOriginAlignment).

This revision is now accepted and ready to land.Jan 14 2022, 12:40 PM
kda accepted this revision.Jan 14 2022, 5:49 PM

LGTM

vitalybuka requested review of this revision.Jan 15 2022, 12:51 AM
vitalybuka added inline comments.
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
1741

Not sure if OriginPtr can be aligned higher then kMinOriginAlignment, as origin_tls base is 4 bytes aligned.
CpOriginPtr can be aligned higher, but I guess it does not matter here.

Also please notice the I changed copy size.

vitalybuka added 1 blocking reviewer(s): eugenis.Jan 15 2022, 12:52 AM

Add FIXME about size

eugenis accepted this revision.Jan 27 2022, 2:08 PM

LGTM

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

yes, you are right

This revision is now accepted and ready to land.Jan 27 2022, 2:08 PM
This revision was landed with ongoing or failed builds.Jan 27 2022, 4:24 PM
This revision was automatically updated to reflect the committed changes.