Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#3540 closed defect (fixed)

Compilation of libavcodec/dirac_arith.h:dirac_get_arith_bit fails for five registers if cmov is fast

Reported by: alucowie Owned by:
Priority: normal Component: avcodec
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: no

Description

Summary of the bug:
The code in function libavcodec/dirac_arith.h:dirac_get_arith_bit needs 6 registers and must be protected by a macro HAVE_6REGS. If we enable PIC (ebx is used by PIC) and check-stack or no-omit-frame-pointer (which use ebp) then we haven't 6 registers but 5 on x86 then the compilation failed as :
In file included from /var/tmp/portage/media-video/ffmpeg-1.2.6/work/ffmpeg-1.2.6/libavcodec/diracdec.c:35:0:
/var/tmp/portage/media-video/ffmpeg-1.2.6/work/ffmpeg-1.2.6/libavcodec/dirac_arith.h: In function ‘dirac_get_arith_bit’:
/var/tmp/portage/media-video/ffmpeg-1.2.6/work/ffmpeg-1.2.6/libavcodec/dirac_arith.h:141:5: error: ‘asm’ operand has impossible constraints
make: * [libavcodec/diracdec.o] Error 1

How to reproduce:

i686-pc-linux-gnu-gcc -I. -I../ffmpeg-1.2.6/ -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -DPIC -DHAVE_AV_CONFIG_H -std=c99 -fPIC -pthread   -I/usr/include/freetype2 -D_REENTRANT -D_GNU_SOURCE=1 -D_REENTRANT -I/usr/include/SDL -pie -fstack-check -MMD -MF libavcodec/diracdec.d -MT libavcodec/diracdec.o -c -o libavcodec/diracdec.o ../ffmpeg-1.2.6/libavcodec/diracdec.c

I test with 1.2.6 but all version since are affected.

Attachments (1)

ffmpeg-1.2.6-dirac_arith_h_6regs.patch (712 bytes) - added by alucowie 5 years ago.
Patch to fix the macro condition

Download all attachments as: .zip

Change History (14)

Changed 5 years ago by alucowie

Patch to fix the macro condition

comment:1 Changed 5 years ago by cehoyos

What is the configure line to reproduce this problem?
(I have already tried this several times but compilation always succeeds here.)

comment:2 Changed 5 years ago by cehoyos

Afaict, the patch is not correct:
I use the following configure line:

$ configure --cc='cc -m32' --disable-optimizations --enable-pic

Now neither EBP nor EBX are available:

$ grep HAVE_EB config.h
#define HAVE_EBP_AVAILABLE 0
#define HAVE_EBX_AVAILABLE 0

This means that HAVE_6REGS is false:

$ grep HAVE_6REGS libavutil/x86/asm.h
#define HAVE_6REGS (ARCH_X86_64 || (HAVE_EBX_AVAILABLE || HAVE_EBP_AVAILABLE))

But compilation of libavcodec/diracdec.o still succeeds:

$ make V=1 libavcodec/diracdec.o
cc -m32 -I. -I./ -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -DPIC -DHAVE_AV_CONFIG_H -std=c99 -fPIC -pthread -D_GNU_SOURCE=1 -D_REENTRANT -I/usr/include/SDL -g -Wdeclaration-after-statement -Wall -Wdisabled-optimization -Wpointer-arith -Wredundant-decls -Wwrite-strings -Wtype-limits -Wundef -Wmissing-prototypes -Wno-pointer-to-int-cast -Wstrict-prototypes -Wempty-body -Wno-parentheses -Wno-switch -Wno-format-zero-length -Wno-pointer-sign  -fno-math-errno -fno-signed-zeros -fno-tree-vectorize -Werror=implicit-function-declaration -Werror=missing-prototypes -Werror=return-type -Werror=vla -Wno-maybe-uninitialized  -MMD -MF libavcodec/diracdec.d -MT libavcodec/diracdec.o -c -o libavcodec/diracdec.o libavcodec/diracdec.c
libavcodec/diracdec.c: In function ‘dirac_decode_picture_header’:
libavcodec/diracdec.c:1716:5: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]
libavcodec/diracdec.c:1722:17: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]
libavcodec/diracdec.c:1730:13: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]
libavcodec/diracdec.c: In function ‘get_delayed_pic’:
libavcodec/diracdec.c:1764:9: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]
libavcodec/diracdec.c: In function ‘dirac_decode_data_unit’:
libavcodec/diracdec.c:1846:9: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]
libavcodec/diracdec.c: In function ‘dirac_decode_frame’:
libavcodec/diracdec.c:1879:9: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]
libavcodec/diracdec.c:1928:9: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]
libavcodec/diracdec.c:1945:13: warning: ‘reference’ is deprecated (declared at ./libavutil/frame.h:253) [-Wdeprecated-declarations]

Tested with current FFmpeg git head and vanilla gcc 4.4.6, vanilla gcc 4.7.2 and vanilla gcc 4.8.2 (and gcc 2.95.3).

comment:3 Changed 5 years ago by cehoyos

  • Component changed from avutil to undetermined

comment:4 Changed 5 years ago by alucowie

I use the following configure line:

$ configure --enable-shared --cc=i686-pc-linux-gnu-gcc --cxx=i686-pc-linux-gnu-g++ --ar=i686-pc-linux-gnu-ar --optflags='-O2 -march=amdfam10 -pipe -fstack-check' --enable-pic

If you disassemble your .o file

$ objdump -D libavcodec/diracdec.o  | grep -2 setae

What do you see ? What registers are used on the 4 last lines ?

comment:5 Changed 5 years ago by cehoyos

I see the following without inlining:

     940:       39 c2                   cmp    %eax,%edx
     942:       0f 93 c0                setae  %al
     945:       0f b6 c0                movzbl %al,%eax
     948:       89 45 e4                mov    %eax,-0x1c(%ebp)

And with inlining:

    374a:       39 d0                   cmp    %edx,%eax
    374c:       0f 93 c0                setae  %al
    374f:       0f b6 c0                movzbl %al,%eax
    3752:       89 45 94                mov    %eax,-0x6c(%ebp)
    38fa:       39 c2                   cmp    %eax,%edx
    38fc:       0f 93 c2                setae  %dl
    38ff:       0f b6 d2                movzbl %dl,%edx
    3902:       89 55 88                mov    %edx,-0x78(%ebp)
    3d0f:       39 c2                   cmp    %eax,%edx
    3d11:       0f 93 c2                setae  %dl
    3d14:       0f b6 d2                movzbl %dl,%edx
    3d17:       89 55 90                mov    %edx,-0x70(%ebp)
    3dfd:       39 d0                   cmp    %edx,%eax
    3dff:       0f 93 c0                setae  %al
    3e02:       0f b6 c0                movzbl %al,%eax
    3e05:       89 45 90                mov    %eax,-0x70(%ebp)
    3ee6:       39 d0                   cmp    %edx,%eax
    3ee8:       0f 93 c0                setae  %al
    3eeb:       0f b6 c0                movzbl %al,%eax
    3eee:       89 45 8c                mov    %eax,-0x74(%ebp)

...

comment:6 Changed 5 years ago by cehoyos

Why are you using -O2?
Is it faster? Then please send a patch to make it the default.

comment:7 Changed 5 years ago by alucowie

Then the macro condition is false (either HAVE_FAST_CMOV or HAVE_INLINE_ASM is false), otherwise you must have:

     a11:       39 fd                   cmp    %edi,%ebp
     a13:       0f 93 c1                setae  %cl
     a16:       0f 42 c6                cmovb  %esi,%eax
     a19:       0f 42 d7                cmovb  %edi,%edx

With -fomit-frame-pointer instead of -fstack-check, or:

     a3c:       39 ca                   cmp    %ecx,%edx
     a3e:       0f 93 c3                setae  %bl
     a41:       0f 42 f8                cmovb  %eax,%edi
     a44:       0f 42 f1                cmovb  %ecx,%esi

Without PIC.
In my case HAVE_FAST_CMOV and HAVE_INLINE_ASM are true then I got the problem and the fix on HAVE_6REGS select the C code instead of asm.

-02 should be faster but I haven't check the fact and it is the same problem with or without.

comment:8 Changed 5 years ago by cehoyos

  • Component changed from undetermined to avcodec
  • Reproduced by developer set
  • Status changed from new to open
  • Version changed from 1.2.6 to git-master

Reproducible with:

$ configure --cc='cc -m32' --disable-optimizations --enable-pic --cpu=pentiumpro
$ make libavcodec/diracdec.o

comment:9 Changed 5 years ago by cehoyos

Please remember that patches are often ignored on the bug tracker, please send them (with git format-patch) to the ffmpeg-devel mailing list.

comment:10 Changed 5 years ago by cehoyos

  • Summary changed from The asm inline code in function libavcodec/dirac_arith.h:dirac_get_arith_bit with PIC and stack-check build options to Compilation of libavcodec/dirac_arith.h:dirac_get_arith_bit fails for five registers if cmov is fast

comment:11 follow-up: Changed 5 years ago by alucowie

More strange, the configure option "--disable-asm" doesn't disable that code.

comment:12 Changed 5 years ago by cehoyos

  • Resolution set to fixed
  • Status changed from open to closed

Fixed by you in d8ab7f31 - thank you for the report!

comment:13 in reply to: ↑ 11 Changed 5 years ago by cehoyos

Replying to alucowie:

More strange, the configure option "--disable-asm" doesn't disable that code.

At least to me this behaviour wasn't known, I opened ticket #3544.

Note: See TracTickets for help on using tickets.