Message ID | 20220429215841.19913-1-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Commit | 4f1d1a48fe7ebe1282d7608a59cb059e1367e584 |
Headers | show |
Series |
|
Related | show |
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 >
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();
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(-)