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

Issue 777391 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: 2017-10-26
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Cannot compile media/filters/source_buffer_stream.cc with GCC

Reported by tomas.po...@gmail.com, Oct 23 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.62 Safari/537.36

Steps to reproduce the problem:
Try to compile Chromium with GCC 6.3 (as well as 5.x).

sh-4.1$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/opt/rh/devtoolset-6/root/usr/libexec/gcc/x86_64-redhat-linux/6.3.1/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,fortran,lto --prefix=/opt/rh/devtoolset-6/root/usr --mandir=/opt/rh/devtoolset-6/root/usr/share/man --infodir=/opt/rh/devtoolset-6/root/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --enable-plugin --with-linker-hash-style=gnu --enable-initfini-array --disable-libgcj --with-default-libstdcxx-abi=gcc4-compatible --with-isl=/builddir/build/BUILD/gcc-6.3.1-20170216/obj-x86_64-redhat-linux/isl-install --enable-libmpx --with-mpc=/builddir/build/BUILD/gcc-6.3.1-20170216/obj-x86_64-redhat-linux/mpc-install --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 6.3.1 20170216 (Red Hat 6.3.1-3) (GCC)

It's actually known bug in GCC and was fixed in GCC 7.2+ - see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66297

What is the expected behavior?

What went wrong?
/opt/rh/devtoolset-6/root/usr/bin/g++ -MMD -MF obj/media/filters/filters/source_buffer_stream.o.d -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DOFFICIAL_BUILD -DCHROMIUM_BUILD -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DMEDIA_IMPLEMENTATION -DUSE_PULSEAUDIO -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_32 -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_26 -DGL_GLEXT_PROTOTYPES -DUSE_GLX -DUSE_EGL -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_HAS_JPEG_LIBRARY -DSK_SUPPORT_GPU=1 -I../.. -Igen -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I../../third_party/khronos -I../../gpu -I../../third_party/libwebp/src -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/encode -I../../third_party/skia/include/gpu -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/third_party/vulkan -I../../third_party/skia/include/codec -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl -I../../third_party/libyuv/include -I../../third_party/ffmpeg/chromium/config/Chromium/linux/x64 -I../../third_party/ffmpeg -I../../third_party/opus/src/include -I../../third_party/libvpx/source/libvpx -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pipe -pthread -m64 -march=x86-64 -Wall -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-unused-parameter -O2 -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -g0 -fvisibility=hidden -std=gnu++14 -Wno-narrowing -fno-rtti -fno-exceptions -fvisibility-inlines-hidden -O2 -g1 -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic  -c ../../media/filters/source_buffer_stream.cc -o obj/media/filters/filters/source_buffer_stream.o
../../media/filters/source_buffer_stream.cc: In member function 'constexpr bool media::SourceBufferStream<RangeClass>::BufferingByPts() [with RangeClass = media::SourceBufferRangeByDts]':
../../media/filters/source_buffer_stream.cc:2099:16: error: enclosing class of constexpr non-static member function 'constexpr bool media::SourceBufferStream<RangeClass>::BufferingByPts() [with RangeClass = media::SourceBufferRangeByDts]' is not a literal type
 constexpr bool SourceBufferStream<SourceBufferRangeByDts>::BufferingByPts() {
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../media/filters/source_buffer_stream.cc:5:0:
../../media/filters/source_buffer_stream.h:54:20: note: 'media::SourceBufferStream<media::SourceBufferRangeByDts>' is not literal because:
 class MEDIA_EXPORT SourceBufferStream {
                    ^~~~~~~~~~~~~~~~~~
../../media/filters/source_buffer_stream.h:54:20: note:   'media::SourceBufferStream<media::SourceBufferRangeByDts>' has a non-trivial destructor
../../media/filters/source_buffer_stream.cc: In member function 'constexpr bool media::SourceBufferStream<RangeClass>::BufferingByPts() [with RangeClass = media::SourceBufferRangeByPts]':
../../media/filters/source_buffer_stream.cc:2104:16: error: enclosing class of constexpr non-static member function 'constexpr bool media::SourceBufferStream<RangeClass>::BufferingByPts() [with RangeClass = media::SourceBufferRangeByPts]' is not a literal type
 constexpr bool SourceBufferStream<SourceBufferRangeByPts>::BufferingByPts() {
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../media/filters/source_buffer_stream.cc:5:0:
../../media/filters/source_buffer_stream.h:54:20: note: 'media::SourceBufferStream<media::SourceBufferRangeByPts>' is not literal because:
 class MEDIA_EXPORT SourceBufferStream {
                    ^~~~~~~~~~~~~~~~~~
../../media/filters/source_buffer_stream.h:54:20: note:   'media::SourceBufferStream<media::SourceBufferRangeByPts>' has a non-trivial destructor                                                                    ^

Did this work before? N/A 

Chrome version: 63.0.3239.9  Channel: dev
OS Version: 
Flash Version:
 
Components: Blink>Media
Labels: Needs-Triage-M62
Cc: wolenetz@chromium.org
Components: -Blink>Media Internals>Media>Source
Cc: kjellander@chromium.org gov...@chromium.org
Owner: wolenetz@chromium.org
Status: Started (was: Unconfirmed)
It seems a quick fix would be to change this to a static (or instance) member method.

+kjellander@ (from crbug 725889) (caveat this isn't libyuv) - is there currently a chromium bot using older GCC (< 7.2) with which I could test before landing such a change? If not, the change would seem trivially testable by the reporter.

+govind@ - Is supporting builds of Chromium using GCC (including < 7.2) something merge-worthy? The build regression for this started in M-63.

Comment 4 by gov...@chromium.org, Oct 24 2017

Labels: M-63
If it is a M63 regression and fix is fully safe to merge to M63, then pls request a merge to M63. Is the change only affect Linux or any others OSs too?
Cc: sande...@chromium.org
@#4: The change will be in all Chrom* builds. The regression being fixed is not exposed by our waterfall/official releases IIUC (unless we really do use older GCC somewhere in our bots/release builders); rather it is an external report.

The fix is trivial by itself. CL for review on trunk is:
https://chromium-review.googlesource.com/c/chromium/src/+/736029

Labels: Needs-Feedback
tomas.popela@ - can you try building with older GCC with the patch in #5 applied? It should land in Chromium trunk shortly, and I'd like further info about its effectiveness for your use case before requesting merge to M63. Thanks!
Labels: -Needs-Feedback
I abandoned my CL and am landing tomas.popela@'s https://chromium-review.googlesource.com/c/chromium/src/+/733121 instead, since that CL ostensibly solves the older GCC compile bug in a slightly different way (static constexpr instead of instance const method).

Comment 8 by gov...@chromium.org, Oct 24 2017

Pls request a merge to M63 when cl is ready.
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 25 2017

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

commit 2083a5d15cbbe84b87e02075e397e6b1dcf1f4ca
Author: Tomas Popela <tomas.popela@gmail.com>
Date: Wed Oct 25 03:28:32 2017

Cannot compile media/filters/source_buffer_stream.cc with GCC

Make the BufferingByPts() static to be able to compile it with GCC 6.3.
Clang compiles fine as well as GCC 7.2+.

Upstream GCC bug - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66297

Bug:  777391 
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: I9a3d0b1ad9abaff245f6a7ac540591acd990f106
Reviewed-on: https://chromium-review.googlesource.com/733121
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511352}
[modify] https://crrev.com/2083a5d15cbbe84b87e02075e397e6b1dcf1f4ca/media/filters/source_buffer_stream.h

Thank you @wolenetz for taking care of this (and also for landing my CL). Is it possible to propose https://bugs.chromium.org/p/chromium/issues/detail?id=776705 to be merged in M63? 
NextAction: 2017-10-26
Status: Fixed (was: Started)
#@10, you're welcome. Request to do that merge (https://bugs.chromium.org/p/chromium/issues/detail?id=776705#c4) is pending.

Normal process:
For this issue's fix (#9), I'll let it bake at least a day on at least a windows canary just to be sure. Earliest such canary is expected to begin later today (PST).

govind@: However, per #8 I'm a little confused about if I need to wait that long until requesting merge. Should I wait or go ahead now and request the merge?

tomas.popela@: Can you confirm that #9 fixes your GCC build issue in trunk?
If CL is landed in trunk and verified, pls request a merge to M63 (usually we wait for Canary coverage but no Canary for Linux) Hoping it will be a safe merge. Thank you.
The NextAction date has arrived: 2017-10-26
@wolenetz yes, I can confirm that #9 fixes the issue in trunk for me..
Labels: Merge-Request-63
Project Member

Comment 16 by sheriffbot@chromium.org, Oct 26 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #14. Please merge ASAP. Thank you.
@#17, thanks, I'll cherry-pick #9 to branch 3239 shortly
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 26 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a5a910c82633484b95c800a892811dbf601b70e

commit 5a5a910c82633484b95c800a892811dbf601b70e
Author: Tomas Popela <tomas.popela@gmail.com>
Date: Thu Oct 26 20:25:51 2017

To M63: Cannot compile media/filters/source_buffer_stream.cc with GCC

Make the BufferingByPts() static to be able to compile it with GCC 6.3.
Clang compiles fine as well as GCC 7.2+.

Upstream GCC bug - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66297

TBR=dalecurtis@chromium.org

Bug:  777391 
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: I9a3d0b1ad9abaff245f6a7ac540591acd990f106
Reviewed-on: https://chromium-review.googlesource.com/733121
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#511352}(cherry picked from commit 2083a5d15cbbe84b87e02075e397e6b1dcf1f4ca)
Reviewed-on: https://chromium-review.googlesource.com/740321
Cr-Commit-Position: refs/branch-heads/3239@{#249}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/5a5a910c82633484b95c800a892811dbf601b70e/media/filters/source_buffer_stream.h

Sign in to add a comment