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.

Sign up for GitHub

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

winapisupp.cpp: Dramatically shrink the function pointer table #2841

Merged
merged 15 commits into from
Jul 12, 2022
Merged

winapisupp.cpp: Dramatically shrink the function pointer table #2841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
805c1bc
Dramatically shrink the `__KERNEL32Functions` table.
StephanTLavavej Jul 2, 2022
c3a7fee
De Morgan.
StephanTLavavej Jul 2, 2022
5765cd9
Move `__KERNEL32Functions` machinery to `winapisupp.cpp`.
StephanTLavavej Jul 2, 2022
9165510
Guard `__KERNEL32Functions` machinery with `#if !defined(_ONECORE)`. …
StephanTLavavej Jul 2, 2022
407a0e7
Use an unnamed namespace instead of `extern "C"` for the `__KERNEL32F…
StephanTLavavej Jul 2, 2022
975c4cf
Cache `GetSystemTimePreciseAsFileTime` only when we need it.
StephanTLavavej Jul 2, 2022
2e13066
Fix return type in `PFNGETCURRENTPACKAGEID` typedef.
StephanTLavavej Jul 2, 2022
da8d4d9
Fuse `DYNAMICGETCACHEDFUNCTION` into `IFDYNAMICGETCACHEDFUNCTION`.
StephanTLavavej Jul 2, 2022
037445b
Always use `decltype` in `IFDYNAMICGETCACHEDFUNCTION`.
StephanTLavavej Jul 2, 2022
86eb557
Use C++17 if-statement with initializer to improve scoping.
StephanTLavavej Jul 2, 2022
c80d039
Avoid emitting empty statements after `STOREFUNCTIONPOINTER`.
StephanTLavavej Jul 2, 2022
1472f80
`winapisupp.cpp` isn't using `<cstdint>` at all.
StephanTLavavej Jul 2, 2022
2f2877b
Simplify `__crt_IsPackagedAppHelper()` with range-for and auto.
StephanTLavavej May 28, 2022
75e974d
`awint.hpp` doesn't need to declare `__crtIsPackagedApp()`.
StephanTLavavej Jul 2, 2022
68e342e
We can always use `decltype` for `GetCurrentPackageId`.
StephanTLavavej Jul 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 0 additions & 77 deletions stl/src/awint.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@

_CRT_BEGIN_C_HEADER

#if !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)
_CRTIMP2 BOOL __cdecl __crtIsPackagedApp();
#endif // !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)

#if _STL_WIN32_WINNT >= _WIN32_WINNT_WIN8

#define __crtGetSystemTimePreciseAsFileTime(lpSystemTimeAsFileTime) \
Expand All @@ -27,79 +23,6 @@ _CRTIMP2 void __cdecl __crtGetSystemTimePreciseAsFileTime(_Out_ LPFILETIME lpSys

#endif // _STL_WIN32_WINNT >= _WIN32_WINNT_WIN8

// This enum should not change, even though some functions are no longer imported dynamically
enum wrapKERNEL32Functions {
eFlsAlloc,
eFlsFree,
eFlsGetValue,
eFlsSetValue,
eInitializeCriticalSectionEx,
eInitOnceExecuteOnce,
eCreateEventExW,
eCreateSemaphoreW,
eCreateSemaphoreExW,
eCreateThreadpoolTimer,
eSetThreadpoolTimer,
eWaitForThreadpoolTimerCallbacks,
eCloseThreadpoolTimer,
eCreateThreadpoolWait,
eSetThreadpoolWait,
eCloseThreadpoolWait,
eFlushProcessWriteBuffers,
eFreeLibraryWhenCallbackReturns,
eGetCurrentProcessorNumber,
eCreateSymbolicLinkW,
#if defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT)
eSetDefaultDllDirectories,
eCompareStringEx,
eEnumSystemLocalesEx,
eGetLocaleInfoEx,
eGetUserDefaultLocaleName,
eIsValidLocaleName,
eLCMapStringEx,
#else // defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT)
eGetCurrentPackageId,
#endif // defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT)
eGetTickCount64,
eGetFileInformationByHandleEx,
eSetFileInformationByHandle,
eGetSystemTimePreciseAsFileTime,
eInitializeConditionVariable,
eWakeConditionVariable,
eWakeAllConditionVariable,
eSleepConditionVariableCS,
eInitializeSRWLock,
eAcquireSRWLockExclusive,
eTryAcquireSRWLockExclusive,
eReleaseSRWLockExclusive,
eSleepConditionVariableSRW,
eCreateThreadpoolWork,
eSubmitThreadpoolWork,
eCloseThreadpoolWork,
#if !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)
eCompareStringEx,
eGetLocaleInfoEx,
eLCMapStringEx,
#endif // !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)
eMaxKernel32Function
};

extern PVOID __KERNEL32Functions[eMaxKernel32Function];

using PFNGETSYSTEMTIMEPRECISEASFILETIME = VOID(WINAPI*)(LPFILETIME);

// Use this macro for caching a function pointer from a DLL
#define STOREFUNCTIONPOINTER(instance, function_name) \
__KERNEL32Functions[e##function_name] = reinterpret_cast<PVOID>(GetProcAddress(instance, #function_name));

// Use this macro as a cached function pointer from a DLL
#define DYNAMICGETCACHEDFUNCTION(function_pointer_type, function_name, variable_name) \
const auto variable_name = reinterpret_cast<function_pointer_type>(__KERNEL32Functions[e##function_name])

#define IFDYNAMICGETCACHEDFUNCTION(function_pointer_type, function_name, variable_name) \
DYNAMICGETCACHEDFUNCTION(function_pointer_type, function_name, variable_name); \
if (variable_name != nullptr)

_CRTIMP2 int __cdecl __crtCompareStringA(_In_z_ LPCWSTR _LocaleName, _In_ DWORD _DwCmpFlags,
_In_reads_(_CchCount1) LPCSTR _LpString1, _In_ int _CchCount1, _In_reads_(_CchCount2) LPCSTR _LpString2,
_In_ int _CchCount2, _In_ int _CodePage);
Expand Down
108 changes: 42 additions & 66 deletions stl/src/winapisupp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,36 @@
#pragma warning(disable : 4265) // non-virtual destructor in base class
#include <wrl/wrappers/corewrappers.h>
#pragma warning(pop)
#include <cstdint>

#if !defined(_ONECORE)
namespace {

enum wrapKERNEL32Functions {
#if !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)
eGetCurrentPackageId,
#endif // !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)

#if _STL_WIN32_WINNT < _WIN32_WINNT_WIN8
eGetSystemTimePreciseAsFileTime,
#endif // _STL_WIN32_WINNT < _WIN32_WINNT_WIN8

eMaxKernel32Function
Comment on lines +21 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it strikes me as not the best idea to preprocess out enumerators like this without explicitly assigning values. If this is ever passed across ABI boundaries (it does not appear to be now) it might blow up pretty immediately

Pre-existing though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, pre-existing. I believe that the risk is low (as you mentioned, this table and its enumerators aren't part of the ABI surface area - also, the conditions are inherently of the form "how is the STL being built"), and it's further mitigated by this PR, as the table and its enumerators are now confined to an unnamed namespace in a single TU.

That said, it now seems to me that the case for having a table is weaker than ever. We never iterate over the table, so we could simply use separate function pointers (which the compiler could still store densely). That would avoid any concerns about varying enumerator values, and actually make debugging potentially easier. I may explore this in a followup PR.

};

PVOID __KERNEL32Functions[eMaxKernel32Function > 0 ? eMaxKernel32Function : 1]{};

// Use this macro for caching a function pointer from a DLL
#define STOREFUNCTIONPOINTER(instance, function_name) \
__KERNEL32Functions[e##function_name] = reinterpret_cast<PVOID>(GetProcAddress(instance, #function_name))

// Use this macro for retrieving a cached function pointer from a DLL
#define IFDYNAMICGETCACHEDFUNCTION(name) \
if (const auto pf##name = reinterpret_cast<decltype(&name)>(__KERNEL32Functions[e##name]); pf##name)

Comment on lines +36 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that we are only doing this a few times it might be easier to just expand these macros.

If we're going to use macros for this we could also use xmacros to define the above enumeration and the below initialization function at the same time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - the primary value of the macros is now enforcing the consistent use of decltype, so I would hesitate to eliminate them completely.

X-macros are definitely a potential technique, although they are quite powerful/opaque. As there are a small number of locations (definition, initialization, usage) and they're slowly-changing, I tend to think that we don't need such machinery at this time - especially now that the number of enumerators is so much smaller.

I'll think about refactoring this in a followup PR. I think it might be feasible to have one macro for definition (using decltype for individual variables) and one for initialization (ensuring that the GetProcAddress uses the correct spelling), and then directly refer to the variable for usage.

Thanks for bringing this up!

} // unnamed namespace
#endif // ^^^ !defined(_ONECORE) ^^^

#if !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)
// GetCurrentPackageId retrieves the current package id, if the app is deployed via a package.
using PFNGETCURRENTPACKAGEID = BOOL(WINAPI*)(UINT32*, BYTE*);

#if !defined _CRT_APP
#if defined _ONECORE
Expand Down Expand Up @@ -46,16 +71,14 @@ extern "C" int __crt_IsPackagedAppHelper() {
L"appmodel.dll" // LNM implementation DLL
};

wchar_t const* const* const first_possible_apiset = possible_apisets;
wchar_t const* const* const last_possible_apiset = possible_apisets + _countof(possible_apisets);
for (wchar_t const* const* it = first_possible_apiset; it != last_possible_apiset; ++it) {
HMODULEHandle const apiset(LoadLibraryExW(*it, nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32));
for (auto& dll : possible_apisets) {
HMODULEHandle const apiset(LoadLibraryExW(dll, nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32));
if (!apiset.IsValid()) {
continue;
}

PFNGETCURRENTPACKAGEID const get_current_package_id =
reinterpret_cast<PFNGETCURRENTPACKAGEID>(GetProcAddress(apiset.Get(), "GetCurrentPackageId"));
auto const get_current_package_id =
reinterpret_cast<decltype(&GetCurrentPackageId)>(GetProcAddress(apiset.Get(), "GetCurrentPackageId"));

if (!get_current_package_id) {
continue;
Expand All @@ -79,8 +102,8 @@ extern "C" int __crt_IsPackagedAppHelper() {
LONG retValue = APPMODEL_ERROR_NO_PACKAGE;
UINT32 bufferLength = 0;

IFDYNAMICGETCACHEDFUNCTION(PFNGETCURRENTPACKAGEID, GetCurrentPackageId, pfn) {
retValue = pfn(&bufferLength, nullptr);
IFDYNAMICGETCACHEDFUNCTION(GetCurrentPackageId) {
retValue = pfGetCurrentPackageId(&bufferLength, nullptr);
}

if (retValue == ERROR_INSUFFICIENT_BUFFER) {
Expand Down Expand Up @@ -110,7 +133,8 @@ extern "C" int __crt_IsPackagedAppHelper() {
//
// Exit:
// TRUE if Packaged app, FALSE if not.
extern "C" BOOL __cdecl __crtIsPackagedApp() {
// TRANSITION, ABI: preserved for binary compatibility
extern "C" _CRTIMP2 BOOL __cdecl __crtIsPackagedApp() {
#ifdef _CRT_APP
return TRUE;
#else
Expand Down Expand Up @@ -349,8 +373,7 @@ extern "C" BOOLEAN __cdecl __crtTryAcquireSRWLockExclusive(_Inout_ PSRWLOCK cons

extern "C" void __cdecl __crtGetSystemTimePreciseAsFileTime(_Out_ LPFILETIME lpSystemTimeAsFileTime) {
// use GetSystemTimePreciseAsFileTime if it is available (only on Windows 8+)...
IFDYNAMICGETCACHEDFUNCTION(
PFNGETSYSTEMTIMEPRECISEASFILETIME, GetSystemTimePreciseAsFileTime, pfGetSystemTimePreciseAsFileTime) {
IFDYNAMICGETCACHEDFUNCTION(GetSystemTimePreciseAsFileTime) {
pfGetSystemTimePreciseAsFileTime(lpSystemTimeAsFileTime);
return;
}
Expand All @@ -370,64 +393,17 @@ extern "C" void __cdecl __crtGetSystemTimePreciseAsFileTime(_Out_ LPFILETIME lpS

#else // defined _ONECORE

extern "C" PVOID __KERNEL32Functions[eMaxKernel32Function] = {nullptr};

static int __cdecl initialize_pointers() {
HINSTANCE hKernel32 = GetModuleHandleW(L"kernel32.dll");
_Analysis_assume_(hKernel32);

STOREFUNCTIONPOINTER(hKernel32, FlsAlloc);
STOREFUNCTIONPOINTER(hKernel32, FlsFree);
STOREFUNCTIONPOINTER(hKernel32, FlsGetValue);
STOREFUNCTIONPOINTER(hKernel32, FlsSetValue);
STOREFUNCTIONPOINTER(hKernel32, InitializeCriticalSectionEx);
STOREFUNCTIONPOINTER(hKernel32, InitOnceExecuteOnce);
STOREFUNCTIONPOINTER(hKernel32, CreateEventExW);
STOREFUNCTIONPOINTER(hKernel32, CreateSemaphoreW);
STOREFUNCTIONPOINTER(hKernel32, CreateSemaphoreExW);
STOREFUNCTIONPOINTER(hKernel32, CreateThreadpoolTimer);
STOREFUNCTIONPOINTER(hKernel32, SetThreadpoolTimer);
STOREFUNCTIONPOINTER(hKernel32, WaitForThreadpoolTimerCallbacks);
STOREFUNCTIONPOINTER(hKernel32, CloseThreadpoolTimer);
STOREFUNCTIONPOINTER(hKernel32, CreateThreadpoolWait);
STOREFUNCTIONPOINTER(hKernel32, SetThreadpoolWait);
STOREFUNCTIONPOINTER(hKernel32, CloseThreadpoolWait);
STOREFUNCTIONPOINTER(hKernel32, FlushProcessWriteBuffers);
STOREFUNCTIONPOINTER(hKernel32, FreeLibraryWhenCallbackReturns);
STOREFUNCTIONPOINTER(hKernel32, GetCurrentProcessorNumber);
STOREFUNCTIONPOINTER(hKernel32, CreateSymbolicLinkW);
#if defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT)
STOREFUNCTIONPOINTER(hKernel32, SetDefaultDllDirectories);
STOREFUNCTIONPOINTER(hKernel32, EnumSystemLocalesEx);
STOREFUNCTIONPOINTER(hKernel32, CompareStringEx);
STOREFUNCTIONPOINTER(hKernel32, GetLocaleInfoEx);
STOREFUNCTIONPOINTER(hKernel32, GetUserDefaultLocaleName);
STOREFUNCTIONPOINTER(hKernel32, IsValidLocaleName);
STOREFUNCTIONPOINTER(hKernel32, LCMapStringEx);
#else
#if !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)
STOREFUNCTIONPOINTER(hKernel32, GetCurrentPackageId);
#endif
STOREFUNCTIONPOINTER(hKernel32, GetTickCount64);
STOREFUNCTIONPOINTER(hKernel32, GetFileInformationByHandleEx);
STOREFUNCTIONPOINTER(hKernel32, SetFileInformationByHandle);
#endif // !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)

#if _STL_WIN32_WINNT < _WIN32_WINNT_WIN8
STOREFUNCTIONPOINTER(hKernel32, GetSystemTimePreciseAsFileTime);
STOREFUNCTIONPOINTER(hKernel32, InitializeConditionVariable);
STOREFUNCTIONPOINTER(hKernel32, WakeConditionVariable);
STOREFUNCTIONPOINTER(hKernel32, WakeAllConditionVariable);
STOREFUNCTIONPOINTER(hKernel32, SleepConditionVariableCS);
STOREFUNCTIONPOINTER(hKernel32, InitializeSRWLock);
STOREFUNCTIONPOINTER(hKernel32, AcquireSRWLockExclusive);
STOREFUNCTIONPOINTER(hKernel32, TryAcquireSRWLockExclusive);
STOREFUNCTIONPOINTER(hKernel32, ReleaseSRWLockExclusive);
STOREFUNCTIONPOINTER(hKernel32, SleepConditionVariableSRW);
STOREFUNCTIONPOINTER(hKernel32, CreateThreadpoolWork);
STOREFUNCTIONPOINTER(hKernel32, SubmitThreadpoolWork);
STOREFUNCTIONPOINTER(hKernel32, CloseThreadpoolWork);
#if !defined(_CRT_WINDOWS) && !defined(UNDOCKED_WINDOWS_UCRT)
STOREFUNCTIONPOINTER(hKernel32, CompareStringEx);
STOREFUNCTIONPOINTER(hKernel32, GetLocaleInfoEx);
STOREFUNCTIONPOINTER(hKernel32, LCMapStringEx);
#endif
#endif // _STL_WIN32_WINNT < _WIN32_WINNT_WIN8

return 0;
}
Expand Down