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

[C++] Fix Windows 32 bits compile and runtime failures #11082

Merged

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Jun 24, 2021

Motivation

C++ source code cannot be compiled successfully for Windows 32-bit build. The compile error is:

fatal error C1021: invalid preprocessor command 'warning'

It's because #warning preprocessor directive is not supported by MSVC compilers.

And even if the related code was remove and the build succeeded, a example producer program would crash. This bug is introduced from #6129. It's because hardware CRC32 implementation on Windows is only provided for 64-bit programs. However, the __cpuid check in crc32_initialize() still returns true for 32-bit build, so finally it went here:

uint32_t crc32c(uint32_t init, const void *buf, size_t len, const chunk_config *config) {
    // SSE 4.2 extension for hw implementation are not present
    abort();
}

The abort() call will terminate the process.

Modifications

  • Use #pragma message() to replace #warning if the compiler is MSVC.
  • Fallback to software implementation of CRC32 checksum algorithm if the hardware implementation is not supported.
  • Add the workflow to build Windows 32-bit C++ library.
  • Fix the wrong document about how to build it for Windows 32-bit.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

@BewareMyPower
Copy link
Contributor Author

BewareMyPower commented Jun 24, 2021

In addition, when I built with Visual Studio 2017 after the fix, the compilation still failed:

CMake Error at lib/CMakeLists.txt:68 (target_link_options):
Unknown CMake command "target_link_options".

It's because target_link_options was firstly introduced from CMake 3.13, see https://cmake.org/cmake/help/latest/command/target_link_options.html. However, Visual Studio 2017's builtin CMake version is 3.12.18081601-MSVC_2.

The minimum version of CMake should be 3.13 but not 3.4. But it might affect existed workflow like CentOS 7 build or docker-build.sh. And it only matters for Windows build. So this PR won't modify it.

@codelipenghui codelipenghui merged commit c8fe1e8 into apache:master Jun 25, 2021
@BewareMyPower BewareMyPower deleted the bewaremypower/fix-cpp-win32 branch June 25, 2021 02:07
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jun 25, 2021
codelipenghui pushed a commit that referenced this pull request Jun 25, 2021
### Motivation

C++ source code cannot be compiled successfully for Windows 32-bit build. The compile error is:

>  fatal error C1021: invalid preprocessor command 'warning'

It's because `#warning` preprocessor directive is not supported by MSVC compilers.

And even if the related code was remove and the build succeeded, a example producer program would crash. This bug is introduced from #6129. It's because hardware CRC32 implementation on Windows is only provided for 64-bit programs. However, the `__cpuid` check in `crc32_initialize()` still returns true for 32-bit build, so finally it went here:

```c++
uint32_t crc32c(uint32_t init, const void *buf, size_t len, const chunk_config *config) {
    // SSE 4.2 extension for hw implementation are not present
    abort();
}
```

The `abort()` call will terminate the process.

### Modifications

- Use `#pragma message()`  to replace `#warning` if the compiler is MSVC.
- Fallback to software implementation of CRC32 checksum algorithm if the hardware implementation is not supported.
- Add the workflow to build Windows 32-bit C++ library.
- Fix the wrong document about how to build it for Windows 32-bit.

(cherry picked from commit c8fe1e8)
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
### Motivation

C++ source code cannot be compiled successfully for Windows 32-bit build. The compile error is:

>  fatal error C1021: invalid preprocessor command 'warning'

It's because `#warning` preprocessor directive is not supported by MSVC compilers.

And even if the related code was remove and the build succeeded, a example producer program would crash. This bug is introduced from apache#6129. It's because hardware CRC32 implementation on Windows is only provided for 64-bit programs. However, the `__cpuid` check in `crc32_initialize()` still returns true for 32-bit build, so finally it went here:

```c++
uint32_t crc32c(uint32_t init, const void *buf, size_t len, const chunk_config *config) {
    // SSE 4.2 extension for hw implementation are not present
    abort();
}
```

The `abort()` call will terminate the process.

### Modifications

- Use `#pragma message()`  to replace `#warning` if the compiler is MSVC.
- Fallback to software implementation of CRC32 checksum algorithm if the hardware implementation is not supported.
- Add the workflow to build Windows 32-bit C++ library.
- Fix the wrong document about how to build it for Windows 32-bit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants