-
-
Notifications
You must be signed in to change notification settings - Fork 421
core.stdcpp.allocator: _Adjust_manually_vector_aligned checks for sentinel unconditionally (Windows only) #3265
Conversation
|
Thanks for your pull request, @n8sh! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "stable + druntime#3265" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for isolating this!
544516a to
6a0b560
Compare
|
Hmm, the test seems to hang on windows |
My guess is that this is because the test is actually failing on Windows and an "abort or debug?" prompt is appearing. IIRC that happened recently on another PR. My guess for why a failure would be is happening is a difference in that, unlike xutility.d, allocator.d does not infer where to use _DEBUG behavior based on mscrtlib . I'll see if changing it fixes this. |
|
nope. |
…ned checks for sentinel unconditionally (Windows only) This bug resulted in assertion failures when deleting large blocks of memory using core.stdcpp.allocator on Windows. _Allocate_manually_vector_aligned only sets a sentinel when version(_DEBUG) so _Adjust_manually_vector_aligned should only check for this sentinel when version(_DEBUG). Additionally change the linkage of those functions from C++ to D to avoid possible linker confusion. Also changed the logic by which allocator.d infers _DEBUG to resemble the the logic for _ITERATOR_DEBUG_LEVEL in utility.d.
|
Should be working now. |
| else | ||
| { | ||
| import core.stdcpp.xutility : __CXXLIB__; | ||
| enum _DEBUG = __CXXLIB__.length && 'd' == __CXXLIB__[$-1]; // libcmtd, msvcrtd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is good. If the logic that drives the codegen doesn't match C++, then you'll get weird link errors that are hard for a user to diagnose.
This bug resulted in assertion failures when deleting large blocks of memory using core.stdcpp.allocator on Windows.
_Allocate_manually_vector_alignedonly sets a sentinel whenversion(_DEBUG)so_Adjust_manually_vector_alignedshould only check for this sentinel whenversion(_DEBUG).Additionally change the linkage of those functions from C++ to D to avoid possible linker confusion.