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

<memory>: _msize & std::allocator don't work together #966

Closed
AlexGuteniev opened this issue Jul 2, 2020 · 5 comments
Closed

<memory>: _msize & std::allocator don't work together #966

AlexGuteniev opened this issue Jul 2, 2020 · 5 comments
Labels
bug Something isn't working

Comments

@AlexGuteniev
Copy link
Contributor

AlexGuteniev commented Jul 2, 2020

Describe the bug
_msize does not work with std::allocator.

std::allocator allocates large buffer with own over-alignment:

__declspec(allocator) void* _Allocate_manually_vector_aligned(const size_t _Bytes) {

so its buffer is not direct malloc

Command-line test case


d:\Temp2>type repro.cpp
#include <memory>
#include <stdio.h>

int main() {
    std::allocator<std::int64_t> a;
    auto p = a.allocate(256 * 256 * 1024);
    auto c = _msize(p); // fails in IsValidHeapPointer
    printf("the execution does not reach here because of the crash");
    return 0;
}
d:\Temp2>cl /MDd repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.27.29009.1 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.27.29009.1
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

d:\Temp2>.\repro.exe

d:\Temp2>

Expected behavior
Should be direct malloc

STL version

Microsoft Visual Studio Professional 2019 Preview
Version 16.7.0 Preview 3.1

Additional context
This item is also tracked on Developer Community as DevCom-380760 and by Microsoft-internal VSO-729275 / AB#729275.

@AlexGuteniev
Copy link
Contributor Author

I think it would go as enhancement, wontfix

@StephanTLavavej StephanTLavavej added bug Something isn't working decision needed We need to choose something before working on this labels Jul 2, 2020
@StephanTLavavej
Copy link
Member

Thanks - I think I'll tag it as bug because the report is that the code doesn't work - but as it's a request for an obscure non-Standard extension to work with std::allocator (where we implemented the over-alignment in conjunction with the compiler backend team to be friendly to autovectorization), I agree that it should probably be wontfix - marking as decision needed to make that determination. Certainly at this point we can't change std::allocator's alignment behavior without breaking ABI.

@AlexGuteniev
Copy link
Contributor Author

AlexGuteniev commented Jul 2, 2020

In theory I think it is possible to make both vector<T, std::allocator<T>> using over-alignment, and std::allocator<T> be compatible with malloc when used outside of vector. But due to obscure control flow, it may be the worst resolution.

@AlexGuteniev
Copy link
Contributor Author

My understanding that since the Standard permits std::allocator to be not direct operator new (for allocations elimination), users who want direct malloc under hood should create their own allocator for that purpose.

@StephanTLavavej StephanTLavavej removed the decision needed We need to choose something before working on this label Nov 2, 2022
@StephanTLavavej
Copy link
Member

We talked about this at the weekly maintainer meeting and decided that this is not a scenario that we should support. std::allocator is fundamentally different from malloc and we've been doing extra work beyond malloc since VS 2015.

@StephanTLavavej StephanTLavavej closed this as not planned Won't fix, can't repro, duplicate, stale Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants