Triple Yang
2012-Dec-21 17:38 UTC
[LLVMdev] A potential bug in helper function "fieldFromInstruction" in tablegen'erated file "XXXGenDisassemblerTables.inc"
Helper function: template<typename InsnType> static InsnType fieldFromInstruction(InsnType insn, unsigned startBit, unsigned numBits) { assert(startBit + numBits <= (sizeof(InsnType)*8) && "Instruction field out of bounds!"); InsnType fieldMask; if (numBits == sizeof(InsnType)*8) fieldMask = (InsnType)(-1LL); else fieldMask = ((1 << numBits) - 1) << startBit; return (insn & fieldMask) >> startBit; } may fail if the last parameter "startBit" is larger than 31 which is likely to occur when instruction sets have encodings more than 32 bits. In "else" statement, RHS is evaluated on 32-bit integers, and thus might result in decoding errors in 32-bit platforms. ************************************************* fieldMask = ((1 << numBits) - 1) << startBit; ************************************************* should be: ******************************************** fieldMask = ((uint64_t(1) << numBits) - 1) << startBit; ******************************************** or something similar. Can someone clarify this situation? Thanks. -- 杨勇勇 (Yang Yongyong)
NAKAMURA Takumi
2012-Dec-26 07:04 UTC
[LLVMdev] A potential bug in helper function "fieldFromInstruction" in tablegen'erated file "XXXGenDisassemblerTables.inc"
Yongyong, fixed in r171101. Thanks for your reporting! ...Takumi 2012/12/22 Triple Yang <triple.yang at gmail.com>:> Helper function: > > template<typename InsnType> > static InsnType fieldFromInstruction(InsnType insn, unsigned startBit, > unsigned numBits) { > assert(startBit + numBits <= (sizeof(InsnType)*8) && > "Instruction field out of bounds!"); > InsnType fieldMask; > if (numBits == sizeof(InsnType)*8) > fieldMask = (InsnType)(-1LL); > else > fieldMask = ((1 << numBits) - 1) << startBit; > return (insn & fieldMask) >> startBit; > } > > may fail if the last parameter "startBit" is larger than 31 which is > likely to occur when instruction sets have encodings more than 32 > bits. > > In "else" statement, RHS is evaluated on 32-bit integers, and thus > might result in decoding errors in 32-bit platforms. > > ************************************************* > fieldMask = ((1 << numBits) - 1) << startBit; > ************************************************* > > should be: > > ******************************************** > fieldMask = ((uint64_t(1) << numBits) - 1) << startBit; > ******************************************** > > or something similar. > > Can someone clarify this situation? Thanks. > > -- > 杨勇勇 (Yang Yongyong) > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev