1052033 - Fix warnings in security/sandbox and mark as FAIL_ON_WARNINGS

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: fix-sandbox-warnings.patch

Fix clang warnings on OS X:

> security/sandbox/mac/Sandbox.mm:42:7 [-Wlogical-not-parentheses] logical not is only applied to the left hand side of this comparison                    
> security/sandbox/mac/Sandbox.mm:63:14 [-Wformat-security] format string is not a string literal (potentially insecure)

Suppress MSVC (false-positive) warning in third-party Chromium code:

> security\sandbox\chromium\base\shim\base\strings\string_piece.h(15) : warning C4717: '`anonymous namespace'::IsStringASCII' : recursive on all control paths, function will cause runtime stack overflow

Suppress MSVC warning in Microsoft header file about DLL exports. TBH, I'm not sure how serious this warning is but we have suppressed it in other bugs like bug 556886:

> c:\tools\msvs10\vc\include\typeinfo(157) : warning C4275: non dll-interface class 'stdext::exception' used as base for dll-interface class 'std::bad_cast'

Comment 1

[Daniel Holbert [:dholbert]]

(Nit: This should be "depends on" bug 557566 (the bug that added FAIL_ON_WARNINGS support), not blocking it.

If we had a meta-bug tracking "add FAIL_ON_WARNINGS to directories", like the metabug for "fix build warnings", then it would make sense to have this block *that* bug. But bug 557566 is not that bug -- it's where we added the ability to annotate directories.  And bugs like this one intuitively *depend* on that feature; they don't block it.)

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8470980 [details] [diff] [review]
fix-sandbox-warnings.patch

This looks fine to me.

-  if (!aInfo.type == MacSandboxType_Plugin) {
+  if (aInfo.type != MacSandboxType_Plugin) {

Thanks especially for catching this!  It's rather embarrassing :-(

Comment 3

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/953635c5e08b

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/953635c5e08b