[libcamera-devel] libcamera: object: Silence gcc false positive error in release mode
diff mbox series

Message ID 20220429215841.19913-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 4f1d1a48fe7ebe1282d7608a59cb059e1367e584
Headers show
Series
  • [libcamera-devel] libcamera: object: Silence gcc false positive error in release mode
Related show

Commit Message

Laurent Pinchart April 29, 2022, 9:58 p.m. UTC
gcc 10 and 11 produce an error when compiling libcamera in release mode:

In file included from ../../src/libcamera/base/object.cpp:13:
../../include/libcamera/base/message.h: In member function ‘void libcamera::Object::notifyThreadMove()’:
../../include/libcamera/base/message.h:58:47: error: array subscript ‘const libcamera::InvokeMessage[0]’ is partly outside array bounds of ‘libcamera::Message [1]’ [-Werror=array-bounds]
   58 |         Semaphore *semaphore() const { return semaphore_; }
      |                                               ^~~~~~~~~~
../../src/libcamera/base/object.cpp:280:17: note: while referencing ‘msg’
  280 |         Message msg(Message::ThreadMoveMessage);
      |                 ^~~

This seems to be a false positive, given that msg->type() can never be
equal to Message::InvokeMessage in Object::message() when called from
Object::notifyThreadMove(), as the message is created there with the
Message::ThreadMoveMessage type. The problem as been reported in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105400, but the error
nonetheless needs to be fixed without waiting for a new gcc release, and
a dynamic_cast does the job with a small additional runtime cost that
shouldn't be a big issue, given that moving objects between threads is a
rare operation.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=125
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/base/object.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Kieran Bingham May 3, 2022, 10:44 a.m. UTC | #1
Hi Laurent,

Quoting Laurent Pinchart via libcamera-devel (2022-04-29 22:58:41)
> gcc 10 and 11 produce an error when compiling libcamera in release mode:
> 
> In file included from ../../src/libcamera/base/object.cpp:13:
> ../../include/libcamera/base/message.h: In member function ‘void libcamera::Object::notifyThreadMove()’:
> ../../include/libcamera/base/message.h:58:47: error: array subscript ‘const libcamera::InvokeMessage[0]’ is partly outside array bounds of ‘libcamera::Message [1]’ [-Werror=array-bounds]
>    58 |         Semaphore *semaphore() const { return semaphore_; }
>       |                                               ^~~~~~~~~~
> ../../src/libcamera/base/object.cpp:280:17: note: while referencing ‘msg’
>   280 |         Message msg(Message::ThreadMoveMessage);
>       |                 ^~~
> 
> This seems to be a false positive, given that msg->type() can never be
> equal to Message::InvokeMessage in Object::message() when called from
> Object::notifyThreadMove(), as the message is created there with the
> Message::ThreadMoveMessage type. The problem as been reported in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105400, but the error
> nonetheless needs to be fixed without waiting for a new gcc release, and
> a dynamic_cast does the job with a small additional runtime cost that
> shouldn't be a big issue, given that moving objects between threads is a
> rare operation.

Agreed, I think we should just merge this as a suitable solution.

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

> Bug: https://bugs.libcamera.org/show_bug.cgi?id=125
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/base/object.cpp | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp
> index ec5b55d125d1..92cecd22fbe9 100644
> --- a/src/libcamera/base/object.cpp
> +++ b/src/libcamera/base/object.cpp
> @@ -189,7 +189,11 @@ void Object::message(Message *msg)
>  {
>         switch (msg->type()) {
>         case Message::InvokeMessage: {
> -               InvokeMessage *iMsg = static_cast<InvokeMessage *>(msg);
> +               /*
> +                * A static_cast should be enough, but gcc 10 and 11 choke on
> +                * it in release mode (with -O2 or -O3).
> +                */
> +               InvokeMessage *iMsg = dynamic_cast<InvokeMessage *>(msg);
>                 Semaphore *semaphore = iMsg->semaphore();
>                 iMsg->invoke();
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>

Patch
diff mbox series

diff --git a/src/libcamera/base/object.cpp b/src/libcamera/base/object.cpp
index ec5b55d125d1..92cecd22fbe9 100644
--- a/src/libcamera/base/object.cpp
+++ b/src/libcamera/base/object.cpp
@@ -189,7 +189,11 @@  void Object::message(Message *msg)
 {
 	switch (msg->type()) {
 	case Message::InvokeMessage: {
-		InvokeMessage *iMsg = static_cast<InvokeMessage *>(msg);
+		/*
+		 * A static_cast should be enough, but gcc 10 and 11 choke on
+		 * it in release mode (with -O2 or -O3).
+		 */
+		InvokeMessage *iMsg = dynamic_cast<InvokeMessage *>(msg);
 		Semaphore *semaphore = iMsg->semaphore();
 		iMsg->invoke();