After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 785628 - v4l2: Build warning on 32-bit ARM in gstv4l2object.c
v4l2: Build warning on 32-bit ARM in gstv4l2object.c
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2017-07-31 12:49 UTC by Kirill Novichikhin
Modified: 2017-08-01 19:09 UTC
See Also:
GNOME target: ---
GNOME version: ---

One possible way to fix the build (888 bytes, patch)
2017-07-31 12:49 UTC, Kirill Novichikhin
none Details | Review
v4l2object: Use mmap64 to match libv4l2 signature (967 bytes, patch)
2017-07-31 16:00 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Kirill Novichikhin 2017-07-31 12:49:04 UTC
Created attachment 356640 [details] [review]
One possible way to fix the build

This is a Yocto build of master branch at 317d3380bbee24763dfeec4bbb9d1c733e519aff (common at 48a5d85ebf4a0bad1c997c83100f710fe2154fbf)

| arm-poky-linux-gnueabi-libtool: compile:  arm-poky-linux-gnueabi-gcc -march=armv7-a -marm -mfpu=neon -mfloat-abi=hard -mcpu=cortex-a9 --sysroot=/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard -DHAVE_CONFIG_H -I. -I../../../git/sys/v4l2 -I../.. -pthread -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gstreamer-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -pthread -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gstreamer-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -I../../../git/gst-libs -pthread -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gstreamer-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -DGST_USE_UNSTABLE_API -DG_THREADS_MANDATORY -DG_DISABLE_DEPRECATED -Wall -Wdeclaration-after-statement -Wvla -Wpointer-arith -Wmissing-declarations -Wmissing-prototypes -Wredundant-decls -Wwrite-strings -Wold-style-definition -Waggregate-return -Winit-self -Wmissing-include-dirs -Waddress -Wno-multichar -Wnested-externs -Werror -DGST_DISABLE_DEPRECATED -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gudev-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -O2 -pipe -g -feliminate-unused-debug-types -fdebug-prefix-map=/home/user/fsl-community-bsp-2.2/build/tmp/work/cortexa9hf-neon-mx6qdl-poky-linux-gnueabi/gstreamer1.0-plugins-good/1.13+gitAUTOINC+317d3380bb-r0=/usr/src/debug/gstreamer1.0-plugins-good/1.13+gitAUTOINC+317d3380bb-r0 -fdebug-prefix-map=/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/x86_64-linux= -fdebug-prefix-map=/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard= -c ../../../git/sys/v4l2/gstv4l2object.c  -fPIC -DPIC -o .libs/libgstvideo4linux2_la-gstv4l2object.o
| ../../../git/sys/v4l2/gstv4l2object.c: In function 'gst_v4l2_object_new':
| ../../../git/sys/v4l2/gstv4l2object.c:512:22: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
|      v4l2object->mmap = mmap;
|                       ^

Last argument to mmap is __off_t, which is unsigned long. Last argument to v4l2_mmap is uint64_t. This mismatch breaks build on 32-bit platforms

Attached: a patch I used to work around the issue, not sure if this should be ifdef'ed on 32-bit platforms or left to the compiler to eliminate.

Alternative patch could use mmap2 to allow for more addressable space if necessary.

Assumed regression after commit 31d8a1d929fecd7ceaa7575be5921f9148ed8afb
Comment 1 Kirill Novichikhin 2017-07-31 14:08:00 UTC
FYI - if ignored, this causes runtime failures on ARM probably due to differences in 32-bit and 64-bit parameter alignment - you would think that length is just truncated to its 32 least significant bits (on little endian platforms), but instead whatever garbage word is on the stack is passed as 'length' parameter and 64-bit 'length' follows that. 
I know because I tried adding a cast to '(void*)' in the 'v4l2object->mmap = mmap' assignment and that didn't work as well as I expected ;)
Comment 2 Nicolas Dufresne (ndufresne) 2017-07-31 14:13:48 UTC
Yes, I know. There is no mmap2 here, but I can see there is a mmap64 which is the same on 64bit platform. Though, this is not in the man-page I got here, I need to figure-out if it's GLIBC specific or if we can find it on other libc. Otherwise we'll just create a wrapper like you proposed on the ML. Will you provide a patch ?
Comment 3 Nicolas Dufresne (ndufresne) 2017-07-31 16:00:52 UTC
Created attachment 356657 [details] [review]
v4l2object: Use mmap64 to match libv4l2 signature

I'm not completly certain this is the right fix, since I don't know if this is
portable across different libc and OS (notably BSD). It's definatly better then
downcasting (hence replacing previous patch). The problem with the blind
downcast is that it could break randomly if you have more then 4G of RAM.
Comment 4 Nicolas Dufresne (ndufresne) 2017-08-01 19:07:44 UTC
Attachment 356657 [details] pushed as b61bba4 - v4l2object: Use mmap64 to match libv4l2 signature
Comment 5 Nicolas Dufresne (ndufresne) 2017-08-01 19:09:19 UTC
Ok, let 's give this a try. On Linux we need _GNU_SOURCE to be defined,
and on BSD, if I read the header properly, mmap64 is always there. It' s also
implemented in uClibc, and v4l2 is not built on Android / bionic, since we 
don 't have permissions to access the define from an Android app anyway.