Skip to content

Commit 64a4e25

Browse files
victorgomesV8 LUCI CQ
victorgomes
authored and
V8 LUCI CQ
committed
[cleanup] Fix UB in BitField::kMax
It is UB to cast integers to enums outside of the valid range of the enum. Clang will turn `-Wenum-constexpr-conversion` into a hard error: llvm/llvm-project#59036 This CL changes BitField::kMax to be the unsigned representation integer type. And all checks "Enum::kA <= BitField::kMax" to "BitField::is_valid(Enum::kA)" Change-Id: I2b0094c72b16fdde3bb2a90901f83e5a6e7d8176 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5816333 Commit-Queue: Victor Gomes <victorgomes@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Auto-Submit: Victor Gomes <victorgomes@chromium.org> Cr-Commit-Position: refs/heads/main@{#95845}
1 parent fed8fb6 commit 64a4e25

23 files changed

Lines changed: 67 additions & 77 deletions

‎src/base/bit-field.h‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,21 +41,14 @@ class BitField final {
4141
static constexpr U kMask = ((U{1} << kShift) << kSize) - (U{1} << kShift);
4242
static constexpr int kLastUsedBit = kShift + kSize - 1;
4343
static constexpr U kNumValues = U{1} << kSize;
44-
45-
// Value for the field with all bits set.
46-
// If clang complains
47-
// "constexpr variable 'kMax' must be initialized by a constant expression"
48-
// on this line, then you're creating a BitField for an enum with more bits
49-
// than needed for the enum values. Either reduce the BitField size,
50-
// or give the enum an explicit underlying type.
51-
static constexpr T kMax = static_cast<T>(kNumValues - 1);
44+
static constexpr U kMax = kNumValues - 1;
5245

5346
template <class T2, int size2>
5447
using Next = BitField<T2, kShift + kSize, size2, U>;
5548

5649
// Tells whether the provided value fits into the bit field.
5750
static constexpr bool is_valid(T value) {
58-
return (static_cast<U>(value) & ~static_cast<U>(kMax)) == 0;
51+
return (static_cast<U>(value) & ~kMax) == 0;
5952
}
6053

6154
// Returns a type U with the bit field value encoded.

‎src/codegen/source-position.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,23 +99,19 @@ class SourcePosition final {
9999
}
100100
void SetExternalLine(int line) {
101101
DCHECK(IsExternal());
102-
DCHECK(line <= ExternalLineField::kMax - 1);
103102
value_ = ExternalLineField::update(value_, line);
104103
}
105104
void SetExternalFileId(int file_id) {
106105
DCHECK(IsExternal());
107-
DCHECK(file_id <= ExternalFileIdField::kMax - 1);
108106
value_ = ExternalFileIdField::update(value_, file_id);
109107
}
110108

111109
void SetScriptOffset(int script_offset) {
112110
DCHECK(IsJavaScript());
113-
DCHECK(script_offset <= ScriptOffsetField::kMax - 2);
114111
DCHECK_GE(script_offset, kNoSourcePosition);
115112
value_ = ScriptOffsetField::update(value_, script_offset + 1);
116113
}
117114
void SetInliningId(int inlining_id) {
118-
DCHECK(inlining_id <= InliningIdField::kMax - 2);
119115
DCHECK_GE(inlining_id, kNotInlined);
120116
value_ = InliningIdField::update(value_, inlining_id + 1);
121117
}

‎src/objects/code-inl.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ RELAXED_UINT32_ACCESSORS(Code, flags, kFlagsOffset)
771771

772772
void Code::initialize_flags(CodeKind kind, bool is_context_specialized,
773773
bool is_turbofanned, int stack_slots) {
774-
CHECK(0 <= stack_slots && stack_slots < StackSlotsField::kMax);
774+
CHECK(StackSlotsField::is_valid(stack_slots));
775775
DCHECK(!CodeKindIsInterpretedJSFunction(kind));
776776
uint32_t value = KindField::encode(kind) |
777777
IsContextSpecializedField::encode(is_context_specialized) |

‎src/objects/feedback-vector.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ class FeedbackVector
226226
NEVER_READ_ONLY_SPACE
227227
DEFINE_TORQUE_GENERATED_OSR_STATE()
228228
DEFINE_TORQUE_GENERATED_FEEDBACK_VECTOR_FLAGS()
229-
static_assert(TieringState::kLastTieringState <= TieringStateBits::kMax);
229+
static_assert(TieringStateBits::is_valid(TieringState::kLastTieringState));
230230

231231
static constexpr uint32_t kFlagsMaybeHasTurbofanCode =
232232
FeedbackVector::MaybeHasTurbofanCodeBit::kMask;
@@ -262,7 +262,7 @@ class FeedbackVector
262262
// the function becomes hotter. When the current loop depth is less than the
263263
// osr_urgency, JumpLoop calls into runtime to attempt OSR optimization.
264264
static constexpr int kMaxOsrUrgency = 6;
265-
static_assert(kMaxOsrUrgency <= OsrUrgencyBits::kMax);
265+
static_assert(OsrUrgencyBits::is_valid(kMaxOsrUrgency));
266266
inline int osr_urgency() const;
267267
inline void set_osr_urgency(int urgency);
268268
inline void reset_osr_urgency();

‎src/objects/js-date-time-format.h‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,23 @@ class JSDateTimeFormat
132132
// Bit positions in |flags|.
133133
DEFINE_TORQUE_GENERATED_JS_DATE_TIME_FORMAT_FLAGS()
134134

135-
static_assert(HourCycle::kUndefined <= HourCycleBits::kMax);
136-
static_assert(HourCycle::kH11 <= HourCycleBits::kMax);
137-
static_assert(HourCycle::kH12 <= HourCycleBits::kMax);
138-
static_assert(HourCycle::kH23 <= HourCycleBits::kMax);
139-
static_assert(HourCycle::kH24 <= HourCycleBits::kMax);
140-
141-
static_assert(DateTimeStyle::kUndefined <= DateStyleBits::kMax);
142-
static_assert(DateTimeStyle::kFull <= DateStyleBits::kMax);
143-
static_assert(DateTimeStyle::kLong <= DateStyleBits::kMax);
144-
static_assert(DateTimeStyle::kMedium <= DateStyleBits::kMax);
145-
static_assert(DateTimeStyle::kShort <= DateStyleBits::kMax);
146-
147-
static_assert(DateTimeStyle::kUndefined <= TimeStyleBits::kMax);
148-
static_assert(DateTimeStyle::kFull <= TimeStyleBits::kMax);
149-
static_assert(DateTimeStyle::kLong <= TimeStyleBits::kMax);
150-
static_assert(DateTimeStyle::kMedium <= TimeStyleBits::kMax);
151-
static_assert(DateTimeStyle::kShort <= TimeStyleBits::kMax);
135+
static_assert(HourCycleBits::is_valid(HourCycle::kUndefined));
136+
static_assert(HourCycleBits::is_valid(HourCycle::kH11));
137+
static_assert(HourCycleBits::is_valid(HourCycle::kH12));
138+
static_assert(HourCycleBits::is_valid(HourCycle::kH23));
139+
static_assert(HourCycleBits::is_valid(HourCycle::kH24));
140+
141+
static_assert(DateStyleBits::is_valid(DateTimeStyle::kUndefined));
142+
static_assert(DateStyleBits::is_valid(DateTimeStyle::kFull));
143+
static_assert(DateStyleBits::is_valid(DateTimeStyle::kLong));
144+
static_assert(DateStyleBits::is_valid(DateTimeStyle::kMedium));
145+
static_assert(DateStyleBits::is_valid(DateTimeStyle::kShort));
146+
147+
static_assert(TimeStyleBits::is_valid(DateTimeStyle::kUndefined));
148+
static_assert(TimeStyleBits::is_valid(DateTimeStyle::kFull));
149+
static_assert(TimeStyleBits::is_valid(DateTimeStyle::kLong));
150+
static_assert(TimeStyleBits::is_valid(DateTimeStyle::kMedium));
151+
static_assert(TimeStyleBits::is_valid(DateTimeStyle::kShort));
152152

153153
DECL_ACCESSORS(icu_locale, Tagged<Managed<icu::Locale>>)
154154
DECL_ACCESSORS(icu_simple_date_format, Tagged<Managed<icu::SimpleDateFormat>>)

‎src/objects/js-display-names-inl.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ ACCESSORS(JSDisplayNames, internal, Tagged<Managed<DisplayNamesInternal>>,
2525
TQ_OBJECT_CONSTRUCTORS_IMPL(JSDisplayNames)
2626

2727
inline void JSDisplayNames::set_style(Style style) {
28-
DCHECK_GE(StyleBits::kMax, style);
28+
DCHECK(StyleBits::is_valid(style));
2929
set_flags(StyleBits::update(flags(), style));
3030
}
3131

@@ -34,7 +34,7 @@ inline JSDisplayNames::Style JSDisplayNames::style() const {
3434
}
3535

3636
inline void JSDisplayNames::set_fallback(Fallback fallback) {
37-
DCHECK_GE(FallbackBit::kMax, fallback);
37+
DCHECK(FallbackBit::is_valid(fallback));
3838
set_flags(FallbackBit::update(flags(), fallback));
3939
}
4040

@@ -44,7 +44,7 @@ inline JSDisplayNames::Fallback JSDisplayNames::fallback() const {
4444

4545
inline void JSDisplayNames::set_language_display(
4646
LanguageDisplay language_display) {
47-
DCHECK_GE(LanguageDisplayBit::kMax, language_display);
47+
DCHECK(LanguageDisplayBit::is_valid(language_display));
4848
set_flags(LanguageDisplayBit::update(flags(), language_display));
4949
}
5050

‎src/objects/js-display-names.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ class JSDisplayNames
8181
// Bit positions in |flags|.
8282
DEFINE_TORQUE_GENERATED_JS_DISPLAY_NAMES_FLAGS()
8383

84-
static_assert(Style::kLong <= StyleBits::kMax);
85-
static_assert(Style::kShort <= StyleBits::kMax);
86-
static_assert(Style::kNarrow <= StyleBits::kMax);
87-
static_assert(Fallback::kCode <= FallbackBit::kMax);
88-
static_assert(Fallback::kNone <= FallbackBit::kMax);
89-
static_assert(LanguageDisplay::kDialect <= LanguageDisplayBit::kMax);
90-
static_assert(LanguageDisplay::kStandard <= LanguageDisplayBit::kMax);
84+
static_assert(StyleBits::is_valid(Style::kLong));
85+
static_assert(StyleBits::is_valid(Style::kShort));
86+
static_assert(StyleBits::is_valid(Style::kNarrow));
87+
static_assert(FallbackBit::is_valid(Fallback::kCode));
88+
static_assert(FallbackBit::is_valid(Fallback::kNone));
89+
static_assert(LanguageDisplayBit::is_valid(LanguageDisplay::kDialect));
90+
static_assert(LanguageDisplayBit::is_valid(LanguageDisplay::kStandard));
9191

9292
DECL_ACCESSORS(internal, Tagged<Managed<DisplayNamesInternal>>)
9393

‎src/objects/js-duration-format-inl.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ ACCESSORS(JSDurationFormat, icu_locale, Tagged<Managed<icu::Locale>>,
2727

2828
#define IMPL_INLINE_SETTER_GETTER(T, n, B, f, M) \
2929
inline void JSDurationFormat::set_##n(T value) { \
30-
DCHECK_GE(B::kMax, value); \
30+
DCHECK(B::is_valid(value)); \
3131
DCHECK_GE(T::M, value); \
3232
set_##f(B::update(f(), value)); \
3333
} \

‎src/objects/js-list-format-inl.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ ACCESSORS(JSListFormat, icu_formatter, Tagged<Managed<icu::ListFormatter>>,
2727
kIcuFormatterOffset)
2828

2929
inline void JSListFormat::set_style(Style style) {
30-
DCHECK_GE(StyleBits::kMax, style);
30+
DCHECK(StyleBits::is_valid(style));
3131
int hints = flags();
3232
hints = StyleBits::update(hints, style);
3333
set_flags(hints);
@@ -38,7 +38,7 @@ inline JSListFormat::Style JSListFormat::style() const {
3838
}
3939

4040
inline void JSListFormat::set_type(Type type) {
41-
DCHECK_GE(TypeBits::kMax, type);
41+
DCHECK(TypeBits::is_valid(type));
4242
int hints = flags();
4343
hints = TypeBits::update(hints, type);
4444
set_flags(hints);

‎src/objects/js-list-format.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ class JSListFormat
8686
// Bit positions in |flags|.
8787
DEFINE_TORQUE_GENERATED_JS_LIST_FORMAT_FLAGS()
8888

89-
static_assert(Style::LONG <= StyleBits::kMax);
90-
static_assert(Style::SHORT <= StyleBits::kMax);
91-
static_assert(Style::NARROW <= StyleBits::kMax);
92-
static_assert(Type::CONJUNCTION <= TypeBits::kMax);
93-
static_assert(Type::DISJUNCTION <= TypeBits::kMax);
94-
static_assert(Type::UNIT <= TypeBits::kMax);
89+
static_assert(StyleBits::is_valid(Style::LONG));
90+
static_assert(StyleBits::is_valid(Style::SHORT));
91+
static_assert(StyleBits::is_valid(Style::NARROW));
92+
static_assert(TypeBits::is_valid(Type::CONJUNCTION));
93+
static_assert(TypeBits::is_valid(Type::DISJUNCTION));
94+
static_assert(TypeBits::is_valid(Type::UNIT));
9595

9696
DECL_PRINTER(JSListFormat)
9797

0 commit comments

Comments
 (0)