CallEvent::isCalled() alternative if a function is called by pointer

Clang Frontend
#1

Hi All,

I hope this is right place to ask such a newbie question like the following.

I am trying to write a checker to catch potential handle leaks (it’s on Windows: a handle is closed by CloseHandle).

So I took SimpleStreamChecker as a base, now my checkPreCall() checks if “CloseHandle” is called. Call.isCalled() works great except one case when CloseHandle is called by pointer. It happens because of using a template class that took a pointer to closing function as template parameter (useful to close different types of handles by appropriate functions: FindClose, CloseHandle etc.).

Call.dump() prints “&CloseHandle(this->m_h)” in this case, so it understands that this a pointer and that this is a pointer of CloseHandle. But how to “extract” the the identifier of CloseHandle?

Thank you in advance!

#2

Hi All,

I hope this is right place to ask such a newbie question like the following.

I am trying to write a checker to catch potential handle leaks (it’s on Windows: a handle is closed by CloseHandle).

So I took SimpleStreamChecker as a base, now my checkPreCall() checks if “CloseHandle” is called. Call.isCalled() works great except one case when CloseHandle is called by pointer. It happens because of using a template class that took a pointer to closing function as template parameter (useful to close different types of handles by appropriate functions: FindClose, CloseHandle etc.).

Call.dump() prints “&CloseHandle(this->m_h)” in this case, so it understands that this a pointer and that this is a pointer of CloseHandle.But how to “extract” the the identifier of CloseHandle?

I’m not quite understanding that dump, it looks a bit weird. What is the actual code under analysis? Could you see if you can get Call.getOriginExpr()->dump() and/or Call.getDecl()->dump()? These should be more informative.

In general the analyzer does indeed understand calls through function pointers, as long as it can at all be tracked by looking at the current execution path.

And when it is tracked, CallEvent::isCalled() should “just work” because it only looks at Call.getDecl() which should be the path-specific decl.

#3

Hi Artem,

It seems I was wrong, the situation is not so simple as I thought initially.

First of all, here the checkPreCall() call, it checks a call against CloseHandle and outputs result along with passed function information:

void HandleChecker::checkPreCall(
const CallEvent &Call,
CheckerContext &C) const
{
if (Call.isCalled(_closeHandleFn))
fprintf(stderr, "It’s CloseHandle: ");
else
fprintf(stderr, "It’s NOT CloseHandle: ");
Call.dump();
}

The first sample uses usual CloseHandle call, without function pointer:
CloseHandle(NULL);

So it works.

My original code that didn’t work has used template class. The simplified code:

typedef BOOL (WINAPI *P_CloseHandle)(HANDLE);

template <P_CloseHandle pCloseHandle> struct AutoCloseHandle
{
AutoCloseHandle(HANDLE h) : _h(h) {}
~AutoCloseHandle() { pCloseHandle(_h); };
HANDLE _h;
};

int main()
{
AutoCloseHandle<&CloseHandle> autoCloseHandle(NULL);
return 1;
}

The output:

It’s NOT CloseHandle: &CloseHandle(this->_h)
It’s NOT CloseHandle: 0
It’s NOT CloseHandle: Call to ~AutoCloseHandle<&CloseHandle>() noexcept {
&CloseHandle(this->_h);
}
It’s NOT CloseHandle: &CloseHandle(this->_h)

Could you see if you can get Call.getOriginExpr()->dump() and/or Call.getDecl()->dump()? These should be more informative.

Sure, I’ve added it. Call.getDecl() is NULL for that call. Call.getOriginExpr() is the following:

CallExpr 0x64ecb10 ‘BOOL’:‘int’

-SubstNonTypeTemplateParmExpr 0x64ecac0 ‘BOOL (*)(HANDLE) attribute((stdcall))’
-UnaryOperator 0x64ecaa8 'BOOL (*)(HANDLE) __attribute__((stdcall))' prefix '&' cannot overflow -DeclRefExpr 0x64eca90 ‘BOOL (HANDLE) attribute((stdcall))’:‘BOOL (HANDLE) attribute((stdcall))’ lvalue Function 0x57b8890 ‘CloseHandle’ ‘BOOL (HANDLE) attribute((stdcall))’:‘BOOL (HANDLE) attribute((stdcall))’
-ImplicitCastExpr 0x64ecb30 'HANDLE':'void *' <LValueToRValue> -MemberExpr 0x64ecae8 ‘HANDLE’:‘void *’ lvalue ->_h 0x64dc710
`-CXXThisExpr 0x64ecad8 ‘struct AutoCloseHandle<&CloseHandle> *’ this

Call.dump()'s result:

&CloseHandle(this->_h)

Thank you!

#4

Aha, interesting. At a glance it seems that we know how to evaluate a function pointer (i.e. ExprEngine::Visit()), but we don’t know how to constant-fold it (i.e. Environment::getSVal() → SValBuilder::getConstantVal()). And we don’t evaluate anything within SubstNonTypeTemplateParmExpr (see ExprEngine::Visit() again) because it’s supposed to be constant-foldable. Constant-folding should be easy, but it should be followed up with a cleanup work to remove the evaluation. Would you be willing to file a bug against me in bugzilla (or fix it)?

#5

Sure, I will file new bug. Just sent a request to bugs-admin@lists.llvm.org to get an access there.

I would be happy to fix it myself, but I really just started working with clang analyzer. Any doc to quick start? Or the code itself is the best documentation? :slight_smile:

Thank you!

#6

Sorry was afk.
Thanks!

Analyzer documentation is very fragmented, but apart from the SimpleStreamChecker video that you’ve probably already watched we do have the manual on the website (), some in-tree high-level overview docs (), and my workbook (). There’s also doxygen. None of these applies directly to the problem though. The correct value for DeclRefExpr to a known FunctionDecl would be a function pointer that is represented in the Analyzer by a value () of loc::MemRegionVal kind that wraps a memory region of FunctionCodeRegion kind (). In order to evaluate DeclRefExpr, i.e. tell our checkers (and also ourselves) that its value is the respective FunctionCodeRegion, we bind the value in the Environment when we encounter it. The proposed solution is to avoid adding an Environment binding, and instead always know, simply by looking at the expression, what its value is. You might not always know what is the value of “x + y” just by looking at the expression, but you’ll always know what is the value of expression “1” - it’s, well, 1. The point is to do the same for function pointers, as i described above, emm, sorry, actually below. I would have probably already fixed the problem if i didn’t have write all this, but you sounded interested and apparently i enjoy talking :slight_smile: