Skip to content

Commit 553d3c1

Browse files
gh-96821: Fix undefined behaviour in audioop.c (#96923)
* gh-96821: Fix undefined behaviour in `audioop.c` Left-shifting negative numbers is undefined behaviour. Fortunately, multiplication works just as well, is defined behaviour, and gets compiled to the same machine code as before by optimizing compilers. Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
1 parent dfcdee4 commit 553d3c1

File tree

2 files changed

+16
-12
lines changed

2 files changed

+16
-12
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix undefined behaviour in ``audioop.c``.

Modules/audioop.c

+15-12
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ static const int16_t seg_uend[8] = {
5959
static int16_t
6060
search(int16_t val, const int16_t *table, int size)
6161
{
62+
assert(0 <= size);
63+
assert(size < INT16_MAX);
6264
int i;
6365

6466
for (i = 0; i < size; i++) {
@@ -170,6 +172,7 @@ st_14linear2ulaw(int16_t pcm_val) /* 2's complement (14-bit range) */
170172
if (seg >= 8) /* out of range, return maximum value. */
171173
return (unsigned char) (0x7F ^ mask);
172174
else {
175+
assert(seg >= 0);
173176
uval = (unsigned char) (seg << 4) | ((pcm_val >> (seg + 1)) & 0xF);
174177
return (uval ^ mask);
175178
}
@@ -300,13 +303,13 @@ static const int stepsizeTable[89] = {
300303
#ifdef WORDS_BIGENDIAN
301304
#define GETINT24(cp, i) ( \
302305
((unsigned char *)(cp) + (i))[2] + \
303-
(((unsigned char *)(cp) + (i))[1] << 8) + \
304-
(((signed char *)(cp) + (i))[0] << 16) )
306+
(((unsigned char *)(cp) + (i))[1] * (1 << 8)) + \
307+
(((signed char *)(cp) + (i))[0] * (1 << 16)) )
305308
#else
306309
#define GETINT24(cp, i) ( \
307310
((unsigned char *)(cp) + (i))[0] + \
308-
(((unsigned char *)(cp) + (i))[1] << 8) + \
309-
(((signed char *)(cp) + (i))[2] << 16) )
311+
(((unsigned char *)(cp) + (i))[1] * (1 << 8)) + \
312+
(((signed char *)(cp) + (i))[2] * (1 << 16)) )
310313
#endif
311314

312315

@@ -347,10 +350,10 @@ static const int stepsizeTable[89] = {
347350
} while(0)
348351

349352

350-
#define GETSAMPLE32(size, cp, i) ( \
351-
(size == 1) ? (int)GETINT8((cp), (i)) << 24 : \
352-
(size == 2) ? (int)GETINT16((cp), (i)) << 16 : \
353-
(size == 3) ? (int)GETINT24((cp), (i)) << 8 : \
353+
#define GETSAMPLE32(size, cp, i) ( \
354+
(size == 1) ? (int)GETINT8((cp), (i)) * (1 << 24) : \
355+
(size == 2) ? (int)GETINT16((cp), (i)) * (1 << 16) : \
356+
(size == 3) ? (int)GETINT24((cp), (i)) * (1 << 8) : \
354357
(int)GETINT32((cp), (i)))
355358

356359
#define SETSAMPLE32(size, cp, i, val) do { \
@@ -1558,7 +1561,7 @@ audioop_ulaw2lin_impl(PyObject *module, Py_buffer *fragment, int width)
15581561

15591562
cp = fragment->buf;
15601563
for (i = 0; i < fragment->len*width; i += width) {
1561-
int val = st_ulaw2linear16(*cp++) << 16;
1564+
int val = st_ulaw2linear16(*cp++) * (1 << 16);
15621565
SETSAMPLE32(width, ncp, i, val);
15631566
}
15641567
return rv;
@@ -1632,7 +1635,7 @@ audioop_alaw2lin_impl(PyObject *module, Py_buffer *fragment, int width)
16321635
cp = fragment->buf;
16331636

16341637
for (i = 0; i < fragment->len*width; i += width) {
1635-
val = st_alaw2linear16(*cp++) << 16;
1638+
val = st_alaw2linear16(*cp++) * (1 << 16);
16361639
SETSAMPLE32(width, ncp, i, val);
16371640
}
16381641
return rv;
@@ -1757,7 +1760,7 @@ audioop_lin2adpcm_impl(PyObject *module, Py_buffer *fragment, int width,
17571760

17581761
/* Step 6 - Output value */
17591762
if ( bufferstep ) {
1760-
outputbuffer = (delta << 4) & 0xf0;
1763+
outputbuffer = (delta * (1 << 4)) & 0xf0;
17611764
} else {
17621765
*ncp++ = (delta & 0x0f) | outputbuffer;
17631766
}
@@ -1875,7 +1878,7 @@ audioop_adpcm2lin_impl(PyObject *module, Py_buffer *fragment, int width,
18751878
step = stepsizeTable[index];
18761879

18771880
/* Step 6 - Output value */
1878-
SETSAMPLE32(width, ncp, i, valpred << 16);
1881+
SETSAMPLE32(width, ncp, i, valpred * (1 << 16));
18791882
}
18801883

18811884
rv = Py_BuildValue("(O(ii))", str, valpred, index);

0 commit comments

Comments
 (0)