On 1/29/26 17:13, Panda Jiang wrote:
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.
Good catch. I will assign *hostp = NULL along that one path.
r~
