search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.07k stars 11.99k forks source link

DynamicLibrary Shutdown unit test crashes when built with /MTd #33804

Open jh7370 opened 7 years ago

jh7370 commented 7 years ago
Bugzilla Link 34456
Version trunk
OS Windows NT

Extended Description

In DynamicLibraryTest.cpp, the unit test "Shutdown" crashes if building with Visual Studio and the cmake variable "LLVM_USE_CRT_DEBUG" is set to /MTd.

The problem appears to be that in this mode, allocating in the main executable's heap and destroying from the DLL (or vice versa) is not allowed. During the test, a vector owned by the main executable is resized by the DLL, resulting in the crash as it deallocates the original vector element.

I attempted to reserve space in the vector to fix the issue, but then the copying of the string within push_back (called during the Global destructor in PipSqueak) resulted in a memory allocation in the DLL, leading to a crash when destroying the vector in the main executable.

Endilll commented 1 year ago

@jh7370 can you reproduce this on trunk?

jh7370 commented 1 year ago

@jh7370 can you reproduce this on trunk?

Unfortunately, I don't have a working LLVM tree at the moment, where I could attempt to reproduce the issue. I also have little to no memory of this incident, so really only have what's in this bug to draw on. Taking a quick look at stuff related to PipSqueak, I stumbled on a couple of possibly related comments in the Support unittests cmake files: From https://github.com/llvm/llvm-project/blob/9deee6bffa9c331f46c68e5dd4cb4abf93dc0716/llvm/unittests/Support/CMakeLists.txt#L134 (introduced in commit 95fb354):

The test doesn't pass when using a custom allocator, PR47881.

and from https://github.com/llvm/llvm-project/blob/9deee6bffa9c331f46c68e5dd4cb4abf93dc0716/llvm/unittests/Support/DynamicLibrary/CMakeLists.txt#L43 with a6a37a2 and b9b954b related to it:

We need to link in the Support lib for the Memory allocator override, otherwise the DynamicLibrary.Shutdown test will fail, because it would allocate memory with the CRT allocator, and release it with our custom allocator (see llvm/lib/Support/Windows/Memory.inc).

I wouldn't be surprised if the commit a6a37a2 fixed the original issue, but I have no way of verifying that.