This is an archive of the discontinued LLVM Phabricator instance.

Fix tests for new str interceptors
ClosedPublic

Authored by m.guseva on Jun 9 2015, 2:41 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

m.guseva updated this revision to Diff 27365.Jun 9 2015, 2:41 AM
m.guseva retitled this revision from to Fix tests for new str interceptors.
m.guseva updated this object.
m.guseva edited the test plan for this revision. (Show Details)
m.guseva added reviewers: eugenis, dvyukov, samsonov, kcc.
m.guseva added subscribers: ygribov, Unknown Object (MLST).
eugenis accepted this revision.Jun 9 2015, 2:09 PM
eugenis edited edge metadata.

OI would be great if someone investigated the "s3" point in my other comment, but this change alone should make the tests much more stable, so LGTM.

test/asan/TestCases/strcasestr-1.c
21 ↗(On Diff #27365)

I missed the original code review, but this "s3" business looks highly suspicious. Firstly, it is not good to rely on the order of variable declarations in the function body, compiler definitely does not promise anything about that. Secondly, if ASan may say that access overflows s3 - I'd treat this as a bug in the tool. Why does it happen? COMMON_INTERCEPTOR_READ_* should point to the first poisoned byte in the memory range, which should be the first byte after the end of s1.

This revision is now accepted and ready to land.Jun 9 2015, 2:09 PM
ygribov added inline comments.Jun 9 2015, 2:18 PM
test/asan/TestCases/strcasestr-1.c
21 ↗(On Diff #27365)

I think we do need to put stopping zero byte somehow to ensure that e.g. underlying Glibc's strcasestr always terminates. Any suggestions how to achieve this in a cleaner way? Perhaps set last byte of s1's redzone to 0 in a noinline no_sanitize_address helper function?

Overflow in s3 is strange indeed.

eugenis added inline comments.Jun 9 2015, 2:34 PM
test/asan/TestCases/strcasestr-1.c
21 ↗(On Diff #27365)

Maybe just allocate a larger buffer for the string and __asan_poison part of it. The report would just say "wild address" though, without a reference to s1/s2 variables.

ygribov added inline comments.Jun 9 2015, 2:43 PM
test/asan/TestCases/strcasestr-1.c
21 ↗(On Diff #27365)

Ah, right. Mash is on vacation so I guess I'll commit what we have meanwhile.

This revision was automatically updated to reflect the committed changes.