[FFmpeg-devel] [PATCH 2/2] configure: Use -fno-sanitize-recover
James Almer
jamrial at gmail.com
Sat Jan 18 01:31:04 EET 2025
On 1/17/2025 8:27 PM, Vitaly Buka via ffmpeg-devel wrote:
> On Fri, Jan 17, 2025 at 3:12 PM James Almer <jamrial at gmail.com> wrote:
>
>> On 1/17/2025 7:53 PM, Vitaly Buka via ffmpeg-devel wrote:
>>> My confusion here is that it looks like ffmpeg developers care about UB,
>> I
>>> see from time to time large cleanups, but there are a bunch of unfixed
>>> reports.
>>> Maybe forcing no-recover by default will improve this situation?
>>
>> It will not change much because the FATE clients currently running with
>> gcc ubsan both manually add this extra option, but with undefined as
>> argument rather than all. See
>>
>> https://fate.ffmpeg.org/report.cgi?time=20250117151351&slot=x86_64-archlinux-gcc-ubsan
>>
>> Like i said, I'm not against adding this extra option, but I'm against
>> adding all the exceptions scattered around the code.
>>
>>
> Sorry. Looks like I fixed my email delivery after your reply.
> I can see it in the archive now:
>
>> Adding this is probably fine, but all the exceptions below to ignore
> issues are not ok.
>
> Does this mean we need to fix those issues first?
Ideally.
> Is it acceptable to make FATE stop passing with UBSAN?
As you can see, there are three currently failing tests if you enable
"-fno-sanitize-recover=undefined", so it's not technically passing as is
either.
>
> Most of them are relatively straight forward, but I failed to quick-guess
> some, like "bounds" one.
Patches for any of them are welcome.
>
>
>
>>>
>>>
>>> On Fri, Jan 17, 2025 at 11:57 AM Frank Plowman <post at frankplowman.com>
>>> wrote:
>>>
>>>> On 16/01/2025 19:12, Vitaly Buka via ffmpeg-devel wrote:
>>>>> UBSAN by default is just prints a mesage and
>>>>> moves on. This hides a few UBs in fate-suite.
>>>>>
>>>>> Signed-off-by: Vitaly Buka <vitalybuka at google.com>
>>>>> ---
>>>>> configure | 4 ++--
>>>>> libavcodec/aacenc_pred.c | 1 +
>>>>> libavcodec/ffv1dec.c | 1 +
>>>>> libavcodec/ffv1enc_template.c | 1 +
>>>>> libavcodec/get_bits.h | 1 +
>>>>> libavcodec/indeo3.c | 2 +-
>>>>> libavcodec/motion_est.c | 1 +
>>>>> libavcodec/mss2dsp.c | 1 +
>>>>> libavcodec/opus/dec.c | 1 +
>>>>> libavcodec/snow.h | 1 +
>>>>> libavcodec/svq1enc.c | 1 +
>>>>> libavfilter/vf_curves.c | 1 +
>>>>> libavfilter/vf_overlay.c | 1 +
>>>>> libavformat/mov.c | 1 +
>>>>> libswscale/input.c | 6 ++++++
>>>>> libswscale/output.c | 4 ++++
>>>>> libswscale/swscale_unscaled.c | 3 +++
>>>>> 17 files changed, 28 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index 3a1e72e1c6..f2b4fd2c62 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -4568,7 +4568,7 @@ set >> $logfile
>>>>> test -n "$valgrind" && toolchain="valgrind-memcheck"
>>>>>
>>>>> enabled ossfuzz && ! echo $CFLAGS | grep -q -- "-fsanitize=" && !
>> echo
>>>> $CFLAGS | grep -q -- "-fcoverage-mapping" &&{
>>>>> - add_cflags -fsanitize=address,undefined
>>>> -fsanitize-coverage=trace-pc-guard,trace-cmp -fno-omit-frame-pointer
>>>>> + add_cflags -fsanitize=address,undefined
>>>> -fsanitize-coverage=trace-pc-guard,trace-cmp -fno-omit-frame-pointer
>>>> -fno-sanitize-recover=all
>>>>> add_ldflags -fsanitize=address,undefined
>>>> -fsanitize-coverage=trace-pc-guard,trace-cmp
>>>>> }
>>>>>
>>>>> @@ -4591,7 +4591,7 @@ add_sanitizer_flags(){
>>>>> add_ldflags -fsanitize=thread
>>>>> ;;
>>>>> usan)
>>>>> - add_cflags -fsanitize=undefined
>>>>> + add_cflags -fsanitize=undefined -fno-sanitize-recover=all
>>>>
>>>> I agree it would be good to return a nonzero exit code on detecting
>>>> undefined behaviour when running FATE, but this sets the flag for any
>>>> --toolchain=*-usan configuration. Personally, I would find it a little
>>>> unexpected that compiling with --toolchain=*-usan results in anything
>>>> but the default behaviour of UBSAN, and one might wish to use UBSAN
>>>> without the flag when testing manually. As an alternative, what about
>>>> instead setting UBSAN_OPTIONS=halt_on_error=1 only when running the FATE
>>>> suite or fuzzing?
>>>>
>>>> --
>>>> Frank
>>>>
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel at ffmpeg.org
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250117/f8e4afe4/attachment.sig>
More information about the ffmpeg-devel
mailing list