Closed Bug 1361750 Opened 8 years ago Closed 8 years ago

[vs2017] build error: iosfwd(356): warning C4365: 'return': conversion from 'int' to 'std::char_traits<wchar_t>::int_type', signed/unsigned mismatch

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: mayhemer, Assigned: Cykesiopka)

References

Details

Attachments

(1 file)

Bug 1361750 - Disable various MSVC 2017 warnings in PSM to unbreak --enable-warnings-as-errors builds.
59 bytes, text/x-review-board-request
keeler
: review+
Details 
0:07.43 C:/PROGRA~2/MIB055~1/2017/COMMUN~1/VC/Tools/MSVC/1410~1.249\include\iosfwd(356): error C2220: warning treated as error - no 'object' file generated
 0:07.43 C:/PROGRA~2/MIB055~1/2017/COMMUN~1/VC/Tools/MSVC/1410~1.249\include\iosfwd(356): warning C4365: 'return': conversion from 'int' to 'std::char_traits<wchar_t>::int_type', signed/unsigned mismatch


win10, both vs2015 and vs2017 installed, sdk 8.1, _NO_ sdk 10
debug build of m-c@d7e40bb852ea
running start-shell.bat


.mozconfig:
mk_add_options MOZ_OBJDIR=c:/_/obj/$(basename $(pwd))/_obj-browser-debug
mk_add_options MOZ_MAKE_FLAGS=-j22

mk_add_options "export COMPILE_PDB_FLAG="
mk_add_options "export HOST_PDB_FLAG="
mk_add_options "export MOZ_DEBUG_FLAGS=-Z7"

ac_add_options --build-backends=-VisualStudio

export MOZ_DEBUG_SYMBOLS=1

ac_add_options --enable-warnings-as-errors
ac_add_options --enable-application=browser
ac_add_options --enable-debug
ac_add_options --disable-optimize

ac_add_options --disable-crashreporter
ac_add_options --disable-profiling
ac_add_options --enable-tests
ac_add_options --enable-chrome-format=flat
ac_add_options --disable-webspeech 




$ ./mach configure
 0:00.42 c:\Mozilla\mozilla-build\mozmake\mozmake.EXE -f client.mk MOZ_PARALLEL_BUILD=22 -s configure
 0:01.99 cd c:/_/obj/mozilla-central1/_obj-browser-debug
 0:02.01 c:/Mozilla/src/mozilla-central1/configure
 0:02.32 Reexecuting in the virtualenv
 0:02.60 Adding configure options from c:\Mozilla\confs\bd
 0:02.61   --build-backends=-VisualStudio
 0:02.61   --enable-warnings-as-errors
 0:02.61   --enable-application=browser
 0:02.61   --enable-debug
 0:02.61   --disable-optimize
 0:02.61   --disable-crashreporter
 0:02.61   --disable-profiling
 0:02.61   --enable-tests
 0:02.61   --enable-chrome-format=flat
 0:02.61   --disable-webspeech
 0:02.61   MOZ_DEBUG_SYMBOLS=1
 0:02.73 checking for a shell... C:/Mozilla/mozilla-build/msys/bin/sh.exe
 0:03.05 checking for host system type... i686-pc-mingw32
 0:03.05 checking for target system type... i686-pc-mingw32
 0:03.08 checking for a shell... C:/Mozilla/mozilla-build/msys/bin/sh.exe
 0:03.39 checking for host system type... i686-pc-mingw32
 0:03.39 checking for target system type... i686-pc-mingw32
 0:03.41 checking whether cross compiling... no
 0:03.45 checking for the target C compiler... 'C:/PROGRA~2/MIB055~1/2017/COMMUN~1/VC/Tools/MSVC/1410~1.249/bin/HostX64/x86/cl.exe'
 0:03.51 checking whether the target C compiler can be used... yes
 0:03.53 checking for llvm-config... not found
 0:03.54 checking for pkg_config... not found
 0:03.55 checking for yasm... c:/Mozilla/mozilla-build/yasm/yasm.exe
 0:03.56 checking yasm version... 1.3.0
 0:03.56 checking the target C compiler version... 19.10.24930
 0:03.60 checking the target C compiler works... yes
 0:03.61 checking for the target C++ compiler... 'C:/PROGRA~2/MIB055~1/2017/COMMUN~1/VC/Tools/MSVC/1410~1.249/bin/HostX64/x86/cl.exe'
 0:03.65 checking whether the target C++ compiler can be used... yes
 0:03.65 checking the target C++ compiler version... 19.10.24930
 0:03.70 checking the target C++ compiler works... yes
 0:03.71 checking for the host C compiler... 'C:/PROGRA~2/MIB055~1/2017/COMMUN~1/VC/Tools/MSVC/1410~1.249/bin/HostX64/x86/cl.exe'
 0:03.75 checking whether the host C compiler can be used... yes
 0:03.75 checking the host C compiler version... 19.10.24930
 0:03.78 checking the host C compiler works... yes
 0:03.79 checking for the host C++ compiler... 'C:/PROGRA~2/MIB055~1/2017/COMMUN~1/VC/Tools/MSVC/1410~1.249/bin/HostX64/x86/cl.exe'
 0:03.83 checking whether the host C++ compiler can be used... yes
 0:03.83 checking the host C++ compiler version... 19.10.24930
 0:03.87 checking the host C++ compiler works... yes
 0:03.92 checking for 64-bit OS... no
 0:03.96 checking for Windows SDK... 0x0603 in 'C:\Program Files (x86)\Windows Kits\8.1\'
 0:03.96 checking for Universal CRT SDK... 10.0.10240.0 in 'C:\Program Files (x86)\Windows Kits\10\'
 0:03.96 checking for the Debug Interface Access SDK... not found
 0:03.97 checking for mt... 'C:/PROGRA~2/WI3CF2~1/8.1/bin/x64/mt.exe'
 0:03.99 checking whether MT is really Microsoft Manifest Tool... yes
 0:03.99 checking for link... 'C:/PROGRA~2/MIB055~1/2017/COMMUN~1/VC/Tools/MSVC/1410~1.249/bin/HostX64/x86/link.exe'
 0:03.99 checking for makecab... c:/Windows/System32/makecab.exe
 0:04.05 checking for stdint.h... yes
 0:04.10 checking for inttypes.h... yes
 0:04.16 checking for malloc.h... yes
 0:04.21 checking for alloca.h... no
 0:04.25 checking for perf_event_open system call... no
 0:04.26 checking for rustc... c:/Users/xmayhemer/.cargo/bin/rustc.exe
 0:04.26 checking for cargo... c:/Users/xmayhemer/.cargo/bin/cargo.exe
 0:04.29 checking rustc version... 1.16.0
 0:04.32 checking cargo version... 0.17.0
 0:04.63 checking for tar... C:/Mozilla/mozilla-build/msys/bin/tar.exe
 0:04.63 checking for unzip... c:/Mozilla/mozilla-build/info-zip/unzip.exe
 0:04.63 checking for zip... c:/Mozilla/mozilla-build/info-zip/zip.exe
 0:04.63 checking for the Mozilla API key... no
 0:04.63 checking for the Google API key... no
 0:04.63 checking for the Bing API key... no
 0:04.63 checking for the Adjust SDK key... no
 0:04.64 checking for awk... C:/Mozilla/mozilla-build/msys/bin/gawk.exe
 0:04.66 checking for perl... C:/Mozilla/mozilla-build/msys/bin/perl.exe
 0:04.67 checking for minimum required perl version >= 5.006... 5.008008
 0:04.68 checking for full perl installation... yes
 0:04.69 checking for gmake... c:/Mozilla/mozilla-build/mozmake/mozmake.EXE
 0:04.70 checking for doxygen... not found
 0:04.70 checking for xargs... C:/Mozilla/mozilla-build/msys/bin/xargs.exe
 0:04.72 checking for makensisu... c:/Mozilla/mozilla-build/nsis-3.0b3/makensis-3.0b3.exe
 0:04.73 checking for NSIS version... 3.0b3
 0:04.73 checking for 32-bit NSIS... yes
 0:04.73 checking for autoconf... C:/Mozilla/mozilla-build/msys/local/bin/autoconf-2.13
 0:06.42 loading cache ./config.cache
 0:06.78 checking host system type... i686-pc-mingw32
 0:06.99 checking target system type... i686-pc-mingw32
 0:07.20 checking build system type... i686-pc-mingw32
 0:07.28 checking for gcc... (cached) cl.exe
 0:07.42 checking whether the C compiler (cl.exe  ) works... yes
 0:07.42 checking whether the C compiler (cl.exe  ) is a cross-compiler... no
 0:07.43 checking whether we are using GNU C... (cached) no
 0:07.43 checking whether cl.exe accepts -g... (cached) no
 0:09.45 checking for c++... (cached) cl.exe
 0:09.59 checking whether the C++ compiler (cl.exe  ) works... yes
 0:09.59 checking whether the C++ compiler (cl.exe  ) is a cross-compiler... no
 0:09.60 checking whether we are using GNU C++... (cached) no
 0:09.60 checking whether cl.exe accepts -g... (cached) no
 0:09.61 checking for ranlib... (cached) :
 0:09.62 checking for ml... (cached) /c/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Tools/MSVC/14.10.24930/bin/HostX64/x86/ml
 0:09.65 checking for ar... no
 0:09.66 checking for strip... no
 0:09.67 checking for windres... no
 0:09.68 checking for otool... no
 0:09.72 checking for midl... (cached) midl
 0:10.10 checking for dia2.h... (cached) no
 0:10.19 checking for overridable _RAISE... (cached) yes
 0:10.45 checking for X... (cached) no
 0:11.58 checking for valid debug flags... yes
 0:11.70 checking for malloc.h... (cached) yes
 0:11.75 checking for strndup... (cached) no
 0:11.77 checking for posix_memalign... (cached) no
 0:11.77 checking for memalign... (cached) no
 0:11.78 checking for malloc_usable_size... (cached) no
 0:11.88 checking whether malloc_usable_size definition can use const argument... yes
 0:12.01 checking for valloc in malloc.h... no
 0:12.10 checking for valloc in unistd.h... no
 0:12.27 checking NSPR selection... source-tree
 0:12.34 checking if app-specific confvars.sh exists... c:/Mozilla/src/mozilla-central1/browser/confvars.sh
 0:13.31 Found D3D compiler in Windows SDK.
 0:13.31 Found MOZ_D3DCOMPILER_VISTA_DLL_PATH: C:\Program Files (x86)\Windows Kits\8.1\/Redist/D3D/x86/d3dcompiler_47.dll
 0:13.31 Found d3dcompiler DLL for Vista+: d3dcompiler_47.dll
 0:13.45 checking for wget... (cached) wget
 0:14.34 checking what kind of list files are supported by the linker... list
 0:14.65 checking for d3d10.h... (cached) yes
 0:15.46 checking for posix_fadvise... (cached) no
 0:15.47 checking for posix_fallocate... (cached) no
 0:15.77 creating ./config.data
 0:16.62
 0:17.12
 0:28.52 ===============================
 0:28.52 Visual Studio Support Available
 0:28.52
 0:28.52 You are building Firefox on Windows. You can generate Visual Studio
 0:28.52 files by running:
 0:28.52
 0:28.52    mach build-backend --backend=VisualStudio
 0:28.52
 0:28.52 ===============================
 0:28.63 Creating config.status
 0:28.63 Reticulating splines...
 0:28.63 Finished reading 1254 moz.build files in 3.24s
 0:28.63 Read 126 gyp files in parallel contributing 0.00s to total wall time
 0:28.63 Processed into 10248 build config descriptors in 3.33s
 0:28.63 RecursiveMake backend executed in 2.75s
 0:28.63   3033 total backend files; 0 created; 0 updated; 3033 unchanged; 0 deleted; 61 -> 1140 Makefile
 0:28.63 FasterMake backend executed in 0.36s
 0:28.63   16 total backend files; 0 created; 0 updated; 16 unchanged; 0 deleted
 0:28.63 Total wall time: 11.21s; CPU time: 11.21s; Efficiency: 100%; Untracked: 1.53s
Configure complete!
Be sure to run |mach build| to pick up any changes
Is there more to the compiler error? Is this MSVC warning on its own header, or is it due to something in our code making it unhappy?

In general, I would advise against building --enable-warnings-as-errors locally, especially if you're not using the exact same toolchain as our CI.

If you'd like to just switch back to using MSVC2015 you can use `--with-visual-studio-version=2015`.
Ouch:

E:\>type foo.cpp
#include <iosfwd>

E:\>cl -nologo -EHsc -we4365 -c foo.cpp
foo.cpp
C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.10.25017\include\iosfwd(356): error C4365: 'return': conversion from 'int' to 'std::char_traits<wchar_t>::int_type', signed/unsigned mismatch
C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.10.25017\include\iosfwd(452): error C4365: 'return': conversion from 'int' to 'std::char_traits<unsigned short>::int_type', signed/unsigned mismatch
That would be a bug worth filing with Microsoft, then, 'cause I think they go to some effort to ensure their headers are warning-free.
Apparently this warning is level 4 and off by default. Doesn't even come up with -W4. You'd have to do -we4365 (which I don't see in the tree) or -Wall.

Why are we doing -Wall though? That sounds like too much, even for --enable-warnings-as-errors. Is this in some third party library?
 0:09.72 pkixcheck_CheckKeyUsage_tests.cpp
 0:09.72 C:/PROGRA~2/MICROS~2/2017/PROFES~1/VC/Tools/MSVC/1410~1.250\include\iosfwd(356): error C2220: warning treated as error - no 'object' file generated

How do I get the cl command line for that? |mach build -v| isn't showing me.
gflags+windbg says:
CommandLine: cl.exe -Fopkixcheck_CheckKeyUsage_tests.obj -c -Ie:/m-c/obj-x86_64-pc-mingw32/dist/stl_wrappers -DDEBUG=1 -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Ie:/m-c/security/pkix/test/gtest -Ie:/m-c/obj-x86_64-pc-mingw32/security/pkix/test/gtest -Ie:/m-c/security/pkix/include -Ie:/m-c/security/pkix/lib -Ie:/m-c/security/pkix/test/lib -Ie:/m-c/obj-x86_64-pc-mingw32/dist/include -Ie:/m-c/obj-x86_64-pc-mingw32/dist/include/nspr -Ie:/m-c/obj-x86_64-pc-mingw32/dist/include/nss -MD -FI e:/m-c/obj-x86_64-pc-mingw32/mozilla-config.h -DMOZILLA_CLIENT -TP -nologo -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -utf-8 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -we4553 -GR- -Zi -Oy- -WX -sdl -Wall -wd4464 -wd4514 -wd4668 -wd4710 -wd4711 -wd4800 -wd4820 -Zc:forScope -Zc:inline -Zc:rvalueCast -Zc:strictStrings -wd4350 -wd4275 -wd4548 -wd4625 -wd4626 -wd4640 -wd4458 -Fdgenerated.pdb -FS e:/m-c/security/pkix/test/gtest/pkixcheck_CheckKeyUsage_tests.cpp -showIncludes
Silencing 4365 merely leads me to:

 0:14.97 pkixcheck_CheckIssuer_tests.cpp
 0:14.98 C:\PROGRA~2\MICROS~2\2017\PROFES~1\VC\Tools\MSVC\1410~1.250\include\eh.h(31): error C2220: warning treated as error - no 'object' file generated
 0:14.98 C:\PROGRA~2\MICROS~2\2017\PROFES~1\VC\Tools\MSVC\1410~1.250\include\eh.h(31): warning C4987: nonstandard extension used: 'throw (...)'

I don't think I want to keep chasing this...
Based on the file that's failing and the conversation on #developers, I assume this is coming from the use of -Wall in PSM.

(In reply to David Major [:dmajor] from comment #4)
> Why are we doing -Wall though? That sounds like too much, even for
> --enable-warnings-as-errors.

security/pkix/ and security/certverifier/ are responsible for TLS cert verification, so for such security critical code I would argue that -Wall isn't too much.

Of course, I imagine the intent wasn't to have warnings come up for non-PSM code.

(In reply to David Major [:dmajor] from comment #8)
> Silencing 4365 merely leads me to:
> I don't think I want to keep chasing this...

If the short term fix to this bug isn't more complicated than "disable various warnings until we're victorious", then I can take it.

I will probably also look into ways to have these warnings apply only to PSM code - ideas welcome.
The changes I just posted for review allow me to build using --enable-warnings-as-errors on both VS 2017 and VS 2015 locally.
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Thanks for taking this on!

Out of curiosity:
'-wd4619', # #pragma warning : there is no warning number 'number'

Which nonexistent warning were we trying to use? I wonder if we should stop trying to use it or maybe it has a typo in the number that we should fix.
I'm sympathetic, but historically we've found that -Wall is just not tractable with --enable-warnings-as-errors, because compilers add new warnings all the time. I think you'd be better served listing out explicit warnings. We have some machinery to do this for gcc/clang:
https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/warnings.configure

but no equivalent for MSVC (although it's generally easier to handle MSVC).
Comment on attachment 8864574 [details]
Bug 1361750 - Disable various MSVC 2017 warnings in PSM to unbreak --enable-warnings-as-errors builds.

https://reviewboard.mozilla.org/r/136240/#review139300

I think this is fine for now (particularly since there's precedent). I would also be open to a more comprehensive solution so we don't have to keep doing this.
Attachment #8864574 - Flags: review?(dkeeler) → review+
(In reply to David Major [:dmajor] from comment #12)
> Thanks for taking this on!
> 
> Out of curiosity:
> '-wd4619', # #pragma warning : there is no warning number 'number'
> 
> Which nonexistent warning were we trying to use? I wonder if we should stop
> trying to use it or maybe it has a typo in the number that we should fix.

C4800 - 'type' : forcing value to bool 'true' or 'false' (performance warning).
It's referenced in at least security/pkix/warnings.mozbuild and our copy of GTest.
I decided to just disable C4619 for now do unbreak stuff ASAP.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #13)
> I'm sympathetic, but historically we've found that -Wall is just not
> tractable with --enable-warnings-as-errors, because compilers add new
> warnings all the time. I think you'd be better served listing out explicit
> warnings. We have some machinery to do this for gcc/clang:
> https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/warnings.
> configure
> 
> but no equivalent for MSVC (although it's generally easier to handle MSVC).

That's fine with me as well, but does that machinery automatically exclude warnings caused by compiler headers, headers in other modules etc? If it does, great, if not, I guess I'll have to find a way to do it manually.
(In reply to David Keeler [:keeler] (use needinfo?) from comment #14)
> I think this is fine for now (particularly since there's precedent). I would
> also be open to a more comprehensive solution so we don't have to keep doing
> this.

Thanks for the review.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef2e5fa63abc2ee116b450f2d843cfc4d9d4c84b
Keywords: checkin-needed
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8659404d72ea
Disable various MSVC 2017 warnings in PSM to unbreak --enable-warnings-as-errors builds. r=keeler
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8659404d72ea
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: