[v1] utils: mojom: Fix build error caused by the mojom tool update
diff mbox series

Message ID 20240215082342.6437-1-naush@raspberrypi.com
State Accepted
Commit 5e4dc46a0c6fc3138c887e9afbabfe7dfa6ebd98
Headers show
Series
  • [v1] utils: mojom: Fix build error caused by the mojom tool update
Related show

Commit Message

Naushir Patuck Feb. 15, 2024, 8:23 a.m. UTC
The update to the mojom tool in commit d17de86904f0 causes build errors
with gcc 12.2 release builds. One such error is:

In file included from src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:18:
In static member function ‘static libcamera::ipa::RPi::ProcessParams libcamera::IPADataSerializer<libcamera::ipa::RPi::ProcessParams>::deserialize(std::vector<unsigned char>::const_iterator, std::vector<unsigned char>::const_iterator, libcamera::ControlSerializer*)’,
    inlined from ‘void IPAProxyRPiWorker::readyRead()’ at src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:302:70:
include/libcamera/ipa/raspberrypi_ipa_serializer.h:1172:32: error: ‘*(uint32_t*)((char*)&ret + offsetof(libcamera::ipa::RPi::ProcessParams, libcamera::ipa::RPi::ProcessParams::buffers.libcamera::ipa::RPi::BufferIds::bayer))’ may be used uninitialized [-Werror=maybe-uninitialized]
 1172 |                         return ret;

The failure is caused by the new auto-generated IPA interface not
initialising POD types to a default value. This is because the updated
mojom library uses a new mojom.ValueKind class to represent POD types,
whereas the interface generator script uses the mojom.Kind class, which
is correct for the older mojom library.

Fix this breakage by switching the interface generator script to use
mojom.ValueKind to test for POD types.

Fixes: d17de86904f0 ("utils: ipc: Update mojo")
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 utils/ipc/generators/mojom_libcamera_generator.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paul Elder Feb. 15, 2024, 10:41 a.m. UTC | #1
Hi Naush,

On Thu, Feb 15, 2024 at 08:23:42AM +0000, Naushir Patuck wrote:
> The update to the mojom tool in commit d17de86904f0 causes build errors
> with gcc 12.2 release builds. One such error is:
> 
> In file included from src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:18:
> In static member function ‘static libcamera::ipa::RPi::ProcessParams libcamera::IPADataSerializer<libcamera::ipa::RPi::ProcessParams>::deserialize(std::vector<unsigned char>::const_iterator, std::vector<unsigned char>::const_iterator, libcamera::ControlSerializer*)’,
>     inlined from ‘void IPAProxyRPiWorker::readyRead()’ at src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:302:70:
> include/libcamera/ipa/raspberrypi_ipa_serializer.h:1172:32: error: ‘*(uint32_t*)((char*)&ret + offsetof(libcamera::ipa::RPi::ProcessParams, libcamera::ipa::RPi::ProcessParams::buffers.libcamera::ipa::RPi::BufferIds::bayer))’ may be used uninitialized [-Werror=maybe-uninitialized]
>  1172 |                         return ret;
> 
> The failure is caused by the new auto-generated IPA interface not
> initialising POD types to a default value. This is because the updated
> mojom library uses a new mojom.ValueKind class to represent POD types,
> whereas the interface generator script uses the mojom.Kind class, which
> is correct for the older mojom library.

Indeed that seems to be the case. (Not sure why it didn't error out for me :S)

> Fix this breakage by switching the interface generator script to use
> mojom.ValueKind to test for POD types.
> 
> Fixes: d17de86904f0 ("utils: ipc: Update mojo")
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Thank you for the fix.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  utils/ipc/generators/mojom_libcamera_generator.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> index 582818c98b5f..99d905de456c 100644
> --- a/utils/ipc/generators/mojom_libcamera_generator.py
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -72,7 +72,7 @@ def ParamsCommaSep(l):
>  def GetDefaultValue(element):
>      if element.default is not None:
>          return element.default
> -    if type(element.kind) == mojom.Kind:
> +    if type(element.kind) == mojom.ValueKind:
>          return '0'
>      if IsFlags(element):
>          return ''
> -- 
> 2.34.1
>
Kieran Bingham Feb. 15, 2024, 10:46 a.m. UTC | #2
Quoting Naushir Patuck (2024-02-15 08:23:42)
> The update to the mojom tool in commit d17de86904f0 causes build errors
> with gcc 12.2 release builds. One such error is:
> 
> In file included from src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:18:
> In static member function ‘static libcamera::ipa::RPi::ProcessParams libcamera::IPADataSerializer<libcamera::ipa::RPi::ProcessParams>::deserialize(std::vector<unsigned char>::const_iterator, std::vector<unsigned char>::const_iterator, libcamera::ControlSerializer*)’,
>     inlined from ‘void IPAProxyRPiWorker::readyRead()’ at src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:302:70:
> include/libcamera/ipa/raspberrypi_ipa_serializer.h:1172:32: error: ‘*(uint32_t*)((char*)&ret + offsetof(libcamera::ipa::RPi::ProcessParams, libcamera::ipa::RPi::ProcessParams::buffers.libcamera::ipa::RPi::BufferIds::bayer))’ may be used uninitialized [-Werror=maybe-uninitialized]
>  1172 |                         return ret;
> 
> The failure is caused by the new auto-generated IPA interface not
> initialising POD types to a default value. This is because the updated
> mojom library uses a new mojom.ValueKind class to represent POD types,
> whereas the interface generator script uses the mojom.Kind class, which
> is correct for the older mojom library.

Interesting, that explains why we had to add
 - https://patchwork.libcamera.org/patch/19446/

but clearly it wasn't a complete solution.


> 
> Fix this breakage by switching the interface generator script to use
> mojom.ValueKind to test for POD types.
> 
> Fixes: d17de86904f0 ("utils: ipc: Update mojo")
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  utils/ipc/generators/mojom_libcamera_generator.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> index 582818c98b5f..99d905de456c 100644
> --- a/utils/ipc/generators/mojom_libcamera_generator.py
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -72,7 +72,7 @@ def ParamsCommaSep(l):
>  def GetDefaultValue(element):
>      if element.default is not None:
>          return element.default
> -    if type(element.kind) == mojom.Kind:
> +    if type(element.kind) == mojom.ValueKind:

Yeah, I have no idea ;-) I'll leave this to Paul as he's the expert
here. But I'm happy to merge if we're sure it's the right fix, and all
the other tests pass.

Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>          return '0'
>      if IsFlags(element):
>          return ''
> -- 
> 2.34.1
>
Kieran Bingham Feb. 15, 2024, 11:46 a.m. UTC | #3
Quoting Naushir Patuck (2024-02-15 08:23:42)
> The update to the mojom tool in commit d17de86904f0 causes build errors
> with gcc 12.2 release builds. One such error is:
> 
> In file included from src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:18:
> In static member function ‘static libcamera::ipa::RPi::ProcessParams libcamera::IPADataSerializer<libcamera::ipa::RPi::ProcessParams>::deserialize(std::vector<unsigned char>::const_iterator, std::vector<unsigned char>::const_iterator, libcamera::ControlSerializer*)’,
>     inlined from ‘void IPAProxyRPiWorker::readyRead()’ at src/libcamera/proxy/worker/raspberrypi_ipa_proxy_worker.cpp:302:70:
> include/libcamera/ipa/raspberrypi_ipa_serializer.h:1172:32: error: ‘*(uint32_t*)((char*)&ret + offsetof(libcamera::ipa::RPi::ProcessParams, libcamera::ipa::RPi::ProcessParams::buffers.libcamera::ipa::RPi::BufferIds::bayer))’ may be used uninitialized [-Werror=maybe-uninitialized]
>  1172 |                         return ret;
> 
> The failure is caused by the new auto-generated IPA interface not
> initialising POD types to a default value. This is because the updated
> mojom library uses a new mojom.ValueKind class to represent POD types,
> whereas the interface generator script uses the mojom.Kind class, which
> is correct for the older mojom library.
> 
> Fix this breakage by switching the interface generator script to use
> mojom.ValueKind to test for POD types.
> 
> Fixes: d17de86904f0 ("utils: ipc: Update mojo")
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

CI-OK: https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1103947 
Merged

--
Thanks.

Kieran


> ---
>  utils/ipc/generators/mojom_libcamera_generator.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> index 582818c98b5f..99d905de456c 100644
> --- a/utils/ipc/generators/mojom_libcamera_generator.py
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -72,7 +72,7 @@ def ParamsCommaSep(l):
>  def GetDefaultValue(element):
>      if element.default is not None:
>          return element.default
> -    if type(element.kind) == mojom.Kind:
> +    if type(element.kind) == mojom.ValueKind:
>          return '0'
>      if IsFlags(element):
>          return ''
> -- 
> 2.34.1
>

Patch
diff mbox series

diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
index 582818c98b5f..99d905de456c 100644
--- a/utils/ipc/generators/mojom_libcamera_generator.py
+++ b/utils/ipc/generators/mojom_libcamera_generator.py
@@ -72,7 +72,7 @@  def ParamsCommaSep(l):
 def GetDefaultValue(element):
     if element.default is not None:
         return element.default
-    if type(element.kind) == mojom.Kind:
+    if type(element.kind) == mojom.ValueKind:
         return '0'
     if IsFlags(element):
         return ''