LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 1188 - BitMask error
Summary: BitMask error
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Interpreter (show other bugs)
Version: 1.8
Hardware: All All
: P normal
Assignee: Reid Spencer
URL:
Keywords: compile-fail
Depends on:
Blocks:
 
Reported: 2007-02-07 18:08 PST by Leo
Modified: 2010-02-22 12:55 PST (History)
1 user (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Leo 2007-02-07 18:08:42 PST
I find there is a potential bitmask error with long long type.
In the INTEGER_ASSIGN macro, the program uses (1ull << BitWidth) -1 to get the
bitmask. If BitWidth = 64, the value is 0. The correct way is to use
~(uint64_t)(0ull) >> (BitWidth-1).

Please review the following patch.
Index: lib/ExecutionEngine/Interpreter/Execution.cpp
===================================================================
RCS file: /var/cvs/llvm/llvm/lib/ExecutionEngine/Interpreter/Execution.cpp,v
retrieving revision 1.167
diff -t -d -u -p -5 -r1.167 Execution.cpp
--- lib/ExecutionEngine/Interpreter/Execution.cpp	2 Feb 2007 02:16:22 -0000	1.167
+++ lib/ExecutionEngine/Interpreter/Execution.cpp	7 Feb 2007 23:58:56 -0000
@@ -1305,11 +1305,11 @@ void Interpreter::visitAShr(BinaryOperat
   SetValue(&I, Dest, SF);
 }
 
 #define INTEGER_ASSIGN(DEST, BITWIDTH, VAL)     \
   {                                             \
-    uint64_t Mask = (1ull << BITWIDTH) - 1;     \
+    uint64_t Mask = ~(uint64_t)(0ull) >> (64-BITWIDTH);     \
     if (BITWIDTH == 1) {                        \
       Dest.Int1Val = (bool) (VAL & Mask);       \
     } else if (BITWIDTH <= 8) {                 \
       Dest.Int8Val = (uint8_t) (VAL & Mask);    \
     } else if (BITWIDTH <= 16) {                \
Index: lib/ExecutionEngine/Interpreter/Interpreter.h
===================================================================
RCS file: /var/cvs/llvm/llvm/lib/ExecutionEngine/Interpreter/Interpreter.h,v
retrieving revision 1.82
diff -t -d -u -p -5 -r1.82 Interpreter.h
--- lib/ExecutionEngine/Interpreter/Interpreter.h	2 Feb 2007 02:16:22 -0000	1.82
+++ lib/ExecutionEngine/Interpreter/Interpreter.h	7 Feb 2007 23:58:56 -0000
@@ -234,11 +234,11 @@ private:  // Helper functions
   void popStackAndReturnValueToCaller(const Type *RetTy, GenericValue Result);
 
 };
 
 inline void maskToBitWidth(GenericValue& GV, unsigned BitWidth) {
-  uint64_t BitMask = (1ull << BitWidth) - 1;
+  uint64_t BitMask = ~(uint64_t)(0ull) >> (64-BitWidth);
   if (BitWidth <= 8)
     GV.Int8Val &= BitMask;
   else if (BitWidth <= 16)
     GV.Int16Val &= BitMask;
   else if (BitWidth <= 32)
Comment 1 Reid Spencer 2007-02-07 18:30:46 PST
Looks right to me. Patches applied. Thanks, Leo.