New issue
Advanced search Search tips

Issue 596846 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 588506



Sign in to add a comment

Consider turning on -Wconversion for blink_platform

Project Member Reported by drott@chromium.org, Mar 22 2016

Issue description

In  issue 595960  a serious integer truncation issue was discovered which could have been avoided using this warning in the platform code. We should probably turn this on for at least blink_platform.

 

Comment 1 by drott@chromium.org, Mar 22 2016

Alternatively, if not possible at all, perhaps we can put this at least on a per file basis for Character.h 

#pragma GCC diagnostic push
#pragma GCC diagnostic warning "-Wconversion"

Comment 2 by drott@chromium.org, Mar 22 2016

Cc: pdr@chromium.org

Comment 3 by thakis@chromium.org, Mar 22 2016

As said on the other issue, let's not do the pragma thing.

Comment 4 by drott@chromium.org, Mar 22 2016

Yes, agree, I mostly put this here as a note/tool on how to do a quick check on a per-file basis, I should have described it that way.
Project Member

Comment 5 by sheriffbot@chromium.org, Mar 22 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been available for more than 365 days, and should be re-evaluated. Please re-triage this issue.
The Hotlist-Recharge-Cold label is applied for tracking purposes, and should not be removed after re-triaging the issue.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 Deleted

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 20

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

commit 4c5bdca5f2995c4ea619c076f18c9c5ad738673d
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Jul 20 18:35:44 2018

Use the appropriate ctor for ElementId.

The constructor was initialized with a int (32 bit) yet the data
type was a uint64_t. Incorrect on sign and width.

BUG=596846

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I3ef3cdd25748ba0b8ca2b3066ed74dcbee6f1b9e
Reviewed-on: https://chromium-review.googlesource.com/1145469
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576936}
[modify] https://crrev.com/4c5bdca5f2995c4ea619c076f18c9c5ad738673d/cc/trees/element_id.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 20

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

commit da3c9037fc53f4c735507f0fd0d1196772681ba9
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Jul 20 19:09:11 2018

Fix type_conversions in test code.

The code was testing C++ to WebIDL type conversions. It should have not
been using C++ long values since these are dynamic based on the
architecture. Use the C++ type that maps correctly to the WebIDL type.

This didn't cause a layout test failure because it was testing a
roundtripped value. If the C++ interface returned a value that was
out of bounds then it could have gotten truncated because on linux
sizeof(long) == 8 but WebIDL long defines it as 4 bytes.

BUG=596846

Change-Id: I92020c1cbbec0fff7c77b5fe0129eb91bbd70c7a
Reviewed-on: https://chromium-review.googlesource.com/1145138
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576944}
[modify] https://crrev.com/da3c9037fc53f4c735507f0fd0d1196772681ba9/third_party/blink/renderer/core/testing/type_conversions.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 20

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

commit c23db9cc57fea6996a9cd2af0cebfc1f2e1f4df4
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Jul 20 19:57:03 2018

Fix ArrayValue accessors that were too wide.

v8::Array's max length is UINTMAX and is uint32_t indexed. The
ArrayValue wrapper used size_t as accessors which would cause
a narrowing on access.

BUG=596846

Change-Id: I0a309d9a3417f73a404f60ff3f6699025202bc2f
Reviewed-on: https://chromium-review.googlesource.com/1145472
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576959}
[modify] https://crrev.com/c23db9cc57fea6996a9cd2af0cebfc1f2e1f4df4/third_party/blink/renderer/bindings/core/v8/array_value.cc
[modify] https://crrev.com/c23db9cc57fea6996a9cd2af0cebfc1f2e1f4df4/third_party/blink/renderer/bindings/core/v8/array_value.h
[modify] https://crrev.com/c23db9cc57fea6996a9cd2af0cebfc1f2e1f4df4/third_party/blink/renderer/modules/mediastream/media_constraints_impl.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 20

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

commit d878db60fb699fef3b80e349468fe456a85fae56
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Jul 20 21:34:52 2018

Change data type to be unsigned on report IDLs.

The value of these attributes was changed to be unsigned in
https://github.com/w3c/reporting/pull/112. This change makes the
implementation match the spec.

BUG=596846
TBR=japhet@chromium.org

Change-Id: I0bfd7e395a9a2c74ea1980afe90338316ab84120
Reviewed-on: https://chromium-review.googlesource.com/1145594
Commit-Queue: Paul Meyer <paulmeyer@chromium.org>
Reviewed-by: Paul Meyer <paulmeyer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576994}
[modify] https://crrev.com/d878db60fb699fef3b80e349468fe456a85fae56/third_party/blink/renderer/core/frame/deprecation_report_body.idl
[modify] https://crrev.com/d878db60fb699fef3b80e349468fe456a85fae56/third_party/blink/renderer/core/frame/intervention_report_body.idl

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 24

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/076fa3438ad4d2ad9de29ec57ac88afb1eeeb815

commit 076fa3438ad4d2ad9de29ec57ac88afb1eeeb815
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Tue Jul 24 13:52:57 2018

Change data type to be unsigned on report IDLs.

The value of these attributes was changed to be unsigned in
https://github.com/w3c/reporting/pull/112. This change makes the
implementation match the spec.

BUG=596846
TBR=dtapuska@chromium.org, japhet@chromium.org

(cherry picked from commit d878db60fb699fef3b80e349468fe456a85fae56)

Change-Id: I0bfd7e395a9a2c74ea1980afe90338316ab84120
Reviewed-on: https://chromium-review.googlesource.com/1145594
Commit-Queue: Paul Meyer <paulmeyer@chromium.org>
Reviewed-by: Paul Meyer <paulmeyer@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#576994}
Reviewed-on: https://chromium-review.googlesource.com/1148017
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#38}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/076fa3438ad4d2ad9de29ec57ac88afb1eeeb815/third_party/blink/renderer/core/frame/deprecation_report_body.idl
[modify] https://crrev.com/076fa3438ad4d2ad9de29ec57ac88afb1eeeb815/third_party/blink/renderer/core/frame/intervention_report_body.idl

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 24

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

commit 5dae36ddb2752c013b75cb63e260e090fd1aa161
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Tue Jul 24 16:37:21 2018

Use proper type for gc_info_index.

gc_info_index was widen to be a size_t in some areas. Whereas the
effective gc_index is restricted to be in the range 1 << 14. It is
more appropriate to use a uint32_t to store this value as opposed
to using a size_t.

BUG=596846

Change-Id: I3eac5fadabe82bbadeb315b3c3d8f1d4cf5a62a3
Reviewed-on: https://chromium-review.googlesource.com/1145232
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577577}
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/gc_info.cc
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/gc_info.h
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/gc_info_test.cc
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/heap.cc
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/heap.h
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/heap_allocator.h
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/heap_page.cc
[modify] https://crrev.com/5dae36ddb2752c013b75cb63e260e090fd1aa161/third_party/blink/renderer/platform/heap/heap_page.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 30

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

commit 6c71549160798f3689f785ca82203786963a7e08
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Mon Jul 30 20:39:00 2018

Adjust media APIs so they aren't using long or size_t data types.

The media long constraints work on the WebIDL definition of long which
is equivalent to int32_t.

The WebMediaPlayer interface would cause a narrowing of values on
32bit platforms which was unnecessary because WebMediaPlayerImpl keeps
its stats in 64 bit values.

BUG=596846

Change-Id: I687d1baf8fa5214d082adacce4e854b09c215819
Reviewed-on: https://chromium-review.googlesource.com/1145550
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579158}
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/content/renderer/media/stream/media_stream_constraints_util_sets_unittest.cc
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/content/renderer/media/stream/media_stream_constraints_util_video_device.cc
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/content/renderer/media/stream/webmediaplayer_ms.cc
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/content/renderer/media/stream/webmediaplayer_ms.h
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/content/renderer/media_capture_from_element/html_video_element_capturer_source_unittest.cc
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/third_party/blink/public/platform/web_media_constraints.h
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/third_party/blink/public/platform/web_media_player.h
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/third_party/blink/public/platform/web_media_stream_source.h
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/third_party/blink/public/platform/web_media_stream_track.h
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/third_party/blink/renderer/modules/mediastream/media_constraints_impl.cc
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/third_party/blink/renderer/platform/exported/web_media_constraints.cc
[modify] https://crrev.com/6c71549160798f3689f785ca82203786963a7e08/third_party/blink/renderer/platform/testing/empty_web_media_player.h

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 24

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

commit 4d19c351a30aadd714245a64ea4383285ffea116
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Fri Aug 24 16:12:36 2018

Fix interface definitions that were too wide for their IDL types.

Make sure the type on the interface matches the IDL type. All were
unsigned long in the case which maps to uint32_t.

BUG=596846

Change-Id: Iecec0190018d18735d7ffe0c0521af8c7f5514f8
Reviewed-on: https://chromium-review.googlesource.com/1188606
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585860}
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/clipboard/data_object.cc
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/clipboard/data_object.h
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/clipboard/data_transfer_item_list.cc
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/clipboard/data_transfer_item_list.h
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/dom/dom_string_list.cc
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/dom/dom_string_list.h
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/dom/named_node_map.cc
[modify] https://crrev.com/4d19c351a30aadd714245a64ea4383285ffea116/third_party/blink/renderer/core/dom/named_node_map.h

Sign in to add a comment