On 1/28/26 9:07 AM, Richard Henderson wrote:
This uninitialized value violates the contract in the
documentation comment, and may lead to a SEGV during
translaton with -d in_asm.
Change the documentation to disallow hostp NULL.
Pass hostp to probe_access_internal directly.
Reported-by: Panda Jiang <[email protected]>
Signed-off-by: Richard Henderson <[email protected]>
Hi Richard,
Thanks a lot for the patch!
---
accel/tcg/internal-common.h | 2 +-
accel/tcg/cputlb.c | 7 +++----
accel/tcg/user-exec.c | 4 +---
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
...
@@ -1545,7 +1545,9 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState
*env, vaddr addr,
(void)probe_access_internal(env_cpu(env), addr, 1, MMU_INST_FETCH,
cpu_mmu_index(env_cpu(env), true), false,
- &p, &full, 0, false);
+ hostp, &full, 0, false);
+
+ p = *hostp;
if (p == NULL) {
return -1;
}
...
I have a small question regarding the implementation choice. My initial
thought was to simply initialize *hostp to NULL at the beginning of
get_page_addr_code_hostp to solve the uninitialized variable issue.
I've been comparing that with your approach of passing 'hostp' directly
into probe_access_internal(). As I see it, the main functional
difference is that in your implementation, if probe_access_internal()
succeeds but the subsequent check
'if (full->lg_page_size < TARGET_PAGE_BITS)' fails, the function
returns -1, but *hostp will hold a valid pointer set by
probe_access_internal(). With the simple initialization approach, *hostp
would always be NULL in any failure case.
Is this behavior intentional? I was wondering if it provides some kind
of performance benefit or gives the caller more context even on this
specific failure path.
Thanks again for the fix!
Best regards,
Panda Jiang
