On Thu, Jan 18, 2018 at 10:20 AM, Peter Maydell <[email protected]> wrote: > On 18 January 2018 at 18:07, Max Filippov <[email protected]> wrote: >> On Thu, Jan 18, 2018 at 4:24 AM, Peter Maydell <[email protected]> >> wrote: >>> Hi. Coverity (CID 1385146) complains about this bit of code: >>> >>>> + opnds = xtensa_opcode_num_operands(isa, opc); >>>> + >>>> + for (opnd = vopnd = 0; opnd < opnds; ++opnd) { >>>> + if (xtensa_operand_is_visible(isa, opc, opnd)) { >>>> + uint32_t v; >>> >>> because xtensa_opcode_num_operands() can return -1 >>> (if the CHECK_OPCODE() fails), and then we will try to use >>> -1 as an upper bound for this loop and it will loop for a very >>> long time. >>> >>> CID 1385148 is similar but for the outer loop where >>> we do "slots = xtensa_format_num_slots()" and then use >>> slots as a loop bound without checking whether we got back -1. >> >> In both cases the check inside libisa should not fail, because >> the caller just made sure that opcode or format is valid. But I >> see, these checks are different. I guess the easiest fix for that >> is to make opnd, opnds, slot and slots signed. > > If you definitely know you won't get back -1 then I think > putting "assert(opnds != XTENSA_UNDEFINED);" etc will express > the intention and should make coverity happy.
I'd rather use data types that match these function return types. -- Thanks. -- Max
