On Thu, Mar 12, 2015 at 03:39:51PM +0100, Christophe Gisquet wrote: > Hi, > > 2015-03-12 14:37 GMT+01:00 Michael Niedermayer <michae...@gmx.at>: > >> > const int mx = h->mv_cache[list][scan8[n]][0] + src_x_offset * > >> > 8; > >> > int my = h->mv_cache[list][scan8[n]][1] + src_y_offset * > >> > 8; > >> > const int luma_xy = (mx & 3) + ((my & 3) << 2); > >> > - ptrdiff_t offset = ((mx >> 2) << pixel_shift) + (my >> 2) * > >> > h->mb_linesize; > >> > + ptrdiff_t offset = (mx >> 2) * (1 << pixel_shift) + (my >> 2) * > >> > h->mb_linesize; > >> > >> > >> Why is this undefined? > > > > Because C sucks > > I actually have an equivalent question to Ronald's. Is there a valid > input that causes the undefined behaviour? For an invalid input (e.g. > DoS tentative), is the behaviour worsened?
i belive any motion vector that points left outside the picture will trigger this one, its also happening with multiple fate samples This issue is about undefined behavor according to the C specification not about current gcc generating actually bad code from it AFAIK > > More in (uninformed) details: > > mx is probably already constrained by AVC specifications. Another > limit to its validness is mx being a bit more than 4 times as large as > the image width. In that case, what would the image width be to cause > this undefined behaviour? It looks to me like for anything vaguely > sensible, it would not fall within the undefined behaviour. > > For invalid inputs (where in particular the content of mv_cache was > not properly validated), let's say you get something that is larger > than the image. So what we get is a correctly computed value, yet > still invalid. I'm probably overlooking this here. > > I'd prefer some fuzzing to actually yield a crash scenario before > acting on such reports (which are not clear to me as actually causing > a degradation). Otherwise, some of those issues are mostly pedantic. > On the other hand, we are only loosing 3 cycles out of several > hundreds. the compiler should optimize the extra operation out, ive confirmed this before posting the patch in some cases but i didnt check all > > If we are going to overengineer such issues, we could have something like: > #if HAVE_FSANITIZED_SHIFT_PLEASURING > # define LEGAL_SHIFT(a, b, type) ( (a) * (((type)1) << (b) ) > #else > # define LEGAL_SHIFT(a, b, type) ( (a) << (b) ) > #endif that could be done it would make the code less readable though [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
