On Mon, 17 Jul 2023 at 17:29, Peter Maydell <peter.mayd...@linaro.org> wrote: > > Coverity points out that in page_table_walk_refill() we can shift by > a negative number, which is undefined behaviour (CID 1452918, > 1452920, 1452922). We already catch the negative directory_shift and > leaf_shift as being a "bail out early" case, but not until we've > already used them to calculated some offset values. > > Move the calculation of the offset values to after we've done the > "return early if directory_shift or leaf_shift are -1" check. > > Since walk_directory() re-calculates these shift values, add an > assert() to tell Coverity that the caller has already ensured they > won't be negative. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > target/mips/tcg/sysemu/tlb_helper.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/target/mips/tcg/sysemu/tlb_helper.c > b/target/mips/tcg/sysemu/tlb_helper.c > index e5e1e9dd3ff..c67c2b09026 100644 > --- a/target/mips/tcg/sysemu/tlb_helper.c > +++ b/target/mips/tcg/sysemu/tlb_helper.c > @@ -643,6 +643,9 @@ static int walk_directory(CPUMIPSState *env, uint64_t > *vaddr, > uint64_t lsb = 0; > uint64_t w = 0; > > + /* The caller should have checked this */ > + assert(directory_shift > 0 && leaf_shift > 0);
Whoops, this should be >= 0 && ... >= 0. -- PMM
