Skip to content
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

invalid free in MSVCP100.dll in Cr Release on exit on several tests #1431

Closed
derekbruening opened this issue Nov 28, 2014 · 3 comments
Closed

Comments

@derekbruening
Copy link
Contributor

From [email protected] on January 30, 2014 11:12:57

My local Release tests never saw this, and I can't reproduce it at all, but
after switching the FYI bots over several tests hit this same thing: http://build.chromium.org/p/chromium.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%283%29/builds/215/steps/memory%20test%3A%20ipc_tests/logs/stdio [ RUN ] IPCFuzzingTest.SanityTest
Valgrind detected, switching to single process mode.
Pass --test-launcher-debug-launcher to valgrind the launcher itself.
[4432:4232:0129/182450:11910645:WARNING:ipc_fuzzing_tests.cc(190)] IPC fuzzer:458774 [43 expect 43]

[4432:4232:0129/182450:11910660:WARNING:ipc_fuzzing_tests.cc(190)] IPC fuzzer:458777 [44 expect 44]

Dr.M
Dr.M Error #1: INVALID HEAP ARGUMENT to free 0x02ce0990
Dr.M # 0 replace_free [e:\b\build\slave\win-builder\drmemory\common\alloc_replace.c:2352]
Dr.M # 1 MSVCP100.dll!std::ctype<>::ctype<> +0x19 (0x6d38c833 <MSVCP100.dll+0xc833>)
Dr.M # 2 std::_Fac_node::
_Fac_node [f:\dd\vctools\crt_bld\self_x86\crt\src\locale0.cpp:23]
Dr.M # 3 MSVCR100.dll!_query_new_mode +0x240 (0x6d2e7a3c <MSVCR100.dll+0x27a3c>)
Dr.M # 4 MSVCR100.dll!exit +0x10 (0x6d2e7b1d <MSVCR100.dll+0x27b1d>)
Dr.M # 5 __tmainCRTStartup [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c:566]
Dr.M # 6 KERNEL32.dll!BaseThreadInitThunk +0x11 (0x75d2336a <KERNEL32.dll+0x1336a>)
Dr.M Note: @0:00:01.264 in thread 4232
[ OK ] IPCFuzzingTest.SanityTest (1419 ms)

At the end of unit_tests: http://build.chromium.org/p/chromium.fyi/builders/Windows&#37;20Unit&#37;20&#37;28DrMemory&#37;20full&#37;29&#37;20&#37;284&#37;29/builds/150/steps/memory&#37;20test&#37;3A&#37;20unit/logs/stdio Dr.M Error #1: INVALID HEAP ARGUMENT to free 0x071bc3a0
Dr.M # 0 replace_free [e:\b\build\slave\win-builder\drmemory\common\alloc_replace.c:2352]
Dr.M # 1 MSVCP100.dll!std::ctype<>::_Tidy +0xf (0x6d72c54f <MSVCP100.dll+0xc54f>)
Dr.M # 2 std::_Fac_node::~_Fac_node [f:\dd\vctools\crt_bld\self_x86\crt\src\locale0.cpp:23]
Dr.M # 3 MSVCR100.dll!_query_new_mode +0x240 (0x6d197a3c <MSVCR100.dll+0x27a3c>)
Dr.M # 4 MSVCR100.dll!exit +0x10 (0x6d197b1d <MSVCR100.dll+0x27b1d>)
Dr.M # 5 __tmainCRTStartup [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c:566]
Dr.M # 6 KERNEL32.dll!BaseThreadInitThunk +0x11 (0x7521339a <KERNEL32.dll+0x1339a>)
Dr.M Note: @0:24:31.860 in thread 3948
Dr.M Note: next higher malloc: 0x071bc3a0-0x071bc5a0

That "next higher malloc" doesn't make sense.

Xref issue #1223

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

@derekbruening
Copy link
Contributor Author

From [email protected] on January 30, 2014 10:52:30

I can repro on the bot with:

_NT_SYMBOL_PATH=e:/b third_party/drmemory/unpacked/bin/drmemory.exe -no_fetch_symbols -no_use_symcache -light -- out/Release/ipc_tests.exe --gtest_filter=IPCFuzzingTest.SanityTest

And I can repro locally with that too: with no symbols.

So w/ symbols we intercept earlier, at MSVCR100!_calloc_impl, and we insert
our Heap for MSVCR100.dll. w/o symbols, the app goes and passes some other
Heap as we intercept at RtlAllocateHeap, but on free it calls libc free and
we there impose our libc Heap.

The question is, why didn't we find msvcr100!_crtheap and use it instead of
creating our own Heap?

Here's the answer:

    /* Determine the pre-us Heap for this pre-existing module, if
     * any (issue #959).
     */
    if (!process_initialized) {
        pre_us_heap = libc_heap_handle(mod);
        LOG(2, "pre-existing Heap for libc set type=%d module=%s is "PFX"\n",
            type, (dr_module_preferred_name(mod) == NULL) ? "<null>" :
            dr_module_preferred_name(mod), pre_us_heap);

And:

/* Indicates whether process initialization is fully complete, including

  • iteration of modules. Thus, we don't set this until we get the
  • first thread init event.
    */
    static bool process_initialized;

So DR issue #1339 in r2428 , which re-ordered the initial thread event to be
prior to the initial module iteration, broke our code here! This is a
regression.

@derekbruening
Copy link
Contributor Author

From [email protected] on January 30, 2014 11:53:02

So I can't use thread init event to say "process initialized" now.
I guess I have to use bb event. To reduce overhead I'll remove it at 1st
call => hit DRi#1356.

***** TODO after fix, get a different error there

Dr.M Error #1: INVALID HEAP ARGUMENT: allocated with Windows API layer, freed with C library layer
Dr.M # 0 drmemorylib.dll!replace_free [d:\derek\drmemory\git\src\common\alloc_replace.c:2352]
Dr.M # 1 MSVCP100.dll!std::ctype<>::ctype<> +0x19 (0x6fd3c833 <MSVCP100.dll+0xc833>)
Dr.M # 2 std::_Fac_node::
_Fac_node [f:\dd\vctools\crt_bld\self_x86\crt\src\locale0.cpp:23]
Dr.M # 3 MSVCR100.dll!_query_new_mode +0x240 (0x70637a3c <MSVCR100.dll+0x27a3c>)
Dr.M # 4 MSVCR100.dll!exit +0x10 (0x70637b1d <MSVCR100.dll+0x27b1d>)

=> issue #960 , right? Just need to expand the suppression.

=>
SUPPRESSIONS USED:
1x: default issue #960 CRT mismatch B
14x: default issue #960 CRT mismatch

@derekbruening
Copy link
Contributor Author

From [email protected] on January 31, 2014 13:20:41

This issue was closed by revision r1691 .

Status: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant