This is an archive of the discontinued LLVM Phabricator instance.

[hwasan] Respect returns attribute when tracking values.
ClosedPublic

Authored by fmayer on Sep 3 2021, 6:26 AM.

Diff Detail

Event Timeline

fmayer created this revision.Sep 3 2021, 6:26 AM
fmayer updated this revision to Diff 370572.Sep 3 2021, 6:27 AM

formatting

fmayer updated this revision to Diff 370574.Sep 3 2021, 6:36 AM

comments

fmayer retitled this revision from [hwasan] Respect returns attribute when following to Alloca. to [hwasan] Respect returns attribute when tracking values..Sep 3 2021, 7:33 AM
fmayer published this revision for review.Sep 3 2021, 8:27 AM
fmayer added a reviewer: vitalybuka.
Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2021, 8:27 AM
fmayer updated this revision to Diff 371593.Sep 9 2021, 7:39 AM

add test

vitalybuka added inline comments.Sep 9 2021, 11:16 AM
llvm/lib/Analysis/StackSafetyAnalysis.cpp
397

Isn't this in the call will make it unsafe anyway?

416

does SCEV can look through such calls?

llvm/lib/Analysis/ValueTracking.cpp
4536

There is "TEST_P(FindAllocaForValueTest, findAllocaForValue*" tests which need to be extended for this test.
Improving findAllocaForValue is a patch by itself and may affect other components, so I ask you to not mix this with StackSafetyAnalysis (if possible)

llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
159

Having that retptr has no body here, SAFETY should make it "call to retptr" unsafe anyway

fmayer updated this revision to Diff 372181.Sep 13 2021, 2:03 AM
fmayer marked 3 inline comments as done.

split change

fmayer marked an inline comment as done.Sep 13 2021, 2:04 AM
fmayer added inline comments.
llvm/lib/Analysis/StackSafetyAnalysis.cpp
397

This is for being able to prove accesses safe, not the whole alloca.

416

Yes, it does.

llvm/test/Instrumentation/HWAddressSanitizer/stack-safety-analysis.ll
159

Yes, but it can tell that the store is safe.

vitalybuka accepted this revision.Sep 13 2021, 11:53 AM
This revision is now accepted and ready to land.Sep 13 2021, 11:53 AM
This revision was automatically updated to reflect the committed changes.