Uh oh!
There was an error while loading. Please reload this page.
gh-96821: Fix undefined behaviour in audioop.c - #96923
Conversation
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 but I won't have time for this in the near future, Mark or Erlend can take a look. |
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
mdickinson
commented
Oct 9, 2022
@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. |
mdickinson
commented
Oct 9, 2022
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. |
matthiasgoergens
commented
Oct 9, 2022
@mdickinson Thanks. I'll revert the lines you asked about. |
@mdickinson@JelleZijlstra I applied the suggested changes. Thanks for having a look! |
* 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.
-fstrict-overflow#96821