Address novel errors found in -G Xcode builds#2729
Conversation
jzulauf-lunarg
requested review from
locke-lunarg,
mikes-lunarg and
richard-lunarg
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Keep indirect copy size multiplication in 64-bit arithmetic
Using uint32_t for draw_call_params_size causes draw_call_params_size * max_draw_count to be evaluated in 32-bit arithmetic before assignment to VkDeviceSize. When very large indirect draw counts are captured (for example, max_draw_count > UINT32_MAX / sizeof(VkDrawIndexedIndirectCommand)), copy_buffer_size can wrap, which under-allocates the cloned buffer while the generated copy regions still cover the full draw range, leading to invalid/out-of-bounds buffer copy operations during replay resource dumping.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Looks pretty sane to me. The AI generated code review that suggests size_t or uint64_t for pointer math seems a reasonable tweak. |
jzulauf-lunarg
force-pushed
the
zulauf-make-G-Xcode-happy
branch
3 times, most recently
from
abab758 to
53b0a17
Compare
|
@richard-lunarg @mikes-lunarg -- review/approve? |
jzulauf-lunarg
added
the
approved-to-run-ci
jzulauf-lunarg
force-pushed
the
zulauf-make-G-Xcode-happy
branch
from
53b0a17 to
ad89c43
Compare
Which also had the potential for errors. Fixed with an extended set of conversion data loss checks (GFXRECON_NARROWING_*) and corrections to types in use and function signatures.
jzulauf-lunarg
force-pushed
the
zulauf-make-G-Xcode-happy
branch
from
ad89c43 to
3623e90
Compare
2 participants
Was experimenting with the XCode generator to build graphics reconstruct, encountered a set of narrowing related "warnings as errors". Updated the CONVERSION_DATA_LOSS to support to easily address and patched the issues, streamlined to cast only operations for release builds. Additionally, some errors involved VkDeviceSize information narrowing through 32bit paths back to VkDeviceSize. Addressed the narrow impacts on hash seeding.