[Openvpn-devel,2/5] Prevent __stdcall name mangling of MSVC

Message ID 20181016102627.18676-2-simon@rozman.si
State Accepted
Headers show
Series [Openvpn-devel,1/5] Set output name to libopenvpnmsica.dll in MSVC builds too | expand

Commit Message

Simon Rozman Oct. 15, 2018, 11:26 p.m. UTC
Using `extern "C" __declspec(dllexport) __stdcall`, Win32 MSVC compiler
exports the functions are as `_name@N`. Exporting functions using
`/EXPORT` linker flag allows us to specify exact function name.

Note: The 64-bit MSVC compiler does not exhibit `__stdcall` name-
mangling.
---
 src/openvpnmsica/openvpnmsica.c | 12 ++++++++++++
 src/openvpnmsica/openvpnmsica.h | 14 +++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

Comments

Kristof Provost via Openvpn-devel Nov. 8, 2018, 11:46 a.m. UTC | #1
Hi Simon,

This approach keeps 'goes in DLL' next to the function itself, which I like. If 
you're interested, another possible approach here is to use .DEF files with 
MSVC, which can also do symbol aliasing:
https://docs.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-def-files

I would expect mingw to support .DEF files as well, but I don't know.

Only x86 Win32 functions are actually __stdcall; x64, ARM32, and ARM64 
all ignore the token because the architectural calling convention makes it
meaningless:
https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx

This is why the names don't get mangled. Of course, it's then inconsistent.

Export name consistency is needed so CustomActions can reference the 
export symbol name, right?

Jon

-----Original Message-----
From: Simon Rozman <simon@rozman.si> 
Sent: Tuesday, October 16, 2018 3:26 AM
To: openvpn-devel@lists.sourceforge.net
Subject: [Openvpn-devel] [PATCH 2/5] Prevent __stdcall name mangling of MSVC

Using `extern "C" __declspec(dllexport) __stdcall`, Win32 MSVC compiler
exports the functions are as `_name@N`. Exporting functions using
`/EXPORT` linker flag allows us to specify exact function name.

Note: The 64-bit MSVC compiler does not exhibit `__stdcall` name-
mangling.
---
 src/openvpnmsica/openvpnmsica.c | 12 ++++++++++++
 src/openvpnmsica/openvpnmsica.h | 14 +++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 82333991..3b90ce05 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -145,6 +145,10 @@ openvpnmsica_setup_sequence_filename(
 UINT __stdcall
 FindTAPInterfaces(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
     MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  TEXT(PACKAGE_VERSION), MB_OK);
 #endif
@@ -247,6 +251,10 @@ cleanup_CoInitialize:
 UINT __stdcall
 EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
     MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  TEXT(PACKAGE_VERSION), MB_OK);
 #endif
@@ -505,6 +513,10 @@ cleanup_exec_seq:
 UINT __stdcall
 ProcessDeferredAction(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
     MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  TEXT(PACKAGE_VERSION), MB_OK);
 #endif
diff --git a/src/openvpnmsica/openvpnmsica.h b/src/openvpnmsica/openvpnmsica.h
index 3a64fbaa..bb8e28ec 100644
--- a/src/openvpnmsica/openvpnmsica.h
+++ b/src/openvpnmsica/openvpnmsica.h
@@ -55,6 +55,14 @@ extern DWORD openvpnmsica_tlsidx_session;
 extern "C" {
 #endif
 
+#ifdef __GNUC__
+#define DLLEXP_DECL __declspec(dllexport)
+#else
+#define DLLEXP_DECL
+#define DLLEXP_EXPORT "/EXPORT:"__FUNCTION__"="__FUNCDNAME__
+#endif
+
+
 /**
  * Find existing TAP interfaces and set TAPINTERFACES property with semicolon delimited list
  * of installed TAP interface GUIDs.
@@ -64,7 +72,7 @@ extern "C" {
  * @return ERROR_SUCCESS on success; An error code otherwise
  *         See: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmsdn.microsoft.com%2Fen-us%2Flibrary%2Fwindows%2Fdesktop%2Faa368072.aspx&amp;data=02%7C01%7Cjkunkee%40microsoft.com%7C726e9ae5072942c989bf08d63351ec71%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636752824301952450&amp;sdata=BXt41C1200ZAAQBQBS7vOiAZ3MzuQGCVHaUuFhYwMCk%3D&amp;reserved=0
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 FindTAPInterfaces(_In_ MSIHANDLE hInstall);
 
 
@@ -77,7 +85,7 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall);
  * @return ERROR_SUCCESS on success; An error code otherwise
  *         See: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmsdn.microsoft.com%2Fen-us%2Flibrary%2Fwindows%2Fdesktop%2Faa368072.aspx&amp;data=02%7C01%7Cjkunkee%40microsoft.com%7C726e9ae5072942c989bf08d63351ec71%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636752824301962454&amp;sdata=vMBtU2AQPrfZ9xnbN4nYZfcAb2wNlIxLPqqYLwDtvUc%3D&amp;reserved=0
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall);
 
 
@@ -89,7 +97,7 @@ EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall);
  * @return ERROR_SUCCESS on success; An error code otherwise
  *         See: https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmsdn.microsoft.com%2Fen-us%2Flibrary%2Fwindows%2Fdesktop%2Faa368072.aspx&amp;data=02%7C01%7Cjkunkee%40microsoft.com%7C726e9ae5072942c989bf08d63351ec71%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636752824301962454&amp;sdata=vMBtU2AQPrfZ9xnbN4nYZfcAb2wNlIxLPqqYLwDtvUc%3D&amp;reserved=0
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 ProcessDeferredAction(_In_ MSIHANDLE hInstall);
 
 #ifdef __cplusplus
Simon Rozman Nov. 8, 2018, 10:07 p.m. UTC | #2
Hi Jon,

> This approach keeps 'goes in DLL' next to the function itself, which I
> like. If you're interested, another possible approach here is to use
> .DEF files with MSVC, which can also do symbol aliasing:
> https://docs.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-
> def-files
> 
> I would expect mingw to support .DEF files as well, but I don't know.

I did use .DEF file initially. However, when adopting the project to build
with MinGW too, my limited knowledge in authoring Linux build systems didn't
find a way to use .DEF files there. After some research, I have put the
export declarations into the source code instead. It's ugly but it keeps
both building systems happy and produces consistent DLL file.

> Only x86 Win32 functions are actually __stdcall; x64, ARM32, and ARM64
> all ignore the token because the architectural calling convention makes
> it
> meaningless:
> https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx
> 
> This is why the names don't get mangled. Of course, it's then
> inconsistent.
> 
> Export name consistency is needed so CustomActions can reference the
> export symbol name, right?

Exactly. That extra effort to prevent name mangling when compiling x86 DLL
file pays off later when authoring MSI package. Otherwise, I would have had
to introduce some "#if-s" in WiX to address different platform-specific
function names.

I had to pick my poison, and I choose to fix issues as close to the source
as possible.

Regards,
Simon
Kristof Provost via Openvpn-devel Nov. 9, 2018, 8:30 a.m. UTC | #3
Hi Jon,

>> This approach keeps 'goes in DLL' next to the function itself, which I
>> like.
>
> It's ugly but it keeps both building systems happy and produces consistent 
> DLL file.

Fantastic. I'm sorry there isn't a more elegant solution, but I'm glad there's
something that works.

> I had to pick my poison, and I choose to fix issues as close to the source
> as possible.

Understood. Thanks for the explanation.

Samuli, LGTM.

Jon
Gert Doering Jan. 17, 2019, 5:12 a.m. UTC | #4
Your patch has been applied to the master branch.  

"LGTM" from Jon (plus patch and explanation seem to make sense :-) )

commit 7d08c33cfc58eeb7286446c8d1ffd02939782332
Author: Simon Rozman
Date:   Tue Oct 16 12:26:24 2018 +0200

     Prevent __stdcall name mangling of MSVC

Acked-by: Jon Kunkee <jkunkee@microsoft.com>
     Message-Id: <20181016102627.18676-2-simon@rozman.si>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17765.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 82333991..3b90ce05 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -145,6 +145,10 @@  openvpnmsica_setup_sequence_filename(
 UINT __stdcall
 FindTAPInterfaces(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
     MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  TEXT(PACKAGE_VERSION), MB_OK);
 #endif
@@ -247,6 +251,10 @@  cleanup_CoInitialize:
 UINT __stdcall
 EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
     MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  TEXT(PACKAGE_VERSION), MB_OK);
 #endif
@@ -505,6 +513,10 @@  cleanup_exec_seq:
 UINT __stdcall
 ProcessDeferredAction(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
     MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  TEXT(PACKAGE_VERSION), MB_OK);
 #endif
diff --git a/src/openvpnmsica/openvpnmsica.h b/src/openvpnmsica/openvpnmsica.h
index 3a64fbaa..bb8e28ec 100644
--- a/src/openvpnmsica/openvpnmsica.h
+++ b/src/openvpnmsica/openvpnmsica.h
@@ -55,6 +55,14 @@  extern DWORD openvpnmsica_tlsidx_session;
 extern "C" {
 #endif
 
+#ifdef __GNUC__
+#define DLLEXP_DECL __declspec(dllexport)
+#else
+#define DLLEXP_DECL
+#define DLLEXP_EXPORT "/EXPORT:"__FUNCTION__"="__FUNCDNAME__
+#endif
+
+
 /**
  * Find existing TAP interfaces and set TAPINTERFACES property with semicolon delimited list
  * of installed TAP interface GUIDs.
@@ -64,7 +72,7 @@  extern "C" {
  * @return ERROR_SUCCESS on success; An error code otherwise
  *         See: https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 FindTAPInterfaces(_In_ MSIHANDLE hInstall);
 
 
@@ -77,7 +85,7 @@  FindTAPInterfaces(_In_ MSIHANDLE hInstall);
  * @return ERROR_SUCCESS on success; An error code otherwise
  *         See: https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall);
 
 
@@ -89,7 +97,7 @@  EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall);
  * @return ERROR_SUCCESS on success; An error code otherwise
  *         See: https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 ProcessDeferredAction(_In_ MSIHANDLE hInstall);
 
 #ifdef __cplusplus