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

fix build on mingw #446

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

fix build on mingw #446

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
765a6da
fix build on mingw
mattn Nov 30, 2022
37f4785
do not include pthread.h
mattn Nov 30, 2022
88679c0
use NOT
mattn Nov 30, 2022
2f615c0
include sys/time.h for mingw
mattn Nov 30, 2022
2189000
test for mingw
mattn Nov 30, 2022
be31c45
disable check
mattn Nov 30, 2022
c1bd5cd
disable check
mattn Nov 30, 2022
8783407
bump GitHub Action
mattn Dec 1, 2022
410660f
Merge branch 'master' into mingw-port
mattn Dec 13, 2022
d911291
use _WIN32
mattn Dec 13, 2022
ff4d2b2
delete target picotls-esni
mattn Dec 13, 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
36 changes: 36 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,39 @@ jobs:
cmake .. -DOPENSSL_ROOT_DIR="$(brew --prefix ${OPENSSL})"
make all VERBOSE=1
make check
windows:
name: "${{ matrix.name }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The name "windows" is a bit confusing, as we also have a direct port on Windows relying on Visual Studio. Maybe use "mingw" for clarity?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that this change broke the CI tests -- they did not run at all after that.

Copy link
Contributor Author

@mattn mattn Nov 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Current implementation of t/cli.c have many UNIX-ish codes. So we can not run it. But this can be useful to check build error for future's changes at least.

runs-on: [windows-latest]
# We want to run on external PRs, but not on our own internal PRs as they'll be run
# by the push to the branch.
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository

strategy:
fail-fast: false
matrix:
include:
- name: "Windows / OpenSSL 3.x"
openssl: openssl@3

timeout-minutes: 20
steps:
- uses: actions/checkout@v2
with:
submodules: recursive

- uses: msys2/setup-msys2@v2
with:
update: true
install: mingw-w64-x86_64-toolchain mingw-w64-x86_64-cmake mingw-w64-x86_64-openssl mingw-w64-x86_64-make
msystem: MINGW64
path-type: inherit

- name: Build
env:
OPENSSL: ${{ matrix.openssl }}
run: |
mkdir -p build
cd build
cmake -G "MSYS Makefiles" ..
mingw32-make all VERBOSE=1
shell: msys2 {0}
27 changes: 21 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,10 @@ IF (OPENSSL_FOUND AND NOT (OPENSSL_VERSION VERSION_LESS "1.0.1"))
INCLUDE_DIRECTORIES(${OPENSSL_INCLUDE_DIR})
ADD_LIBRARY(picotls-openssl lib/openssl.c)
TARGET_LINK_LIBRARIES(picotls-openssl ${OPENSSL_CRYPTO_LIBRARIES} picotls-core ${CMAKE_DL_LIBS})
ADD_EXECUTABLE(cli t/cli.c lib/pembase64.c)
TARGET_LINK_LIBRARIES(cli picotls-openssl picotls-core)
IF (NOT WIN32)
ADD_EXECUTABLE(cli t/cli.c lib/pembase64.c)
TARGET_LINK_LIBRARIES(cli picotls-openssl picotls-core)
ENDIF ()
ADD_EXECUTABLE(picotls-esni src/esni.c)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure why you need to exclude "pembase64.c" on Windows builds. It works well with the direct Windows port, with one caveat: you have to define the preprocessor flag "_WINDOWS".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. t/cli is not possible to build on Visual Studio.

picotls/t/cli.c

Lines 27 to 41 in 7e97d3e

#include <arpa/inet.h>
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
#include <getopt.h>
#include <inttypes.h>
#include <stdio.h>
#include <string.h>
#include <strings.h>
#include <sys/select.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/time.h>
#include <sys/types.h>
#include <unistd.h>

TARGET_LINK_LIBRARIES(picotls-esni picotls-openssl picotls-core ${OPENSSL_CRYPTO_LIBRARIES} ${CMAKE_DL_LIBS})

Expand Down Expand Up @@ -180,17 +182,30 @@ IF (NOT WITH_FUSION)
SET_TARGET_PROPERTIES(ptlsbench PROPERTIES EXCLUDE_FROM_ALL 1)
ENDIF ()

ADD_CUSTOM_TARGET(check env BINARY_DIR=${CMAKE_CURRENT_BINARY_DIR} prove --exec '' -v ${CMAKE_CURRENT_BINARY_DIR}/*.t t/*.t WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} DEPENDS ${TEST_EXES} cli)
IF (NOT WIN32)
ADD_CUSTOM_TARGET(check env BINARY_DIR=${CMAKE_CURRENT_BINARY_DIR} prove --exec '' -v ${CMAKE_CURRENT_BINARY_DIR}/*.t t/*.t WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} DEPENDS ${TEST_EXES} cli)
ENDIF ()

IF (CMAKE_SYSTEM_NAME STREQUAL "Linux")
SET(CMAKE_C_FLAGS "-D_GNU_SOURCE -pthread ${CMAKE_C_FLAGS}")
ELSEIF ("${CMAKE_SYSTEM_NAME}" MATCHES "SunOS")
TARGET_LINK_LIBRARIES(cli "socket" "nsl")
ENDIF ()

FIND_LIBRARY(LIBC_RESOLV_LIB "resolv")
IF (OPENSSL_FOUND AND LIBC_RESOLV_LIB)
TARGET_LINK_LIBRARIES(cli ${LIBC_RESOLV_LIB})
IF (NOT WIN32)
FIND_LIBRARY(LIBC_RESOLV_LIB "resolv")
IF (OPENSSL_FOUND AND LIBC_RESOLV_LIB)
TARGET_LINK_LIBRARIES(cli ${LIBC_RESOLV_LIB})
ENDIF ()
ENDIF ()

IF (WIN32)
INCLUDE_DIRECTORIES(picotlsvs/picotls)
TARGET_LINK_LIBRARIES(test-minicrypto.t bcrypt ws2_32)
TARGET_LINK_LIBRARIES(test-openssl.t bcrypt ws2_32)
TARGET_LINK_LIBRARIES(test-fusion.t bcrypt ws2_32)
TARGET_LINK_LIBRARIES(ptlsbench bcrypt ws2_32)
TARGET_LINK_LIBRARIES(picotls-esni ws2_32)
ENDIF ()

IF (BUILD_FUZZER)
Expand Down
4 changes: 2 additions & 2 deletions deps/cifra/src/bitops.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include <stdint.h>
#include <stddef.h>

#ifdef _WINDOWS
#ifdef _WIN32
#include <intrin.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not change this code, that would break the native Visual Studio implementation.

If you really prefer using the string "_WIN32", then you could write:

#if defined(_WIN32) || defined(_WINDOWS)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You don't need to define _WINDOWS since _WIN32 is always set on Windows. (even if Windows 64 bit)

#endif

Expand Down Expand Up @@ -298,7 +298,7 @@ static inline void copy_bytes_unaligned(uint8_t *out, const uint8_t *in, size_t

static inline uint32_t count_trailing_zeroes(uint32_t x)
{
#ifdef _WINDOWS
#ifdef _WIN32
uint32_t r = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same remark as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I replaced all _WINDOWS to _WIN32 since _WINDOWS is not required for mingw and Visual Studio both.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am dubious about this because the CI checks are not running after your changes in ci.yml. I see that the appveyor tests were succeeding before the commit test for mingw, which implies that the Visual Studio build was verified, so we may well be good. But we will not know until we have CI tests working gain...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason the ci didn't run after this change is because I merged this change from another branch.

https://github.com/mattn/picotls/actions

@kazuho could you please re-run the action for latest changes ?

Copy link
Member

Choose a reason for hiding this comment

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

@mattn Mind pushing an empty commit? I think @huitema is wondering if we could rerun CI on appveyor.

Regarding _WIN32 vs. _WINDOWS, to me it seems we can switch to _WIN32.

Official documentation does state that _WIN32 will always be defined by MSVC compiler (https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170). OTOH, _WINDOWS does not appear in the list. I wonder if it is something Vistual Studio sets (or if it is a convention in the project-level setting).

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, it is not something that Visual Studio sets. It is something that I kind of invented 5 years ago as I was trying to make things work. I think that to be complete, we should also edit the Visual Studio projects that use the _WINDOWS and _WINDOWS64 macros. But I could do that in a different PR.

As soon as I see a correct test run, I will approve the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, _WIN32 can be used for checking Windows OS from 12 years ago at least. For example, I used _WIN32 in 2010 that works with both compilers.

https://github.com/mattn/gtktweeter

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mattn I do realize that I made a mistake 5 years ago. As I said, I just need to see the test run...

_BitScanReverse(&r, x);
return (31 - r);
Expand Down
10 changes: 5 additions & 5 deletions include/picotls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
extern "C" {
#endif

#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#endif

Expand Down Expand Up @@ -60,7 +60,7 @@ extern "C" {

#define PTLS_ELEMENTSOF(x) (PTLS_ASSERT_IS_ARRAY_EXPR(x) * sizeof(x) / sizeof((x)[0]))

#ifdef _WINDOWS
#ifdef _WIN32
#define PTLS_THREADLOCAL __declspec(thread)
#else
#define PTLS_THREADLOCAL __thread
Expand Down Expand Up @@ -917,7 +917,7 @@ typedef enum en_ptls_early_data_acceptance_t {
/**
* optional arguments to client-driven handshake
*/
#ifdef _WINDOWS
#ifdef _MSC_VER
/* suppress warning C4201: nonstandard extension used: nameless struct/union */
#pragma warning(push)
#pragma warning(disable : 4201)
Expand Down Expand Up @@ -1002,10 +1002,10 @@ typedef struct st_ptls_handshake_properties_t {
*/
int (*collected_extensions)(ptls_t *tls, struct st_ptls_handshake_properties_t *properties, ptls_raw_extension_t *extensions);
} ptls_handshake_properties_t;
#ifdef _WINDOWS
#ifdef _MSC_VER
#pragma warning(pop)
#endif
#ifdef _WINDOWS
#ifdef _MSC_VER
/* suppress warning C4293: >> shift count negative or too big */
#pragma warning(disable : 4293)
#endif
Expand Down
2 changes: 1 addition & 1 deletion include/picotls/ptlsbcrypt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ extern "C" {

#include "../picotls.h"

#ifdef _WINDOWS
#ifdef _WIN32
#include <bcrypt.h>

extern ptls_cipher_algorithm_t ptls_bcrypt_aes128ecb;
Expand Down
2 changes: 1 addition & 1 deletion lib/asn1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* Basic ASN1 validation and optional print-out
*/

#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#else
#include <sys/time.h>
Expand Down
4 changes: 2 additions & 2 deletions lib/cifra/random.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#else
#include <unistd.h>
Expand All @@ -35,7 +35,7 @@
#include "picotls.h"
#include "picotls/minicrypto.h"
#include <stdio.h>
#ifdef _WINDOWS
#ifdef _WIN32
#ifdef _WINDOWS_XP
/* The modern BCrypt API is only available on Windows Vista and later versions.
* If compiling on Windows XP, we need to use the olded "wincrypt" API */
Expand Down
2 changes: 1 addition & 1 deletion lib/ffx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#else
#include <unistd.h>
Expand Down
4 changes: 2 additions & 2 deletions lib/fusion.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
#define NO_SANITIZE_ADDRESS
#endif

#ifdef _WINDOWS
#ifdef _WIN32
#define aligned_alloc(a, s) _aligned_malloc((s), (a))
#endif

Expand Down Expand Up @@ -2165,7 +2165,7 @@ ptls_aead_algorithm_t ptls_non_temporal_aes256gcm = {"AES256-GCM",
sizeof(struct aesgcm_context),
non_temporal_aes256gcm_setup};

#ifdef _WINDOWS
#ifdef _WIN32
/**
* ptls_fusion_is_supported_by_cpu:
* Check that the CPU has extended instructions for PCMUL, AES and AVX2.
Expand Down
2 changes: 1 addition & 1 deletion lib/minicrypto-pem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#else
#include <unistd.h>
Expand Down
4 changes: 2 additions & 2 deletions lib/openssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* IN THE SOFTWARE.
*/

#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#else
#include <unistd.h>
Expand All @@ -45,7 +45,7 @@
#include "picotls.h"
#include "picotls/openssl.h"

#ifdef _WINDOWS
#ifdef _MSC_VER
#ifndef _CRT_SECURE_NO_WARNINGS
#define _CRT_SECURE_NO_WARNINGS
#endif
Expand Down
4 changes: 2 additions & 2 deletions lib/pembase64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/*
* Manage Base64 encoding.
*/
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#else
#include <sys/time.h>
Expand Down Expand Up @@ -301,7 +301,7 @@ int ptls_load_pem_objects(char const *pem_fname, const char *label, ptls_iovec_t
FILE *F;
int ret = 0;
size_t count = 0;
#ifdef _WINDOWS
#ifdef _WIN32
errno_t err = fopen_s(&F, pem_fname, "r");
if (err != 0) {
ret = -1;
Expand Down
8 changes: 4 additions & 4 deletions lib/picotls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*/
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#endif
#include <assert.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#ifndef _WINDOWS
#ifndef _WIN32
#include <errno.h>
#include <pthread.h>
#include <unistd.h>
Expand Down Expand Up @@ -491,7 +491,7 @@ void ptls_buffer__release_memory(ptls_buffer_t *buf)
{
ptls_clear_memory(buf->base, buf->off);
if (buf->is_allocated) {
#ifdef _WINDOWS
#ifdef _WIN32
if (buf->align_bits != 0) {
_aligned_free(buf->base);
} else {
Expand Down Expand Up @@ -523,7 +523,7 @@ int ptls_buffer_reserve_aligned(ptls_buffer_t *buf, size_t delta, uint8_t align_
new_capacity *= 2;
}
if (align_bits != 0) {
#ifdef _WINDOWS
#ifdef _WIN32
if ((newp = _aligned_malloc(new_capacity, (size_t)1 << align_bits)) == NULL)
return PTLS_ERROR_NO_MEMORY;
#else
Expand Down
4 changes: 2 additions & 2 deletions lib/ptlsbcrypt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* IN THE SOFTWARE.
*/

#ifndef _WINDOWS
#ifndef _WIN32
/* This module is only defined for windows.
* It is an implementation of the main crypto algorithms
* using windows crypto libraries */
Expand Down Expand Up @@ -839,4 +839,4 @@ ptls_key_exchange_algorithm_t ptls_bcrypt_x25519 = {PTLS_GROUP_X25519, evp_keyex
ptls_key_exchange_algorithm_t *ptls_bcrypt_key_exchanges[] = {&ptls_bcrypt_secp256r1, NULL};
#endif

#endif /* _WINDOWS */
#endif /* _WIN32 */
2 changes: 1 addition & 1 deletion lib/uecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#else
#include <unistd.h>
Expand Down
6 changes: 5 additions & 1 deletion picotlsvs/picotls/wincompat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
#include <ws2tcpip.h>
#include <malloc.h>

#ifndef _MSC_VER
#include <sys/time.h>
#else
#ifndef gettimeofday
#define gettimeofday wintimeofday

Expand All @@ -32,6 +35,7 @@ int wintimeofday(struct timeval *tv, struct timezone *tz);
} /* extern "C" */
#endif

#endif
#endif

#endif /* WINCOMPAT_H */
#endif /* WINCOMPAT_H */
4 changes: 2 additions & 2 deletions src/esni.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include <inttypes.h>
#include <stdio.h>
#include <string.h>
#ifdef _WINDOWS
#ifdef _MSC_VER
#include "..\picotls\wincompat.h"
#ifndef _CRT_SECURE_NO_WARNINGS
#define _CRT_SECURE_NO_WARNINGS
Expand Down Expand Up @@ -203,7 +203,7 @@ int main(int argc, char **argv)
lifetime *= 86400; /* convert to seconds */
break;
case 'p':
#ifdef _WINDOWS
#ifdef _WIN32
if (sscanf_s(optarg, "%" SCNu16, &padded_length) != 1 || padded_length == 0) {
fprintf(stderr, "padded length must be a positive integer\n");
exit(1);
Expand Down
2 changes: 1 addition & 1 deletion t/fusion.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ static void test_generated(void)
ptls_cipher_context_t *rand = ptls_cipher_new(&ptls_minicrypto_aes128ctr, 1, zero);
ptls_cipher_init(rand, zero);
int i;
#ifdef _WINDOWS
#ifdef _WIN32
const int nb_runs = 1000;
#else
const int nb_runs = 10000;
Expand Down
2 changes: 1 addition & 1 deletion t/hpke.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*/
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#endif
#include <assert.h>
Expand Down
2 changes: 1 addition & 1 deletion t/openssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*/
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#endif
#include <assert.h>
Expand Down
2 changes: 1 addition & 1 deletion t/picotls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*/
#ifdef _WINDOWS
#ifdef _WIN32
#include "wincompat.h"
#endif
#include <assert.h>
Expand Down
Loading