[joshuaswarren]

READY TO MERGE

Head: b87e7456cdbb7abdea2f360cdcba0e2a56316f3f.

GitHub reports MERGEABLE against main: no text conflicts. This is not a test pass.

The exact-head package and native tests are recorded below. The three transform-feedback CTS chunks cover 2,719 cases on both M1 chips: all 18 former crashes pass, with no other result changes. The six submitted-call probes also pass. The head-bound source review is GREEN. No build/test CI is configured for this head. These are targeted checks, not full CTS. Land this first in the agreed order: #10, #8, #9, then #6. An authorized maintainer must merge it; the author account has pull-only access.

Earlier review, source and test notes are kept below with their original bindings. They do not change this status.

---

What this fixes

vkCmdBeginTransformFeedbackEXT and vkCmdEndTransformFeedbackEXT crash the application when pCounterBuffers is NULL and counterBufferCount is not zero. The driver reads pCounterBuffers[0] and faults at address 0.

The reference page for vkCmdBeginTransformFeedbackEXT says pCounterBuffers is NULL or a pointer to an array of handles. It also says that with NULL, transform feedback starts at byte offset zero in all bound transform feedback buffers. The reference page for vkCmdEndTransformFeedbackEXT allows NULL in the same way, but it does not say what NULL means for End. Here End then records no counter.

In the conformance group transform_feedback, 18 cases pass NULL with a count of 1 and crash. They are resume_1_131072, resume_1_256 and resume_1_512, each in the null_counter_buffers and the null_ptrs_end_xfb variant, in the simple, simple_fast_gpl and simple_optimized_gpl groups. In the test source at VK-GL-CTS commit 9335a4e4e455, vktTransformFeedbackSimpleTests.cpp lines 1420 and 1433 to 1443, the null_counter_buffers variant calls Begin with a count of 1 and NULL arrays, and the null_ptrs_end_xfb variant calls End with a count of 1 and NULL arrays. I did not check that the test binary on my machines was built from that unmodified tree.

Stored core files of four of these crashes on the 16-inch M1 Max show the same fault. The first frame is the load of pCounterBuffers[0] in hk_begin_end_xfb, with pCounterBuffers equal to 0 and the count equal to 1.

What this changes

The loop over the counter buffers in hk_begin_end_xfb now runs only when pCounterBuffers is not NULL. Begin still sets the four offsets to zero, because the copy to the offsets runs with no source for each of them. End still copies nothing. A call with an array, and an array with a null handle in it, take the same path as before.

Test results

A small test program, run on the 16-inch M1 Max, one boot, one fresh process per case. It records and submits Begin and End with a bound counter buffer that starts at 0x1000, and reads the buffer after the wait.

Case	Main	This change
Begin and End with NULL arrays, count 1	crash	pass
Begin with NULL, End with the buffer	crash	pass, buffer reads 0x0
Begin with the buffer, End with NULL	crash	pass, buffer reads 0x1000
Begin and End with the buffer	pass, 0x1000	pass, 0x1000
A null handle in a non-NULL array	pass	pass
Count 0 with NULL	pass	pass

Main crashes the same way as a package build and as a native build. The program draws nothing, so "Begin starts at zero" is shown by the End copy reading 0 and not by a captured draw.

Three chunks of the transform_feedback group that hold the 18 cases: chunk 117, chunk 125 and chunk 133, with the library bytes of built packages, one run per arm. The 16-inch M1 Max and the 13-inch M1 give the same tables.

Chunk	Cases	Main Pass	Main Crash	This change Pass	This change Crash
117	1000	716	6	722	0
125	1000	709	6	715	0
133	719	538	6	544	0

The other cases in each chunk are NotSupported: 278, 285 and 175. Exactly 18 of the 2719 cases differ on both chips, all from Crash to Pass, and 2701 are unchanged. No row gets worse.

The control is the package of main. The candidate is the package of this commit on main. Both packages were built with the same build options.

What this does not show

• These are three chunks, 2719 of the 133,719 cases of the group. The 133,719 cases are in 134 chunks.
• The NotSupported cases were not executed, so a defect behind them is not seen. Other transform feedback functions were not searched for NULL pointer defects.
• The test program ran on the 16-inch M1 Max only. The 13-inch M1 ran the three chunks.
• There is no unit test for this change, because a Vulkan entry point has no test harness in this tree.

[joshuaswarren]

GREEN at b87e745, with two non-blocking notes. I read the full diff against main (7cda785) and ran no build.

Checked:

• The change is one condition in the loop of hk_begin_end_xfb: pCounterBuffers && i < counterBufferCount. Begin and End both go through this function, so both are covered.
• With a NULL array, Begin still gives each of the four offsets a destination and no source. The copy kernel writes src ? *src : 0, so every counter starts at zero. End copies nothing and dispatches nothing.
• A real array, and an array with a null handle in it, run the same instructions as before.
• hk_CmdBindTransformFeedbackBuffersEXT already handles a NULL pSizes. I found no second NULL pointer defect there. I did not read the other transform feedback entry points.
• The numbers in the body match the raw per-chunk results from both machines. Chunks 117, 125 and 133 hold 2719 cases. Exactly 18 differ, all Crash to Pass. 2701 are unchanged and no row gets worse. The group has 133,719 cases in 134 chunks.
• This branch merges with main and with the heads of honeykrisp: map placed memory at the allocation size #8 and honeykrisp: give capture and replay buffers their own address range #9 without conflicts.

Notes:

1. cmd_idx = firstCounterBuffer + i is not checked against 4. Valid usage limits it, and the same holds for a non-NULL array before this change.
2. The test program draws nothing, so "Begin starts at zero" is shown through the End copy reading 0. The conformance chunks are the draw-level check, as the body says.

Merging is the maintainers' call.

[joshuaswarren]

Both notes are open items. I did not change the branch.

1. The index firstCounterBuffer + i is not checked against 4, with a NULL array as with a real one. Valid usage bounds it, and the code before this change did the same. A bounds check is a separate change. A new commit changes the head id, and the library bytes carry that id, so the package result above is tied to this head.

2. The test program draws nothing. It shows the Begin zeroing through the End copy, which reads 0. The conformance chunks are the draw-level check, as the body says.

[joshuaswarren]

Maintainer routing request for PR 10. This is a request for a merge contact, not a claim that review is finished.

It fixes a crash when pCounterBuffers is NULL in the transform feedback begin and end calls. What was run on the head b87e7456cdbb and what was not run is in the description and in the two review comments above; I have not changed it since. PR 10 belongs to a merge order of PR 8, PR 9 and PR 10, then PR 6, then PR 11. A whole-suite run of PR 6 and the PR 11 commit on top of PRs 8, 9 and 10, against PRs 8, 9 and 10 alone, is queued. Its results will be posted on the PR 6 thread.

Question: who can merge into this repository's main branch, and in which of these PRs would you like to start? My account has pull access only.

[joshuaswarren]

Second routing request for PR 10, about 14 hours after the first. It is still a request for a merge contact, not a claim that review is finished.

Nothing on the head has changed since my first note above. The review notes and the list of what was run are in the description and the two review comments.

Question: who can merge into this repository's main branch? If there is a different place to ask, a link is enough. My account has pull access only, so I cannot merge or mark anything ready.

[joshuaswarren]

Status, 25 Oct: unchanged, independent of the rest. It is included in #12 with its original commit and is in both arms of the #6 whole-suite run. It can land as is, or be closed as included once #12's branch lands.

[joshuaswarren]

Status, 25 Oct, for the landing order (#10 first, then #8, then #9, then #6).

Head b87e7456cdbb. It merges cleanly on main 7cda7850ed (checked with a merge of the whole sequence #10, #8, #9, #6: no textual conflicts at any step).

MEASURED: the whole-suite CTS of the stack that contains this change (#8, #9, #10 plus #6, sha 518212284e) ran on a 16-inch M1 Max and a 13-inch M1. The control arm was main plus #8, #9 and #10. No test in the compute, transform feedback or pipeline groups changed from Pass to Fail because of this change. The run tells nothing about #10 alone.

NOT measured: the CTS with only this change, and a vkQuake or vkmark run.

[joshuaswarren]

Batch 1 for main: #10, then #8, then #9, in that order.

All three apply to main at 7cda7850ed with no conflicts. They touch different code and git applies them in any order. The order is the agreed landing order. The #9 device run below is on top of #10 and #8. The #10 and #8 results below were measured on each branch alone. We checked the three together as a local merge (tip 59c25f62c9d). #19 went into integration/aurora, not main, so this base has not moved.

PR	Head	Change	Device result at that head
#10	b87e7456	accept a NULL array of transform feedback counter buffers	three transform_feedback chunks, 2,719 cases, on a 16-inch M1 Max and a 13-inch M1: the 18 former crashes pass, no other change
#8	ddfb4cf1	map placed memory at the allocation size	memory chunk 0, 1000 cases, both chips: 4 Fail and 1 Crash become Pass, no other row differs
#9	11ae82d8	give capture and replay buffers their own address range	binding_model chunk 0, 1000 cases, 13-inch M1, run at this exact head: 122 ResourceError become Pass, no case gets worse

Stack check, one boot on a 13-inch M1, binding_model chunk 0: main gives 340 Pass, 538 NotSupported and 122 ResourceError. Main with #10, #8 and #9 gives 462 Pass, 538 NotSupported and 0 ResourceError. The 122 changes are all ResourceError to Pass. This shows the three together. It does not isolate #9 from #8 and #10.

These are targeted checks, not the full conformance suite. The CTS binary's source commit is not independently verified.

#6 is not part of this batch. Its run over the touched test groups at its current head has not finished, and three shader_object.performance timing cases still need a decision. We will post that result separately. #3, #4 and #5 are superseded by #6 and will be closed after it merges.

The author account on these PRs has pull-only access, so a maintainer has to merge. Tell us if you want a different order or a different check first.

[joshuaswarren]

@maralcbr could you review this current head for landing first? The exact-head test evidence and remaining scope limits are in the PR body. The requested landing order is #10, then #8, then #9. Our account cannot assign formal reviewers or merge here; please route this to a main-branch maintainer if needed.