This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Make the memtag sanitizer require the memtag extension
ClosedPublic

Authored by chill on Jul 31 2019, 5:25 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

chill created this revision.Jul 31 2019, 5:25 AM
vitalybuka added inline comments.
clang/lib/Driver/ToolChains/Clang.cpp
1561 ↗(On Diff #212558)

Please move back Driver closer to the first use

1624 ↗(On Diff #212558)

maybe better place is somewhere near llvm/tools/clang/lib/Driver/SanitizerArgs.cpp:1020

clang/test/Driver/fsanitize.c
194 ↗(On Diff #212558)

Can you also add command line where driver accepts flags?

chill updated this revision to Diff 213597.Aug 6 2019, 6:57 AM
chill marked 3 inline comments as done.Aug 6 2019, 7:00 AM
chill added inline comments.
clang/lib/Driver/ToolChains/Clang.cpp
1624 ↗(On Diff #212558)

The check is target specific, I prefer to keep it in a function that's already target specific anyway.

chill marked an inline comment as done.Aug 6 2019, 7:01 AM
chill added inline comments.
clang/lib/Driver/ToolChains/Clang.cpp
1632 ↗(On Diff #213597)

I've already fixed the line length here.

vitalybuka added inline comments.Aug 6 2019, 11:13 AM
clang/lib/Driver/ToolChains/Clang.cpp
1624 ↗(On Diff #212558)

My thoughts here are that the check is both sanitizer specific and platform specific. However it's going to be always sanitizers specific, but in theory we may get more platforms supporting "mte".

Also right now err_stack_tagging_requires_hardware_feature is very platform specific.
Something less specific?

'-fsanitize=memtag' requires hardware support (+memtag)
chill updated this revision to Diff 214185.Aug 8 2019, 10:36 AM
chill marked 2 inline comments as done.
vitalybuka accepted this revision.Aug 8 2019, 10:47 AM
This revision is now accepted and ready to land.Aug 8 2019, 10:47 AM
eugenis added inline comments.Aug 8 2019, 3:19 PM
clang/lib/Driver/SanitizerArgs.cpp
1042 ↗(On Diff #214185)

I'm a bit worried that this check can be satisfied by, say, a source file or a relative include path of "+mte".

chill updated this revision to Diff 214390.Aug 9 2019, 9:21 AM
chill marked 2 inline comments as done.
chill added inline comments.
clang/lib/Driver/SanitizerArgs.cpp
1042 ↗(On Diff #214185)

Fair enough, I've updated the patch.

chill marked an inline comment as done.Aug 9 2019, 9:22 AM
eugenis accepted this revision.Aug 9 2019, 9:40 AM

LGTM

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 7:20 AM