Skip to content

fix: make cobra-core build under MSVC - #50

Open
w00tzenheimer wants to merge 1 commit into
trailofbits:masterfrom
w00tzenheimer:fix-msvc-build
Open

fix: make cobra-core build under MSVC#50
w00tzenheimer wants to merge 1 commit into
trailofbits:masterfrom
w00tzenheimer:fix-msvc-build

Conversation

@w00tzenheimer

Copy link
Copy Markdown

Problem

cobra-core does not compile with MSVC. Two independent issues, both surfaced
by a build on windows-latest (VS 2022):

1. __builtin_popcountll is a GCC/Clang extension.
lib/core/StructureRecovery.cpp:169 calls it directly:

error C3861: '__builtin_popcountll': identifier not found

Four further errors cascade from it as the surrounding auto &src fails to
deduce (C2530, C3536, C2446 x2).

2. /sdl promotes C4146 to an error.
CMakeLists.txt:41 enables /sdl, which turns this warning into a hard error:

error C4146: unary minus operator applied to unsigned type, result still unsigned

It fires at five sites that deliberately compute two's-complement negation for
Expr::Kind::kNeg, all of the form (-x) & mask:

lib/core/SignatureEval.cpp:71
lib/core/TemplateDecomposer.cpp:184
lib/core/TemplateDecomposer.cpp:1122
lib/core/SemilinearSignature.cpp:46
lib/core/SemilinearSignature.cpp:144

Negation of an unsigned value is well defined, and the masking makes the intent
explicit.

Fix

  1. Use std::popcount from <bit>. It is available since C++20 and the
    project already sets CMAKE_CXX_STANDARD 23, so no feature test is needed.
    The operand is uint64_t, making this a direct substitution.

  2. Add /wd4146 for MSVC. Suppressing reads better than rewriting the idiom at
    five sites: the negation is intentional, and the warning carries little
    signal in code that does modular arithmetic throughout. The comment in
    CMakeLists.txt records why.

Verification

Builds clean on Windows (MSVC, VS 2022) where it previously failed.

Rebuilt on macOS arm64 to confirm no behaviour change:

cmake --build build --target cobra-core
./build/tools/cobra-cli/cobra-cli --mba "(x|y)-(x&y)" --bitwidth 32
x ^ y

Also green on macOS x86_64 and Linux (manylinux_2_28).

Note

Independent of the change itself, building the dependency superbuild on a
machine without a system LLVM also requires the companion fix in #49 to
dependencies/CMakeLists.txt.

Two portability defects surfaced by building on windows-latest:

1. StructureRecovery.cpp used __builtin_popcountll, a GCC/Clang extension
   with no MSVC equivalent (error C3861, plus four cascading errors as the
   surrounding reference failed to deduce). Replaced with std::popcount from
   <bit> -- available since C++20 and the project already requires C++23.

2. /sdl promotes C4146 'unary minus applied to unsigned type' to an error,
   breaking five sites that deliberately compute two's-complement negation
   for Expr::Kind::kNeg ((-x) & mask, well-defined for unsigned). Added
   /wd4146 rather than rewriting the idiom at each site.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

3 participants