gh-96821: Fix undefined behaviour in audioop.c by matthiasgoergens · Pull Request #96923 · python/cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation20 Commits11 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

matthiasgoergens

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.

@matthiasgoergens

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.

@blurb-it

@matthiasgoergens

@matthiasgoergens

This was referenced

Sep 21, 2022

@matthiasgoergens

@kumaraditya303

Sorry but I won't have time for this in the near future, Mark or Erlend can take a look.

@matthiasgoergens

JelleZijlstra

@matthiasgoergens

@matthiasgoergens

mdickinson

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes LGTM at least in principle; I haven't done much in the way of testing.

@matthiasgoergens

mdickinson

mdickinson

mdickinson

@mdickinson

@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

Can we revert the changes at lines 173 and 262 of the original code?

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

@mdickinson Thanks. I'll revert the lines you asked about.

@matthiasgoergens

JelleZijlstra

mpage pushed a commit to mpage/cpython that referenced this pull request

Oct 11, 2022

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>