On 14-08-11 10:15 AM, Alexei Podtelezhnikov wrote:> On Fri, Aug 8, 2014 at 4:56 PM, Behdad Esfahbod <[email protected]> wrote: >> commit 177982e933ed6f2ab96163e14f4267f8abe89efd > > Seriously though, my commit does not change much, both FT_MSB and > ft_highpow2 used to return nonsense on zero input even before this > commit. I think I am going to apply some patches to protect against > reaching this code with zero argument, but I am not willing to change > FT_MSB itself.
Non-sense is not the same as undefined. FT_MSB used to return 0 for 0. Now it's undefined. See below. On 14-08-09 02:05 PM, Alexei Podtelezhnikov wrote:>> * src/pfr/pfrobjs.c, do you think count can be 0 here? > > FT_UInt power = 1 << FT_MSB( count ); > > I doubt very highly that undefined behavior can mean a number between > 0 and 31. Actually it can. Please take the time to understand what it means when something is undefined in C. > So we'll get power = 0 if count = 0, which is fine too. Even if count becomes, say, 100, the shift operation 1 << 100 is itself undefined in C. In fact, that produces non-zero on existing x86 CPUs. Here, I fixed exactly that in glib in 2006: https://bugzilla.gnome.org/show_bug.cgi?id=371631 > Every case is safe, or so it seems to me. You haven't reasoned them as being safe so far. Plus, I'd rather the macro API not change into undefined behavior because of an optimization that most probably is never ever measurable. On 14-08-09 01:50 PM, Alexei Podtelezhnikov wrote: > On Fri, Aug 8, 2014 at 4:56 PM, Behdad Esfahbod <[email protected]> wrote: >> >> * src/base/ftcalc.c (FT_MSB): Utilize gcc builtins. >> >> introduced undefined behavior, since __builtin_clz and __builtin_ctz have >> undefined behavior if passed in value is zero. >> >> This can actually happen with degenerate glyph outlines. It might be >> harmless, but it's something that should be fixed. > > There is just a handful of places where FT_MSB is used. > * ftoutln.c, FT_MSB is used for scaling and you cannot really > scale/shift 0 unsafely or undefinedly. You are missing the point. It's possible to end up calling FT_MSB(0) from this call-site, and that has become undefined. Please fix that. > * fttrigon.c, the same reasoning applies > * ftbbox.c, the same reasoning applies > * src/pfr/pfrobjs.c, do you think count can be 0 here? Didn't check these ones. -- behdad http://behdad.org/ _______________________________________________ Freetype-devel mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/freetype-devel
