[FFmpeg-devel] [PATCH 1/5] lavu/common.h: Fix UB in av_clipl_int32_c()

[Tomas Härdin]

tor 2024-05-30 klockan 17:28 +0300 skrev Rémi Denis-Courmont:
> 
> 
> Le 30 mai 2024 17:07:21 GMT+03:00, "Tomas Härdin" <git at haerdin.se> a
> écrit :
> > > We should depend on punning as long as it conforms to the
> > > standard.
> > 
> > My mistake, I forgot type punning is allowed in C. It's UB in C++
> > 
> > > > The standard compliant way
> > > > is to use memcpy()
> > > 
> > > That's way worse than union in terms of how proactively the
> > > compiler
> > > needs to optimise, and both approaches are as confirming.
> > 
> > A good compiler will do the same thing
> 
> True, and I don't care very much about memcpy vs union, as they both
> rely on matching representation. AFAIR, FFmpeg tends to use unions
> though.
> 
> > 
> > Maybe I can get the riscv version covered by Eva as well. That's
> > beyond
> > the scope of this patchset
> 
> IMHO, this specific patch (and the following one) are beating dead
> horses. Sure there may be theoretical UB in the current code, but if
> there is a *better* implementation, better switch to that than bike
> shedding the fix for the UB.

Are you saying that UB is acceptable? You know the compiler is free to
assume signed arithmetic doesn't overflow, right? If so then what other
UB might we accept?

/Tomas