Skip to content

Commit 9e6127f

Browse files
committed
Merge #74: Avoid >> above type width in BitWriter
fe1040f Drop -Wno-shift-count-overflow compile flag (Pieter Wuille) 154bcd4 Avoid >> above type width in BitWriter (Pieter Wuille) Pull request description: For tiny fields (bits <= 8), the table type is `uint8_t`, which results in `BitWriter` using uint8_t logic as well. This is technically UB, as it performs right shifts of up to 8 positions. Avoid this by performing the BitWriter logic in either the table type, or `unsigned`, whichever is larger. This is an alternative to #71. ACKs for top commit: hebasto: re-ACK fe1040f. Only rebased since my [recent](#74 (review)) review. Tree-SHA512: fb41e125253d11f1662af77e9fc6e89ea7f4712f4fa5d500f483da3c6b325760d3ffedabedb7f15a7702223c86bc651b26004763e44c98d379cc2966ea0919a3
2 parents 67b87ac + fe1040f commit 9e6127f

File tree

2 files changed

+16
-9
lines changed

2 files changed

+16
-9
lines changed

configure.ac

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,6 @@ esac
104104
AX_CHECK_COMPILE_FLAG([-Wall],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"],,[[$CXXFLAG_WERROR]])
105105
AX_CHECK_COMPILE_FLAG([-fvisibility=hidden],[CXXFLAGS="$CXXFLAGS -fvisibility=hidden"],[],[$CXXFLAG_WERROR])
106106

107-
## Some compilers (gcc) ignore unknown -Wno-* options, but warn about all
108-
## unknown options if any other warning is produced. Test the -Wfoo case, and
109-
## set the -Wno-foo case if it works.
110-
AX_CHECK_COMPILE_FLAG([-Wshift-count-overflow],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-shift-count-overflow"],,[[$CXXFLAG_WERROR]])
111-
112107
if test "x$use_ccache" != "xno"; then
113108
AC_MSG_CHECKING(if ccache should be used)
114109
if test x$CCACHE = x; then

src/int_utils.h

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,10 @@ class BitWriter {
5555
int offset = 0;
5656
unsigned char* out;
5757

58-
public:
59-
BitWriter(unsigned char* output) : out(output) {}
60-
6158
template<int BITS, typename I>
62-
inline void Write(I val) {
59+
inline void WriteInner(I val) {
60+
// We right shift by up to 8 bits below. Verify that's well defined for the type I.
61+
static_assert(std::numeric_limits<I>::digits > 8, "BitWriter::WriteInner needs I > 8 bits");
6362
int bits = BITS;
6463
if (bits + offset >= 8) {
6564
state |= ((val & ((I(1) << (8 - offset)) - 1)) << offset);
@@ -78,6 +77,19 @@ class BitWriter {
7877
offset += bits;
7978
}
8079

80+
81+
public:
82+
BitWriter(unsigned char* output) : out(output) {}
83+
84+
template<int BITS, typename I>
85+
inline void Write(I val) {
86+
// If I is smaller than an unsigned int, invoke WriteInner with argument converted to unsigned.
87+
using compute_type = typename std::conditional<
88+
(std::numeric_limits<I>::digits < std::numeric_limits<unsigned>::digits),
89+
unsigned, I>::type;
90+
return WriteInner<BITS, compute_type>(val);
91+
}
92+
8193
inline void Flush() {
8294
if (offset) {
8395
*(out++) = state;

0 commit comments

Comments
 (0)