replace wide char versions of string and mem routines

[derekbruening]

From bruen...@google.com on April 19, 2011 11:49:19

should replace wmemcmp, wcslen, wmemchr, wmemcpy, wmemset, wcscmp, wcsncmp

Original issue: http://code.google.com/p/drmemory/issues/detail?id=350

[derekbruening]

From timurrrr@google.com on April 25, 2011 05:27:25

The implementation should be trivial.

Do you have any particular app or test that has false positives on these functions? Not sure how to test it better, I couldn't reproduce false positives yet.

btw, (how) did you test the non-wide memcmp, memchr, ... functions?

Status: Started
Owner: timurrrr@google.com
Cc: bruen...@google.com

[derekbruening]

From bruen...@google.com on April 25, 2011 07:08:20

patterns.c has some string/mem routine usage back from when I pattern-matched. don't have nice unit tests of string/mem corner-cases: spec2k6 and other large apps have been the tests in the past.

in net_unittests (did not confirm this is from optimized routine, could be data from unhandled syscall instead):

Error #3634: UNINITIALIZED READ: reading register edx @0:06:42.658 in thread 3252 0x01a88dfb <net_unittests.exe+0xc38dfb> net_unittests.exe!wcslen f:\dd\vctools\crt_bld\self_x86\crt\src\wcslen.c:44
0x00f16b9c <net_unittests.exe+0xc6b9c> net_unittests.exe!std::char_traits::length c:\program files (x86)\microsoft visual studio 9.0\vc\include\iosfwd:334
0x0101e17d <net_unittests.exe+0x1ce17d> net_unittests.exe!std::basic_string<wchar_t,std::char_traits,std::allocator >::assign c:\program files (x86)\microsoft visual studio 9.0\vc\include\xstring:1085
0x0101de3f <net_unittests.exe+0x1cde3f> net_unittests.exe!std::basic_string<wchar_t,std::char_traits,std::allocator >::basic_string<wchar_t,std::char_traits,std::allocator > c:\program files (x86)\microsoft visual studio 9.0\vc\include\xstring:653
0x01791771 <net_unittests.exe+0x941771> net_unittests.exe!base::Time::FromString e:\src\chromium\src\base\time.cc:103
0x01890719 <net_unittests.exe+0xa40719> net_unittests.exe!net::HttpResponseHeaders::GetTimeValuedHeader e:\src\chromium\src\net\http\http_response_headers.cc:1051
0x018905a8 <net_unittests.exe+0xa405a8> net_unittests.exe!net::HttpResponseHeaders::GetDateValue e:\src\chromium\src\net\http\http_response_headers.cc:1033
0x0188ffc5 <net_unittests.exe+0xa3ffc5> net_unittests.exe!net::HttpResponseHeaders::GetCurrentAge e:\src\chromium\src\net\http\http_response_headers.cc:977
0x0188f983 <net_unittests.exe+0xa3f983> net_unittests.exe!net::HttpResponseHeaders::RequiresValidation e:\src\chromium\src\net\http\http_response_headers.cc:847
0x01a0cf56 <net_unittests.exe+0xbbcf56> net_unittests.exe!net::HttpCache::Transaction::RequiresValidation e:\src\chromium\src\net\http\http_cache_transaction.cc:1600
0x01a0c167 <net_unittests.exe+0xbbc167> net_unittests.exe!net::HttpCache::Transaction::BeginCacheValidation e:\src\chromium\src\net\http\http_cache_transaction.cc:1443
0x01a0c384 <net_unittests.exe+0xbbc384> net_unittests.exe!net::HttpCache::Transaction::BeginPartialCacheValidation e:\src\chromium\src\net\http\http_cache_transaction.cc:1477

[derekbruening]

From timurrrr@google.com on April 26, 2011 06:13:58

Done in r260 : wmemcmp, wcslen, wmemchr, wmemcpy, wmemset, wcscmp, wcsncmp

Maybe we should also replace: wcschr, wcsrchr, wcscpy, wcsncpy, wcscat, wcsncat, wmemmove and non-wide functions: strlcat, strstr, stpcpy (probably on ad-hoc basis)

Labels: -Priority-Medium Priority-Low

[derekbruening]

From timurrrr@google.com on May 17, 2011 07:59:36

lstrcmpW and other lstr* functions? Oooh it looks harder this time: they need to access the current locale http://msdn.microsoft.com/en-us/library/ms647488(v=vs.85).aspx We can do a different approach, like the one we use in TSan https://code.google.com/p/data-race-test/source/browse/trunk/tsan/ts_replace.h?spec=svn3450&r=3245#95 -> the memory accesses in the replace routines are ignored and they manually notify the tool about the read/write accesses.

WDYT about the "different approach"?

[derekbruening]

From bruen...@google.com on May 23, 2011 07:53:10

(project was read-only when I tried to edit last week)

let's wait and see if these cause any problems: they may not be as optimized.

[derekbruening]

From bruen...@google.com on June 06, 2013 20:16:30

A user reported on the mailing list that VS2012 wcsstr is optimized enough to need replacing (xref issue #1243 ):

int _tmain(int argc, _TCHAR* argv[]) { wstring s(L"foo\r\nbar"); const wchar_t* p1 = wcsstr(s.c_str(), L"\r\n"); const wchar_t* p2 = wcsstr(L"foo\r\nbar", L"\r\n"); return 0; }

First call to wcsstr gives error, second does not:

     Dr. Memory version 1.5.1
     Running ""C:\Users\paul\Documents\Visual Studio 2012\Projects\ConsoleApplication2\Release\ConsoleApplication2.exe""

     Error `#1`: UNINITIALIZED READ: reading 0x0047b95a-0x0047b95e 4 byte(s) within 0x0047b94e-0x0047b95e
     MSVCR110.dll!wcsstr      
         ??:0
     wmain                  
         c:\users\paul\documents\visual studio 2012\projects\consoleapplication2\consoleapplication2\consoleapplication2.cpp(58):
     Note: @0:00:00.194 in thread 9272
     Note: instruction: movdqu (%edi) -> %xmm1

[derekbruening]

We added interceptors for most of these a long time ago. The only one missing was wcsnlen which is now resulting in false positives with VS2015: #1872, #1869. Let's close this and let #1869 cover wscnlen.

[derekbruening]

Actually we're still missing a couple:

/* XXX i#350: add wrappers for wcsncpy, wcscat,
 * wcsncat, wmemmove.
 */