Reduce memory consumption of system_category().message() (#457)

This avoids the allocation in _Winerror_message, and introduces a new version of it which uses FORMAT_MESSAGE_ALLOCATE_BUFFER to completely avoid overallocating memory

Fixes #434

Co-authored-by: Billy O'Neal <billy.oneal@gmail.com>
Co-authored-by: Casey Carter <cartec69@gmail.com>

stl/CMakeLists.txt

@@ -3,6 +3,7 @@
 
 set(HEADERS
     ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_all_public_headers.hpp
+    ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_system_error_abi.hpp
     ${CMAKE_CURRENT_LIST_DIR}/inc/algorithm
     ${CMAKE_CURRENT_LIST_DIR}/inc/any
     ${CMAKE_CURRENT_LIST_DIR}/inc/array
@@ -242,6 +243,7 @@ set(IMPLIB_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/nothrow.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/parallel_algorithms.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/sharedmutex.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/syserror_import_lib.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/vector_algorithms.cpp
 )
 

stl/inc/__msvc_all_public_headers.hpp

@@ -6,8 +6,8 @@
 //
 // This file may be changed, renamed, or removed at any time.
 
-#ifndef _MSVC_ALL_PUBLIC_HEADERS_HPP
-#define _MSVC_ALL_PUBLIC_HEADERS_HPP
+#ifndef __MSVC_ALL_PUBLIC_HEADERS_HPP
+#define __MSVC_ALL_PUBLIC_HEADERS_HPP
 
 #pragma warning(push)
 #pragma warning(1 : 4668) // 'MEOW' is not defined as a preprocessor macro, replacing with '0' for '#if/#elif'
@@ -211,4 +211,4 @@
 
 #pragma warning(pop)
 
-#endif // _MSVC_ALL_PUBLIC_HEADERS_HPP
+#endif // __MSVC_ALL_PUBLIC_HEADERS_HPP

stl/inc/__msvc_system_error_abi.hpp

@@ -0,0 +1,41 @@
+// __msvc_system_error_abi.hpp internal header (core)
+
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#pragma once
+#ifndef __MSVC_SYSTEM_ERROR_ABI_HPP
+#define __MSVC_SYSTEM_ERROR_ABI_HPP
+#include <yvals_core.h>
+#if _STL_COMPILER_PREPROCESSOR
+#include <stddef.h>
+
+#pragma pack(push, _CRT_PACKING)
+#pragma warning(push, _STL_WARNING_LEVEL)
+#pragma warning(disable : _STL_DISABLED_WARNINGS)
+_STL_DISABLE_CLANG_WARNINGS
+#pragma push_macro("new")
+#undef new
+
+#ifdef _M_CEE_PURE
+#define __CLRCALL_PURE_OR_STDCALL __clrcall
+#else
+#define __CLRCALL_PURE_OR_STDCALL __stdcall
+#endif
+
+_EXTERN_C
+_NODISCARD size_t __CLRCALL_PURE_OR_STDCALL __std_get_string_size_without_trailing_whitespace(
+    const char* _Str, size_t _Size) noexcept;
+
+_NODISCARD size_t __CLRCALL_PURE_OR_STDCALL __std_system_error_allocate_message(
+    unsigned long _Message_id, char** _Ptr_str) noexcept;
+void __CLRCALL_PURE_OR_STDCALL __std_system_error_deallocate_message(char* _Str) noexcept;
+_END_EXTERN_C
+
+#pragma pop_macro("new")
+_STL_RESTORE_CLANG_WARNINGS
+#pragma warning(pop)
+#pragma pack(pop)
+
+#endif // _STL_COMPILER_PREPROCESSOR
+#endif // __MSVC_SYSTEM_ERROR_ABI_HPP

stl/inc/system_error

@@ -8,9 +8,10 @@
 #define _SYSTEM_ERROR_
 #include <yvals_core.h>
 #if _STL_COMPILER_PREPROCESSOR
+#include <__msvc_system_error_abi.hpp>
 #include <cerrno>
-#include <cstdlib> // for strerror
-#include <stdexcept> // for runtime_error
+#include <cstdlib>
+#include <stdexcept>
 #include <xcall_once.h>
 #include <xerrc.h>
 
@@ -432,8 +433,21 @@ protected:
 
 _CRTIMP2_PURE const char* __CLRCALL_PURE_OR_CDECL _Syserror_map(int);
 _CRTIMP2_PURE int __CLRCALL_PURE_OR_CDECL _Winerror_map(int);
-_CRTIMP2_PURE unsigned long __CLRCALL_PURE_OR_CDECL _Winerror_message(
-    unsigned long _Message_id, char* _Narrow, unsigned long _Size);
+
+struct _System_error_message {
+    char* _Str;
+    size_t _Length;
+
+    explicit _System_error_message(const unsigned long _Ec) noexcept
+        : _Str(nullptr), _Length(_CSTD __std_system_error_allocate_message(_Ec, &_Str)) {}
+
+    _System_error_message(const _System_error_message&) = delete;
+    _System_error_message& operator=(const _System_error_message&) = delete;
+
+    ~_System_error_message() {
+        _CSTD __std_system_error_deallocate_message(_Str);
+    }
+};
 
 // CLASS _Generic_error_category
 class _Generic_error_category : public error_category { // categorize a generic error
@@ -464,7 +478,9 @@ public:
 
     _NODISCARD virtual string message(int _Errcode) const override {
         if (_Errcode == static_cast<int>(io_errc::stream)) {
-            return "iostream stream error";
+            static constexpr char _Iostream_error[] = "iostream stream error";
+            constexpr size_t _Iostream_error_length = sizeof(_Iostream_error) - 1; // TRANSITION, DevCom-906503
+            return string(_Iostream_error, _Iostream_error_length);
         } else {
             return _Generic_error_category::message(_Errcode);
         }
@@ -483,18 +499,14 @@ public:
     }
 
     _NODISCARD virtual string message(int _Errcode) const override {
-        const unsigned long _Size = 32767;
-        string _Narrow(_Size, '\0');
-
-        const unsigned long _Val = _Winerror_message(static_cast<unsigned long>(_Errcode), &_Narrow[0], _Size);
-        if (_Val == 0) {
-            _Narrow = "unknown error";
+        const _System_error_message _Msg(static_cast<unsigned long>(_Errcode));
+        if (_Msg._Length == 0) {
+            static constexpr char _Unknown_error[] = "unknown error";
+            constexpr size_t _Unknown_error_length = sizeof(_Unknown_error) - 1; // TRANSITION, DevCom-906503
+            return string(_Unknown_error, _Unknown_error_length);
         } else {
-            _Narrow.resize(_Val);
+            return string(_Msg._Str, _Msg._Length);
         }
-
-        _Narrow.shrink_to_fit();
-        return _Narrow;
     }
 
     _NODISCARD virtual error_condition default_error_condition(int _Errval) const noexcept override {

stl/msbuild/stl_base/stl.files.settings.targets

@@ -173,6 +173,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
             $(CrtRoot)\github\stl\src\nothrow.cpp;
             $(CrtRoot)\github\stl\src\parallel_algorithms.cpp;
             $(CrtRoot)\github\stl\src\sharedmutex.cpp;
+            $(CrtRoot)\github\stl\src\syserror_import_lib.cpp;
             $(CrtRoot)\github\stl\src\vector_algorithms.cpp;
             ">
             <BuildAs>nativecpp</BuildAs>

stl/src/syserror.cpp

@@ -3,19 +3,24 @@
 
 // system_error message mapping
 
-#include <cstdlib>
-#include <memory>
 #include <system_error>
 
 #include <Windows.h>
 
+// TRANSITION, MSBuild
+// MSBuild has a hard requirement against including the same file in both a DLL and its import lib, so we include
+// the import lib .cpp here to make those functions available for internal use by other parts of our DLL.
+#ifdef _DLL
+#include "syserror_import_lib.cpp"
+#endif
+
 _STD_BEGIN
 struct _Win_errtab_t { // maps Windows error to Posix error
     int _Windows;
     errc _Posix;
 };
 
-static const _Win_errtab_t _Win_errtab[] = {
+static constexpr _Win_errtab_t _Win_errtab[] = {
     // table of Windows/Posix pairs
     {ERROR_ACCESS_DENIED, errc::permission_denied},
     {ERROR_ALREADY_EXISTS, errc::file_exists},
@@ -108,31 +113,15 @@ _CRTIMP2_PURE int __CLRCALL_PURE_OR_CDECL _Winerror_map(int _Errcode) {
     return 0;
 }
 
+// TRANSITION, ABI: _Winerror_message() is preserved for binary compatibility
 _CRTIMP2_PURE unsigned long __CLRCALL_PURE_OR_CDECL _Winerror_message(
     unsigned long _Message_id, char* _Narrow, unsigned long _Size) {
     // convert to name of Windows error, return 0 for failure, otherwise return number of chars written
     // pre: _Size < INT_MAX
-    const unique_ptr<wchar_t[]> _Wide(new wchar_t[_Size]); // not using make_unique because we want default-init
-    const auto _First = _Wide.get();
-    unsigned long _Wide_chars =
-        FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, 0, _Message_id, 0, _First, _Size, 0);
+    const unsigned long _Chars = FormatMessageA(
+        FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, _Message_id, 0, _Narrow, _Size, nullptr);
 
-    // remove any trailing whitespace
-    while (_Wide_chars != 0) {
-        switch (_First[--_Wide_chars]) {
-        case L' ':
-        case L'\n':
-        case L'\r':
-        case L'\t':
-        case L'\0':
-            continue;
-        default:
-            return static_cast<unsigned long>(
-                WideCharToMultiByte(CP_ACP, 0, _First, _Wide_chars + 1, _Narrow, _Size, 0, 0));
-        }
-    }
-
-    return 0;
+    return static_cast<unsigned long>(_CSTD __std_get_string_size_without_trailing_whitespace(_Narrow, _Chars));
 }
 
 struct _Sys_errtab_t { // maps error_code to NTBS
@@ -140,7 +129,7 @@ struct _Sys_errtab_t { // maps error_code to NTBS
     const char* _Name;
 };
 
-static const _Sys_errtab_t _Sys_errtab[] = {
+static constexpr _Sys_errtab_t _Sys_errtab[] = {
     // table of Posix code/name pairs
     {errc::address_family_not_supported, "address family not supported"},
     {errc::address_in_use, "address in use"},

stl/src/syserror_import_lib.cpp

@@ -0,0 +1,58 @@
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+// This must be as small as possible, because its contents are
+// injected into the msvcprt.lib and msvcprtd.lib import libraries.
+// Do not include or define anything else here.
+// In particular, basic_string must not be included here.
+
+#include <__msvc_system_error_abi.hpp>
+
+#include <Windows.h>
+
+namespace {
+    struct _Whitespace_bitmap_t {
+        bool _Is_whitespace[256];
+
+        constexpr _Whitespace_bitmap_t() noexcept : _Is_whitespace{} {
+            _Is_whitespace[' ']  = true;
+            _Is_whitespace['\n'] = true;
+            _Is_whitespace['\r'] = true;
+            _Is_whitespace['\t'] = true;
+            _Is_whitespace['\0'] = true;
+        }
+
+        _NODISCARD constexpr bool _Test(const char _Ch) const noexcept {
+            return _Is_whitespace[static_cast<unsigned char>(_Ch)];
+        }
+    };
+
+    constexpr _Whitespace_bitmap_t _Whitespace_bitmap;
+} // unnamed namespace
+
+_EXTERN_C
+_NODISCARD size_t __CLRCALL_PURE_OR_STDCALL __std_get_string_size_without_trailing_whitespace(
+    const char* const _Str, size_t _Size) noexcept {
+    while (_Size != 0 && _Whitespace_bitmap._Test(_Str[_Size - 1])) {
+        --_Size;
+    }
+
+    return _Size;
+}
+
+_NODISCARD size_t __CLRCALL_PURE_OR_STDCALL __std_system_error_allocate_message(
+    const unsigned long _Message_id, char** const _Ptr_str) noexcept {
+    // convert to name of Windows error, return 0 for failure, otherwise return number of chars in buffer
+    // __std_system_error_deallocate_message should be called even if 0 is returned
+    // pre: *_Ptr_str == nullptr
+    const unsigned long _Chars =
+        FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+            nullptr, _Message_id, 0, reinterpret_cast<char*>(_Ptr_str), 0, nullptr);
+
+    return _CSTD __std_get_string_size_without_trailing_whitespace(*_Ptr_str, _Chars);
+}
+
+void __CLRCALL_PURE_OR_STDCALL __std_system_error_deallocate_message(char* const _Str) noexcept {
+    LocalFree(_Str);
+}
+_END_EXTERN_C