1233981 - rijndael.c needs to #include <intrin.h> for _xgetvb()

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[(no longer active)]

Without this, clang-cl generates:

rijndael.obj : error LNK2019: unresolved external symbol _xgetbv referenced in function check_xcr0_ymm

Comment 1

[(no longer active)]

Attachment: Add #include <intrin.h> to rijndael.c for _xgetvb()

Comment 2

[Wan-Teh Chang]

Comment on attachment 8700328 [details] [diff] [review]
Add #include <intrin.h> to rijndael.c for _xgetvb()

Review of attachment 8700328 [details] [diff] [review]:
-----------------------------------------------------------------

Ehsan: thanks for the patch. I suggest two changes.

::: security/nss/lib/freebl/rijndael.c
@@ +31,5 @@
>  static int has_intel_clmul = 0;
>  static PRBool use_hw_gcm = PR_FALSE;
> +#if defined(_MSC_VER)
> +#if !defined(_M_IX86)
> +#include <intrin.h>

I would simplify the ifdef as #if defined(_MSC_VER) && !defined(_M_IX86).

According to the MSDN page at https://msdn.microsoft.com/en-us/library/hh977022.aspx, we should include the header immintrin.h. Could you please try that? Thanks.

Comment 3

[(no longer active)]

Thanks for the review!  Should I land on the nss repository with your suggestions or would you like to look at another version of the patch?

Comment 4

[Wan-Teh Chang]

Ehsan: I don't think you have commit access to the NSS repository
(https://hg.mozilla.org/projects/nss). Please attach a new version
of the patch, and I'll check it in for you. Thanks.

Comment 5

[(no longer active)]

clang-cl currently declares _xgetbv() in intrin.h not in immintrin.h, like many other intrinsics.  In other similar situations we have just included intrin.h, even though that's not necessary for MSVC.  I prefer to do the same here, if possible.

Comment 6

[(no longer active)]

Attachment: Add #include <intrin.h> to rijndael.c for _xgetvb()

Comment 7

[Wan-Teh Chang]

Attachment: Add #include <intrin.h> to rijndael.c for _xgetbv(), by Ehsan Akhgari

Ehsan: I added a comment to the #include statement and checked in your patch:
https://hg.mozilla.org/projects/nss/rev/9e482f91fcbd

Comment 8

[(no longer active)]

Thanks, Wan-Teh!

Is there a bug tracking the next NSS merge on mozilla-central?

Comment 9

[Wan-Teh Chang]

Ehsan: I just did a search for "NSS 3.22" in Bugzilla and found
the tracking bug: bug 1228410.