Bug 174628 - [EME] Build failure with Clang-3.8 on InitDataRegistry.cpp
Summary: [EME] Build failure with Clang-3.8 on InitDataRegistry.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-18 07:28 PDT by Carlos Alberto Lopez Perez
Modified: 2017-07-19 10:27 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.49 KB, patch)
2017-07-18 07:33 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (1.54 KB, patch)
2017-07-19 06:56 PDT, Carlos Alberto Lopez Perez
zan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2017-07-18 07:28:25 PDT
Building WPE with clang-3.8 gives this error:

Building CXX object Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o
FAILED: Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o 
/usr/lib/ccache/clang++   -DBUILDING_WITH_CMAKE=1 -DBUILDING_WPE__=1 -DBUILDING_WebCore -DDATA_DIR=\"share\" -DGETTEXT_PACKAGE=\"WPE\" -DHAVE_CONFIG_H=1 -DSTATICALLY_LINKED_WITH_PAL=1 -isystem ../DependenciesWPE/Root/include/gstreamer-1.0 -isystem ../DependenciesWPE/Root/include/glib-2.0 -isystem ../DependenciesWPE/Root/lib/glib-2.0/include  [.....]  Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o -MF Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o.d -o Source/WebCore/CMakeFiles/WebCore.dir/Modules/encryptedmedia/InitDataRegistry.cpp.o -c /home/clopez/webkit/wpe/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp
ICECC[13551] 16:24:49: no server found behind given hostname 192.168.10.152:10245
ICECC[13551] 16:24:49: got exception 2 (192.168.10.152) 
/home/clopez/webkit/wpe/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:45:34: error: non-constant-expression cannot be narrowed from type 'size_t' (aka 'unsigned long') to 'unsigned int' in initializer list [-Wc++11-narrowing]
    String json { buffer.data(), buffer.size() };
                                 ^~~~~~~~~~~~~
/home/clopez/webkit/wpe/Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:45:34: note: insert an explicit cast to silence this issue
    String json { buffer.data(), buffer.size() };
                                 ^~~~~~~~~~~~~
                                 static_cast<unsigned int>( )
1 error generated.
Comment 1 Carlos Alberto Lopez Perez 2017-07-18 07:33:04 PDT
Created attachment 315793 [details]
Patch
Comment 2 Michael Catanzaro 2017-07-18 07:52:30 PDT
Comment on attachment 315793 [details]
Patch

I'm not sure about this. Clearly a String cannot hold as much data as a SharedBuffer, but perhaps extractKeyIDsKeyids should return an error instead of silently truncating the data?

It's odd that the String class uses unsigned rather than size_t.
Comment 3 Carlos Alberto Lopez Perez 2017-07-18 08:04:47 PDT
I'm also not sure.
Comment 4 Zan Dobersek 2017-07-19 06:18:31 PDT
In practice it's unlikely that such a large JSON input would be passed in. If it was truncated, it's again unlikely that it would be correctly parsed  in the parseJSON() call.

But for the sake of tidiness the input SharedBuffer should have its size tested, returning immediately if it exceeds the maximum value that fits into the unsigned type. After that we can static_cast<> the size_t value to unsigned without an issue.
Comment 5 Carlos Alberto Lopez Perez 2017-07-19 06:56:11 PDT
Created attachment 315917 [details]
Patch
Comment 6 Zan Dobersek 2017-07-19 09:37:31 PDT
Comment on attachment 315917 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315917&action=review

> Source/WebCore/Modules/encryptedmedia/InitDataRegistry.cpp:45
> +    if (buffer.size() > UINT_MAX)

I think std::numeric_limits<unsigned>::max() would fit better, but up to you.
Comment 7 Carlos Alberto Lopez Perez 2017-07-19 10:27:20 PDT
Committed r219658: <http://trac.webkit.org/changeset/219658>