Skip to content
This repository has been archived by the owner on Jan 27, 2022. It is now read-only.

Fix compilation with clang-3.3 #63

Closed
wants to merge 2 commits into from

Conversation

sebastinas
Copy link
Contributor

While testing PR #62 on different platforms, I noticed that it failed to compile with clang-3.3. clang-3.3 is stricter regarding the second argument of _mm_shuffle_epi32. This arguments needs to be an immediate value.

clang-3.3 is stricter regarding the second argument of _mm_shuffle_epi32.

Signed-off-by: Sebastian Ramacher <sebastian+dev@ramacher.at>
clang provides the same constant as bit_AESNI in some versions, and doesn't
provide it at all in others.

Signed-off-by: Sebastian Ramacher <sebastian+dev@ramacher.at>
@msabramo
Copy link
Contributor

Hey @sebastinas,

Nice work! This fixes build problems related to _mm_shuffle_epi32 that I was having with the master branch.

I'm on OS X 10.8.5 with Xcode 5.0.1 and what I guess is clang 5.0?

marca@marca-mac:~/dev/git-repos/pycrypto$ clang --version
Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin12.5.0
Thread model: posix

I was getting compile errors related to _mm_shuffle_epi32. Note I'm using tox with the tox.ini file that I added in #64.

marca@marca-mac:~/dev/git-repos/pycrypto$ tox -e py27
...
    building 'Crypto.Cipher._AESNI' extension
    gcc -g -O3 -g -O2 -Wall -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -DHAVE_CONFIG_H -Isrc/ -I/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -c src/AESNI.c -o build/temp.macosx-10.6-intel-2.7/src/AESNI.o -maes
    src/AESNI.c:51:16: error: index for __builtin_shufflevector must be a constant integer
        keygened = _mm_shuffle_epi32(keygened, shuf);
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/5.0/include/emmintrin.h:1276:12: note: expanded from macro '_mm_shuffle_epi32'
      (__m128i)__builtin_shufflevector((__v4si)__a, (__v4si) _mm_set1_epi32(0), \
               ^
    src/AESNI.c:51:14: error: assigning to '__m128i' from incompatible type 'void'
        keygened = _mm_shuffle_epi32(keygened, shuf);
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    2 errors generated.
    error: command 'gcc' failed with exit status 1
...

With your changes, now the build works!

marca@marca-mac:~/dev/git-repos/pycrypto$ grep pull .git/config
    fetch = +refs/pull/*:refs/remotes/origin/pr/*
marca@marca-mac:~/dev/git-repos/pycrypto$ git remote update
...
marca@marca-mac:~/dev/git-repos/pycrypto$ git checkout origin/pr/63/merge
...
HEAD is now at 72cf738... Merge 53cb3e231b2a88f04d20a4298124ff06481566f4 into af058ee6f5da391a05275470ab4a4a96aa22b350
marca@marca-mac:~/dev/git-repos/pycrypto$ tox -e py25,py26,py27,pypy
...
  py25: commands succeeded
  py26: commands succeeded
  py27: commands succeeded
  pypy: commands succeeded

Nice work!

@msabramo msabramo mentioned this pull request Nov 12, 2013
@msabramo
Copy link
Contributor

I did a little more testing to verify that this works in all of the various Python versions, including Python 3.4.0a4.

Here's a sample tox run on OS X. I'm using a local branch which has PR #63 (this PR), PR #64 ("Add support for tox"), and PR #65 ("Use different method for getting ext_suffix" -- this fixes a problem on Python 3.4.0a4) merged in:

marca@marca-mac:~/dev/git-repos/pycrypto$ git --no-pager log -n 4
commit 3c916ff4c54279b68d30c22db293206e3d787624
Merge: 72cf738 84bb4aa
Author: Marc Abramowitz <marc@marc-abramowitz.com>
Date:   Tue Nov 12 08:02:12 2013 -0800

    Merge branch 'gracefully_handle_lack_of_SO_distutils_config_var' into pr/63

commit 84bb4aa0a4f04d754959aa4ae0c8286b33d11df7
Author: Marc Abramowitz <marc@marc-abramowitz.com>
Date:   Mon Nov 11 22:45:42 2013 +0000

    Refactor 3 places handling fastmath ImportError

    so that they call `Crypto.SelfTest.st_common.handle_fastmath_import_error`,
    thereby eliminiating duplicate code.

commit 65a86343a89292cfe615309ee0d0329dab7e3432
Author: Marc Abramowitz <marc@marc-abramowitz.com>
Date:   Mon Nov 11 21:57:06 2013 +0000

    Use different method for getting ext_suffix

    ```
    ext_suffix = get_config_var("EXT_SUFFIX") or get_config_var("SO")
    ```

    because `get_config_var("SO")` returns None in Python 3.4.0a4 because the "SO"
    variable is deprecated and "EXT_SUFFIX" is the new way to get this information
    (see: http://bugs.python.org/issue19555)

    This fixes `TypeError: Can't convert 'NoneType' object to str implicitly`
    errors when running the tests on Python 3.4.0a4.

commit 72cf7388cbf22b9258ae3b7f3620409ad6fa8eb7
Merge: af058ee 53cb3e2
Author: Sebastian Ramacher <sebastian+dev@ramacher.at>
Date:   Wed Nov 6 08:25:10 2013 -0800

    Merge 53cb3e231b2a88f04d20a4298124ff06481566f4 into af058ee6f5da391a05275470ab4a4a96aa22b350
marca@marca-mac:~/dev/git-repos/pycrypto$ tox
...
  py25: commands succeeded
  py26: commands succeeded
  py27: commands succeeded
  py32: commands succeeded
  py33: commands succeeded
  py34: commands succeeded
  pypy: commands succeeded
  congratulations :)

@isghe
Copy link

isghe commented Apr 6, 2014

great, it works for me too :-)

@chuckcarpenter
Copy link

I've run into this as well, this fix worked for me.

@dlitz
Copy link
Member

dlitz commented Jun 23, 2014

Cherry-picked these as:

Thanks!

@dlitz dlitz closed this Jun 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants