Fix double-scaled pointer arithmetic in ETDumpGen constructor #18782
Fix double-scaled pointer arithmetic in ETDumpGen constructor #18782
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18782
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 3 New Failures, 2 Unrelated FailuresAs of commit 3d9d308 with merge base 21d9c64 ( NEW FAILURES - The following jobs have failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following job failed but was present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
bot
added
the
CLA Signed
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
Fixes an incorrect pointer-arithmetic expression in ETDumpGen’s constructor that could advance the builder pointer far beyond the intended location when computing the start of the aligned buffer region.
Changes:
- Replace
builder_ + sizeof(struct flatcc_builder)withbuilder_ + 1to avoid double-scaling in typed pointer arithmetic. - Preserve the intended “advance by one builder struct, then align” behavior when computing
buffer_with_builder.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
devtools/etdump/etdump_flatcc.cpp
Outdated
| (struct flatcc_builder*)internal::align_pointer(buffer.data(), 64); | ||
| uintptr_t buffer_with_builder = (uintptr_t)internal::align_pointer( | ||
| builder_ + sizeof(struct flatcc_builder), 64); | ||
| builder_ + 1, 64); |
There was a problem hiding this comment.
why are we advancing the pointer at all?
There was a problem hiding this comment.
I think there's separate memory to hold the flatcc_builder which has context to build the flatbuffer, and then the working arena.
…ECUTORCH-32) The expression `builder_ + sizeof(struct flatcc_builder)` double-scales the offset because `builder_` is a `struct flatcc_builder*` -- the compiler already multiplies by `sizeof(struct flatcc_builder)` for typed pointer arithmetic. The result advances far past the intended location, potentially into unallocated memory. Replace with `builder_ + 1`, which correctly advances by exactly one `sizeof(struct flatcc_builder)` element. This PR was authored with the assistance of Claude.
builder_ + sizeof(struct flatcc_builder)results inbuilder_ + sizeof(struct flatcc_builder) * sizeof(struct flatcc_builder)Because C/C++ arithmetic builder_ + N advances by N*sizeof(type) where type is the type of builder_. This means we get a pointer that advances past the intended memory location, potentially into unallocated memory.
Replace with
builder_ + 1, which correctly advances by exactly onesizeof(struct flatcc_builder)element.This PR was authored with the assistance of Claude.