Skip to content

Commit

Permalink
Fix lossy bitmask computation in OptimizationGuide UKM
Browse files Browse the repository at this point in the history
This was caught by UBSan but also just computed the wrong value.
1 << ... is computing the shift as an int, which is 32-bit, but these
values exceed 31. Not only is thus UB, but even if the compiler doesn't
cause mishaps, we'd lose any values 32 and higher.

Bug: 1394755
Change-Id: I3f39f580e09b3f7783996854ac299cd811cd95d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4986623
Auto-Submit: David Benjamin <davidben@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216617}
  • Loading branch information
davidben authored and Chromium LUCI CQ committed Oct 28, 2023
1 parent d278651 commit a5e0288
Showing 1 changed file with 2 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ void OptimizationGuideNavigationData::RecordOptimizationGuideUKM() const {
if (!registered_optimization_types_.empty()) {
int64_t types_bitmask = 0;
for (const auto& optimization_type : registered_optimization_types_) {
types_bitmask |= (1 << static_cast<int>(optimization_type));
types_bitmask |= (int64_t{1} << static_cast<int>(optimization_type));
}
builder.SetRegisteredOptimizationTypes(types_bitmask);
did_record_metric = true;
}
if (!registered_optimization_targets_.empty()) {
int64_t targets_bitmask = 0;
for (const auto& optimization_target : registered_optimization_targets_) {
targets_bitmask |= (1 << static_cast<int>(optimization_target));
targets_bitmask |= (int64_t{1} << static_cast<int>(optimization_target));
}
builder.SetRegisteredOptimizationTargets(targets_bitmask);
did_record_metric = true;
Expand Down

0 comments on commit a5e0288

Please sign in to comment.