Conversation
sebastianpoeplau
left a comment
sebastianpoeplau
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
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.
Does this mean we don't handle negative numbers?
runtime/LibcWrappers.cpp
Outdated
| return result; | ||
| } | ||
|
|
||
| long int SYM(atol)(const char *s) { |
There was a problem hiding this comment.
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.
This is really minor, but isn't srcLen always less than or equal to n as per the definition of strnlen?
sebastianpoeplau
force-pushed
the
more_wrappers
branch
from
7396053 to
496dac1
Compare
sebastianpoeplau
force-pushed
the
more_wrappers
branch
from
0510c5b to
99a7410
Compare
3 participants
add 7 common glibc function wrappers, including
strcmp,strncmp,strlen,atoi,atol,strcpyandstrncpyEvery wrapper was provided with a
test_xxx.cto verity its correctness.