Skip to content

Conversation

@lmilne-qnx
Copy link

Copy the QNX Probes proto configuration so traced_qnx_probes can be used with the main Perfetto project. This has been tested as working with traced_qnx_probes build from QNX Ports Perfetto repo and traced + perfetto build from this project

As part of this commit all changes in order to get Perfetto compiling on QNX were ported. These changes allow for building on QNX7.1 and QNX8.0 The changes are as listed

  • Fix the toolchain to not specify a gcc version as QNX7.1 and QNX8.0 differ
  • Add no-stringop-overflow for is_gcc in upd
  • in perfetto_cmd.cc prealloc vector to avoid -Warray-bounds for emplaced string this change won't affect the behaviour of the code but remove the error.
  • Add x86_64 support for QNX cross compiling.
  • Fix getpeereid to no longer take a NULL gid, this is because the change from iopkt to io-sock removes the ability to ignore NULL input args. No functional for iopkt but will fix segfault in libsocket for io-sock platforms.

@lmilne-qnx lmilne-qnx requested a review from a team as a code owner January 21, 2026 19:15
@google-cla
Copy link

google-cla bot commented Jan 21, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Member

@primiano primiano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very exciting. I have few very minor comments, but other than that LGTM.
Also note you need to sign the CLA in order to be able to contrib

#if PERFETTO_BUILDFLAG(PERFETTO_OS_QNX)
int fd = sock_raw_.fd();
int res = getpeereid(fd, &peer_uid_, nullptr);
gid_t peer_gid = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to add a fallback gid_t to base/sys_types.h or this might break windows (well this one line is behind ifdef... but if you use it elsewhere)

int res = getpeereid(fd, &peer_uid_, nullptr);
gid_t peer_gid = 0;
int res = getpeereid(fd, &peer_uid_, &peer_gid);
PERFETTO_CHECK(res == 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a ignore_result(peer_gid); ? I'm surprised the compiler doesn't warn about it being unused

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added to be more explicit that we don't care about the result but i expect it has something to do with passing it to the getpeereid function as the compiler doesn't know if it is used internally due to external linkage

TraceConfig test_config;
ConfigOptions opts;
opts.time = "2s";
opts.categories.reserve(4);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change? this is some code that is almsot never hit. this optimization here seems very debatable. This is by far NOT a perf-sensitive code.
I'd argue this it's not wroth neither the extra binary size, nor the extra line of scrolling while reading the code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely fair points, but I am not trying to optimize the code here 😄. on QNX8.0 the emplace of the const char* was causing an -Warray-bounds error, where it wasn't in QNX7.1 this is most likely a bug in the gcc version we are using but it is causing the build to fail. This fix seemed to be the least invasive while suppressing the error. I am open to other options here as well if you have any, but if possible I would like to avoid disabling the warning on the whole project for QNX as it still provides useful utility.

Another option is to use push instead of emplace which doesn't seem to have this issue, and should provide the same performance as previously as the const char* will need to be cast to a std::string anyways. That being said I think no matter what solution we come up with here won't affect performance anyways as you mentioned this is almost never hit and will at most get hit once during the run AFAIK.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand where the problem comes though. there is notthing bad doing push_back without reserve.
I am mainly curious if you have a minified repro on how that hits -Warray-bounds error

but also keep that as an aside, i'm okay keeping this, but curious what the warning is about as I can't see the problem here

@lmilne-qnx lmilne-qnx force-pushed the dev/lmilne/traced_qnx_probes_config branch from 43e1e87 to eb6371c Compare January 22, 2026 14:09

// Begin of protos/perfetto/config/qnx/qnx_config.proto

message QnxConfig {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@primiano are you okay with QNX providing a description to the proto and a link to the traced_qnx_probes repo. I think having a link to the actual source code would provide very meaningful context to this proto.

If not here then it would be nice for a reference to QNX's traced_qnx_probes repo to exists within the Perfetto repo, so that it is possible to go deeper into QNX's integration of Perfetto.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From our side I think this is a great idea, but will wait for confirmation before integrating the change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed change, leaving the solution generic so we won't need to update in case our branch name changes.

also still working on getting the CLA signed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@primiano are you okay with QNX providing a description to the proto and a link to the traced_qnx_probes repo. I think having a link to the actual source code would provide very meaningful context to this proto.

Yeah aboslutely. I think it's great to be able to see where this comes from.

@lmilne-qnx lmilne-qnx force-pushed the dev/lmilne/traced_qnx_probes_config branch from eb6371c to 68db0c3 Compare January 23, 2026 21:21
Copy the QNX Probes proto configuration so traced_qnx_probes can be
used with the main perfetto project. This has been tested as working
with traced_qnx_probes build from QNX repo and traced + perfetto build
from this project

As part of this commit all changes in order to get Perfetto compiling on
QNX were ported. These changes allow for building on QNX7.1 and QNX8.0
The changes are as listed
- Fix the toolchain to not specify a gcc version as QNX7.1 and QNX8.0 differ
- Disable no-stringop-overflow for is_gcc in upd
- in perfetto_cmd.cc prealloc vector to avoid -Warray-bounds for emplaced string
  this change won't affect the behaviour of the code but remove the error.
- Add x86_64 support for QNX cross compiling.
- Fix getpeereid to no longer take a NULL gid, this is because the change from
  iopkt to io-sock removes the ability to ignore NULL input args. No functional
  for iopkt but will fix segfault in libsocket for io-sock platforms.
@lmilne-qnx lmilne-qnx force-pushed the dev/lmilne/traced_qnx_probes_config branch from 68db0c3 to b280b11 Compare January 28, 2026 19:21
Copy link
Member

@primiano primiano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LG. land it whenever you sort out the CLA (I can't do anything about that)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants