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

DECLSPEC_IMPORT make all curl_cv_func_recv_test Failed when building in windows #4764

Closed
Ron2014 opened this issue Dec 27, 2019 · 6 comments
Closed
Labels

Comments

@Ron2014
Copy link

@Ron2014 Ron2014 commented Dec 27, 2019

When I built curl in my project, I found all the recv_test/send_test failed.
But I can build successfully in curl-curl-7-67-0.git.
So I message something in OtherTests.cmake and CheckCSourceCompiles.cmake to check what's the different between these two project. And I found this:

-- Performing Test curl_cv_func_recv_test
====== CMAKE_BINARY_DIR = E:/TestCode/curl-curl-7_67_0/build
====== CMAKE_FILES_DIRECTORY = /CMakeFiles
====== CMAKE_REQUIRED_DEFINITIONS =  -D_WINSOCKAPI_= -DSECURITY_WIN32
====== CHECK_C_SOURCE_COMPILES_ADD_LINK_OPTIONS =
====== CHECK_C_SOURCE_COMPILES_ADD_LIBRARIES = LINK_LIBRARIES;ws2_32
====== MACRO_CHECK_FUNCTION_DEFINITIONS = -Dcurl_cv_func_recv_test
====== CHECK_C_SOURCE_COMPILES_ADD_INCLUDES = -DINCLUDE_DIRECTORIES:STRING=E:/TestCode/curl-curl-7_67_0/include
====== COMPILE_DEFINITIONS =
====== CMAKE_FLAGS =
====== _FAIL_REGEX =
====== OUTPUT = Change Dir: E:/TestCode/curl-curl-7_67_0/build/CMakeFiles/CMakeTmp

Run Build Command(s):D:/Program Files (x86)/Microsoft Visual Studio/2017/Community/MSBuild/15.0/Bin/MSBuild.exe cmTC_8df82.vcxproj /p:Configuration=Debug /p:Platform=Win32 /p:VisualStudioVersion=15.0 /v:m && Microsoft (R) Build Engine version 15.9.21+g9802d43bc3 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27034 for x86
  Copyright (C) Microsoft Corporation.  All rights reserved.

  cl /c /I"E:\TestCode\curl-curl-7_67_0\include" /Zi /W3 /WX- /diagnostics:classic /Od /Ob0 /Oy- /D WIN32 /D _WINDOWS /D curl_cv_func_recv_test /D _WINSOCKAPI_= /D SECURITY_WIN32 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"cmTC_8df82.dir\Debug\\" /Fd"cmTC_8df82.dir\Debug\cmTC_8df82.pdb" /Gd /TC /analyze- /errorReport:queue "E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\src.c"
  src.c

E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\src.c(10): warning C4273: 'recv': inconsistent dll linkage [E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\cmTC_8df82.vcxproj]
  D:\Windows Kits\10\Include\10.0.17763.0\um\winsock2.h(2002): note: see previous definition of 'recv'
  cmTC_8df82.vcxproj -> E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\Debug\cmTC_8df82.lib


-- Performing Test curl_cv_func_recv_test - Success
-- Tested: int recv(SOCKET, char *, int, int)

in curl-curl-7-67-0.git, recv can be tested successfully but with an warning, and in my project, treat warning as error, so all recv test failed.

To fix this waning, I check the WinSock2.h and found that function recv/send have WINSOCK_API_LINKAGE description like this:

#if INCL_WINSOCK_API_PROTOTYPES
WINSOCK_API_LINKAGE
int
WSAAPI
recv(
    _In_ SOCKET s,
    _Out_writes_bytes_to_(len, return) __out_data_source(NETWORK) char FAR * buf,
    _In_ int len,
    _In_ int flags
    );
#endif /* INCL_WINSOCK_API_PROTOTYPES */

and then I found WINSOCK_API_LINKAGE == DECLSPEC_IMPORT == __declspec (dllimport)

BUT, we don't need anything in front of the function recv/send, RIGHT?

so I change OtherTests.cmake to:

macro(add_header_include check header)
  if(${check})
    set(_source_epilogue "${_source_epilogue}\n#define DECLSPEC_IMPORT\n#include <${header}>")
  endif()
endmacro()

build again and no warning. SO I recommend this change.

Did anybody deal with the same problem?

@jay
Copy link
Member

@jay jay commented Dec 28, 2019

I don't have that problem. This computer has an old version of Visual Studio but I used a recent cmake, cmake-3.15.5-win64-x64.msi. Try from scratch.

mkdir X:\j\curl\cmake\cmake
cd /d X:\j\curl\cmake\cmake
set PATH=X:\j\curl\cmake\cmake\src\Debug;X:\j\curl\cmake\cmake\lib\Debug;X:\j\curl\cmake\openssl\bin;%PATH%
cmake ..\..\curl -G "Visual Studio 10" -DCMAKE_USE_OPENSSL=ON -DOPENSSL_ROOT_DIR=X:\j\curl\cmake\openssl -DBUILD_TESTING=OFF -DCURL_ZLIB=OFF -DENABLE_THREADED_RESOLVER=ON -DCMAKE_INSTALL_PREFIX:PATH=X:\j\curl\cmake\curl-install-dir
-- Performing Test curl_cv_recv
-- Performing Test curl_cv_recv - Success
-- Performing Test curl_cv_func_recv_test
-- Performing Test curl_cv_func_recv_test - Success
-- Tested: int recv(SOCKET, char *, int, int)
-- Performing Test curl_cv_send
-- Performing Test curl_cv_send - Success
-- Performing Test curl_cv_func_send_test
-- Performing Test curl_cv_func_send_test - Success
-- Tested: int send(SOCKET, const char *, int, int)
@jay jay added the cmake label Dec 28, 2019
@Ron2014
Copy link
Author

@Ron2014 Ron2014 commented Dec 28, 2019

I don't have that problem. This computer has an old version of Visual Studio but I used a recent cmake, cmake-3.15.5-win64-x64.msi. Try from scratch.

mkdir X:\j\curl\cmake\cmake
cd /d X:\j\curl\cmake\cmake
set PATH=X:\j\curl\cmake\cmake\src\Debug;X:\j\curl\cmake\cmake\lib\Debug;X:\j\curl\cmake\openssl\bin;%PATH%
cmake ..\..\curl -G "Visual Studio 10" -DCMAKE_USE_OPENSSL=ON -DOPENSSL_ROOT_DIR=X:\j\curl\cmake\openssl -DBUILD_TESTING=OFF -DCURL_ZLIB=OFF -DENABLE_THREADED_RESOLVER=ON -DCMAKE_INSTALL_PREFIX:PATH=X:\j\curl\cmake\curl-install-dir
-- Performing Test curl_cv_recv
-- Performing Test curl_cv_recv - Success
-- Performing Test curl_cv_func_recv_test
-- Performing Test curl_cv_func_recv_test - Success
-- Tested: int recv(SOCKET, char *, int, int)
-- Performing Test curl_cv_send
-- Performing Test curl_cv_send - Success
-- Performing Test curl_cv_func_send_test
-- Performing Test curl_cv_func_send_test - Success
-- Tested: int send(SOCKET, const char *, int, int)

Let me explain it.

I treat warning as error in my project. That the key. Which means I use /WX in MSVC like this:

set(CMAKE_C_FLAGS "/MP /WX /TC /errorReport:queue /DWIN32")

What made me test recv failed is the warning C4273. Generally, the warning isn't printed.

Why I can get this warning printed, like this-----

====== OUTPUT = Change Dir: E:/TestCode/curl-curl-7_67_0/build/CMakeFiles/CMakeTmp

Run Build Command(s):D:/Program Files (x86)/Microsoft Visual Studio/2017/Community/MSBuild/15.0/Bin/MSBuild.exe cmTC_8df82.vcxproj /p:Configuration=Debug /p:Platform=Win32 /p:VisualStudioVersion=15.0 /v:m && Microsoft (R) Build Engine version 15.9.21+g9802d43bc3 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27034 for x86
  Copyright (C) Microsoft Corporation.  All rights reserved.

  cl /c /I"E:\TestCode\curl-curl-7_67_0\include" /Zi /W3 /WX- /diagnostics:classic /Od /Ob0 /Oy- /D WIN32 /D _WINDOWS /D curl_cv_func_recv_test /D _WINSOCKAPI_= /D SECURITY_WIN32 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"cmTC_8df82.dir\Debug\\" /Fd"cmTC_8df82.dir\Debug\cmTC_8df82.pdb" /Gd /TC /analyze- /errorReport:queue "E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\src.c"
  src.c

E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\src.c(10): warning C4273: 'recv': inconsistent dll linkage [E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\cmTC_8df82.vcxproj]
  D:\Windows Kits\10\Include\10.0.17763.0\um\winsock2.h(2002): note: see previous definition of 'recv'
  cmTC_8df82.vcxproj -> E:\TestCode\curl-curl-7_67_0\build\CMakeFiles\CMakeTmp\Debug\cmTC_8df82.lib

Because I used Everything to find the file CheckCSourceCompiles.cmake in my cmake installation path which defined macro check_c_source_compiles you used in OtherTests.cmake. like include(CheckCSourceCompiles).

image

I added a message here and see the content of ${OUTPUT} have a warning C4273.

image

In Brief,

  1. Add /WX complie flag for your MSVC project and you will test fail.
  2. message the OUTPUT in file CheckCSourceCompiles.cmake in the CMake installation path and you will see the warning which made you test fail.

Pls try it.

PS.
Why you test successfully??
I don't see the /WX flag, maybe when we use cmake . command, it use /WX- as default, which means don't treat warnings as errors.

image

@jay
Copy link
Member

@jay jay commented Dec 28, 2019

I've retested with -DCURL_WERROR=ON (which adds /WX) and there was no change. I am not too familiar with cmake and so I don't know if what you are doing is correct. Hopefully someone else that follows this project and knows about cmake can help.

@Ron2014
Copy link
Author

@Ron2014 Ron2014 commented Dec 30, 2019

I've retested with -DCURL_WERROR=ON (which adds /WX) and there was no change. I am not too familiar with cmake and so I don't know if what you are doing is correct. Hopefully someone else that follows this project and knows about cmake can help.

Pay attention to the lineno of code in CMakeLists.txt

/WX is set after include(OtherTests.cmake), so the flag doesn't work for the recv/send test.

1130: include(CMake/OtherTests.cmake)
...
1154: if(CURL_WERROR)
1155:      if(MSVC_VERSION)
1156:          set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX")
...

So test with -DCURL_WERROR=ON is not a proper way, write set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX") at the first line of CMakeLists.txt and you will get the fails I got.

This is what happend in my work. I use cURL source code as subproject for my MSVC solution. In the main project, I set /WX in the CMakeLists.txt of root directory, which will be passed through add_subdirectory(curl) and finally it works for the recv/send test.

For the familiar with cmake, recommend official tutorial. What I told above and used in my project CMakeLists.txt files all come from the first five steps.

@jay
Copy link
Member

@jay jay commented Dec 31, 2019

Ok I can reproduce that here. I have no idea if that's allowed or expected. Someone that works with libcurl's cmake build system will have to review this.

@jay jay added the build label Dec 31, 2019
@bagder bagder added the help wanted label Feb 10, 2020
bradking added a commit to bradking/curl that referenced this issue Apr 13, 2020
We use `check_c_source_compiles` to check possible send/recv signatures
by reproducing the forward declarations from system headers.  On Windows
the `winsock2.h` header adds dll linkage settings to its forward
declaration.  If ours does not match the compiler warns:

    warning C4273: 'recv': inconsistent dll linkage

Add `WINSOCK_API_LINKAGE` to our test signatures when it is defined so
that our linkage is consistent with that from `winsock2.h`.

Fixes: curl#4764
@bradking
Copy link
Contributor

@bradking bradking commented Apr 13, 2020

@Ron2014 please see an alternative fix in #5232. We should match the linkage to make sure the test binary can actually link to the symbol.

@bagder bagder removed the help wanted label Apr 13, 2020
@bagder bagder closed this in 89f1e63 Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.