Skip to content

feat(libc): implement timingsafe_bcmp - #12586

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
voidlakhan:feat/libc-timingsafe-bcmp
Nov 2, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
voidlakhan:feat/libc-timingsafe-bcmp

Conversation

@voidlakhan

Copy link
Copy Markdown
Contributor

What

Implement timingsafe_bcmp (NID aWl82o9ammc) in libc matching FreeBSD 11.4 behavior, and add test coverage in GuestStrings.cpp.

Reference:
https://github.com/freebsd/freebsd-src/blob/release/11.4.0/lib/libc/string/timingsafe_bcmp.c

Tested

Windows 10, core/libs/tests/GuestStrings.cpp checking equal, non-equal at start/middle/end, reversed order, sub-lengths, empty buffers, and high-bit byte sequences.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Progress report (26fa121 → c08fe41)

📈 System libraries: 26.23% (+0.0022%, +1 implemented functions)

✅ 1 implemented
Library Name
libc timingsafe_bcmp_nid_postfix

Changed libraries (implemented exports)

Library Before After Change
libc 1484/1500 (98.93%) 1485/1501 (98.93%) +0.0007%

@adriadam10 adriadam10 self-assigned this Oct 28, 2026
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@voidlakhan thanks, the port reads well: the body matches FreeBSD 11.4's timingsafe_bcmp.c, has no early exit, the NID recomputes, and the new assertions are registered.

One thing before it can go in: the repository owner decided on 28 Oct that code ported from FreeBSD or other permissively licensed sources keeps its original copyright notice and licence text, verbatim. timingsafe_bcmp.c is ISC-licensed (Copyright (c) 2010 Damien Miller). Could you add that notice and the ISC licence text to Strings.cpp? Put it next to the function, or at the top of the file with a line naming the function and the source file and revision it came from.

@rooklz rooklz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I ran a host-side check on macOS arm64 (Apple clang, -fsanitize=address,undefined). The harness holds the function body from 33b5bee and the 11 new Require lines word for word. They all pass, and the NID aWl82o9ammc recomputes from timingsafe_bcmp.

Mutants against the new assertions:

Mutant Result
loop starts at 1 killed (safeD)
loop stops one byte early killed
& instead of ^ killed
ret == 0 inverted killed
p1[i] - p2[i] instead of ^ equivalent (non-zero exactly when the bytes differ)
return ret; (raw OR of the XORs) survives
return -(ret != 0); survives

The body follows FreeBSD 11.4, where the function returns (ret != 0), so it gives exactly 1 when the buffers differ. The tests only check != 0, so nothing would catch a change that returns some other non-zero value. If 1 is the intended contract, two lines pin it down and kill both survivors:

     Require(timingsafe_bcmp_nid_postfix(highB, highA, 2) != 0);
+    Require(timingsafe_bcmp_nid_postfix(safeA, safeB, 6) == 1);
+    Require(timingsafe_bcmp_nid_postfix(highA, highB, 2) == 1);

Apart from that, and the licence notice @oneandonlydean asked for, this looks right to me. I did not build guest_strings itself (the macOS lane here has no submodules), and I did not test on a console.

AI-assisted: yes

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 10113 docs/dev/debt/libc.md L270
PR 10226 docs/dev/debt/libc.md L270
PR 10349 docs/dev/debt/libc.md L270
PR 14235 docs/dev/debt/libc.md L270

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 5021 core/libs/prx/libc/src/Strings.cpp
docs/dev/debt/libc.md
PR 7081 docs/dev/debt/libc.md
PR 7386 docs/dev/debt/libc.md
PR 7447 docs/dev/debt/libc.md
PR 7715 docs/dev/debt/libc.md
PR 8150 docs/dev/debt/libc.md
PR 8965 core/libs/prx/libc/src/Strings.cpp
core/libs/tests/GuestStrings.cpp
docs/dev/debt/libc.md
PR 9340 docs/dev/debt/libc.md
PR 9371 docs/dev/debt/libc.md
PR 9447 docs/dev/debt/libc.md
PR 9526 docs/dev/debt/libc.md
PR 9929 docs/dev/debt/libc.md
PR 10405 docs/dev/debt/libc.md
PR 10987 docs/dev/debt/libc.md
PR 11332 docs/dev/debt/libc.md
PR 11625 docs/dev/debt/libc.md
PR 11831 core/libs/prx/libc/include/Api.hpp
docs/dev/debt/libc.md
PR 12560 docs/dev/debt/libc.md
PR 14212 docs/dev/debt/libc.md

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 28, 2026
@voidlakhan
voidlakhan force-pushed the feat/libc-timingsafe-bcmp branch from 5b02f8a to 6b3ffd4 Compare October 29, 2026 07:53
@voidlakhan

Copy link
Copy Markdown
Contributor Author

@oneandonlydean @rooklz Updated:

  1. Added the verbatim ISC copyright notice for timingsafe_bcmp (Copyright (c) 2010 Damien Miller) and attribution to FreeBSD 11.4's timingsafe_bcmp.c directly above timingsafe_bcmp_nid_postfix in Strings.cpp. Added the corresponding technical debt entry in docs/dev/debt/libc.md.
  2. Added the two suggested exact-return assertions (== 1) in GuestStrings.cpp.
  3. Rebased onto current main (10bec5c322). Local convention checks and the guest_strings tests pass.

@voidlakhan
voidlakhan force-pushed the feat/libc-timingsafe-bcmp branch from 6b3ffd4 to 1264958 Compare October 30, 2026 08:03
@voidlakhan

Copy link
Copy Markdown
Contributor Author

@adriadam10 The merge conflict has now been resolved. I've rebased PR #12586 onto the latest main, preserved all existing debt entries, and pushed the updated branch.

The focused guest_strings test and convention checks pass locally. The updated Build and Progress report workflows are now running.

Could you please take another look at the updated head when convenient? Thanks!

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 30, 2026
@voidlakhan

Copy link
Copy Markdown
Contributor Author

Investigated the macOS write-watch CI failure.

This failure is unrelated to the timingsafe_bcmp changes in this PR.

The failure was caused by duplicate CTest registrations in core/libs/tests/write_watch/CMakeLists.txt for:

  • write_watch_execute-fault
  • write_watch_read-fault

The duplicate registrations caused CMake configuration to fail before the build or tests could run.

This was a pre-existing issue on the base branch and was addressed separately in PR #13220 (test(libc): register the macOS write watch fault tests once).

No changes to the timingsafe_bcmp implementation are needed to address this failure.

@oneandonlydean oneandonlydean self-assigned this Oct 30, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Nov 1, 2026
# Conflicts:
#	docs/dev/debt/libc.md
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Nov 1, 2026
@oneandonlydean
oneandonlydean merged commit 25aeb9f into boykopovar:main Nov 2, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants