DynamoRIO / DynamoRIO/drmemory

heap mismatch due to VS2015 aliasing to delete[] a function that calls delete

Open
#1,874 0 comments 0 reactions 0 assignees View on GitHub
Bug-FalsePositive Hotlist-Release OpSys-Windows
Dominant language
C
Stars
2.7k
Forks
290
PR merge metrics
No merged PRs in 30d

Description

While investigating #1870, running outside of the chrome_tests script, content_unittests FileSystemDirURLRequestJobTest.DirectoryListing (and probably any other test) has this mismatch reported:

```
[==========] 1 test from 1 test case ran. (3714 ms total)
[ PASSED ] 1 test.
~~Dr.M~~
~~Dr.M~~ Error #1: INVALID HEAP ARGUMENT: allocated with operator new, freed with operator delete[]
~~Dr.M~~ # 0 drmemorylib.dll!replace_operator_delete_array [d:\derek\drmemory\git\src\common\alloc_replace.c:2998]
~~Dr.M~~ # 1 v8.dll!v8::internal::DateCache::`scalar deleting destructor'
~~Dr.M~~ # 2 v8.dll!v8::internal::Isolate::~Isolate [z:\derek\chromium\src\v8\src\isolate.cc:2016]
~~Dr.M~~ # 3 v8.dll!v8::internal::Isolate::TearDown [z:\derek\chromium\src\v8\src\isolate.cc:1908]
~~Dr.M~~ # 4 webcore_shared.dll!blink::V8PerIsolateData::~V8PerIsolateData [z:\derek\chromium\src\third_party\webkit\source\bindings\core\v8\v8perisolatedata.cpp:151]
~~Dr.M~~ # 5 webcore_shared.dll!blink::V8Initializer::shutdownMainThread [z:\derek\chromium\src\third_party\webkit\source\bindings\core\v8\v8initializer.cpp:398]
~~Dr.M~~ # 6 content::UnitTestTestSuite::~UnitTestTestSuite [z:\derek\chromium\src\content\public\test\unittest_test_suite.cc:45]
~~Dr.M~~ # 7 __scrt_common_main_seh [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:264]
~~Dr.M~~ # 8 KERNEL32.dll!BaseThreadInitThunk
~~Dr.M~~ Note: @0:00:30.576 in thread 3700
~~Dr.M~~ Note: memory was allocated here:
~~Dr.M~~ Note: # 0 drmemorylib.dll!replace_operator_new [d:\derek\drmemory\git\src\common\alloc_replace.c:2899]
~~Dr.M~~ Note: # 1 v8.dll!v8::base::OS::CreateTimezoneCache [z:\derek\chromium\src\v8\src\base\platform\platform-win32.cc:454]
~~Dr.M~~ Note: # 2 v8.dll!v8::internal::Snapshot::Initialize [z:\derek\chromium\src\v8\src\snapshot\snapshot-common.cc:61]
~~Dr.M~~ Note: # 3 v8.dll!v8::Isolate::New [z:\derek\chromium\src\v8\src\api.cc:7277]
~~Dr.M~~ Note: # 4 gin.dll!gin::IsolateHolder::IsolateHolder [z:\derek\chromium\src\gin\isolate_holder.cc:41]
~~Dr.M~~ Note: # 5 gin.dll!gin::IsolateHolder::IsolateHolder [z:\derek\chromium\src\gin\isolate_holder.cc:28]
~~Dr.M~~ Note: # 6 content::TestBlinkWebUnitTestSupport::TestBlinkWebUnitTestSupport [z:\derek\chromium\src\content\test\test_blink_web_unit_test_support.cc:121]
~~Dr.M~~ Note: # 7 content::UnitTestTestSuite::UnitTestTestSuite [z:\derek\chromium\src\content\public\test\unittest_test_suite.cc:39]
~~Dr.M~~ Note: # 8 main [z:\derek\chromium\src\content\test\run_all_unittests.cc:26]
```

Relevant source code:

```
return new TimezoneCache();

delete date_cache_;

void OS::DisposeTimezoneCache(TimezoneCache* cache) {
delete cache;
}
```

Analysis:

```
% grep delete logs/symcache/v8.dll.txt
operator delete,0x10db3b
operator delete[] nothrow,0x0
operator delete[],0x10afb0
operator delete nothrow,0x0

0:000> x v8!operator delete*
6565f11d v8!operator delete (void *, unsigned int)
6565db3b v8!operator delete (void *)
6565afb0 v8!operator delete[] (void *)

intercepting operator delete @0x6565db3b type 12 in module v8.dll
intercepting operator delete[] @0x6565afb0 type 13 in module v8.dll

in event_basic_block(tag=0x6566f84c)
in event_basic_block(tag=0x6566fd70)
set range 0x003af990-0x003af994 => 0x0
replace_operator_delete_array 0x043b8c90
replace_operator_delete 0x043b8c90
addr_is_debug_delete: 0x6566fd82 => v8::internal::DateCache::
addr_is_debug_delete: 0x6565db3b => operator delete

Error #1: INVALID HEAP ARGUMENT: allocated with operator new, freed with operator delete[]
#0 drmemorylib.dll!replace_operator_delete_array [d:\derek\drmemory\git\src\common\alloc_replace.c:2998] (0x73964130 ) modid:117
#1 v8.dll!v8::internal::DateCache::`scalar deleting destructor' (0x6566fd87 ) modid:57
#2 v8.dll!v8::internal::Isolate::~Isolate [z:\derek\chromium\src\v8\src\isolate.cc:2016] (0x6566f852 ) modid:57

v8!v8::internal::DateCache::`scalar deleting destructor':
6566fd70 55 push ebp
6566fd71 8bec mov ebp,esp
6566fd73 56 push esi
6566fd74 8bf1 mov esi,ecx
6566fd76 ffb62c020000 push dword ptr [esi+22Ch]
6566fd7c c70648039e65 mov dword ptr [esi],offset v8!v8::internal::DateCache::`vftable' (659e0348)
6566fd82 e829b2feff call v8!v8::base::OS::DisposeTimezoneCache (6565afb0)
6566fd87

v8!v8::base::OS::DisposeTimezoneCache [z:\derek\chromium\src\v8\src\base\platform\platform-win32.cc @ 458]:
6565afb0 e9862b0000 jmp v8!operator delete (6565db3b)

v8!operator delete [f:\dd\vctools\crt\vcstartup\src\heap\delete_scalar.cpp @ 15]:
6565db3b e998160000 jmp v8!free (6565f1d8)
```

So v8::base::OS::DisposeTimezoneCache == operator delete[]. This is hard to solve, b/c here we have separate delete and delete[] and it looks at the surface like we can successfully detect mismatches. How can we possibly know that the compiler is going to place DisposeTimezoneCache where delete[] is instead of delete? In some sense it seems like a bug, though with no regular consequences.

This will not show up on Cr bots b/c we run with -no_check_delete_mismatch for Release build.
That was due to another related issue in v8: #1625.

For non-Cr DrMem: it is not clear what we can do other than suggest that heap mismatch only be enabled for non-VS-optimized builds or something.

This just adds more nails in the coffin of the whole delete vs delete[] mismatch detection feature with Visual Studio: the compiler's lack of concern for operator separation makes it impractical.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.