Skip to content
This repository was archived by the owner on Feb 3, 2020. It is now read-only.

ExternalDispatcher: add handlers for external functions #18

Open
cinemamoon5 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ExternalDispatcher: add handlers for external functions #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions include/klee/ExternalDispatcher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ namespace klee {
class ExternalDispatcher {
private:
public:
typedef uint64_t (*external_fcn_t)(...);
typedef llvm::SmallVector<uint64_t, 8> Arguments;

ExternalDispatcher();
virtual ~ExternalDispatcher();

virtual void *resolveSymbol(const std::string &name);
virtual bool call(external_fcn_t targetFunction, const Arguments &args, uint64_t *result, std::stringstream &err);
virtual bool call(const std::string& targetName, void* targetAddr, const Arguments &args, uint64_t *result, std::stringstream &err);

@vitalych vitalych Dec 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run clang-format.


uint64_t double_to_rawbits(double value);

@vitalych vitalych Dec 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like utility functions. They don't need to be inside a class.


double rawbits_to_double(uint64_t bits);
};
}

Expand Down
12 changes: 4 additions & 8 deletions lib/Core/Executor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,6 @@ void Executor::callExternalFunction(ExecutionState &state, KInstruction *target,
}

ExternalDispatcher::Arguments cas;

unsigned i = 1;
for (std::vector<ref<Expr>>::iterator ai = arguments.begin(), ae = arguments.end(); ai != ae; ++ai, ++i) {
ref<Expr> arg = state.toUnique(*ai);
Expand Down Expand Up @@ -1742,28 +1741,25 @@ void Executor::callExternalFunction(ExecutionState &state, KInstruction *target,
os << ", ";
}
os << ")" << std::dec;

klee_warning_external(function, "%s", os.str().c_str());
}

uint64_t result;
external_fcn_t targetFunction = (external_fcn_t) externalDispatcher->resolveSymbol(function->getName());
if (!targetFunction) {
uint64_t result = 0;
void* targetAddr = externalDispatcher->resolveSymbol(function->getName());
if (!targetAddr) {
std::stringstream ss;
ss << "Could not find address of external function " << function->getName().str();
terminateState(state, ss.str());
return;
}

std::stringstream ss;
if (!externalDispatcher->call(targetFunction, cas, &result, ss)) {
if (!externalDispatcher->call(function->getName(), targetAddr, cas, &result, ss)) {
ss << ": " << function->getName().str();
terminateState(state, ss.str());
return;
}

Type *resultType = target->inst->getType();

if (resultType != Type::getVoidTy(function->getContext())) {
ref<Expr> resultExpr;
auto resultWidth = getWidthForLLVMType(resultType);
Expand Down
119 changes: 81 additions & 38 deletions lib/Core/ExternalDispatcher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@
//===----------------------------------------------------------------------===//

#include <klee/ExternalDispatcher.h>
#include <klee/Common.h>
#include <llvm/Support/DynamicLibrary.h>

#include <iostream>
#include <sstream>

#include <setjmp.h>
#include <signal.h>
#include <sstream>
#include <dlfcn.h>

namespace klee {

ExternalDispatcher::ExternalDispatcher() {
}

Expand All @@ -39,54 +41,95 @@ void *ExternalDispatcher::resolveSymbol(const std::string &name) {
if (addr) {
return addr;
}

// If it has an asm specifier and starts with an underscore we retry
// without the underscore. I (DWD) don't know why.
if (name[0] == 1 && str[0] == '_') {
++str;
addr = llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(str);
}

if (!addr) {
addr = dlsym(RTLD_DEFAULT, str);
if (addr) {
llvm::sys::DynamicLibrary::AddSymbol(str, addr);

@vitalych vitalych Dec 10, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work for symbol resolution. You may also need to extend function invocation to handle functions that take or return floating point data (float/double/long double). These may be passed in FP registers and the current implementation does not support that.

@cinemamoon5 cinemamoon5 Dec 13, 2019 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I need to implement several function prototypes like "external_func_t" to deal with functions returning/taking floating point data. I guess I can get function return/parameter types from llvm::Function class.
Is it a efficient/right approach to this?

And... is the original KLEE supporting external functions with floating point data? They generate stubs before calling external functions. Probably, the stub enables KLEE to support external functions with floating point data.

Thank you.

@vitalych vitalych Dec 13, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they generated stubs, but that's very expensive to do, it was showing on the profile and I removed them.
The KLEE version in S2E is very specialized, it does not need to handle all types of functions and can only restrict itself to those used by helpers. Therefore, you can hard-code all these external functions, e.g,:
switch (functionName) {
case "log": return logl(..);
case "sin": ...
}

See the issue description to get a complete list of these functions.
It's important to have a whitelist though, that helps catching ones we don't support and that may have a weird calling convention.

}
}

return addr;
}

bool ExternalDispatcher::call(external_fcn_t targetFunction, const Arguments &args, uint64_t *result,
std::stringstream &err) {
extern "C" {
typedef double (*external_fcn_dd_t)(double);
typedef uint32_t (*external_fcn_u32d_t)(double);
typedef double (*external_fcn_ddu32_t)(double, uint32_t);
typedef double (*external_fcn_ddd_t)(double, double);
typedef uint64_t (*external_fcn_t)(...);
}

uint64_t ExternalDispatcher::double_to_rawbits(double value) {
uint64_t bits = 0;
memcpy(&bits, &value, 8);

@vitalych vitalych Dec 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use sizeof(bits) instead of 8.

return bits;
}

double ExternalDispatcher::rawbits_to_double(uint64_t bits) {
double value = 0.0;
memcpy(&value, &bits, 8);
return value;
}

switch (args.size()) {
case 0:
*result = targetFunction();
break;
case 1:
*result = targetFunction(args[0]);
break;
case 2:
*result = targetFunction(args[0], args[1]);
break;
case 3:
*result = targetFunction(args[0], args[1], args[2]);
break;
case 4:
*result = targetFunction(args[0], args[1], args[2], args[3]);
break;
case 5:
*result = targetFunction(args[0], args[1], args[2], args[3], args[4]);
break;
case 6:
*result = targetFunction(args[0], args[1], args[2], args[3], args[4], args[5]);
break;
case 7:
*result = targetFunction(args[0], args[1], args[2], args[3], args[4], args[5], args[6]);
break;
case 8:
*result = targetFunction(args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7]);
break;
default: {
err << "External function has too many parameters";
return false;
bool ExternalDispatcher::call(const std::string& targetName, void *targetAddr, const Arguments &args, uint64_t *result,
std::stringstream &err) {
if (targetName == "exp2" || targetName == "log" || targetName == "tan" ||

@vitalych vitalych Dec 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A dictionary of name => function pointer would be more efficient

@vitalych vitalych Dec 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps look at the argument types instead:
if (retType == double && paramCount==1 && param[0] == double) {
...
}

targetName == "rint" || targetName == "fabs" || targetName == "floor" ||
targetName == "ceil" || targetName == "sin" || targetName == "cos") { // double func(double)

@vitalych vitalych Dec 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you assert that the LLVM function prototype actually takes double and not floats? AFAIK, you can have cos(double), cos(float), etc.

@vitalych vitalych Dec 23, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, if you use if (retType == double && paramCount==1 && param[0] == double) {..., it is not necessary.

auto targetFunc = (external_fcn_dd_t) targetAddr;
*result = double_to_rawbits(targetFunc(rawbits_to_double(args[0])));
} else if (targetName == "isinf" || targetName == "isnan") { // uint32_t func(double)
auto targetFunc = (external_fcn_u32d_t) targetAddr;
*result = targetFunc(rawbits_to_double(args[0]));
} else if (targetName == "ldexp") { // double func(double, uint32_t)
auto targetFunc = (external_fcn_ddu32_t) targetAddr;
*result = double_to_rawbits(targetFunc(rawbits_to_double(args[0]), args[1]));
} else if (targetName == "atan2") { // double func(double, double)
auto targetFunc = (external_fcn_ddd_t) targetAddr;
*result = double_to_rawbits(targetFunc(rawbits_to_double(args[0]), rawbits_to_double(args[1])));
} else {
auto targetFunc = (external_fcn_t) targetAddr;
switch (args.size()) {
case 0:
*result = targetFunc();
break;
case 1:
*result = targetFunc(args[0]);
break;
case 2:
*result = targetFunc(args[0], args[1]);
break;
case 3:
*result = targetFunc(args[0], args[1], args[2]);
break;
case 4:
*result = targetFunc(args[0], args[1], args[2], args[3]);
break;
case 5:
*result = targetFunc(args[0], args[1], args[2], args[3], args[4]);
break;
case 6:
*result = targetFunc(args[0], args[1], args[2], args[3], args[4], args[5]);
break;
case 7:
*result = targetFunc(args[0], args[1], args[2], args[3], args[4], args[5], args[6]);
break;
case 8:
*result = targetFunc(args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7]);
break;
default: {
err << "External function has too many parameters";
return false;
}
}
}

return true;
}
}
}