This is an archive of the discontinued LLVM Phabricator instance.

[EfficiencySanitizer] Adds shadow memory parameters for 40-bit virtual memory address.
ClosedPublic

Authored by slthakur on Aug 23 2016, 6:18 AM.

Details

Summary

Adding 40-bit shadow memory parameters because MIPS64 uses 40-bit virtual memory addresses.

Diff Detail

Repository
rL LLVM

Event Timeline

slthakur updated this revision to Diff 68978.Aug 23 2016, 6:18 AM
slthakur retitled this revision from to [EfficiencySanitizer] Adds shadow memory parameters for 40-bit virtual memory address..
slthakur updated this object.
slthakur added reviewers: aizatsky, bruening.
slthakur set the repository for this revision to rL LLVM.
slthakur added a project: Restricted Project.
bruening added inline comments.Aug 25 2016, 8:44 PM
lib/Transforms/Instrumentation/EfficiencySanitizer.cpp
102

Style: { on prior line

slthakur updated this revision to Diff 69359.Aug 26 2016, 5:53 AM
slthakur marked an inline comment as done.
bruening accepted this revision.Aug 26 2016, 10:58 AM
bruening edited edge metadata.
This revision is now accepted and ready to land.Aug 26 2016, 10:58 AM
slthakur closed this revision.Sep 7 2016, 2:56 AM
slthakur updated this revision to Diff 70522.Sep 7 2016, 5:46 AM
slthakur edited edge metadata.

Removed default case for reporting error for unsupported architectures as this was causing failures on aarch64, powerpc64 build bots

slthakur reopened this revision.Sep 7 2016, 5:51 AM
This revision is now accepted and ready to land.Sep 7 2016, 5:51 AM
slthakur updated this revision to Diff 70647.Sep 7 2016, 10:48 PM

Enabled the instrumentation tests only for mips and x86.

slthakur requested a review of this revision.Sep 8 2016, 3:33 AM
slthakur edited edge metadata.
rengolin requested changes to this revision.Sep 8 2016, 4:58 AM
rengolin added a reviewer: rengolin.

Can you elaborate why this patch is restricting AArch64?

This revision now requires changes to proceed.Sep 8 2016, 4:58 AM
bruening edited edge metadata.Sep 8 2016, 1:00 PM
bruening added a subscriber: zhaoqin.

Theres no good reason for restricting architectures other than x86 and mips. Since the instrumentation tests are running cleanly without these shadow memory parameters, I am okay with removing the default case which is reporting the fatal error.

slthakur updated this revision to Diff 70790.Sep 9 2016, 12:17 AM
slthakur edited edge metadata.

Removed default case which was reporting fatal error on aarch64, powerpc64 etc.

slthakur updated this revision to Diff 71133.Sep 13 2016, 3:30 AM
slthakur edited edge metadata.

Using if ... else construct instead of switch case to avoid warning about unhandled enumeration values.

slthakur updated this revision to Diff 71134.Sep 13 2016, 3:32 AM

Increased patch context.

slthakur removed a subscriber: zhaoqin.

I assume your ping is to rengolin as his request for changes is still there.

@rengolin: Could you please find some time to review this?

rengolin accepted this revision.Oct 4 2016, 3:13 AM
rengolin edited edge metadata.

Sorry, I was out for more than a week. The patch looks ok now, assuming it still passes all tests on X86_64, AArch64 and MIPS64.

This revision is now accepted and ready to land.Oct 4 2016, 3:13 AM
slthakur closed this revision.Oct 6 2016, 3:01 AM

Committed revision 283433