Thanks Cristian. Your explanation makes sense so I guess there's nothing really wrong with the ConstantExpr::getZExtValue() method implementation but I do think the description could be improved as " Return the constant value for a limited number of bits." makes it sounds as if the bit parameter to the method will let you truncate the Constant to that many bits. Perhaps something like this...
///Returns the constant value zero extended to the return type of this method. ///\param bits - optional parameter that can be used to check that the number of bits used by this constant is <= to the parameter value. This is useful for checking that type casts won't truncate useful bits. /// /// Example: /// unit8_t byte= (unit8_t) constant->getZExtValue(8); Thanks, Dan Liew. On 11 September 2012 11:20, Cristian Cadar <[email protected]> wrote: > Hi Dan, > > Thanks for your message. I agree with your fixes to getTrue() and > getFalse(); I applied your patch in r163606: > http://llvm.org/viewvc/llvm-**project?rev=163606&view=rev<http://llvm.org/viewvc/llvm-project?rev=163606&view=rev> > > Regarding ConstantExpr::getZExtValue(), I think the current version is > correct. The bits argument simply states the assumption that the constant > fits within that many bits. For example, if you look at the call at > Memory.cpp:421 "write8(offset, (uint8_t) CE->getZExtValue(8));" the assert > in getZExtValue ensures that the constant fits within 8 bits and the cast > to uint8_t does not truncate the value. Does this make sense or am I > misunderstanding your comments? > > Best wishes, > Cristian > > > On 08/07/2012 10:42 PM, Delcypher wrote: > >> Hi, >> >> I've been doing some work on KLEE recently and I believe I've found >> several bugs in the code for the class ConstantExpr (include/klee/Expr.h). >> >> I need to be able to determine whether or not an instance of a >> ConstantExpr is a boolean true/ boolean false or a integer constant. >> >> The ConstantExpr::isTrue() andConstantExpr::isFalse() methods would seem >> >> to provide this however using them can easily cause assertion failures. >> >> Consider calling ConstantExpr::isTrue() on a ConstantExpr of width 64 >> (i.e. it's not an integer constant). This will execute >> >> return getZExtValue(1) == 1 >> >> Which is turn executes the following assert statement >> assert(getWidth() <= bits && "Value may be out of range!"); >> >> This will fail because it is checking (64 <= 1) which is false! In fact >> the only case where calling ConstantExpr::isTrue() or >> ConstantExpr::isFalse() won't result in assertion failure is if the >> ConstantExpr is width 1. >> >> So there is definitely a bug in bothConstantExpr::isTrue() and >> >> ConstantExpr::isFalse(). >> >> To fix this I present a patch here : >> https://github.com/delcypher/**klee/commit/** >> bcdda366096e6c617a809a58dc8232**b8e533c822.patch<https://github.com/delcypher/klee/commit/bcdda366096e6c617a809a58dc8232b8e533c822.patch> >> >> The other bugs I believe are in the ConstantExpr::getZExtValue(**unsigned >> bits) method.. >> >> * It's description is "- Return the constant value for a limited number >> of bits.". I think this is incredibly mis-leading as I interpret this >> description to mean return the an integer with bits [0,bits -1] and the >> remaining bits set to zero. >> >> The function actually calls llvm::APInt::getZExtValue() which returns >> the integer as a uint64_t regardless of what the "bits" parameter is set >> to. >> >> * The assert statement doesn't really make any sense >> - callingllvm::APInt::**getZExtValue() on allvm::APInt of width greater >> >> than 64 will cause an assertion failure according to the documentation ( >> http://llvm.org/docs/doxygen/**html/classllvm_1_1APInt.html#** >> a7dc983ebf0eb2d255fa90a67063c7**2e2<http://llvm.org/docs/doxygen/html/classllvm_1_1APInt.html#a7dc983ebf0eb2d255fa90a67063c72e2> >> ). >> So an assert really isn't necessary. >> >> * The assert itself doesn't make sense surely it should be testing >> assert(bits <= getWidth() ) instead? >> >> Regards, >> Dan Liew. >> >> >> >> ______________________________**_________________ >> klee-dev mailing list >> [email protected] >> http://keeda.Stanford.EDU/**mailman/listinfo/klee-dev<http://keeda.Stanford.EDU/mailman/listinfo/klee-dev> >> >
_______________________________________________ klee-dev mailing list [email protected] http://keeda.Stanford.EDU/mailman/listinfo/klee-dev
