This is an archive of the discontinued LLVM Phabricator instance.

[Sanitizers] Intercept snprintf_l() on FreeBSD
ClosedPublic

Authored by kutuzov.viktor.84 on Nov 26 2014, 5:15 AM.

Details

Summary

In contrast to Linux, FreeBSD does not omit the *printf_l() functions so we have to intercept them. Particularly, libcxx leverage snprintf_l() for verbalization of numeric values. Since gtest use the standard i/o streams, we get mass false positives on Msan unit tests.

Diff Detail

Repository
rL LLVM

Event Timeline

kutuzov.viktor.84 retitled this revision from to [Sanitizers] Intercept snprintf_l() on FreeBSD.
kutuzov.viktor.84 updated this object.
kutuzov.viktor.84 edited the test plan for this revision. (Show Details)
kutuzov.viktor.84 added a subscriber: Unknown Object (MLST).
emaste added inline comments.Nov 26 2014, 5:50 AM
lib/sanitizer_common/sanitizer_common_interceptors.inc
906–908 ↗(On Diff #16647)

Just to confirm, the issue reported in this comment is not directly tied to the new *printf_l functionality?

eugenis edited edge metadata.Nov 27 2014, 12:18 AM

Please add a test, either in test/asan or in lib/asan/tests/asan_test.cc

lib/sanitizer_common/sanitizer_common_interceptors.inc
892 ↗(On Diff #16647)

Why not use VSNPRINTF_INTERCEPTOR_IMPL directly?

Also, what about all the other *printf*_l functions? They do not have to be done in 1 patch, but you'll probably run into issues with them at some point, too.

906–908 ↗(On Diff #16647)

It's not.

kutuzov.viktor.84 edited edge metadata.

Rewritten with VSNPRINTF_INTERCEPTOR_IMPL.

Also, what about all the other *printf*_l functions? They do not have to be done in 1 patch, but you'll probably run into issues with them at some point, too.

Yes, I've added them to my TODO and will provide another patch for the rest of the funcs some time later.

Now it seems I have to add a test case to this patch...

eugenis added inline comments.Dec 18 2014, 7:42 AM
lib/asan/tests/asan_test.cc
1289 ↗(On Diff #17442)

Capital W after an underscore looks unusual.

1298 ↗(On Diff #17442)

This does not really test much (does it fail before this patch?).
See test/asan/TestCases/printf-*.c for inspiration. No need to repeat all 5 of them for the _l variant, but please add something that verifies that the whole prinf sanitizing machinery is applied in this case.

Updated. Thanks, Evgeniy.

eugenis accepted this revision.Dec 19 2014, 4:45 AM
eugenis edited edge metadata.

LGTM

This revision is now accepted and ready to land.Dec 19 2014, 4:45 AM
This revision was automatically updated to reflect the committed changes.