This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] CallEvent: Add partially working methods for obtaining the callee stack frame.
ClosedPublic

Authored by NoQ on Jul 23 2018, 7:27 PM.

Details

Summary

These methods allow reasoning about the stack frame that will be (or was) used when the function will be (or was) inlined, even if it will not be (or was not) inlined.

They are implemented in a fairly ugly manner, and while i plan to address some of the issues in follow-up patches (at least, do something about virtual calls), i don't think it'll work perfectly soon. A proper implementation would require a large amount of refactoring for Call::getDecl() and Call::getRuntimeDefinition() and the logic in ExprEngine's call modeling and i didn't accomplish much in a couple days of working on it, so i'm putting a proper implementation on hold for now, safely returning nullptr when the proper Decl cannot be reliably obtained.

This was ultimately necessary to obtain the parameter regions on the future stack frame for the purposes of D49443. We don't need this function to be perfect because a lot of other things are imperfect and we're fine with falling back to a conservative behavior.

Diff Detail

Repository
rL LLVM

Event Timeline

NoQ created this revision.Jul 23 2018, 7:27 PM
george.karpenkov requested changes to this revision.Jul 24 2018, 12:05 PM

Minor nits mostly.

include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
411 ↗(On Diff #156957)

Could we be more succinct here?
e.g. \return Best-effort getter for AnalysisDeclContext, returns null on failure.

Same for all the methods below.

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
776 ↗(On Diff #156957)

Could we group it with other public methods at the top?

lib/StaticAnalyzer/Core/CallEvent.cpp
193 ↗(On Diff #156957)

"we cannot have" comments seem redundant, it's usually implied by the code.
This one is up to you though.

197 ↗(On Diff #156957)

Can we remove "maybe" / "that would have been" comments?

This revision now requires changes to proceed.Jul 24 2018, 12:05 PM
NoQ updated this revision to Diff 157416.Jul 25 2018, 6:55 PM
NoQ marked 4 inline comments as done.

Address comments by editing comments.

NoQ added inline comments.Jul 26 2018, 11:39 AM
lib/StaticAnalyzer/Core/CallEvent.cpp
197 ↗(On Diff #156957)

Changed them into a TODO because they're rather important to keep. Like, that's a flaw in this code.

NoQ updated this revision to Diff 157550.Jul 26 2018, 12:21 PM

Add one more handy method, getAdjustedParameterIndex(), which helps using getParameterLocation().

MTC added a subscriber: MTC.Jul 26 2018, 7:58 PM
This revision is now accepted and ready to land.Jul 30 2018, 11:49 AM
This revision was automatically updated to reflect the committed changes.
NoQ added inline comments.Aug 1 2018, 7:31 PM
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
431

Whoops, this is wrong: should say getLocationContext().