As the ELF parsing code uses non-const pointers to the ELF mapping, we
have to map the module in private read-write mode. This causes issues
with valgrind, due to the IPA manager mapping the module in shared
read-only mode and valgrind having trouble loading debugging symbols
later at dlopen time due to conflicting mappings.
This is likely a bug in valgrind (reported as [1]), but we can easily
work around it by using shared read-only mappings only. As such a
mapping shouldn't be less efficient than private read-write mappings,
switch the mapping type. This requires modifying the ELF parsing
functions to operate on const memory, which is a good idea anyway as
they're not supposed to modify the ELF file.
[1] https://bugs.kde.org/show_bug.cgi?id=422601
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Given how the elfSection() function uses the sub-expression
(idx * eHdr->e_shentsize)
it has effectively two (16 bits, unsigned) operands.
The sub-expression is promoted to type int (32 bits, signed) for
multiplication and then added to eHdr->e_shoff, which is uint32_t on
32-bit platforms and uint64_t on 64-bit platforms. Since eHdr->e_shoff
is unsigned, the integer conversion rules dictate that the other signed
operand (i.e. the result of aforementioned sub-expression) will be
converted to unsigned type too. This causes sign-extension for both of
the above operands to match eHdr->e_shoff's type and should be avoided.
The solution is to explicitly cast one of the operands of the
sub-expression with unsigned int type. Hence, the other operand will be
integer promoted and the resultant will also be of unsigned int type,
not requiring to bother about a sign-extension.
Reported-by: Coverity CID=280008
Reported-by: Coverity CID=280009
Reported-by: Coverity CID=280010
Signed-off-by: Umang Jain <email@uajain.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The libcamera internal headers are located in src/libcamera/include/.
The directory is added to the compiler headers search path with a meson
include_directories() directive, and internal headers are included with
(e.g. for the internal semaphore.h header)
#include "semaphore.h"
All was well, until libcxx decided to implement the C++20
synchronization library. The __threading_support header gained a
#include <semaphore.h>
to include the pthread's semaphore support. As include_directories()
adds src/libcamera/include/ to the compiler search path with -I, the
internal semaphore.h is included instead of the pthread version.
Needless to say, the compiler isn't happy.
Three options have been considered to fix this issue:
- Use -iquote instead of -I. The -iquote option instructs gcc to only
consider the header search path for headers included with the ""
version. Meson unfortunately doesn't support this option.
- Rename the internal semaphore.h header. This was deemed to be the
beginning of a long whack-a-mole game, where namespace clashes with
system libraries would appear over time (possibly dependent on
particular system configurations) and would need to be constantly
fixed.
- Move the internal headers to another directory to create a unique
namespace through path components. This causes lots of churn in all
the existing source files through the all project.
The first option would be best, but isn't available to us due to missing
support in meson. Even if -iquote support was added, we would need to
fix the problem before a new version of meson containing the required
support would be released.
The third option is thus the only practical solution available. Bite the
bullet, and do it, moving headers to include/libcamera/internal/.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Jacopo Mondi <jacopo@jmondi.org>
Make the IPAModule class inherit from Loggable to log the IPA module
name in all messages.
Before:
[14:19:55.743795676] [20250] ERROR IPAModule ipa_module.cpp:279 IPA module is not an ELF file
After:
[14:20:22.343795887] [20270] ERROR IPAModule ipa_module.cpp:279 ipa_vimc.so: IPA module is not an ELF file
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
The IPAModuleInfo::name field is currently a free-formed string that has
little use. Tighten its usage rules to make it suitable for building
file system paths to IPA-specific resources by matching the directory
name of the IPA module.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Load the signature from the .sign file, if available, when loading the
IPA module information and store it in the IPAModule class.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
The IPAModule class passes pointers to data and the corresponding size
as thwo different variables to several functions. Tie them together in a
Span.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
C++14 introduced useful type traits helpers named std::*_t as aliases to
std::*<...>::type. Use them to simplify the code.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Access the ELF types corresponding to the native word size using the
ElfW() macro instead of template types. This is the standard method and
simplifies the code.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Switch IPA communication to the plain C API. As the IPAInterface class
is easier to use for pipeline handlers than a plain C API, retain it and
add an IPAContextWrapper that translate between the C++ and the C APIs.
On the IPA module side usage of IPAInterface may be desired for IPAs
implemented in C++ that want to link to libcamera. For those IPAs, a new
IPAInterfaceWrapper helper class is introduced to wrap the IPAInterface
implemented internally by the IPA module into an ipa_context,
ipa_context_ops and ipa_callback_ops.
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
The IPAModule::loadIPAModuleInfo() function includes a *data pointer
which is used as a null-pointer comparison in the error path with a
conditional statement of "if (ret || !data)".
The data variable is not initialised, and a single error path evaluates
this as "if (true || uninitialised)".
Whilst this error path does not incorrectly utilise the uninitialised
data, as the ret evaluates to true already, it does leave a statement
which includes an uninitialised variable.
Help the static analysers by initialising the data variable when it is
defined.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Fix a number of spelling errors and word duplications throughout the comments
within libcamera.
These were picked up with spellintian.
Also one capitalisation of the first word of a \return statement picked
up by checkstyle.py while creating this patch.
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The second argument to std::array is the size of the array, not of the
elements it contains. Fix this by turning the std::array into a simple
array of const char pointers.
Fixes: 099815b853 ("libcamera: ipa_module: add isOpenSource")
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Add a method to IPAModule to check if the module is open source.
This uses the license field of the member IPAModuleInfo.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Add a field to IPAModuleInfo to contain the license of the module.
This license field will be used to determine whether the IPA module
should be run in an isolated process or not. If the license is open
source, then the IPA module will be allowed to run without process
isolation, if the user enables it. If the license is not open source,
then the IPA module will be run with process isolation.
Update the dummy IPA and IPA test to conform to the new struct layout.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Make elfLoadSymbol more generic by making the symbol size an output
rather than an input. Also move the memcpy out of elfLoadSymbol.
If the size of struct IPAModuleInfo changes between versions, we still
want to be able to load it and perhaps do conversions for backwards
compatibility. In this case the size should not be a restriction when
searching for the symbol.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Add functions for loading the IPAInterface factory function from an IPA
module shared object, and for creating an instance of an IPAInterface.
These functions will be used by IPAManager, from which a PipelineHandler
will request an IPAInterface.
Also update meson to add the "-ldl" linker argument, to allow loading of
the factory function from a shared object.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The IPA module API version determines the layout of struct
IPAModuleInfo. If this version number does not match, then it means that
the IPA module information structure can't be interpreted, and the
module can't be used. Validate this version number upon loading the
IPA module info from the IPA shared object.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
We need a way to match pipelines with IPA modules, so add fields in
IPAModuleInfo to hold the IPA module API version number, the pipeline
name, and the pipeline version.
The module API version is used to determine the layout of struct
IPAModuleInfo.
Also update IPA module tests and Doxygen accordingly. Doxygen needs to
be updated to accomodate __attribute__((packed)).
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Implement a class to wrap around an IPA module shared object.
For now, just load a struct IPAModuleInfo with symbol name
ipaModuleInfo from an IPA module .so shared object.
Also provide a public header file including the struct IPAModuleInfo,
structured such that both C and C++ IPA modules are supported.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
