llvm dev - Nov 2011 - [LLVMdev] [PATCH] LLVM 3.0 broken in lib/Support/Windows/DynamicLibrary.inc

I don't know since when, but this file has been changed to remove all the
trickery (aka defines) needed for MinGW-w64 (and probably everything else
that as forgotten) to succesfully compile it.

Attached is a patch that reintroduces the compiler checking. I would like
to see this in LLVM 3.0, otherwise (by the looks of the reintroduced code)
anything newer than _MSC_VER_1500 will be broken.

Thanks!

Ruben

PS: any replies please "reply-to-all", only subscribed to cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111102/5e2fe363/attachment.html>
-------------- next part --------------
diff --git a/lib/Support/Windows/DynamicLibrary.inc
b/lib/Support/Windows/DynamicLibrary.inc
index 83da82a..dba81b4 100644
--- a/lib/Support/Windows/DynamicLibrary.inc
+++ b/lib/Support/Windows/DynamicLibrary.inc
@@ -43,10 +43,34 @@ static DenseSet<HMODULE> *OpenedHandles;
 
 extern "C" {
 
-  static BOOL CALLBACK ELM_Callback(WIN32_ELMCB_PCSTR ModuleName,
-                                    ULONG_PTR ModuleBase,
+// Use old callback if:
+//  - Not using Visual Studio
+//  - Visual Studio 2005 or earlier but only if we are not using the Windows
SDK
+//    or Windows SDK version is older than 6.0
+// Use new callback if:
+//  - Newer Visual Studio (comes with newer SDK).
+//  - Visual Studio 2005 with Windows SDK 6.0+
+#if defined(_MSC_VER)
+  #if _MSC_VER < 1500 && (!defined(VER_PRODUCTBUILD) ||
VER_PRODUCTBUILD < 6000)
+    #define OLD_ELM_CALLBACK_DECL 1
+  #endif
+#elif defined(__MINGW64__)
+  // Use new callback.
+#elif defined(__MINGW32__)
+  #define OLD_ELM_CALLBACK_DECL 1
+#endif
+
+#ifdef OLD_ELM_CALLBACK_DECL
+  static BOOL CALLBACK ELM_Callback(PSTR  ModuleName,
+                                    DWORD_PTR ModuleBase,
+                                    ULONG ModuleSize,
+                                    PVOID UserContext)
+#else
+  static BOOL CALLBACK ELM_Callback(PCSTR  ModuleName,
+                                    DWORD_PTR ModuleBase,
                                     ULONG ModuleSize,
                                     PVOID UserContext)
+#endif
   {
     // Ignore VC++ runtimes prior to 7.1.  Somehow some of them get loaded
     // into the process.

Hi Ruben,
> I don't know since when, but this file has been changed to remove all
the
> trickery (aka defines) needed for MinGW-w64 (and probably everything else
> that as forgotten) to succesfully compile it.
Will you please check which commit introduces the breakage?

Thanks!

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

2011/11/2 Anton Korobeynikov <anton at korobeynikov.info>
> Hi Ruben,
>
> > I don't know since when, but this file has been changed to remove
all the
> > trickery (aka defines) needed for MinGW-w64 (and probably everything
else
> > that as forgotten) to succesfully compile it.
> Will you please check which commit introduces the breakage?
>
My git clone of LLVM shows
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk at
13065791177308-0d34-0410-b5e6-96231b3b80d8
as the "offender". It seems this code was replaced by a configure-time
check. That check apparently misses out when configuring on msys like this:
$ /m/Development/Source/LLVM/configure --host=x86_64-w64-mingw32
--enable-optimized --disable-assertions --disable-pthreads CC=clang
CXX=clang++

For some reason unknown to me. I was wondering why the CMake build was
fine...

Ruben
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111102/f00f302a/attachment.html>

Hi,

On Wed, Nov 2, 2011 at 3:22 PM, Ruben Van Boxem wrote:
> I don't know since when, but this file has been changed to remove all
the
> trickery (aka defines) needed for MinGW-w64 (and probably everything else
> that as forgotten) to succesfully compile it.
That line was changed in r130657:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/DynamicLibrary.inc?view=diff&r1=130656&r2=130657
>
> Attached is a patch that reintroduces the compiler checking. I would like
to
> see this in LLVM 3.0, otherwise (by the looks of the reintroduced code)
> anything newer than _MSC_VER_1500 will be broken.
I believe this is not the right way to fix it. "Trickery" is in place
elsewhere:

$ grep WIN32_ELMCB_PCSTR -rn ../llvm/ | grep -v -- -base
../llvm/configure:19278:#define WIN32_ELMCB_PCSTR $llvm_cv_win32_elmcb_pcstr
../llvm/cmake/config-ix.cmake:370:    set(WIN32_ELMCB_PCSTR "PCSTR")
../llvm/cmake/config-ix.cmake:372:    set(WIN32_ELMCB_PCSTR "PSTR")
../llvm/lib/Support/Windows/DynamicLibrary.inc:46:  static BOOL
CALLBACK ELM_Callback(WIN32_ELMCB_PCSTR ModuleName,
../llvm/include/llvm/Config/config.h.cmake:684:#cmakedefine
WIN32_ELMCB_PCSTR ${WIN32_ELMCB_PCSTR}
../llvm/include/llvm/Config/config.h.in:685:#undef WIN32_ELMCB_PCSTR
../llvm/autoconf/configure.ac:1331:
AC_DEFINE_UNQUOTED([WIN32_ELMCB_PCSTR],$llvm_cv_win32_elmcb_pcstr,[Type
of 1st arg on ELM Callback])
../llvm/projects/sample/configure:19228:#define WIN32_ELMCB_PCSTR
$llvm_cv_win32_elmcb_pcstr
../llvm/projects/sample/autoconf/configure.ac:1264:
AC_DEFINE_UNQUOTED([WIN32_ELMCB_PCSTR],$llvm_cv_win32_elmcb_pcstr,[Type
of 1st arg on ELM Callback])

I have to say that since the only difference appears to be the type of
the first parameter, it is much cleaner to define a macro for it,
rather than a split declaration of the entire function definition.

Csaba
-- 
GCS a+ e++ d- C++ ULS$ L+$ !E- W++ P+++$ w++$ tv+ b++ DI D++ 5++
The Tao of math: The numbers you can count are not the real numbers.
Life is complex, with real and imaginary parts.
"Ok, it boots. Which means it must be bug-free and perfect. " -- Linus
Torvalds
"People disagree with me. I just ignore them." -- Linus Torvalds