dom/crypto/CryptoKey.cpp(750): warning C4838: conversion from 'nsTArray_base<Alloc,nsTArray_CopyChooser<E>::Type>::size_type' to 'CK_ULONG' requires a narrowing conversion

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gps, Assigned: ttaubert)

Tracking

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

3 years ago
Warning 4838 is getting umasked in bug 1124033. It is turned into an error when run in automation with Visual Studio 2015 Update 1.

 14:03:20     INFO -  Unified_cpp_dom_crypto0.cpp
 14:03:20     INFO -  c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/dom/crypto/CryptoKey.cpp(750): error C2220: warning treated as error - no 'object' file generated
 14:03:20     INFO -  Warning: C4838 in c:\builds\moz2_slave\try-w64-0000000000000000000000\build\src\dom\crypto\CryptoKey.cpp: conversion from 'nsTArray_base<Alloc,nsTArray_CopyChooser<E>::Type>::size_type' to 'CK_ULONG' requires a narrowing conversion
 14:03:20     INFO -  c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/dom/crypto/CryptoKey.cpp(750): warning C4838: conversion from 'nsTArray_base<Alloc,nsTArray_CopyChooser<E>::Type>::size_type' to 'CK_ULONG' requires a narrowing conversion
 14:03:20     INFO -          with
 14:03:20     INFO -          [
 14:03:20     INFO -              Alloc=nsTArrayFallibleAllocator,
 14:03:20     INFO -              E=uint8_t
 14:03:20     INFO -          ]
 14:03:20     INFO -  c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/dom/crypto/CryptoKey.cpp(750): note: to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings
 14:03:20     INFO -  Warning: C4838 in c:\builds\moz2_slave\try-w64-0000000000000000000000\build\src\dom\crypto\CryptoKey.cpp: conversion from 'nsTArray_base<Alloc,nsTArray_CopyChooser<E>::Type>::size_type' to 'CK_ULONG' requires a narrowing conversion
 14:03:20     INFO -  c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/dom/crypto/CryptoKey.cpp(804): warning C4838: conversion from 'nsTArray_base<Alloc,nsTArray_CopyChooser<E>::Type>::size_type' to 'CK_ULONG' requires a narrowing conversion
 14:03:20     INFO -          with
 14:03:20     INFO -          [
 14:03:20     INFO -              Alloc=nsTArrayFallibleAllocator,
 14:03:20     INFO -              E=uint8_t
 14:03:20     INFO -          ]
 14:03:20     INFO -  c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/dom/crypto/CryptoKey.cpp(804): note: to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings
14:03:20 INFO - c:/builds/moz2_slave/try-w64-0000000000000000000000/build/src/config/rules.mk:918: recipe for target 'Unified_cpp_dom_crypto0.obj' failed
Reporter

Comment 1

3 years ago
As part of unblocking building with VS2015u1 in automation, I'm mass
disabling compiler warnings that are turned into errors. This is not
the preferred mechanism to fix compilation warnings. So hopefully
this patch never lands because someone insists on fixing the underlying
problem instead. But if it does land, hopefully the workaround is
only temporary.

Review commit: https://reviewboard.mozilla.org/r/40597/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40597/
Attachment #8731433 - Flags: review?(rlb)
I agree we should not ship this patch.  Tim, could you look at this?  I think it might just need a "(CK_ULONG)" in front of "d.Length()" here:

https://dxr.mozilla.org/mozilla-central/source/dom/crypto/CryptoKey.cpp?from=CryptoKey.cpp#749
Flags: needinfo?(ttaubert)
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Component: Security: PSM → Security
Flags: needinfo?(ttaubert)
Comment on attachment 8731642 [details] [diff] [review]
0001-Bug-1257325-Silence-VS2015-compiler-warnings-in-Cryp.patch

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

This looks fine, but before landing, we should check that it fixes the error.  It looks from StackOverflow like you might need to use static_cast<CK_ULONG>.

http://stackoverflow.com/questions/4434140/narrowing-conversions-in-c0x-is-it-just-me-or-does-this-sound-like-a-breakin  

gps: Suggestions for how best to test this?
Attachment #8731642 - Flags: review?(rlb) → review+
Flags: needinfo?(gps)
Attachment #8731433 - Attachment is obsolete: true
Attachment #8731433 - Flags: review?(rlb)
Reporter

Comment 6

3 years ago
Comment on attachment 8731642 [details] [diff] [review]
0001-Bug-1257325-Silence-VS2015-compiler-warnings-in-Cryp.patch

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

Try says this successfully silenced the warning!
Attachment #8731642 - Flags: feedback+

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3b4efd3e713c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.