New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 759336 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocking:
issue 718641



Sign in to add a comment

CHECK failure: buffers_.empty() || CanAppendBuffersToEnd(new_buffers, new_buffers_group_start_t

Project Member Reported by ClusterFuzz, Aug 26 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=4634068246593536

Fuzzer: libFuzzer_mediasource_WEBM_VP9_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  buffers_.empty() || CanAppendBuffersToEnd(new_buffers, new_buffers_group_start_t
  SignalHandler
  media::SourceBufferRange::AppendBuffersToEnd
  
Sanitizer: memory (MSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=497057:497112

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4634068246593536

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Cc: msrchandra@chromium.org
Labels: M-62 Test-Predator-Wrong
Owner: wolenetz@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using Code Search for the file, "source_buffer_range.cc" assigning to the concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/c485d6c706fe27268948c43e71ef9f68e743bcf7

@wolenetz -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
 Issue 759314  has been merged into this issue.
Components: Internals>Media>Source
Status: Started (was: Assigned)
Investigating. 
Confirmed repro (with debug ASAN tooling, not MSAN, since I already had that built...)
Cc: chcunningham@chromium.org
After debugging quite a bit locally, I understand the problem:

https://chromium.googlesource.com/chromium/src/+/565106b4e643be1d181958320b3ed9d8fc78c330/media/filters/source_buffer_stream.cc#509 needs to be more careful:

In the case where the last append is *not* removed by the TruncateAt, below, next append might belong to the current, not the new range (if the last append is in the current, not the new range).

Labels: -Pri-1 Pri-2
This is hitting a CHECK, not a DCHECK. Lowering priority.
Blocking: 718641
Preliminary CL that repros the issue with new unit tests (not yet including a fix): https://chromium-review.googlesource.com/c/chromium/src/+/651352

This fix impacts how SBS performs overlap appends (impacting PTS/DTS too), so I'm choosing to attempt a fix before forking the SBS logic to handle overlap appends by PTS range (separately from soon-to-be-legacy-by-DTS-range) to reduce code churn.
Labels: -M-62 M-63
Fixing this issue includes fixing tightly-related  bug 589295  and  bug 758802 .
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2b16265430412636e3856901ffafab66541d9b6

commit c2b16265430412636e3856901ffafab66541d9b6
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Wed Sep 06 21:07:46 2017

MSE: Remove the +1us in SBS::PotentialNextAppendTimestamp()

That +1us was enough to trigger a problem when prepending a previous
buffer with a couple new buffers that are 1us long and are in their own
continuous coded frame group. Removing the +1us seems to fix the problem
without regressing any other test cases (caveat [0]). Also, removing the
+1us was previously tracked by  bug 589295  as part of residual future
work leftover from relaxing the keyframe restriction ( bug 249412 ). See
related CR comment thread [1].

New tests for this required microsecond-granularity timestamp and
duration test modifications, included too.

[0] An immediate result of this +1us removal is that both of the new test
    cases trigger  bug 758802  (one more than before), so I fixed that issue
    in this CL as well: buffers can have 0 duration (consider WebM alt-ref).
    Fixing  bug 758802  essentially lets estimated buffers be adjusted to
    have 0 duration minimum (instead of 1 microsecond minimum).

[1] https://codereview.chromium.org/1670033002/diff/80001/media/filters/source_buffer_stream.cc#newcode442

BUG= 759336 ,  589295 ,  758802 
TEST=2 new SourceBufferStreamTests, both of which failed prior to this CL;
fuzzer cases in 759336 and 758802 no longer repro locally.

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If04dbdf605a3c6d4876afb015d521ad60863bafa
Reviewed-on: https://chromium-review.googlesource.com/651352
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500086}
[modify] https://crrev.com/c2b16265430412636e3856901ffafab66541d9b6/media/filters/source_buffer_range.cc
[modify] https://crrev.com/c2b16265430412636e3856901ffafab66541d9b6/media/filters/source_buffer_stream.cc
[modify] https://crrev.com/c2b16265430412636e3856901ffafab66541d9b6/media/filters/source_buffer_stream_unittest.cc

Status: Fixed (was: Started)
#9 should fix this. Pending CF verification...
Project Member

Comment 11 by ClusterFuzz, Sep 7 2017

ClusterFuzz has detected this issue as fixed in range 500065:500110.

Detailed report: https://clusterfuzz.com/testcase?key=4634068246593536

Fuzzer: libFuzzer_mediasource_WEBM_VP9_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  buffers_.empty() || CanAppendBuffersToEnd(new_buffers, new_buffers_group_start_t
  SignalHandler
  media::SourceBufferRange::AppendBuffersToEnd
  
Sanitizer: memory (MSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=497057:497112
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=500065:500110

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4634068246593536

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 12 by ClusterFuzz, Sep 7 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 5713298032164864 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 13 by ClusterFuzz, Sep 7 2017

ClusterFuzz has detected this issue as fixed in range 500065:500110.

Detailed report: https://clusterfuzz.com/testcase?key=4634068246593536

Fuzzer: libFuzzer_mediasource_WEBM_VP9_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  buffers_.empty() || CanAppendBuffersToEnd(new_buffers, new_buffers_group_start_t
  SignalHandler
  media::SourceBufferRange::AppendBuffersToEnd
  
Sanitizer: memory (MSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=497057:497112
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=500065:500110

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4634068246593536

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

Sign in to add a comment