Make UBSan warnings fatal and fix the existing ones

Make UBSan warnings fatal and fix the existing ones

The warnings were (all signed integer overflow):
webrtc/common_audio/signal_processing/levinson_durbin.c:46:25
12 * 268435456 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core.cc:930:69
522240 * 6115 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:455:36
72293096 * 50 cannot be represented in type 'int'
webrtc/modules/pacing/alr_detector.cc:70:48
1000000000 * 65 cannot be represented in type 'int'
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:947:20
1929277286 + 321546521 cannot be represented in type 'int'

BUG=webrtc:8195
Review-Url: https://codereview.webrtc.org/3005003002
Cr-Commit-Position: refs/heads/master@{#19670}
Committed: https://chromium.googlesource.com/external/webrtc/+/ba09f79ba31c543d0dcf3a6d824992685087005a

[oprypin_webrtc, 2017-09-01 13:58:57]

Description was changed from

==========
Surface the UBSan warnings by making them fatal

BUG=webrtc:8195
==========

to

==========
Make UBSan warnings fatal and fix the existing ones

BUG=webrtc:8195
==========

[oprypin_webrtc, 2017-09-01 13:58:58]

oprypin@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org, kjellander@webrtc.org, kwiberg@webrtc.org,
niklas.enbom@webrtc.org

[oprypin_webrtc, 2017-09-01 14:06:38]

Description was changed from

==========
Make UBSan warnings fatal and fix the existing ones

BUG=webrtc:8195
==========

to

==========
Make UBSan warnings fatal and fix the existing ones

The warnings were:
webrtc/common_audio/signal_processing/levinson_durbin.c:46:25
signed integer overflow: 12 * 268435456 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core.cc:930:69
signed integer overflow: 522240 * 6115 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:455:36
signed integer overflow: 72293096 * 50 cannot be represented in type 'int'
webrtc/modules/pacing/alr_detector.cc:70:48
signed integer overflow: 1000000000 * 65 cannot be represented in type 'int'
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:947:20
signed integer overflow: 1929277286 + 321546521 cannot be represented in type
'int'

BUG=webrtc:8195
==========

[oprypin_webrtc, 2017-09-01 14:38:50]

Description was changed from

==========
Make UBSan warnings fatal and fix the existing ones

The warnings were:
webrtc/common_audio/signal_processing/levinson_durbin.c:46:25
signed integer overflow: 12 * 268435456 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core.cc:930:69
signed integer overflow: 522240 * 6115 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:455:36
signed integer overflow: 72293096 * 50 cannot be represented in type 'int'
webrtc/modules/pacing/alr_detector.cc:70:48
signed integer overflow: 1000000000 * 65 cannot be represented in type 'int'
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:947:20
signed integer overflow: 1929277286 + 321546521 cannot be represented in type
'int'

BUG=webrtc:8195
==========

to

==========
Make UBSan warnings fatal and fix the existing ones

The warnings were (all signed integer overflow):
webrtc/common_audio/signal_processing/levinson_durbin.c:46:25
12 * 268435456 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core.cc:930:69
522240 * 6115 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:455:36
72293096 * 50 cannot be represented in type 'int'
webrtc/modules/pacing/alr_detector.cc:70:48
1000000000 * 65 cannot be represented in type 'int'
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:947:20
1929277286 + 321546521 cannot be represented in type 'int'

BUG=webrtc:8195
==========

[oprypin_webrtc, 2017-09-01 22:39:11]

The CQ bit was checked by oprypin@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-09-01 22:39:19]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-09-01 23:06:09]

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

[commit-bot: I haz the power, 2017-09-01 23:06:10]

Dry run: Try jobs failed on following builders:
  android_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/27313)

[kwiberg-webrtc, 2017-09-03 02:39:47]

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aecm/aecm_core_c.cc (right):

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:457:
(static_cast<int64_t>(tmp32no1) * 50) >> 8);
If you write int64_t{x} instead of static_cast<int64_t>(x), the compiler will
complain unless all possible values of x are preserved.

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/pacing/alr...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/pacing/alr...
webrtc/modules/pacing/alr_detector.cc:72:
alr_budget_.set_target_rate_kbps(rtc::dchecked_cast<int>(target_rate_kbps));
Why not use int64_t here instead of double? It'll be faster, more deterministic,
and give results bit-identical to those of the current implementation.

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:928: double avg_delay_ms = 0;
Again, int64_t?

[oprypin_webrtc, 2017-09-04 08:36:07]

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aecm/aecm_core_c.cc (right):

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:457:
(static_cast<int64_t>(tmp32no1) * 50) >> 8);
On 2017/09/03 02:39:47, kwiberg-webrtc wrote:
> If you write int64_t{x} instead of static_cast<int64_t>(x), the compiler will
> complain unless all possible values of x are preserved.

OK thanks!

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/pacing/alr...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/pacing/alr...
webrtc/modules/pacing/alr_detector.cc:72:
alr_budget_.set_target_rate_kbps(rtc::dchecked_cast<int>(target_rate_kbps));
On 2017/09/03 02:39:47, kwiberg-webrtc wrote:
> Why not use int64_t here instead of double? It'll be faster, more
deterministic,
> and give results bit-identical to those of the current implementation.

Done.

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/3005003002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:928: double avg_delay_ms = 0;
On 2017/09/03 02:39:47, kwiberg-webrtc wrote:
> Again, int64_t?

Yeah makes sense. My logic was that `double` is safer for a much larger range of
values and still has exact precision for up to 52 bits. But int64 will work.

[kwiberg-webrtc, 2017-09-04 08:54:00]

lgtm

https://codereview.webrtc.org/3005003002/diff/40001/webrtc/modules/pacing/alr...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/3005003002/diff/40001/webrtc/modules/pacing/alr...
webrtc/modules/pacing/alr_detector.cc:70: int64_t target_rate_kbps =
int64_t{bitrate_bps} * bandwidth_usage_percent_ /
Consider replacing the first "int64_t" with "auto". That way, you're guaranteed
to not get a narrowing conversion when you save the value to a temporary.

(Also consider making the temporary const. It's not all that necessary given
that it only lives one more line, but it's a good habit to get into.)

[oprypin_webrtc, 2017-09-04 09:09:21]

https://codereview.webrtc.org/3005003002/diff/40001/webrtc/modules/pacing/alr...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/3005003002/diff/40001/webrtc/modules/pacing/alr...
webrtc/modules/pacing/alr_detector.cc:70: int64_t target_rate_kbps =
int64_t{bitrate_bps} * bandwidth_usage_percent_ /
On 2017/09/04 08:53:59, kwiberg-webrtc wrote:
> Consider replacing the first "int64_t" with "auto". That way, you're
guaranteed
> to not get a narrowing conversion when you save the value to a temporary.
> 
> (Also consider making the temporary const. It's not all that necessary given
> that it only lives one more line, but it's a good habit to get into.)

Done.

[oprypin_webrtc, 2017-09-04 09:09:25]

The CQ bit was checked by oprypin@webrtc.org

[oprypin_webrtc, 2017-09-04 09:09:25]

The patchset sent to the CQ was uploaded after l-g-t-m from kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3005003002/#ps60001 (title:
"Use 'auto' per suggestion")

[commit-bot: I haz the power, 2017-09-04 09:09:35]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-09-04 10:21:06]

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

[commit-bot: I haz the power, 2017-09-04 10:21:07]

Try jobs failed on following builders:
  android_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/27390)

[oprypin_webrtc, 2017-09-04 12:58:24]

Patchset #5 (id:80001) has been deleted

[oprypin_webrtc, 2017-09-04 13:03:32]

On 2017/09/04 10:21:07, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   android_dbg on master.tryserver.webrtc (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/27390)

I had to revert one of the fixes and blacklist the error instead.
Needs further investigation --
https://bugs.chromium.org/p/webrtc/issues/detail?id=8200
Will submit this soon if there are no objections.

[oprypin_webrtc, 2017-09-04 13:38:41]

The CQ bit was checked by oprypin@webrtc.org

[oprypin_webrtc, 2017-09-04 13:38:41]

The patchset sent to the CQ was uploaded after l-g-t-m from kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3005003002/#ps100001 (title:
"Revert one fix because the overflow was relied on")

[commit-bot: I haz the power, 2017-09-04 13:38:51]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-09-04 15:32:44]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1504532321311160,
"parent_rev": "9762d4f2e551ba6d32df693b047c8bac867b904c", "commit_rev":
"ba09f79ba31c543d0dcf3a6d824992685087005a"}

[commit-bot: I haz the power, 2017-09-04 15:33:00]

Description was changed from

==========
Make UBSan warnings fatal and fix the existing ones

The warnings were (all signed integer overflow):
webrtc/common_audio/signal_processing/levinson_durbin.c:46:25
12 * 268435456 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core.cc:930:69
522240 * 6115 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:455:36
72293096 * 50 cannot be represented in type 'int'
webrtc/modules/pacing/alr_detector.cc:70:48
1000000000 * 65 cannot be represented in type 'int'
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:947:20
1929277286 + 321546521 cannot be represented in type 'int'

BUG=webrtc:8195
==========

to

==========
Make UBSan warnings fatal and fix the existing ones

The warnings were (all signed integer overflow):
webrtc/common_audio/signal_processing/levinson_durbin.c:46:25
12 * 268435456 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core.cc:930:69
522240 * 6115 cannot be represented in type 'int'
webrtc/modules/audio_processing/aecm/aecm_core_c.cc:455:36
72293096 * 50 cannot be represented in type 'int'
webrtc/modules/pacing/alr_detector.cc:70:48
1000000000 * 65 cannot be represented in type 'int'
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:947:20
1929277286 + 321546521 cannot be represented in type 'int'

BUG=webrtc:8195

Review-Url: https://codereview.webrtc.org/3005003002
Cr-Commit-Position: refs/heads/master@{#19670}
Committed:
https://chromium.googlesource.com/external/webrtc/+/ba09f79ba31c543d0dcf3a6d8...
==========

[commit-bot: I haz the power, 2017-09-04 15:33:02]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/external/webrtc/+/ba09f79ba31c543d0dcf3a6d8...

[kamilinimukharji, 2017-09-04 17:37:28]

On 2017/09/04 15:33:02, commit-bot: I haz the power wrote:
> Committed patchset #5 (id:100001) as
>
https://chromium.googlesource.com/external/webrtc/+/ba09f79ba31c543d0dcf3a6d8...

Get Chromium from Appvn. Download it from https://appvn.vip/