Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC false-positives for Werror=maybe_uninitialized #4027

Open
jleibs opened this issue Oct 26, 2023 · 1 comment
Open

GCC false-positives for Werror=maybe_uninitialized #4027

jleibs opened this issue Oct 26, 2023 · 1 comment
Labels
😤 annoying Something in the UI / SDK is annoying to use 🌊 C++ API C/C++ API specific

Comments

@jleibs
Copy link
Member

jleibs commented Oct 26, 2023

Overview

We have a handful of places where we need to disabled -Wmaybe-uninitialized primarily related to usage of std::optional.

It seems that move-assigning to the std::optional can lead to situations where gcc thinks we are using uninitialized-memory because the memory in the optional hasn't been initialized.

This issue is to track those usages and maybe review or better work around them in the future.

Archetypes

  • Inside of with_component() builders, the std::move(*this) move-constructs from a possible uninitialized optional. This sadly needs to be code-generated into every archetype.

ComponentBatches

  • Inside the move-assignment, the call to this->swap(other); seems like it's possibly happening on uninitialized memory. This one in particular seems suspicious.

TranslationRotationScale

  • Contains optional union-types that seem to cause problems during copy-construction.

Future Work

I suspect one approach to mitigating these would be to avoid some of our usages of swap. We aren't required to swap data back into the moved-from objects. Rather we should free anything necessary immediately (rather than counting on the moved-from-r-values destructor), copy the data over from the r-value, and then explicitly set it to an uninitialized state.

@jleibs jleibs added 🎄 tracking issue issue that tracks a bunch of subissues 😤 annoying Something in the UI / SDK is annoying to use 🌊 C++ API C/C++ API specific labels Oct 26, 2023
jleibs added a commit that referenced this issue Oct 26, 2023
…zed memory in std::optionals (#4030)

### What
We have a handful of places where we need to disable
`-Wmaybe-uninitialized` primarily related to usage of `std::optional`.

It seems that move-assigning to the std::optional can lead to situations
where gcc thinks we are using uninitialized-memory because the memory in
the optional hasn't been initialized.

Tracking these now in: #4027

A few of these still make me uncomfortable, but seems better than having
a pile of unusable warning spam.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/4030) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4030)
- [Docs
preview](https://rerun.io/preview/f7c01adf1874fed69c13f904d1c810da91e69b0a/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/f7c01adf1874fed69c13f904d1c810da91e69b0a/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
@Wumpf
Copy link
Member

Wumpf commented Jan 23, 2025

The problem with with_component on archetypes went away with eager serialization. Those suppressions got removed in

Others are still around. Might need another pass.

@Wumpf Wumpf removed the 🎄 tracking issue issue that tracks a bunch of subissues label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use 🌊 C++ API C/C++ API specific
Projects
None yet
Development

No branches or pull requests

2 participants