Commit 30e5d69e authored by Artem Bolgar's avatar Artem Bolgar Committed by Chromium LUCI CQ

Fixing "Abort message: '[FATAL:decoder_stream.cc(524)] OnDecodeDone<audio>: 0" in dev/debug

Dev and debug versions throw DCHECK in OnDecodeDone due to wrong syntax of logging statement:
```
FUNCTION_DVLOG(status.is_ok() ? 3 : 1) << ": " << status.code();
```
The FUNCTION_DVLOG is defined as follows:
```
  DVLOG(level) << __func__ << "<" << GetStreamTypeString() << ">"
```

The issue is that the condition "status.is_ok() ? 3 : 1" should become "level", but in reality, since there are no braces around it, it gets parsed incorrectly and the number "3" becomes a severity level (see base/logging.h), which corresponds to "FATAL" severity level, which ends up which crash / assert.

Here is the code after preprocessor for that line 524 BEFORE the fix:

  !(((status.is_ok() ? 3 : 1) <= ::logging::GetVlogLevel("../../media/filters/decoder_stream.cc")) && (true)) ? (void) 0 : ::logging::LogMessageVoidify() & (::logging::LogMessage("../../media/filters/decoder_stream.cc", 524, -status.is_ok() ? 3 : 1).stream()) << __func__ << "<" << GetStreamTypeString() << ">" << ": " << status.code();

and this is the fixed version:

  !((((status.is_ok() ? 3 : 1)) <= ::logging::GetVlogLevel("../../media/filters/decoder_stream.cc")) && (true)) ? (void) 0 : ::logging::LogMessageVoidify() & (::logging::LogMessage("../../media/filters/decoder_stream.cc", 524, -(status.is_ok() ? 3 : 1)).stream()) << __func__ << "<" << GetStreamTypeString() << ">" << ": " << status.code();

Pay attention to this part:

LogMessage("../../media/filters/decoder_stream.cc", 524, -status.is_ok() ? 3 : 1).stream())

and

LogMessage("../../media/filters/decoder_stream.cc", 524, -(status.is_ok() ? 3 : 1)).stream())

So, basically, before the fix the negative sign is applied to the wrong statement, therefore the LogMessage will get +3 instead of -3. +3 means FATAL, while -3 means verbosity level 3, as far as I understand.

Fixed: 1157701, 1156942
Change-Id: I3132dde1dbd7d18460b74715016b74228e52ad7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586656Reviewed-by: default avatardanakj <danakj@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Commit-Queue: Artem Bolgar <artyom@fb.com>
Cr-Commit-Position: refs/heads/master@{#836357}
parent ff2da9d7
......@@ -454,7 +454,7 @@ const LogSeverity LOGGING_0 = LOGGING_ERROR;
// The VLOG macros log with negative verbosities.
#define VLOG_STREAM(verbose_level) \
::logging::LogMessage(__FILE__, __LINE__, -verbose_level).stream()
::logging::LogMessage(__FILE__, __LINE__, -(verbose_level)).stream()
#define VLOG(verbose_level) \
LAZY_STREAM(VLOG_STREAM(verbose_level), VLOG_IS_ON(verbose_level))
......@@ -465,11 +465,11 @@ const LogSeverity LOGGING_0 = LOGGING_ERROR;
#if defined (OS_WIN)
#define VPLOG_STREAM(verbose_level) \
::logging::Win32ErrorLogMessage(__FILE__, __LINE__, -verbose_level, \
::logging::Win32ErrorLogMessage(__FILE__, __LINE__, -(verbose_level), \
::logging::GetLastSystemErrorCode()).stream()
#elif defined(OS_POSIX) || defined(OS_FUCHSIA)
#define VPLOG_STREAM(verbose_level) \
::logging::ErrnoLogMessage(__FILE__, __LINE__, -verbose_level, \
::logging::ErrnoLogMessage(__FILE__, __LINE__, -(verbose_level), \
::logging::GetLastSystemErrorCode()).stream()
#endif
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment