re-evaluate support for Chromium Release builds

_From [bruen...@google.com](https://code.google.com/u/109494838902877177630/) on January 29, 2014 13:37:58_

**\* TODO re-evaluate support for Chromium Release builds

We'd like to switch to Chromium Release builds as they are 2x faster than
Debug and we'd need fewer bots. However, in the past we've had a lot of
trouble with Release buids.

The downsides of Release are:

1) Optimized code leading to uninitialized read false positives

2) FPO, inlining, and tailcalls resulting in confusing callstacks for
developers and messing up our heap interception

Let's look at each in detail.

Beyond Chromium, while we recommend unoptimized builds to general users, we
want to try to support 3rd party libraries that are optimized, and we've
put in a lot of work to support all versions of the VS C library and other
heap layers. For general users, we're at the point where their use of an
optimized build generally only hurts their analysis of errors we report
(from missing frames in callstacks) and does not affect Dr. Memory
correctness.

***\* TODO false positives from optimized code: negligible risk

I ran all the Chromium tests locally in both Release and Debug.

Light mode saw absolutely no extra errors reported beyond Debug, and no
extra failing tests or problems of any kind.

Full mode saw no extra errors in all the tests smaller than unit_tests. In
unit_tests, there were 2 extra uninits: one a "fld;xor;fstp" pattern fixed
by a simple issue #471 heuristic extension and the other a real bug in Chromium
(crbug.com/338739)!

browser_tests does have a handful of extra uninits: another fld issue we'll
likely have to suppress, and 3 related webkit uninits that have not been
analyzed. Given that Debug browser_tests full mode also has a bunch of
outstanding uninits, including some more disturbing ones than Release (I
hit something in libc that shouldn't be reported), this does not seem like
enough to outweigh the 2x performance increase from Release.

browser_tests does have 2 failing tests in Release that don't fail for me
locally in Debug. Strangely they seem to time out.

In conclusion: there does not seem to be much risk from false positives in
Release buids beyond what we can easily handle or suppress. Some
problematic code should already have annotations (e.g., re2) or
suppressions. Valgrind is after all running Release (at -O1 and
-fno-inline -fno-omit-frame-pointer).

Thus I see no need to consider compiling as /Od in Release ( issue #725 ).

****\* TODO messed-up callstacks

We are building with "/O2 /Oy- /Ob1'. So we have no FPO, and no inlining
except for functions explicitly marked __inline. That doesn't seem too bad
to me.

From my recent experience, we're as likely to see a bad callstack in Debug
as Release. For example, from Debug browser_tests:

[ RUN ] InitiallyRestored/AcceleratorCommandsPlatformAppFullscreenBrowserTest.ToggleFullscreen/0

Error `#1`: UNINITIALIZED READ: reading 0x0018ba84-0x0018ba88 4 byte(s) within 0x0018ba84-0x0018ba88
#0 system call NtUserGetObjectInformation parameter `#4`
#1 MSCTF.dll!GetWindowStationName
#2 webkit.dll!WebCore::DocumentMarkerDescription::create [third_party\webkit\source\core\dom\documentmarker.cpp:58]
#3 MSCTF.dll!IsMutexForWinSta
#4 MSCTF.dll!ExecuteLoader
#5 MSCTF.dll!CThreadInputMgr::ActivateEx_P
#6 MSCTF.dll!CicBridge::ActivateIMMX
#7 MSCTF.dll!_CtfImeCreateThreadMgr
#8 IMM32.dll!CtfImmTIMActivate
#9 IMM32.dll!InternalImmLockIMC

That webkit frame looks wrong. I have not analyze that fully yet.

Looking backward at the history of bad callstacks:

Xref issue #1054 : Oct-Nov 2012, where we had to abandon /Ob0

Xref issue #557 : bogus callstack frames displayed in Chrome Release build
"try /Oy- /Ob1: better but not good enough"
"try /Oy- /Ob0...still seeing crappy callstacks"

Xref issue #722 : INVALID HEAP ARGUMENT b/c at /O1 ~basic_streambuf calls
_DebugHeapDelete which tailcalls free

Xref crbug.com/108155: Investigate Debug + /O1 compatibility with memory tools
"/O1 in Debug does do tailcalls (so does Release /O*): any way to disable?"
Xref issue #725 : compare Release /Od perf to /O2 and Debug natively and under DrMem

Xref issue #582 : UNADDRs beyond TOS in WebCore::RenderStyle::resetBorder*: compiler
reads beyond TOS before setting up frame with "/O2 /Ob0 /Oy-"

Most of those issues were in 2011.

What we've done since then:
1) Improved frame scanning w/ definedness info ( issue #557 ) and other tweaks
2) Fixed heap layer tailcall issues ( issue #722 _DebugHeapDelete)
3) Added -zero_retaddr which really helps avoid bogus frames
4) Build with /Ob1 which eliminates issue #582 and issue #1054 In conclusion: we've had to improve our callstack walking just for light
mode on Debug Chromium and for non-Chromium uses, to the point that it can
handle Release Chromium now.

_Original issue: http://code.google.com/p/drmemory/issues/detail?id=1429_