Commit 8ea17a0
committed
menu/ozone: clamp sidebar ticker field_width, root-cause UBSan trips
UBSan-instrumented run reported, on the same arithmetic at two
sites in ozone_draw_sidebar():
menu/drivers/ozone.c:3658:41: runtime error: -20.4167 is
outside the range of representable values of type 'unsigned int'
menu/drivers/ozone.c:3806:41: runtime error: -20.4167 is
outside the range of representable values of type 'unsigned int'
Two pre-existing TODO/FIXME comments in this file already noted
the same trip (with a -12.549 sample) but punted on the fix.
Same situation also lurks in the sibling non-smooth-ticker
branches, which UBSan didn't happen to catch only because
ticker.len is size_t rather than unsigned and the smooth path is
the default.
Where the float comes from
--------------------------
ozone->dimensions_sidebar_width is a *float* "animated field"
(declared so at line 578) that tweens between 0 / collapsed /
normal sidebar widths via the gfx_animation system. It's
initialised to 0.0f at startup (line 9324), so on the very first
frame after init -- and during every collapse/expand animation
-- it holds a small or fractional value.
Through line 3568,
entry_width = (unsigned)dimensions_sidebar_width
- sidebar_padding_horizontal * 2;
so entry_width is integer but tracks the float. Then both
ticker computations evaluate
entry_width - sidebar_entry_icon_size - 40 * scale_factor
scale_factor is float (last_scale_factor at line 580), so
40 * scale_factor is float, so the whole sum is float. When
entry_width is small (mid-animation, or sidebar collapsed) the
sum lands below zero -- UBSan saw -20.4167 (40 * 1.5 - the
~icon_size offset against an entry_width near zero).
The implicit float-to-unsigned (or float-to-size_t) conversion
of a negative is UB per C11 6.3.1.4 p1. Modern compilers wrap
to ~UINT_MAX/SIZE_MAX, which the ticker then treats as "plenty
of room", drawing too much text for one or two animation frames
until dimensions_sidebar_width settles. Visually mostly
invisible; UBSan-fatally noisy.
Fix
---
Compute the available width in signed int, with the float
intruder explicitly cast through int
avail_width = entry_width - icon_size - (int)(40.0f * scale_factor);
and clamp to zero before assignment to the unsigned/size_t
field:
ticker_field_width = avail_width > 0 ? (unsigned)avail_width : 0;
This produces a defined zero-width when there's no room (the
correct semantic -- the ticker has nothing to draw and skips
rather than scribbling at a wrap-around offset). Apply
identically at both sites (system tab loop and horizontal-list
loop) and use the same local in both the smooth and non-smooth
branches, which removes the duplicated arithmetic and applies
the same clamp to ticker.len's previously-buggy non-smooth
branch. The two TODO/FIXME comments are removed since they're
now fixed rather than known-broken.
Also adds a glyph_width != 0 guard around the non-smooth divide.
glyph_width is populated during font init and is normally non-
zero by the time draw_sidebar runs, but a font that failed to
load would leave it at 0 and the existing code would divide by
zero -- belt-and-suspenders since we're touching the line
anyway.
Address-of-root-cause vs whack-a-mole
-------------------------------------
The deeper root cause is dimensions_sidebar_width being a float
that flows into integer pixel math, rather than these specific
sites. Normalising the animated width to an int field would
require restructuring how the gfx_animation tween writes back
(its subject is a float* by API), so it's not a drive-by fix.
For now, every site that mixes scale_factor into integer pixel
arithmetic is a candidate UBSan trip; this commit handles the
two reported plus their immediate non-smooth counterparts.
Future trips will need similar local clamps.1 parent b3618d6 commit 8ea17a0
1 file changed
Lines changed: 52 additions & 34 deletions
File tree
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3643 | 3643 | | |
3644 | 3644 | | |
3645 | 3645 | | |
| 3646 | + | |
| 3647 | + | |
| 3648 | + | |
| 3649 | + | |
| 3650 | + | |
| 3651 | + | |
| 3652 | + | |
| 3653 | + | |
| 3654 | + | |
| 3655 | + | |
| 3656 | + | |
| 3657 | + | |
| 3658 | + | |
| 3659 | + | |
| 3660 | + | |
| 3661 | + | |
| 3662 | + | |
| 3663 | + | |
3646 | 3664 | | |
3647 | 3665 | | |
3648 | 3666 | | |
| |||
3651 | 3669 | | |
3652 | 3670 | | |
3653 | 3671 | | |
3654 | | - | |
3655 | | - | |
3656 | | - | |
3657 | | - | |
3658 | | - | |
3659 | | - | |
3660 | | - | |
| 3672 | + | |
3661 | 3673 | | |
3662 | 3674 | | |
3663 | 3675 | | |
| |||
3666 | 3678 | | |
3667 | 3679 | | |
3668 | 3680 | | |
3669 | | - | |
3670 | | - | |
3671 | | - | |
3672 | | - | |
| 3681 | + | |
| 3682 | + | |
| 3683 | + | |
3673 | 3684 | | |
3674 | 3685 | | |
3675 | 3686 | | |
| |||
3796 | 3807 | | |
3797 | 3808 | | |
3798 | 3809 | | |
3799 | | - | |
| 3810 | + | |
| 3811 | + | |
| 3812 | + | |
| 3813 | + | |
| 3814 | + | |
| 3815 | + | |
3800 | 3816 | | |
3801 | | - | |
3802 | | - | |
3803 | | - | |
3804 | | - | |
3805 | | - | |
3806 | | - | |
| 3817 | + | |
3807 | 3818 | | |
3808 | | - | |
3809 | | - | |
3810 | | - | |
3811 | | - | |
| 3819 | + | |
| 3820 | + | |
| 3821 | + | |
3812 | 3822 | | |
3813 | | - | |
3814 | | - | |
3815 | | - | |
3816 | | - | |
3817 | | - | |
3818 | | - | |
3819 | | - | |
3820 | | - | |
3821 | | - | |
3822 | | - | |
3823 | | - | |
| 3823 | + | |
| 3824 | + | |
| 3825 | + | |
| 3826 | + | |
| 3827 | + | |
| 3828 | + | |
| 3829 | + | |
3824 | 3830 | | |
3825 | | - | |
| 3831 | + | |
| 3832 | + | |
| 3833 | + | |
| 3834 | + | |
| 3835 | + | |
| 3836 | + | |
| 3837 | + | |
| 3838 | + | |
| 3839 | + | |
| 3840 | + | |
| 3841 | + | |
| 3842 | + | |
| 3843 | + | |
3826 | 3844 | | |
3827 | 3845 | | |
3828 | 3846 | | |
| |||
0 commit comments