[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 27/43] target/loongarch: Add TLB instruction support
From: |
Peter Maydell |
Subject: |
Re: [PULL 27/43] target/loongarch: Add TLB instruction support |
Date: |
Thu, 7 Nov 2024 17:33:30 +0000 |
On Tue, 7 Jun 2022 at 00:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> From: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>
> This includes:
> - TLBSRCH
> - TLBRD
> - TLBWR
> - TLBFILL
> - TLBCLR
> - TLBFLUSH
> - INVTLB
Hi; running the loongarch functional tests on a build with
the clang undefined-behaviour sanitizer enabled reveals an
attempt to shift by an out-of-range amount in
helper_invtlb_page_asid_or_g():
../../target/loongarch/tcg/tlb_helper.c:470:31: runtime error: shift
exponent 244 is too large for 64-bit type 'uint64_t' (aka 'unsigned
long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../target/loongarch/tcg/tlb_helper.c:470:31 in
> +void helper_invtlb_page_asid_or_g(CPULoongArchState *env,
> + target_ulong info, target_ulong addr)
> +{
> + uint16_t asid = info & 0x3ff;
> +
> + for (int i = 0; i < LOONGARCH_TLB_MAX; i++) {
> + LoongArchTLB *tlb = &env->tlb[i];
> + uint8_t tlb_g = FIELD_EX64(tlb->tlb_entry0, TLBENTRY, G);
> + uint16_t tlb_asid = FIELD_EX64(tlb->tlb_misc, TLB_MISC, ASID);
> + uint64_t vpn, tlb_vppn;
> + uint8_t tlb_ps, compare_shift;
> +
> + if (i >= LOONGARCH_STLB) {
> + tlb_ps = FIELD_EX64(tlb->tlb_misc, TLB_MISC, PS);
> + } else {
> + tlb_ps = FIELD_EX64(env->CSR_STLBPS, CSR_STLBPS, PS);
> + }
We read here a field from the guest, which can be 0.
> + tlb_vppn = FIELD_EX64(tlb->tlb_misc, TLB_MISC, VPPN);
> + vpn = (addr & TARGET_VIRT_MASK) >> (tlb_ps + 1);
> + compare_shift = tlb_ps + 1 - R_TLB_MISC_VPPN_SHIFT;
If tlb_ps is 0, then "tlb_ps + 1 - R_TLB_MISC_VPPN_SHIFT"
is tlb_ps + 1 - 13 == tlb_ps - 12. When converted back to
uint8_t this is 244.
> +
> + if ((tlb_g || (tlb_asid == asid)) &&
> + (vpn == (tlb_vppn >> compare_shift))) {
Here we shift tlb_vppn by 244, which is undefined behaviour
and triggers the sanitizer.
> + tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, E, 0);
> + }
> + }
> + tlb_flush(env_cpu(env));
> +}
What's the intended behaviour here?
This likely applies also to other similar functions; this
is just the one that I found.
thanks
-- PMM
- Re: [PULL 27/43] target/loongarch: Add TLB instruction support,
Peter Maydell <=