Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/5] lavu/common.h: Fix UB in av_clipl_int32_c()
Date: Thu, 30 May 2024 21:07:13 +0200
Message-ID: <20240530190713.GB2821752@pb2> (raw)
In-Reply-To: <3421890.a5N6fE6ukZ@basile.remlab.net>


[-- Attachment #1.1: Type: text/plain, Size: 1773 bytes --]

On Thu, May 30, 2024 at 08:49:12PM +0300, Rémi Denis-Courmont wrote:
> Le torstaina 30. toukokuuta 2024, 19.48.13 EEST Tomas Härdin a écrit :
> > > > 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?
> > > 
> > > He did not say that... He said we should switch to a better
> > > implementation rather than trying to fix the existing potentially
> > > buggy one.
> > 
> > I have a fix for demonstrable UB and Rémi is problematizing it.
> 
> Andreas made cosmetic arguments against this patch before I had even seen the 
> patch, forget comment on it.
> 

> > It is not a "theoretical" UB - that's not how UB works.
> 
> It is a *theoretical* UB if you can not prove that it leads to misbehaviour in 

If the function doesnt get called with values triggering UB then its not UB.

If the function gets called with values triggering the signed overflow then its UB
And its a bug unless the applications intended behavior is undefined.

also i would not bet on that the function produces the correct output for
input values that trigger UB on every platform

The case where this really could be a problem is if its used with compile
time constants that would trigger the overflow because in these cases
the optimizer can assume the whole codepath leading to it can be
removed.

IMHO we should simply fix UB instead of arguing over how bad it could be
or when.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2024-05-30 19:08 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-29 22:13 Tomas Härdin
2024-05-29 22:13 ` [FFmpeg-devel] [PATCH 2/5] lavu/common.h: Fix UB in av_clip_intp2_c() Tomas Härdin
2024-05-29 22:24   ` Andreas Rheinhardt
2024-05-29 23:08     ` Tomas Härdin
2024-05-29 22:14 ` [FFmpeg-devel] [PATCH 3/5] lavu/common.h: Fix UB in av_clip_uintp2_c() Tomas Härdin
2024-05-31  0:27   ` Michael Niedermayer
2024-05-29 22:14 ` [FFmpeg-devel] [PATCH 4/5] lavu/intmath.h: Fix UB in ff_ctz_c() and ff_ctzll_c() Tomas Härdin
2024-05-30  7:54   ` Rémi Denis-Courmont
2024-05-30  9:50     ` Tomas Härdin
2024-05-30 12:29       ` Hendrik Leppkes
2024-05-30 13:06       ` Rémi Denis-Courmont
2024-05-30 14:03         ` Tomas Härdin
2024-05-30 14:32           ` Rémi Denis-Courmont
2024-05-31  0:43           ` Ronald S. Bultje
2024-05-31  0:41     ` Michael Niedermayer
2024-05-31  5:48       ` Rémi Denis-Courmont
2024-05-31  0:31   ` Michael Niedermayer
2024-05-29 22:15 ` [FFmpeg-devel] [PATCH 5/5] lavu/mathematics: Return early if either a or b is zero Tomas Härdin
2024-05-31  0:22   ` Michael Niedermayer
2024-05-31 15:21     ` Tomas Härdin
2024-05-29 22:31 ` [FFmpeg-devel] [PATCH 1/5] lavu/common.h: Fix UB in av_clipl_int32_c() Andreas Rheinhardt
2024-05-30  9:48   ` Tomas Härdin
2024-05-30  6:41 ` Rémi Denis-Courmont
2024-05-30  9:40   ` Tomas Härdin
2024-05-30 11:50     ` Rémi Denis-Courmont
2024-05-30 14:07       ` Tomas Härdin
2024-05-30 14:28         ` Rémi Denis-Courmont
2024-05-30 15:32           ` Tomas Härdin
2024-05-30 15:38             ` Rémi Denis-Courmont
2024-05-31  1:03               ` Michael Niedermayer
2024-06-03  7:32                 ` Rémi Denis-Courmont
2024-06-03 21:21                   ` Michael Niedermayer
2024-05-30 15:42             ` James Almer
2024-05-30 16:48               ` Tomas Härdin
2024-05-30 17:49                 ` Rémi Denis-Courmont
2024-05-30 19:07                   ` Michael Niedermayer [this message]
2024-05-30 19:20                     ` Rémi Denis-Courmont
2024-05-31 15:23                   ` Tomas Härdin
2024-06-14 12:31 ` Tomas Härdin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240530190713.GB2821752@pb2 \
    --to=michael@niedermayer.cc \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git