ExternalDispatcher: add handlers for external functions - #18
Conversation
|
Moon seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| if(!addr) { | ||
| addr = dlsym(RTLD_DEFAULT, str); | ||
| if(addr) { | ||
| llvm::sys::DynamicLibrary::AddSymbol(str, addr); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
- handlers for functions with floating point data
395db35 to
00e1186
Compare
|
Could you review my modification again? |
|
Sure, did you get a chance to sign the CLA? I won't be able to merge your fixes without it. |
| 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" || |
There was a problem hiding this comment.
A dictionary of name => function pointer would be more efficient
There was a problem hiding this comment.
Or perhaps look at the argument types instead:
if (retType == double && paramCount==1 && param[0] == double) {
...
}
| 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); | ||
|
|
||
| uint64_t double_to_rawbits(double value); |
There was a problem hiding this comment.
These look like utility functions. They don't need to be inside a class.
|
|
||
| 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); |
| std::stringstream &err) { | ||
| if (targetName == "exp2" || targetName == "log" || targetName == "tan" || | ||
| targetName == "rint" || targetName == "fabs" || targetName == "floor" || | ||
| targetName == "ceil" || targetName == "sin" || targetName == "cos") { // double func(double) |
There was a problem hiding this comment.
Could you assert that the LLVM function prototype actually takes double and not floats? AFAIK, you can have cos(double), cos(float), etc.
There was a problem hiding this comment.
Actually, if you use if (retType == double && paramCount==1 && param[0] == double) {..., it is not necessary.
|
|
||
| uint64_t ExternalDispatcher::double_to_rawbits(double value) { | ||
| uint64_t bits = 0; | ||
| memcpy(&bits, &value, 8); |
There was a problem hiding this comment.
Use sizeof(bits) instead of 8.
|
@cinemamoon5 any progress on this? |
3 participants
A response to issue "S2E/s2e-env#346" I reported before "https://groups.google.com/forum/#!topic/s2e-dev/CccWw4cPQK4"
This is a very simple fix in ExternalDispathcer::resolveSymbol function that finds external function using dlsym function.
Please let me know if it needs some changes.