Improve EAT_STREAM_PARAMETERS for Windows x86

Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret

After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret

Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699
Review-Url: https://codereview.chromium.org/2559323007
Committed: https://crrev.com/3c957a583f692a71024bddbcf50b39a69af89d6e
Cr-Commit-Position: refs/heads/master@{#437777}

[scottmg, 2016-12-09 18:06:54]

Description was changed from

==========
Attempt to improve EAT_STREAM_PARAMETERS for Windows x86

BUG=672699
==========

to

==========
Attempt to improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699
==========

[scottmg, 2016-12-09 18:14:37]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 18:15:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dcheng, 2016-12-09 18:16:55]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2016-12-09 18:16:56]

Seems reasonable to me, let's see how the trybots like it.

[scottmg, 2016-12-09 18:24:45]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 18:24:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 18:48:53]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 18:49:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 19:04:21]

For reference, primiano came up with another approach we discussed offline that
we could try if this one doesn't work:

#define EAT_STREAM_PARAMETERS                                    \
  true ? (void)0                                                 \
       : ::logging::LogMessageVoidify() & LOG_STREAM(FATAL) true \
             ? (void)0                                           \
             : ::logging::LogMessageVoidify() &                  \
                   (*reinterpret_cast<std::ostream*>(0))

My only small concern is the deref of 0. I'm not enough of a language lawyer to
know if it's possible for a compiler to take that as an indication of undefined
behaviour and decide that it can instead format c: (or otherwise wreak havoc).

[scottmg, 2016-12-09 19:05:25]

On 2016/12/09 19:04:21, scottmg wrote:
> For reference, primiano came up with another approach we discussed offline
that
> we could try if this one doesn't work:
> 
> #define EAT_STREAM_PARAMETERS                                    \
>   true ? (void)0                                                 \
>        : ::logging::LogMessageVoidify() & LOG_STREAM(FATAL) true \
>              ? (void)0                                           \
>              : ::logging::LogMessageVoidify() &                  \
>                    (*reinterpret_cast<std::ostream*>(0))
> 
> My only small concern is the deref of 0. I'm not enough of a language lawyer
to
> know if it's possible for a compiler to take that as an indication of
undefined
> behaviour and decide that it can instead format c: (or otherwise wreak havoc).

Bah, copy-paste error, here's the real idea:

#define EAT_STREAM_PARAMETERS \
  true                        \
      ? (void)0               \
      : ::logging::LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0))

[dcheng, 2016-12-09 19:12:53]

On 2016/12/09 19:05:25, scottmg wrote:
> On 2016/12/09 19:04:21, scottmg wrote:
> > For reference, primiano came up with another approach we discussed offline
> that
> > we could try if this one doesn't work:
> > 
> > #define EAT_STREAM_PARAMETERS                                    \
> >   true ? (void)0                                                 \
> >        : ::logging::LogMessageVoidify() & LOG_STREAM(FATAL) true \
> >              ? (void)0                                           \
> >              : ::logging::LogMessageVoidify() &                  \
> >                    (*reinterpret_cast<std::ostream*>(0))
> > 
> > My only small concern is the deref of 0. I'm not enough of a language lawyer
> to
> > know if it's possible for a compiler to take that as an indication of
> undefined
> > behaviour and decide that it can instead format c: (or otherwise wreak
havoc).
> 
> Bah, copy-paste error, here's the real idea:
> 
> #define EAT_STREAM_PARAMETERS \
>   true                        \
>       ? (void)0               \
>       : ::logging::LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0))

I prefer the current approach, as the latter requires language lawyering to
figure out if we're going to end up triggering undefined behavior one day.

[scottmg, 2016-12-09 19:44:25]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[scottmg, 2016-12-09 19:44:33]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-12-09 19:45:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 19:47:28]

Patchset #3 (id:60001) has been deleted

[scottmg, 2016-12-09 19:48:18]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 19:49:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 20:16:13]

Description was changed from

==========
Attempt to improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699
==========

to

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699
==========

[scottmg, 2016-12-09 21:04:49]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 21:05:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 21:43:07]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 21:43:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 21:49:08]

Patchset #5 (id:120001) has been deleted

[commit-bot: I haz the power, 2016-12-09 21:51:28]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[scottmg, 2016-12-09 21:53:42]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 21:54:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 22:37:40]

On 2016/12/09 19:12:53, dcheng wrote:
> On 2016/12/09 19:05:25, scottmg wrote:
> > On 2016/12/09 19:04:21, scottmg wrote:
> > > For reference, primiano came up with another approach we discussed offline
> > that
> > > we could try if this one doesn't work:
> > > 
> > > #define EAT_STREAM_PARAMETERS                                    \
> > >   true ? (void)0                                                 \
> > >        : ::logging::LogMessageVoidify() & LOG_STREAM(FATAL) true \
> > >              ? (void)0                                           \
> > >              : ::logging::LogMessageVoidify() &                  \
> > >                    (*reinterpret_cast<std::ostream*>(0))
> > > 
> > > My only small concern is the deref of 0. I'm not enough of a language
lawyer
> > to
> > > know if it's possible for a compiler to take that as an indication of
> > undefined
> > > behaviour and decide that it can instead format c: (or otherwise wreak
> havoc).
> > 
> > Bah, copy-paste error, here's the real idea:
> > 
> > #define EAT_STREAM_PARAMETERS \
> >   true                        \
> >       ? (void)0               \
> >       : ::logging::LogMessageVoidify() &
(*reinterpret_cast<std::ostream*>(0))
> 
> I prefer the current approach, as the latter requires language lawyering to
> figure out if we're going to end up triggering undefined behavior one day.

PS#3 was sorta fine, *except* that there's a handful of static
operator<<(ostream) implementations, which in turn cause warnings like this when
the ostream one doesn't look like it's used:


FAILED:
obj/components/autofill/core/browser/browser/autofill_profile_comparator.o 
... 
-c ../../components/autofill/core/browser/autofill_profile_comparator.cc -o
obj/components/autofill/core/browser/browser/autofill_profile_comparator.o
../../components/autofill/core/browser/autofill_profile_comparator.cc:36:15:
error: 'std::ostream& autofill::{anonymous}::operator<<(std::ostream&, const
i18n::phonenumbers::PhoneNumber&)' defined but not used
[-Werror=unused-function]
 std::ostream& operator<<(std::ostream& os,
               ^
cc1plus.elf: all warnings being treated as errors


So, instead I went with something more like Primiano's. In place of an inline
*nullptr though, I extern'd a std::ostream* to avoid any run-ins with the C++
police.

PTAL.

[scottmg, 2016-12-09 22:38:30]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 22:39:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dcheng, 2016-12-09 23:16:48]

https://codereview.chromium.org/2559323007/diff/160001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/2559323007/diff/160001/base/logging.h#newcode431
base/logging.h:431: #define EAT_STREAM_PARAMETERS \
Can we add a comment that mentions the subtleties involved here? I'm going to do
some language lawyering in the meantime to make sure this is considered OK.

[scottmg, 2016-12-09 23:30:27]

https://codereview.chromium.org/2559323007/diff/160001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/2559323007/diff/160001/base/logging.h#newcode431
base/logging.h:431: #define EAT_STREAM_PARAMETERS \
On 2016/12/09 23:16:48, dcheng wrote:
> Can we add a comment that mentions the subtleties involved here? I'm going to
do
> some language lawyering in the meantime to make sure this is considered OK.

Sure. Are you concerned about not actually assigning to the pointer in the .cc
file? We could also easily do that to be safe if you think that's a good idea.

[dcheng, 2016-12-09 23:35:18]

On 2016/12/09 23:30:27, scottmg wrote:
> https://codereview.chromium.org/2559323007/diff/160001/base/logging.h
> File base/logging.h (right):
> 
>
https://codereview.chromium.org/2559323007/diff/160001/base/logging.h#newcode431
> base/logging.h:431: #define EAT_STREAM_PARAMETERS \
> On 2016/12/09 23:16:48, dcheng wrote:
> > Can we add a comment that mentions the subtleties involved here? I'm going
to
> do
> > some language lawyering in the meantime to make sure this is considered OK.
> 
> Sure. Are you concerned about not actually assigning to the pointer in the .cc
> file? We could also easily do that to be safe if you think that's a good idea.

I just want to make sure the standard doesn't say anything about reading
uninitialized variables and/or compilers making weird assumptions in that area.

[scottmg, 2016-12-09 23:39:49]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 23:40:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-09 23:41:53]

On 2016/12/09 23:35:18, dcheng wrote:
> On 2016/12/09 23:30:27, scottmg wrote:
> > https://codereview.chromium.org/2559323007/diff/160001/base/logging.h
> > File base/logging.h (right):
> > 
> >
>
https://codereview.chromium.org/2559323007/diff/160001/base/logging.h#newcode431
> > base/logging.h:431: #define EAT_STREAM_PARAMETERS \
> > On 2016/12/09 23:16:48, dcheng wrote:
> > > Can we add a comment that mentions the subtleties involved here? I'm going
> to
> > do
> > > some language lawyering in the meantime to make sure this is considered
OK.
> > 
> > Sure. Are you concerned about not actually assigning to the pointer in the
.cc
> > file? We could also easily do that to be safe if you think that's a good
idea.
> 
> I just want to make sure the standard doesn't say anything about reading
> uninitialized variables and/or compilers making weird assumptions in that
area.

OK, if you're at all concerned, I don't see any reason not to leak a single
stream instance by initializing it in BaseInitLoggingImpl() or somewhere.

(added a comment too)

[scottmg, 2016-12-09 23:46:41]

On 2016/12/09 23:41:53, scottmg wrote:
> On 2016/12/09 23:35:18, dcheng wrote:
> > On 2016/12/09 23:30:27, scottmg wrote:
> > > https://codereview.chromium.org/2559323007/diff/160001/base/logging.h
> > > File base/logging.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2559323007/diff/160001/base/logging.h#newcode431
> > > base/logging.h:431: #define EAT_STREAM_PARAMETERS \
> > > On 2016/12/09 23:16:48, dcheng wrote:
> > > > Can we add a comment that mentions the subtleties involved here? I'm
going
> > to
> > > do
> > > > some language lawyering in the meantime to make sure this is considered
> OK.
> > > 
> > > Sure. Are you concerned about not actually assigning to the pointer in the
> .cc
> > > file? We could also easily do that to be safe if you think that's a good
> idea.
> > 
> > I just want to make sure the standard doesn't say anything about reading
> > uninitialized variables and/or compilers making weird assumptions in that
> area.
> 
> OK, if you're at all concerned, I don't see any reason not to leak a single
> stream instance by initializing it in BaseInitLoggingImpl() or somewhere.
> 
> (added a comment too)

[ We could even do if (volatile-but-always-runtime-false-thing) {
g_swallow_stream = new ...; }, but maybe that's getting too cute. I don't see
how the compiler could possibly be allowed to do something bad in that case
though. ]

[dcheng, 2016-12-10 00:39:33]

https://codereview.chromium.org/2559323007/diff/180001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/2559323007/diff/180001/base/logging.h#newcode431
base/logging.h:431: // Note that g_swallow_stream is used instead of an
arbitrary LOG() stream to
I did some more thinking about this, and both jschuh and I agree that having
undefined behavior in a branch that's not taken *should* be ok. So I think we
can just use nullptr / 0 directly on the RHS. Sorry about the waffling back and
forth on this point.

https://codereview.chromium.org/2559323007/diff/180001/base/logging.h#newcode438
base/logging.h:438: // works to avoid these instructions, however this causes
warnings on statically
Nit: this comma should be a period or a semicolon =P

[scottmg, 2016-12-10 00:56:59]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-10 00:57:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-10 00:57:31]

https://codereview.chromium.org/2559323007/diff/180001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/2559323007/diff/180001/base/logging.h#newcode431
base/logging.h:431: // Note that g_swallow_stream is used instead of an
arbitrary LOG() stream to
On 2016/12/10 00:39:33, dcheng wrote:
> I did some more thinking about this, and both jschuh and I agree that having
> undefined behavior in a branch that's not taken *should* be ok. So I think we
> can just use nullptr / 0 directly on the RHS. Sorry about the waffling back
and
> forth on this point.

Done.

https://codereview.chromium.org/2559323007/diff/180001/base/logging.h#newcode438
base/logging.h:438: // works to avoid these instructions, however this causes
warnings on statically
On 2016/12/10 00:39:33, dcheng wrote:
> Nit: this comma should be a period or a semicolon =P

Done.

[commit-bot: I haz the power, 2016-12-10 01:24:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-10 01:24:04]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[scottmg, 2016-12-10 01:27:07]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-10 01:27:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-10 03:20:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-10 03:20:06]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2016-12-10 03:44:03]

LGTM

https://codereview.chromium.org/2559323007/diff/220001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/2559323007/diff/220001/base/logging.h#newcode432
base/logging.h:432: // pointless instructions to be emitted even at full
optimization level, and
Nit: remove 'and'

[scottmg, 2016-12-10 05:02:01]

https://codereview.chromium.org/2559323007/diff/220001/base/logging.h
File base/logging.h (right):

https://codereview.chromium.org/2559323007/diff/220001/base/logging.h#newcode432
base/logging.h:432: // pointless instructions to be emitted even at full
optimization level, and
On 2016/12/10 03:44:03, dcheng wrote:
> Nit: remove 'and'

Oops, done.

[scottmg, 2016-12-10 05:02:23]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-12-10 05:02:23]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2559323007/#ps240001
(title: "nit")

[commit-bot: I haz the power, 2016-12-10 05:02:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-10 06:32:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-10 06:32:20]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dcheng, 2016-12-10 06:42:11]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-12-10 06:42:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-10 08:29:30]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1481352131034120,
"parent_rev": "f62b93366b920f79994f35aec8fa2c10f792dfd6", "commit_rev":
"30c6282747afaae16d4396bc7319ac007138f183"}

[commit-bot: I haz the power, 2016-12-10 08:30:16]

Description was changed from

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699
==========

to

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Review-Url: https://codereview.chromium.org/2559323007
==========

[commit-bot: I haz the power, 2016-12-10 08:30:17]

Committed patchset #10 (id:240001)

[findit-for-me, 2016-12-10 08:52:52]

FYI: Findit identified this CL at revision 437763 as the culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...

[Primiano Tucci (use gerrit), 2016-12-10 10:28:35]

Ahhh I think we need to be slightly smarter to trick compilers that run with
lower -O levels.
Some bots failed when hitting this when official_build = false but dcheck is
still off (EAT_STREAM_PARAMS  still applies to  D*LOG in release builds).
The failure looks the following:
In file included from ../../media/formats/mp4/box_reader.cc:5:
../../media/formats/mp4/box_reader.h:254:3: error: binding dereferenced null
pointer to reference has undefined behavior [-Werror,-Wnull-dereference]
  DVLOG(2) << "Found " << children->size() << " "
  ^~~~~~~~
../../base/logging.h:695:29: note: expanded from macro 'DVLOG'
#define DVLOG(verboselevel) DVLOG_IF(verboselevel, VLOG_IS_ON(verboselevel))
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:677:43: note: expanded from macro 'DVLOG_IF'
#define DVLOG_IF(verboselevel, condition) EAT_STREAM_PARAMETERS
                                          ^~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:441:43: note: expanded from macro 'EAT_STREAM_PARAMETERS'
      : ::logging::LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0))
                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.


I think this is still functionally right, however the compiler is trying to
outsmart us, but not in a too clever way, as opposite to what happens in
official build.

My interpretation of the situation is the following:
Given 

#define DVLOG(verboselevel) DVLOG_IF(verboselevel, VLOG_IS_ON(verboselevel))
#define DVLOG_IF(verboselevel, condition) EAT_STREAM_PARAMETERS
#define EAT_STREAM_PARAMETERS  true ? (void) 0 : LogMessageVoidify() &
(*reinterpret_cast<std::ostream*>(0))

The following code gets emitted for 
DVLOG(2) << "Found " << children->size() << " "...

true ? (void) 0 : LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0)) <<
"Found " << children->size() << " ";
Which really is:

if (true) {
  (void) 0;
} else {
  LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0)) << "Found " <<
children->size() << " "... ;
}


Now the compiler is right that dereferencing nullptr is undefined behavior.
However the problem now is that it doesn't realize (I think due to the lower
optimization level) that such statement with undefined behavior cannot possibly
be hit.


I think that the possible solutions to this are:
1) reinterpret_cast<std::ostream*>(0x100)  // At this point the compiler cannot
assume what is located at 100
but this is really horrible to see in a civilised codebase

2) Introduce an extern exported global variable and define it to be nullptr in
the .cc file, something like
logging.h:
 extern BASE_EXPORT std::ostream* g_null_ostream;
 LogMessageVoidify() & (*g_null_ostream)
logging.cc
  std::ostream* g_null_ostream = nullptr;

At this point, if g_null_ostream is a publicly accessible variable, in theory
anybody could set it to a NON-nullptr value, so the compiler cannot make any
assumption around that and should shut up.

In the meantime I'm going to revert this due to the breakages. -EAGAIN

[Primiano Tucci (use gerrit), 2016-12-10 10:29:40]

A revert of this CL (patchset #10 id:240001) has been created in
https://codereview.chromium.org/2569583002/ by primiano@chromium.org.

The reason for reverting is: Broke D*LOG on a bunch of bots building
non-official builds.
See https://codereview.chromium.org/2559323007/#msg60
and https://codereview.chromium.org/2559323007/#msg61
for more context..

[scottmg, 2016-12-10 15:16:53]

Description was changed from

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Review-Url: https://codereview.chromium.org/2559323007
==========

to

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Review-Url: https://codereview.chromium.org/2559323007
==========

[scottmg, 2016-12-10 15:18:21]

On 2016/12/10 10:28:35, Primiano Tucci wrote:
> Ahhh I think we need to be slightly smarter to trick compilers that run with
> lower -O levels.
> Some bots failed when hitting this when official_build = false but dcheck is
> still off (EAT_STREAM_PARAMS  still applies to  D*LOG in release builds).
> The failure looks the following:
> In file included from ../../media/formats/mp4/box_reader.cc:5:
> ../../media/formats/mp4/box_reader.h:254:3: error: binding dereferenced null
> pointer to reference has undefined behavior [-Werror,-Wnull-dereference]
>   DVLOG(2) << "Found " << children->size() << " "
>   ^~~~~~~~
> ../../base/logging.h:695:29: note: expanded from macro 'DVLOG'
> #define DVLOG(verboselevel) DVLOG_IF(verboselevel, VLOG_IS_ON(verboselevel))
>                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../base/logging.h:677:43: note: expanded from macro 'DVLOG_IF'
> #define DVLOG_IF(verboselevel, condition) EAT_STREAM_PARAMETERS
>                                           ^~~~~~~~~~~~~~~~~~~~~
> ../../base/logging.h:441:43: note: expanded from macro 'EAT_STREAM_PARAMETERS'
>       : ::logging::LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0))
>                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> 
> 
> I think this is still functionally right, however the compiler is trying to
> outsmart us, but not in a too clever way, as opposite to what happens in
> official build.
> 
> My interpretation of the situation is the following:
> Given 
> 
> #define DVLOG(verboselevel) DVLOG_IF(verboselevel, VLOG_IS_ON(verboselevel))
> #define DVLOG_IF(verboselevel, condition) EAT_STREAM_PARAMETERS
> #define EAT_STREAM_PARAMETERS  true ? (void) 0 : LogMessageVoidify() &
> (*reinterpret_cast<std::ostream*>(0))
> 
> The following code gets emitted for 
> DVLOG(2) << "Found " << children->size() << " "...
> 
> true ? (void) 0 : LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0))
<<
> "Found " << children->size() << " ";
> Which really is:
> 
> if (true) {
>   (void) 0;
> } else {
>   LogMessageVoidify() & (*reinterpret_cast<std::ostream*>(0)) << "Found " <<
> children->size() << " "... ;
> }
> 
> 
> Now the compiler is right that dereferencing nullptr is undefined behavior.
> However the problem now is that it doesn't realize (I think due to the lower
> optimization level) that such statement with undefined behavior cannot
possibly
> be hit.
> 
> 
> I think that the possible solutions to this are:
> 1) reinterpret_cast<std::ostream*>(0x100)  // At this point the compiler
cannot
> assume what is located at 100
> but this is really horrible to see in a civilised codebase
> 
> 2) Introduce an extern exported global variable and define it to be nullptr in
> the .cc file, something like
> logging.h:
>  extern BASE_EXPORT std::ostream* g_null_ostream;
>  LogMessageVoidify() & (*g_null_ostream)
> logging.cc
>   std::ostream* g_null_ostream = nullptr;
> 
> At this point, if g_null_ostream is a publicly accessible variable, in theory
> anybody could set it to a NON-nullptr value, so the compiler cannot make any
> assumption around that and should shut up.

Yeah, that's what I had in ps#7. :p I'll try that.

> In the meantime I'm going to revert this due to the breakages. -EAGAIN

[scottmg, 2016-12-10 15:24:06]

Patchset #11 (id:260001) has been deleted

[scottmg, 2016-12-10 15:29:17]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-10 15:29:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-12-10 15:49:50]

The CQ bit was unchecked by scottmg@chromium.org

[scottmg, 2016-12-10 15:50:23]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-12-10 15:50:24]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2559323007/#ps280001
(title: "back to ps7, extern ostream*")

[commit-bot: I haz the power, 2016-12-10 15:50:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-10 17:08:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-10 17:08:56]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dcheng, 2016-12-10 19:51:30]

Sigh.

Still LGTM

[dcheng, 2016-12-10 19:51:41]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-12-10 19:51:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-10 20:58:04]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1481399501611850,
"parent_rev": "03bd1f6cc5386abb9313deabd28fc7349cee756c", "commit_rev":
"54e01f0a75bf59583a1a563357d386fc30c5a230"}

[commit-bot: I haz the power, 2016-12-10 20:58:33]

Description was changed from

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Review-Url: https://codereview.chromium.org/2559323007
==========

to

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Review-Url: https://codereview.chromium.org/2559323007
Review-Url: https://codereview.chromium.org/2559323007
==========

[commit-bot: I haz the power, 2016-12-10 20:58:35]

Committed patchset #11 (id:280001)

[commit-bot: I haz the power, 2016-12-12 15:06:45]

Description was changed from

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret



After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret


Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Review-Url: https://codereview.chromium.org/2559323007
Review-Url: https://codereview.chromium.org/2559323007
==========

to

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret

After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret

Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Committed: https://crrev.com/39e086ae3eda235a490200b73b1702cbf5d2cc93
Cr-Commit-Position: refs/heads/master@{#437763}
==========

[commit-bot: I haz the power, 2016-12-12 15:06:46]

Patchset 11 (id:??) landed as
https://crrev.com/39e086ae3eda235a490200b73b1702cbf5d2cc93
Cr-Commit-Position: refs/heads/master@{#437763}

[commit-bot: I haz the power, 2016-12-12 15:07:20]

Description was changed from

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret

After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret

Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Committed: https://crrev.com/39e086ae3eda235a490200b73b1702cbf5d2cc93
Cr-Commit-Position: refs/heads/master@{#437763}
==========

to

==========
Improve EAT_STREAM_PARAMETERS for Windows x86

Dumps of check_example.exe

Current:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EDC: 55                 push        ebp
  00404EDD: 8B EC              mov         ebp,esp
  00404EDF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EE3: 75 07              jne         00404EEC
  00404EE5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EEC: 5D                 pop         ebp
  00404EED: C3                 ret
?DoCheck@@YAX_N@Z:
  00404EEE: 55                 push        ebp
  00404EEF: 8B EC              mov         ebp,esp
  00404EF1: 51                 push        ecx
  00404EF2: 83 65 FC 00        and         dword ptr [ebp-4],0
  00404EF6: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EFA: 75 07              jne         00404F03
  00404EFC: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404F03: 8B E5              mov         esp,ebp
  00404F05: 5D                 pop         ebp
  00404F06: C3                 ret
_main:
  00404F07: 55                 push        ebp
  00404F08: 8B EC              mov         ebp,esp
  00404F0A: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404F0E: 53                 push        ebx
  00404F0F: 0F 9F C3           setg        bl
  00404F12: 53                 push        ebx
  00404F13: E8 D6 FF FF FF     call        ?DoCheck@@YAX_N@Z
  00404F18: 53                 push        ebx
  00404F19: E8 BE FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404F1E: 59                 pop         ecx
  00404F1F: 59                 pop         ecx
  00404F20: 33 C0              xor         eax,eax
  00404F22: 5B                 pop         ebx
  00404F23: 5D                 pop         ebp
  00404F24: C3                 ret

After this CL:

?DoBlinkReleaseAssert@@YAX_N@Z:
  00404EAC: 55                 push        ebp
  00404EAD: 8B EC              mov         ebp,esp
  00404EAF: 80 7D 08 00        cmp         byte ptr [ebp+8],0
  00404EB3: 75 07              jne         00404EBC
  00404EB5: C6 05 00 00 00 00  mov         byte ptr ds:[0],0
            00
  00404EBC: 5D                 pop         ebp
  00404EBD: C3                 ret
_main:
  00404EBE: 55                 push        ebp
  00404EBF: 8B EC              mov         ebp,esp
  00404EC1: 83 7D 08 02        cmp         dword ptr [ebp+8],2
  00404EC5: 53                 push        ebx
  00404EC6: 0F 9F C3           setg        bl
  00404EC9: 53                 push        ebx
  00404ECA: E8 DD FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ECF: 53                 push        ebx
  00404ED0: E8 D7 FF FF FF     call        ?DoBlinkReleaseAssert@@YAX_N@Z
  00404ED5: 59                 pop         ecx
  00404ED6: 59                 pop         ecx
  00404ED7: 33 C0              xor         eax,eax
  00404ED9: 5B                 pop         ebx
  00404EDA: 5D                 pop         ebp
  00404EDB: C3                 ret

Amusingly, I was confused because I thought I was going crazy when
DoCheck wasn't showing up in the /disasm. But of course, it's because it
got COMDAT'd with the Blink one, as we want. :)

R=primiano@chromium.org
BUG=672699

Review-Url: https://codereview.chromium.org/2559323007
Committed: https://crrev.com/3c957a583f692a71024bddbcf50b39a69af89d6e
Cr-Commit-Position: refs/heads/master@{#437777}
==========

[commit-bot: I haz the power, 2016-12-12 15:07:21]

Patchset 11 (id:??) landed as
https://crrev.com/3c957a583f692a71024bddbcf50b39a69af89d6e
Cr-Commit-Position: refs/heads/master@{#437777}

[danakj, 2016-12-14 17:02:40]

Thank you for this :)