Skip to content

Conversation

@justinzhuguangwen
Copy link

Description

This PR fixes Windows build compatibility issues when using Clang as the compiler on Windows (with the MSVC toolchain). The build system was previously attempting to use Linux-specific compiler and linker flags that are not supported on Windows, causing build failures or warnings. This change adds conditional logic to use platform-appropriate flags based on the target platform.

Context:
When Clang is used on Windows (not MSVC compiler, but Clang with MSVC toolchain), it has different capabilities and limitations compared to Clang on Linux. The previous code assumed a Linux-like environment, which caused issues on Windows.

Key Changes:

  1. DEF File Format Handling (lines 41-48):

    • On Windows, use Windows .def file format via CMAKE_LINK_DEF_FILE_FLAG instead of Linux version-script format
    • Use win${TBB_ARCH} prefix for Windows DEF files instead of lin${TBB_ARCH}
    • Maintains backward compatibility for non-Windows platforms
  2. Compile Flags (lines 72-78):

    • Excludes -fPIC on Windows (not supported by Clang with MSVC toolchain)
    • Excludes -fstack-clash-protection and -fcf-protection on Windows (not supported)
    • Adds -D_CRT_SECURE_NO_WARNINGS to suppress CRT security warnings for deprecated functions (e.g., getenv, strncpy) that TBB uses
    • Retains -fstack-protector-strong and format security checks on Windows
  3. Library Compile Flags (line 80):

    • Explicitly excludes Windows from -fstack-clash-protection and -fcf-protection=full flags
    • These flags are only applied on non-Apple, non-Android, x86/x64 platforms that are not Windows
  4. Linker Flags (lines 84-88):

    • Excludes Windows from -z linker options (relro, now, noexecstack)
    • These options are Linux-specific and not supported by lld-link on Windows
    • Updated comments to clarify that -z switch is not supported on both macOS and Windows

Impact:

  • ✅ Windows builds with Clang (MSVC toolchain) now succeed without unsupported flag errors
  • ✅ Proper Windows .def file format is used instead of Linux version-script
  • ✅ CRT security warnings are suppressed for deprecated functions that TBB uses
  • ✅ No impact on Linux, macOS, or other platforms (backward compatible)

Fixes # - issue number(s) if exists

Type of change

Choose one or multiple, leave empty if none of the other choices apply

Add a respective label(s) to PR if you have permissions

  • bug fix - change that fixes an issue
  • new feature - change that adds functionality
  • tests - change in tests
  • infrastructure - change in infrastructure and CI
  • documentation - documentation update

Tests

  • added - required for new features and some bug fixes
  • not needed

Documentation

  • updated in # - add PR number
  • needs to be updated
  • not needed

Breaks backward compatibility

  • Yes
  • No
  • Unknown

Notify the following users

List users with @ to send notifications

Other information

Testing:

  • ✅ Verified that the changes maintain existing behavior on Linux and macOS
  • ✅ Tested that Windows builds with Clang (MSVC toolchain) now succeed without unsupported flag errors
  • ✅ Confirmed that Windows-specific DEF file format is correctly used
  • ✅ Verified that all conditional branches work correctly for their respective platforms

Technical Details:

  • The changes use WIN32 CMake variable to detect Windows platform (works for both native Windows and cross-compilation)
  • Windows-specific flags are only applied when WIN32 is true, ensuring other platforms are unaffected
  • The -z linker options are Linux ELF-specific and not supported by lld-link on Windows
  • -fPIC is not needed on Windows as DLLs use a different linking model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant