-
Notifications
You must be signed in to change notification settings - Fork 139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More wrappers #79
base: master
Are you sure you want to change the base?
More wrappers #79
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very nice, thank you!
I'll be happy to work on resolving the few comments I had, rebase, and merge.
runtime/LibcWrappers.cpp
Outdated
auto aShadowIt = ReadOnlyShadow(a, strlen(a)).begin_non_null(); | ||
auto bShadowIt = ReadOnlyShadow(b, strlen(b)).begin_non_null(); | ||
auto *allEqual = _sym_build_equal(*aShadowIt, *bShadowIt); | ||
for (size_t i = 1; i < strlen(a); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably stop at the minimum of the two string lengths - otherwise we'll crash while trying to assemble the expression.
runtime/LibcWrappers.cpp
Outdated
if (isConcrete(a, strlen(a)) && isConcrete(b, strlen(b))) | ||
return result; | ||
|
||
auto aShadowIt = ReadOnlyShadow(a, strlen(a)).begin_non_null(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include the null byte?
runtime/LibcWrappers.cpp
Outdated
++aShadowIt; | ||
++bShadowIt; | ||
allEqual = | ||
_sym_build_bool_and(allEqual, _sym_build_equal(*aShadowIt, *bShadowIt)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't capture the exact semantics (especially for the case where the strings aren't equal), but it's much better than what we currently have 😉 Maybe we can just add a comment saying what would have to be done for an accurate symbolic representation.
runtime/LibcWrappers.cpp
Outdated
* return (int)strtol(str, (char **)NULL, 10); | ||
* } | ||
* */ | ||
auto result = strtol(s, (char **)NULL, 10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's nothing wrong with that function summary, but why don't we call atoi
directly?
runtime/LibcWrappers.cpp
Outdated
size_t length = strlen(s); | ||
size_t num_len = 0; | ||
for (size_t i = 0; i < length; i++) { | ||
if ('0' <= (char)s[i] && (char)s[i] <= '9') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean we don't handle negative numbers?
return result; | ||
} | ||
|
||
long int SYM(atol)(const char *s) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really similar to the wrapper for atoi
. Should we merge the two?
runtime/LibcWrappers.cpp
Outdated
_sym_set_return_expression(nullptr); | ||
|
||
size_t srcLen = strnlen(src, n); | ||
size_t copied = std::min(n, srcLen); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really minor, but isn't srcLen
always less than or equal to n
as per the definition of strnlen
?
7396053
to
496dac1
Compare
0510c5b
to
99a7410
Compare
add 7 common glibc function wrappers, including
strcmp
,strncmp
,strlen
,atoi
,atol
,strcpy
andstrncpy
Every wrapper was provided with a
test_xxx.c
to verity its correctness.