New issue
Advanced search Search tips

Issue 701384 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Fix instances of -Wbitfield-enum-conversions in Chromium

Project Member Reported by r...@chromium.org, Mar 14 2017

Issue description

I recently prototyped a new warning, and it finds bugs in Chromium. Here's the first one I found, it looks like a true positive:

FAILED: obj/third_party/WebKit/Source/core/layout/layout/ng_constraint_space_builder.obj
../../../../llvm/build_stage2/bin/clang-cl.exe /nologo /showIncludes /FC @obj/third_party/WebKit/Source/core/layout/layout/ng_constraint_space_builder.obj.rsp /c ../../third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc /Foobj/third_party/WebKit/Source/core/layout/layout/ng_constraint_space_builder.obj /Fd"obj/third_party/WebKit/Source/core/layout/layout_cc.pdb"
C:\src\chromium\src\third_party\WebKit\Source\core\layout\ng\ng_constraint_space_builder.cc(18,7):  error: bit-field 'parent_writing_mode_' is not wide enough to store all enumerators of enum type 'NGWritingMode' [-Werror,-Wbitfield-enum-conversion]
      parent_writing_mode_(parent_space->WritingMode()),
      ^
C:\src\chromium\src\third_party\WebKit\Source\core\layout\ng\ng_constraint_space_builder.cc(34,7):  error: bit-field 'parent_writing_mode_' is not wide enough to store all enumerators of enum type 'NGWritingMode' [-Werror,-Wbitfield-enum-conversion]
      parent_writing_mode_(writing_mode),
      ^
2 errors generated.

The bitfield is two bits wide, which fails to represent the last enumerator:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h?rcl=96ff8f1f6f00e57db01f58b3bbc4ee0fe3e90ee7&l=75
  unsigned parent_writing_mode_ : 2;

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/ng/ng_writing_mode.h?q=NGWritingMode+package:%5Echromium$&dr=CSs&l=18

enum NGWritingMode {
  kHorizontalTopBottom = 0,
  kVerticalRightLeft = 1,
  kVerticalLeftRight = 2,
  kSidewaysRightLeft = 3,
  kSidewaysLeftRight = 4
};

 

Comment 1 by r...@chromium.org, Mar 14 2017

This instance is possibly a false positive:

FAILED: obj/third_party/WebKit/Source/core/svg/svg/SVGAnimatedProperty.obj
../../../../llvm/build_stage2/bin/clang-cl.exe /nologo /showIncludes /FC @obj/third_party/WebKit/Source/core/svg/svg/SVGAnimatedProperty.obj.rsp /c ../../third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.cpp /Foobj/third_party/WebKit/Source/core/svg/svg/SVGAnimatedProperty.obj /Fd"obj/third_party/WebKit/Source/core/svg/svg_cc.pdb"
C:\src\chromium\src\third_party\WebKit\Source\core\svg\properties\SVGAnimatedProperty.cpp(43,7):  error: bit-field 'm_cssPropertyId' is not wide enough to store all enumerators of enum type 'CSSPropertyID' [-Werror,-Wbitfield-enum-conversion]
      m_cssPropertyId(cssPropertyId),
      ^
1 error generated.

The CSSPropertyID enum has many high alias enumerators which are presumably invalid at this point, and the bitfield is only intended to store the low half of the enumerators. However, there is no assertion or DCHECK to guard against truncation.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 14 2017

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

commit 782364b2af9f1c3c7f0a39d5dd9fa1df34cdfb09
Author: rnk <rnk@chromium.org>
Date: Tue Mar 14 21:49:09 2017

Fix instances of clang -Wbitfield-enum-conversion in Blink

Widen parent_writing_mode_ to fit all enumerator values. This looks like
a real bug.

Add static_cast<unsigned> when assigning m_cssPropertyId. This
suppresses the warning. It looks like enumerators larger than
lastCXXProperty should never appear here.

R=pdr@chromium.org,thakis@chromium.org
BUG=701384

Review-Url: https://codereview.chromium.org/2747193002
Cr-Commit-Position: refs/heads/master@{#456848}

[modify] https://crrev.com/782364b2af9f1c3c7f0a39d5dd9fa1df34cdfb09/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h
[modify] https://crrev.com/782364b2af9f1c3c7f0a39d5dd9fa1df34cdfb09/third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.cpp

Status: Assigned (was: Untriaged)

Sign in to add a comment