Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve CI #4

Open
kbingham opened this issue Jan 23, 2020 · 0 comments
Open

Improve CI #4

kbingham opened this issue Jan 23, 2020 · 0 comments

Comments

@kbingham
Copy link
Owner

Gitlab:

Great examples of using FD.o ci-templates in libinput to get really wide build coverage:

https://gitlab.freedesktop.org/wayland/ci-templates
https://gitlab.freedesktop.org/libinput/libinput/tree/master

Git hub:

Seems a bit more complex, but might be useful in the future if anyone wants to tackle it.
kbingham added a commit that referenced this issue Feb 6, 2020
@kbingham
qcam: Fix ViewFinder memory leak
f0b2582 
When setting the format on the ViewFinder, a new image_ is allocated.
Any change in format deletes the existing allocation, but it is not
cleaned up on shutdown:

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x7f0bf8a7e17f in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10e17f)
    #1 0x564c7205f7b0 in ViewFinder::setFormat(unsigned int, unsigned int, unsigned int) ../src/qcam/viewfinder.cpp:43
    #2 0x564c71fec467 in MainWindow::startCapture() ../src/qcam/main_window.cpp:152
    #3 0x564c71fe6c1a in MainWindow::MainWindow(libcamera::CameraManager*, OptionsParser::Options const&) ../src/qcam/main_window.cpp:40
    #4 0x564c71fdf133 in main ../src/qcam/main.cpp:76
    #5 0x7f0bf5944b6a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x26b6a)

Provide a ViewFinder destructor, and delete the allocation as
appropriate.

Fixes: 97e8b3a ("qcam: Add Qt-based GUI application")
Reviewed-by: Jacopo Mondi <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Kieran Bingham <[email protected]>
kbingham pushed a commit that referenced this issue Apr 17, 2021
@pinchartl
libcamera: bound_method: Please the gcc undefined behaviour sanitizer
5a76290 
Enabling the gcc undefined behaviour sanitizer (with the meson configure
-Db_sanitize=undefined option) causes many tests to fail, with errors
such as the following (for test/object-invoke):

------------------------------------------------------------------------
../../include/libcamera/bound_method.h:198:27: runtime error: member access within address 0x55fcd7bfbd38 which does not point to an object of type 'BoundMethodBase'
0x55fcd7bfbd38: note: object has invalid vptr
 fc 55 00 00  2a 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  31 00 00 00 00 00 00 00  4b c6 72 88
              ^~~~~~~~~~~~~~~~~~~~~~~
              invalid vptr
../../include/libcamera/bound_method.h:198:41: runtime error: member call on null pointer of type 'struct InvokedObject'
../../include/libcamera/bound_method.h:198:41: runtime error: member access within null pointer of type 'struct InvokedObject'
Segmentation fault
------------------------------------------------------------------------

or

------------------------------------------------------------------------
../../include/libcamera/bound_method.h:198:27: runtime error: member access within address 0x603000006628 which does not point to an object of type 'BoundMethodBase'
0x603000006628: note: object has invalid vptr
 70 55 00 00  2a 00 00 00 be be be be  03 02 00 00 18 00 00 00  01 00 00 60 00 00 00 00  05 00 80 07
              ^~~~~~~~~~~~~~~~~~~~~~~
              invalid vptr
=================================================================
==941==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000006630 at pc 0x55704e461371 bp 0x7fff539b9040 sp 0x7fff539b9030
READ of size 8 at 0x603000006630 thread T0
    #0 0x55704e461370 in libcamera::BoundMethodMember<InvokedObject, void, int>::invoke(int) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x47370)
    #1 0x55704e4622ca in void libcamera::BoundMethodArgs<void, int>::invokePack<0ul>(libcamera::BoundMethodPackBase*, std::integer_sequence<unsigned long, 0ul>) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x482ca)
    #2 0x55704e460a93 in libcamera::BoundMethodArgs<void, int>::invokePack(libcamera::BoundMethodPackBase*) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x46a93)
    #3 0x7fdc38a5fec4 in libcamera::InvokeMessage::invoke() ../../src/libcamera/message.cpp:154
    #4 0x7fdc38a62faf in libcamera::Object::message(libcamera::Message*) ../../src/libcamera/object.cpp:183
    #5 0x7fdc38ad3742 in libcamera::Thread::dispatchMessages(libcamera::Message::Type) ../../src/libcamera/thread.cpp:575
    #6 0x7fdc38972d8d in libcamera::EventDispatcherPoll::processEvents() ../../src/libcamera/event_dispatcher_poll.cpp:148
    #7 0x55704e44bc15 in ObjectInvokeTest::run() (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x31c15)
    #8 0x55704e4630ab in Test::execute() ../../test/libtest/test.cpp:28
    #9 0x55704e44965b in main ../../test/object-invoke.cpp:204
    #10 0x7fdc36090eba in __libc_start_main ../csu/libc-start.c:314
    #11 0x55704e449359 in _start (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x2f359)

0x603000006630 is located 0 bytes to the right of 32-byte region [0x603000006610,0x603000006630)
allocated by thread T0 here:
    #0 0x7fdc3ad757c7 in operator new(unsigned long) /var/tmp/portage/sys-devel/gcc-11.0.1_pre9999/work/gcc-11.0.1_pre9999/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x55704e45afea in __gnu_cxx::new_allocator<std::_Sp_counted_ptr_inplace<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, (__gnu_cxx::_Lock_policy)2> >::allocate(unsigned long, void const*) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x40fea)
    #2 0x55704e45a45d in std::allocator_traits<std::allocator<std::_Sp_counted_ptr_inplace<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, (__gnu_cxx::_Lock_policy)2> > >::allocate(std::allocator<std::_Sp_counted_ptr_inplace<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, (__gnu_cxx::_Lock_policy)2> >&, unsigned long) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x4045d)
    #3 0x55704e458339 in std::__allocated_ptr<std::allocator<std::_Sp_counted_ptr_inplace<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, (__gnu_cxx::_Lock_policy)2> > > std::__allocate_guarded<std::allocator<std::_Sp_counted_ptr_inplace<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, (__gnu_cxx::_Lock_policy)2> > >(std::allocator<std::_Sp_counted_ptr_inplace<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, (__gnu_cxx::_Lock_policy)2> >&) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x3e339)
    #4 0x55704e4574ad in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, int&>(libcamera::BoundMethodPack<void, int>*&, std::_Sp_alloc_shared_tag<std::allocator<libcamera::BoundMethodPack<void, int> > >, int&) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x3d4ad)
    #5 0x55704e4569c7 in std::__shared_ptr<libcamera::BoundMethodPack<void, int>, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<libcamera::BoundMethodPack<void, int> >, int&>(std::_Sp_alloc_shared_tag<std::allocator<libcamera::BoundMethodPack<void, int> > >, int&) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x3c9c7)
    #6 0x55704e455f9d in std::shared_ptr<libcamera::BoundMethodPack<void, int> >::shared_ptr<std::allocator<libcamera::BoundMethodPack<void, int> >, int&>(std::_Sp_alloc_shared_tag<std::allocator<libcamera::BoundMethodPack<void, int> > >, int&) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x3bf9d)
    #7 0x55704e454eb5 in std::shared_ptr<libcamera::BoundMethodPack<void, int> > std::allocate_shared<libcamera::BoundMethodPack<void, int>, std::allocator<libcamera::BoundMethodPack<void, int> >, int&>(std::allocator<libcamera::BoundMethodPack<void, int> > const&, int&) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x3aeb5)
    #8 0x55704e454220 in std::shared_ptr<libcamera::BoundMethodPack<void, int> > std::make_shared<libcamera::BoundMethodPack<void, int>, int&>(int&) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x3a220)
    #9 0x55704e450e60 in libcamera::BoundMethodMember<InvokedObject, void, int>::activate(int, bool) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x36e60)
    #10 0x55704e44efb2 in void libcamera::Object::invokeMethod<InvokedObject, void, int, int, (void*)0>(void (InvokedObject::*)(int), libcamera::ConnectionType, int) (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x34fb2)
    #11 0x55704e44b7cc in ObjectInvokeTest::run() (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x317cc)
    #12 0x55704e4630ab in Test::execute() ../../test/libtest/test.cpp:28
    #13 0x55704e44965b in main ../../test/object-invoke.cpp:204
    #14 0x7fdc36090eba in __libc_start_main ../csu/libc-start.c:314

SUMMARY: AddressSanitizer: heap-buffer-overflow (libcamera/build/x86-gcc-11.0.1/test/object-invoke+0x47370) in libcamera::BoundMethodMember<InvokedObject, void, int>::invoke(int)
Shadow bytes around the buggy address:
  0x0c067fff8c70: 00 fa fa fa 00 00 06 fa fa fa fd fd fd fd fa fa
  0x0c067fff8c80: 00 00 06 fa fa fa 00 00 03 fa fa fa 00 00 00 05
  0x0c067fff8c90: fa fa 00 00 04 fa fa fa 00 00 00 00 fa fa fd fd
  0x0c067fff8ca0: fd fd fa fa fd fd fd fd fa fa fd fd fd fd fa fa
  0x0c067fff8cb0: fd fd fd fd fa fa 00 00 00 00 fa fa 00 00 00 00
=>0x0c067fff8cc0: fa fa 00 00 00 00[fa]fa fd fd fd fa fa fa fa fa
  0x0c067fff8cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8ce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c067fff8d10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==941==ABORTING
------------------------------------------------------------------------

The root cause isn't clear, but this change fixes the issue. It may be a
bug in gcc.

Signed-off-by: Laurent Pinchart <[email protected]>
Acked-by: Kieran Bingham <[email protected]>
kbingham pushed a commit that referenced this issue Aug 16, 2021
@kbingham
pipeline: vimc: Force complete of request on cancelled buffers
b8a5109 
When the stream is stopped, the V4L2VideoDevice sends back all
the queued buffers with FrameMetadata::FrameCancelled status.
It is the responsibility of the pipeline handler to handle
these buffers with FrameMetadata::FrameCancelled. VIMC is
currently missing this handling path.

As the FrameMetadata::FrameCancelled is set when the stream is
stopped, we can be sure that no more queued and re-use of request
shall happen.  Hence, cancel all the requests' buffers force a
complete with completeBuffer().

The issue is caught by the gstreamer_single_stream_test.cpp running
with vimc. During the check with meson built-in option
	'-Db_sanitize=address,undefined'
it was observed:

==118003==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e000037108 at pc 0x7f225160c9ac bp 0x7f224a47b620 sp 0x7f224a47b618
READ of size 4 at 0x60e000037108 thread T1
    #0 0x7f225160c9ab in libcamera::Request::sequence() const ../include/libcamera/request.h:55
    #1 0x7f22518297aa in libcamera::VimcCameraData::bufferReady(libcamera::FrameBuffer*) ../src/libcamera/pipeline/vimc/vimc.cpp:577
    #2 0x7f225183b1ef in libcamera::BoundMethodMember<libcamera::VimcCameraData, void, libcamera::FrameBuffer*>::activate(libcamera::FrameBuffer*, bool) ../include/libcamera/base/bound_method.h:194
    #3 0x7f22515cc91f in libcamera::Signal<libcamera::FrameBuffer*>::emit(libcamera::FrameBuffer*) ../include/libcamera/base/signal.h:126
    #4 0x7f22515c3305 in libcamera::V4L2VideoDevice::streamOff() ../src/libcamera/v4l2_videodevice.cpp:1605
    #5 0x7f225181f345 in libcamera::PipelineHandlerVimc::stop(libcamera::Camera*) ../src/libcamera/pipeline/vimc/vimc.cpp:365

The VimcCameraData::bufferReady seems to emit even after the stream
is stopped. It's primarily due to vimc's lack of handling
FrameMetadata::FrameCancelled in its pipeline handler.

Signed-off-by: Umang Jain <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Tested-by: Laurent Pinchart <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
kbingham pushed a commit that referenced this issue Aug 16, 2021
pipeline: vimc: Force complete of request on cancelled buffers
5420e35 
When the stream is stopped, the V4L2VideoDevice sends back all
the queued buffers with FrameMetadata::FrameCancelled status.
It is the responsibility of the pipeline handler to handle
these buffers with FrameMetadata::FrameCancelled. VIMC is
currently missing this handling path.

As the FrameMetadata::FrameCancelled is set when the stream is
stopped, we can be sure that no more queued and re-use of request
shall happen.  Hence, cancel all the requests' buffers force a
complete with completeBuffer().

The issue is caught by the gstreamer_single_stream_test.cpp running
with vimc. During the check with meson built-in option
	'-Db_sanitize=address,undefined'
it was observed:

==118003==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e000037108 at pc 0x7f225160c9ac bp 0x7f224a47b620 sp 0x7f224a47b618
READ of size 4 at 0x60e000037108 thread T1
    #0 0x7f225160c9ab in libcamera::Request::sequence() const ../include/libcamera/request.h:55
    #1 0x7f22518297aa in libcamera::VimcCameraData::bufferReady(libcamera::FrameBuffer*) ../src/libcamera/pipeline/vimc/vimc.cpp:577
    #2 0x7f225183b1ef in libcamera::BoundMethodMember<libcamera::VimcCameraData, void, libcamera::FrameBuffer*>::activate(libcamera::FrameBuffer*, bool) ../include/libcamera/base/bound_method.h:194
    #3 0x7f22515cc91f in libcamera::Signal<libcamera::FrameBuffer*>::emit(libcamera::FrameBuffer*) ../include/libcamera/base/signal.h:126
    #4 0x7f22515c3305 in libcamera::V4L2VideoDevice::streamOff() ../src/libcamera/v4l2_videodevice.cpp:1605
    #5 0x7f225181f345 in libcamera::PipelineHandlerVimc::stop(libcamera::Camera*) ../src/libcamera/pipeline/vimc/vimc.cpp:365

The VimcCameraData::bufferReady seems to emit even after the stream
is stopped. It's primarily due to vimc's lack of handling
FrameMetadata::FrameCancelled in its pipeline handler.

Signed-off-by: Umang Jain <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Tested-by: Laurent Pinchart <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
Tested-by: Kieran Bingham <[email protected]>
kbingham pushed a commit that referenced this issue Jun 3, 2024
@mz-pdm @pinchartl
libcamera: software_isp: Remove TODO about internal representation
a52d818 
TODO #4 was recorded at a time where the IPA module computed gain values
and the ISP computed the look up tables.  The gains were higher-level
parameters.  Now that the look up tables are computed in the IPA module,
the IPA and ISP are more tightly coupled and the TODO item is less
relevant.

Let's drop the TODO item.  We may or may not need to switch to a
different representation in future but there is currently no good need
for this and the conversion of the values would be just waste of CPU
cycles.

Signed-off-by: Milan Zamazal <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Laurent Pinchart <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@kbingham