Skip to content

build: default to a 64-bit only build for the Capstone decoder - #41

Closed
lvaroqui wants to merge 1 commit into
masterfrom
build/capstone-only64bit-default
Closed

lvaroqui wants to merge 1 commit into
masterfrom
build/capstone-only64bit-default

Conversation

@lvaroqui

@lvaroqui lvaroqui commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Capstone is compiled for the primary platform only, and cycledecode.c does not build without it. On a bi-arch host with 32-bit multilib, a plain ./configure also enabled the secondary x86 Callgrind, which then failed in make with a missing capstone/capstone.h and the CLG_WITH_CAPSTONE #error.

Default to --enable-only64bit unless --enable-only64bit or --enable-only32bit was given either way, and fail at configure time with a clear message if a secondary platform is still requested.

Capstone is compiled for the primary platform only, and cycledecode.c does
not build without it. On a bi-arch host with 32-bit multilib, a plain
./configure also enabled the secondary x86 Callgrind, which then failed in
make with a missing capstone/capstone.h and the CLG_WITH_CAPSTONE #error.

Default to --enable-only64bit unless --enable-only64bit or
--enable-only32bit was given either way, and fail at configure time with a
clear message if a secondary platform is still requested.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Changes build configuration defaults for the decoder.

The PR appears safe to merge, with non-blocking test coverage needed for its new default behavior.

Fix All in Claude CodeFindings

  1. P2 Default lacks CI coverage ▶
Fix with agent prompt
### Issue 1
configure.ac:336
Both CI builds pass `--enable-only64bit`, so neither tests what happens when `./configure` receives no architecture flag. If the new default stops selecting a single platform, CI could pass while a plain build fails when the secondary Callgrind target compiles without Capstone. Please add a test that configures without either architecture flag and checks that no secondary platform is selected.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR makes an unqualified configure select a 64-bit-only build and rejects secondary-platform builds before compilation.

  • The platform guard aligns with the current primary-only Capstone linkage.
  • CI does not exercise the new no-flag default.

Reviews (1) · Last reviewed commit: "build: default to a 64-bit only build fo..."

Comment thread configure.ac
# used. --njn]

# CodSpeed: 64-bit only by default. See m4/codspeed_capstone.m4.
CODSPEED_ONLY64BIT_DEFAULT

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Default lacks CI coverage Both CI builds pass --enable-only64bit, so neither tests what happens when ./configure receives no architecture flag. If the new default stops selecting a single platform, CI could pass while a plain build fails when the secondary Callgrind target compiles without Capstone. Please add a test that configures without either architecture flag and checks that no secondary platform is selected.

Prompt To Fix With AI
This is a comment left during a code review.
Path: configure.ac
Line: 336

Comment:
**Default lacks CI coverage** Both CI builds pass `--enable-only64bit`, so neither tests what happens when `./configure` receives no architecture flag. If the new default stops selecting a single platform, CI could pass while a plain build fails when the secondary Callgrind target compiles without Capstone. Please add a test that configures without either architecture flag and checks that no secondary platform is selected.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

@lvaroqui

lvaroqui commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #42, which uses the COD-3590 branch name.

@lvaroqui lvaroqui closed this Oct 1, 2026
@codspeed

codspeed Bot commented Oct 1, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 84 untouched benchmarks
⏩ 60 skipped benchmarks1


Comparing build/capstone-only64bit-default (e60746a) with master (4adb224)

Open in CodSpeed

Footnotes

  1. 60 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

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