paullouisageneau / libdatachannel

C/C++ WebRTC network library featuring Data Channels, Media Transport, and WebSockets
https://libdatachannel.org/
Mozilla Public License 2.0
1.81k stars 366 forks source link

Windows MSVC needs /EHsc in CMakeLists.txt #786

Open WalkerWhite opened 1 year ago

WalkerWhite commented 1 year ago

The CMake files need the compiler flag /EHsc for Windows MSVC builds. Withou itt, exceptions do not cleanly fail and therefore the test suite fails on the C API.

paullouisageneau commented 1 year ago

If I understand correctly the corresponding MSVC documentation, you need to set /EHsc flag to make the compiler generate exception-handling code compliant with the C++ standard? Technical decisions from Microsoft never cease to amaze me.

However, it looks like CMake hardcodes the flag for MSVC, so I'm surprised it's not set by default.

What versions of MSVC and CMake do you use? Do you override the CXX flags in some way?

WalkerWhite commented 1 year ago

I am using CMake 3.23.1 and Microsoft Visual Studio 2022.

I agree that Windows is weird. I am investigating using libdatachannels for a course that has to compile on multiple platforms. Windows always gives me problems. I have gotten this library to build and pass the tests on Linux, macOS, iOS, and Android. Though I had problems with the OpenSSL prefab on the latter and ended up building OpenSSL from source.

But Windows fails the tests. It passes the C++ tests, but as soon as it reaches the C API tests it fails with a "device busy" error. It looks like an exception failed to release a lock of some sort. To solve the problem, to the WIN32 section (line 264 of CMake) I added

if (MSVC)
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc")
endif()
Tim-S commented 1 year ago

Looking at my CMakeCache.txt I can tell that for Microsoft Visual Studio 2019 / MSVC 14.29.30133 cmake version 3.25.1 /EHsc is set by default without explicitly setting it and the C API test succeeds CMakeCacheEHscSetByDefault

pkviet commented 1 year ago

Hello, ~~could this be added to CMakeLists.txt ? set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHsc") context: trying to build libdatachannel with msvc 2022 for obs (obs dev here) and failing miserably without this line~~ Edit: ignore for now, the issue could be related to mbedtls (i'm testing the mbedtls PR atm).