From 2ddb8bd3593a2a2797fea16f358cc89c140f6154 Mon Sep 17 00:00:00 2001
From: sprang <sprang@webrtc.org>
Date: Thu, 4 Feb 2016 03:59:52 -0800
Subject: [PATCH] Avoid undefined behavior in vp8 screenshare_layers

active_layer_ could be dereferenced while being -1...
Also added som DCHECKs

BUG=webrtc:5490

Review URL: https://codereview.webrtc.org/1656233002

Cr-Commit-Position: refs/heads/master@{#11486}
---
 .../codecs/vp8/screenshare_layers.cc           | 18 +++++++++++-------
 .../codecs/vp8/screenshare_layers_unittest.cc  |  4 ++++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc b/webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc
index 536587a13e..2480a502e8 100644
--- a/webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/screenshare_layers.cc
@@ -57,8 +57,8 @@ ScreenshareLayers::ScreenshareLayers(int num_temporal_layers,
       max_qp_(-1),
       max_debt_bytes_(0),
       frame_rate_(-1) {
-  assert(num_temporal_layers > 0);
-  assert(num_temporal_layers <= 2);
+  RTC_CHECK_GT(num_temporal_layers, 0);
+  RTC_CHECK_LE(num_temporal_layers, 2);
 }
 
 int ScreenshareLayers::CurrentLayerId() const {
@@ -147,7 +147,8 @@ bool ScreenshareLayers::ConfigureBitrates(int bitrate_kbps,
     }
 
     // Don't reconfigure qp limits during quality boost frames.
-    if (layers_[active_layer_].state != TemporalLayer::State::kQualityBoost) {
+    if (active_layer_ == -1 ||
+        layers_[active_layer_].state != TemporalLayer::State::kQualityBoost) {
       min_qp_ = cfg->rc_min_quantizer;
       max_qp_ = cfg->rc_max_quantizer;
       // After a dropped frame, a frame with max qp will be encoded and the
@@ -169,6 +170,10 @@ bool ScreenshareLayers::ConfigureBitrates(int bitrate_kbps,
 void ScreenshareLayers::FrameEncoded(unsigned int size,
                                      uint32_t timestamp,
                                      int qp) {
+  if (number_of_temporal_layers_ == 1)
+    return;
+
+  RTC_DCHECK_NE(-1, active_layer_);
   if (size == 0) {
     layers_[active_layer_].state = TemporalLayer::State::kDropped;
     return;
@@ -198,6 +203,7 @@ void ScreenshareLayers::PopulateCodecSpecific(bool base_layer_sync,
     vp8_info->layerSync = false;
     vp8_info->tl0PicIdx = kNoTl0PicIdx;
   } else {
+    RTC_DCHECK_NE(-1, active_layer_);
     vp8_info->temporalIdx = active_layer_;
     if (base_layer_sync) {
       vp8_info->temporalIdx = 0;
@@ -218,10 +224,7 @@ void ScreenshareLayers::PopulateCodecSpecific(bool base_layer_sync,
 }
 
 bool ScreenshareLayers::TimeToSync(int64_t timestamp) const {
-  if (active_layer_ != 1) {
-    RTC_NOTREACHED();
-    return false;
-  }
+  RTC_DCHECK_EQ(1, active_layer_);
   RTC_DCHECK_NE(-1, layers_[0].last_qp);
   if (layers_[1].last_qp == -1) {
     // First frame in TL1 should only depend on TL0 since there are no
@@ -248,6 +251,7 @@ bool ScreenshareLayers::TimeToSync(int64_t timestamp) const {
 bool ScreenshareLayers::UpdateConfiguration(vpx_codec_enc_cfg_t* cfg) {
   if (max_qp_ == -1 || number_of_temporal_layers_ <= 1)
     return false;
+  RTC_DCHECK_NE(-1, active_layer_);
 
   // If layer is in the quality boost state (following a dropped frame), update
   // the configuration with the adjusted (lower) qp and set the state back to
diff --git a/webrtc/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
index f31ed5e4d8..e5e88942e4 100644
--- a/webrtc/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
@@ -8,6 +8,8 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include <vector>
+
 #include "gtest/gtest.h"
 #include "vpx/vpx_encoder.h"
 #include "vpx/vp8cx.h"
@@ -41,6 +43,8 @@ class ScreenshareLayerTest : public ::testing::Test {
                    CodecSpecificInfoVP8* vp8_info,
                    int* flags) {
     *flags = layers_->EncodeFlags(timestamp);
+    if (*flags == -1)
+      return;
     layers_->PopulateCodecSpecific(base_sync, vp8_info, timestamp);
     ASSERT_NE(-1, frame_size_);
     layers_->FrameEncoded(frame_size_, timestamp, kDefaultQp);