gh-96821: Fix undefined behaviour in audioop.c - #96923
Conversation
Sorry, something went wrong.
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.
|
@kumaraditya303 @mdickinson @erlend-aasland Would you please have a look? Thanks! |
Sorry, something went wrong.
|
Sorry but I won't have time for this in the near future, Mark or Erlend can take a look. |
Sorry, something went wrong.
mdickinson
left a comment
There was a problem hiding this comment.
The changes LGTM at least in principle; I haven't done much in the way of testing.
Sorry, something went wrong.
|
@matthiasgoergens Can we revert the changes at lines 173 and 262 of the original code? I'm fairly sure that those changes aren't necessary, because the quantity being shifted is always nonnegative. |
Sorry, something went wrong.
To be clear, I'm happy with the rest of the changes as-is, so if @JelleZijlstra is also happy, I think we can merge after those two reversions. |
Sorry, something went wrong.
|
@mdickinson Thanks. I'll revert the lines you asked about. |
Sorry, something went wrong.
|
@mdickinson @JelleZijlstra I applied the suggested changes. Thanks for having a look! |
Sorry, something went wrong.
* pythongh-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>
Left-shifting negative numbers is undefined behaviour.
Fortunately, multiplication works just as well, is (implementation) defined behaviour, and gets compiled to the same machine code as before by optimizing compilers.