Repository navigation
Conversation
|
@JJenkx the change matches your native comparison and the null guards on
Optional, for #12593 and #12594 alike: #12589's debt entry records that the behaviour is the titles' |
|
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.
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.
|
|
@oneandonlydean thanks, you're right: the loop ran (timer, null) twice. Fixed in I also added the debt entry you suggested. It records the titles' rule from PPSA07923's On Windows, AI-assisted: yes (Claude Code). |
rooklz
left a comment
There was a problem hiding this comment.
I ran this on macOS x86_64 (Apple clang, ASan+UBSan, -fno-sanitize-recover), at head 704acff4af. It's a standalone build of Time.cpp, TimeExtras.cpp, Environment.cpp and Multibyte.cpp with the target's -include HostStandardStreams.hpp, plus a two-function shim: __error_nid_postfix returns &errno, and set_constraint_handler_s_nid_postfix calls SonySetConstraintHandlerS.
guest_time_conversionandguest_time_extraspass at head (3/3 runs).- If I swap in
main'sTime.cpp,guest_time_conversionfails withnull argument did not report a bad argument, as your body says. git merge-treeagainstmain319621ba2eis clean, and conventions pass.- Every internal caller of
gmtime_s/localtime_s(tzset,mktime,timegm,SonyCtimeS) passes non-null pointers, so the new handler call can't fire from inside the library.
Mutants. I ran 12 against the two tests. 5 are killed: main's Time.cpp, gmtime_s reporting only a null timer, a message of "gmtime_s", ERANGE for EINVAL, and result passed as the handler pointer. These 7 survive:
| Mutant | Survives because |
|---|---|
gmtime_s, both null → silent nullptr |
(null, null) is never exercised; which == 2 repeats which == 0, as oneandonlydean noted |
localtime_s, both null → silent nullptr |
same |
libc_gmtime / libc_localtime without the new timer == nullptr guard |
no test passes a null timer to the non-_s functions with a recording handler installed |
gmtime_r / localtime_r guarding only result |
same |
(If gmtime_r loses its guard completely, guest_time_extras catches it, because its null-result call aborts in the default handler.)
The body says gmtime, localtime, gmtime_r and localtime_r "keep returning null for a null argument without calling the handler". That is half of this PR, but no test pins it. Here is a test-only diff. It includes oneandonlydean's (null, null) fix, and it checks that the four non-_s entry points (plus libc_gmtime/libc_localtime) return null for every null combination while the recording handler is installed, without calling it:
@@ -51,13 +51,22 @@ static void CheckNullArguments() {
const char* expected = convert == gmtime_s_nid_postfix ? "gmtime_s: bad argument" : "localtime_s: bad argument";
for (int which = 0; which < 3; ++which) {
g_constraintCalls = 0;
- Require(convert(which == 1 ? nullptr : &timer, which == 0 ? nullptr : which == 1 ? &result : nullptr) == nullptr,
+ Require(convert(which == 0 ? &timer : nullptr, which == 1 ? &result : nullptr) == nullptr,
"null argument was accepted");
Require(g_constraintCalls == 1 && g_constraintMessage == expected, "null argument did not report a bad argument");
}
g_constraintCalls = 0;
Require(convert(&timer, &result) == &result && g_constraintCalls == 0, "valid conversion reported a violation");
}
+ g_constraintCalls = 0;
+ for (const auto convert : {libc_gmtime_nid_postfix, gmtime_nid_postfix, libc_localtime_nid_postfix, localtime_nid_postfix}) {
+ Require(convert(nullptr) == nullptr, "null timer was accepted");
+ }
+ for (const auto convert : {gmtime_r_nid_postfix, localtime_r_nid_postfix}) {
+ Require(convert(nullptr, &result) == nullptr && convert(&timer, nullptr) == nullptr && convert(nullptr, nullptr) == nullptr,
+ "null argument was accepted");
+ }
+ Require(g_constraintCalls == 0, "non-_s conversion reported a violation");
set_constraint_handler_s_nid_postfix(previous);
}With this diff all 12 mutants are killed, and head stays green (3/3 under ASan+UBSan).
One question about provenance. Your native comparison covers gmtime_s and localtime_s. Did the scratch loader also call the titles' gmtime/localtime/gmtime_r/localtime_r with a null argument and a recording handler installed? If it did, a line in Tested would back up the new assertions. If it didn't, the non-_s side is main's behaviour kept unchanged, which is fine, but the body could say so.
|
@rooklz thanks. Your diff is applied:
Provenance: I hadn't measured those four before, so I did now. The titles'
So the silent null return is AI-assisted: yes (Claude Code). |
jotitadev
left a comment
There was a problem hiding this comment.
First Linux run of this head (the thread has Windows and a standalone macOS build).
Ubuntu 26.04, i7-8700, GCC 15.2, Release, BUILD_TESTING=ON. Head b8ea6ed9cc merged (--no-commit) onto current main ed1e0ed08f; the merge is clean.
guest_time_conversion: passes 3/3.guest_time_*,guest_mktime,guest_strftime,guest_crtandguest_locale: 10/10 pass.public_headers: passes.- Control: with
Time.cppandTimeExtras.cppfrommainand this PR's test kept,guest_time_conversionaborts 3/3 atnull argument did not report a bad argument.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
A Windows run with GCC 16: Windows 11 Pro 10.0.22631, i9-9900K, MSYS2 UCRT64 GCC 16.1.0, Release, this PR (
AI-assisted: yes (Claude Code) |
…traint handler The libc.prx that titles ship (PPSA07923) calls the constraint handler with "gmtime_s: bad argument" or "localtime_s: bad argument", EINVAL and a null pointer when either argument of gmtime_s or localtime_s is null, and returns null. main returned null without calling it. gmtime, localtime, gmtime_r and localtime_r, which are built on them, keep returning null for a null argument without a call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 704acff)
The loop repeated the null-result case instead of passing both arguments null; it now runs (timer, null), (null, result) and (null, null). The debt notes record the measured rule. Both from oneandonlydean's review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit db1c1b2)
…r _r forms gmtime, localtime, libc_gmtime, libc_localtime, gmtime_r and localtime_r must return null for a null argument without calling the installed constraint handler. Suggested by rooklz in review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit b8ea6ed)
b8ea6ed to
23fcc2d
Compare
|
Current-main refresh — 2026-11-02. Published head Rebuilt actual libc/Ampr and selected tests from the final input tree. Linux-7.2.11-arch1-1-x86_64-with-glibc2.44, Ryzen 9 7950X, GCC 16.2.1, Ninja Release, BUILD_TESTING=ON. Focused Linux CPU qualification only. Older Windows/module/reference reports retain their original pins; no fresh Windows/macOS, SDK-module, console, title/GPU or speed claim. AI-assisted: yes (Codex). Evidence: https://github.com/JJenkx/AnyPS5/releases/download/linux-validation-20261023/math-six-main-a4eca-linux-extra10321-20261102.tar.gz; SHA-256 525ccb31a5d3d0f5adced5fda58818d8432f06c0afa7290139adb60ae6664b54. |
|
Windows check of the rebased head Setup: Windows 11 Enterprise 10.0.22621, Ryzen 9 5950X, WinLibs GCC 15.2.0 UCRT r7 (the CI toolchain), Release,
🤖 Generated with Claude Code |
|
The failing
AI-assisted: yes. |
✅ AMD GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (15, not counted against this PR)Validation IDs already present on main (4 IDs in 765 tests)Skipped (61)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
What
gmtime_sandlocaltime_snow report a nulltimerorresultto the constraint handler, as titles'libc.prxdoes. The handler gets"gmtime_s: bad argument"or"localtime_s: bad argument",EINVALand a null pointer, and the function returns null. Onmainit returned null without calling the handler.gmtime,localtime,gmtime_randlocaltime_rare built on these two. They keep returning null for a null argument without calling the handler, exactly as onmain.The default constraint-handler prerequisite is fulfilled on current main
b11c7bf84f5d2f636ff60166cc3371bd071e24d5: #9935 was closed after its changes landed through #10270. Merging its final head into this main produces the unchanged main tree. Earlier test and title measurements retain their recorded source pins.Overlap: ntamas94's #10409 (36-byte
struct tm,tm_isdst = -1fromgmtime_s) also edits these two functions. It currently conflicts withmainon its own; the change here is the four-line null check at the top of each function.Tested
Current-main refresh — 2026-11-02. Published head
23fcc2d857f40e9c7f2b0cbc451c89d574bddb30includes pinned maina4eca2a056bf3bbbec98ab42e9d28163e4ea9cb8. The time-conversion constraint implementation and null argument fixtures are retained with main’s Time.cpp updates and timelocal debt. Quiet null returns of ordinary gmtime/localtime and _r forms remain the documented host compatibility behavior, not a new title-library assertion.Rebuilt actual libc/Ampr and selected tests from the final input tree.
guest_crt,guest_time_conversion,guest_time_extras,guest_mktime,guest_strftime,guest_localeeach pass three consecutive runs: 18 runtime passes, zero skips, plus public_headers. Conventions and whitespace pass. Source/input manifests, exact dependencies, library/test hashes, logs/JUnit and head-leased push receipts are attached. The same pinned main passes 18 selected groups three times (54 runtime passes) plus headers. All 34 recursive dependency trees/31,551 tracked blobs verify with zero errors; QuickJS’s update=none test262 corpus is explicitly excluded because it is not a library build input.Linux-7.2.11-arch1-1-x86_64-with-glibc2.44, Ryzen 9 7950X, GCC 16.2.1, Ninja Release, BUILD_TESTING=ON. Focused Linux CPU qualification only. Older Windows/module/reference reports retain their original pins; no fresh Windows/macOS, SDK-module, console, title/GPU or speed claim. AI-assisted: yes (Codex).
Evidence: https://github.com/JJenkx/AnyPS5/releases/download/linux-validation-20261023/math-six-main-a4eca-linux-extra10321-20261102.tar.gz; SHA-256 525ccb31a5d3d0f5adced5fda58818d8432f06c0afa7290139adb60ae6664b54.
Earlier dated qualifications below retain their recorded source/platform pins.
Windows 11, WinLibs GCC 15.2.0 UCRT r7, Release,
main319621ba2e.gmtime_s: 50,000 random calls with times across the 64-bit range and null arguments. Calls with a null argument differ in 4,960 cases onmainand 0 with this PR.tm_isdst(titles -1,main0), which fix(libc): write a 36-byte struct tm as the console does #10409 addresses;gmtime_sgets those dates wrong (it returns wrapped years even forINT64_MAX), which this build does not copy.localtime_s: the titles report"localtime_s: bad argument"for a nulltimer, a nullresultand both. Its non-null path needs a time-zone import the loader doesn't provide, so it isn't compared._sfunctions: the titles'gmtime,localtime,gmtime_randlocaltime_rdon't check for null at all. Each of the eight null cases (one for each single-argument function, three for each_rfunction) ends in an access violation on PPSA07923. Returning null without a handler call for these four is thereforemain's existing, safer behaviour, which this PR keeps, not a titles rule.guest_time_conversiongainsCheckNullArguments, which covers each null combination for both functions and a valid call with no handler call. With the previousTime.cppit fails (null argument did not report a bad argument).guest_time_conversion,guest_time_extras,guest_mktime,guest_strftime,guest_crtandguest_localepass 3/3. Conventions pass.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code