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

Bug 5232: Fix GCC v12 build [-Wuse-after-free] #1136

Conversation

eduard-bagdasaryan
Copy link
Contributor

@eduard-bagdasaryan eduard-bagdasaryan commented Sep 8, 2022

This warning is a false positive. Introduced in GCC v12, use-after-free
checks have attracted dozens of bug reports, with several reports still
open as of GCC v12.2, including one that looks very similar to our case:
https://gcc.gnu.org/bugzilla//show_bug.cgi?id=106776

Removing (excessive) "inline" works around this GCC bug. We do not have
a better workaround right now. If we see more false positives like this,
we may disable -Wuse-after-free until its support matures.

@eduard-bagdasaryan eduard-bagdasaryan marked this pull request as draft September 8, 2022 22:01
@eduard-bagdasaryan
Copy link
Contributor Author

This patch should fix the gcc-12 build problem, but it is not still clear why it made gcc-12 happy. We need some more time to figure out what the underlying problem is and whether this solution is the correct one (or come up with another solution).

Copy link
Contributor

@yadij yadij left a comment

Choose a reason for hiding this comment

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

Looks good and confirmed it fixes the build.

@yadij
Copy link
Contributor

yadij commented Sep 9, 2022

IMO commit this to fix the immediate build. RefCount has a number of issues identified long ago by static analysis, and this may just be a side effect of those that the newest GCC are more picky about.

@rousskov
Copy link
Contributor

rousskov commented Sep 9, 2022

Since there is no emergency here (AFAICT), I support spending a bit more time on trying to figure out what is really going on, now that we have a much easier-to-reproduce case than the Coverity ones.

  • If this is a false positive, then Coverity and GCC are making the same kind of mistake and are likely to keep making similar mistakes. It would be good to develop a better understanding of the underlying issue instead of just kicking the can down the road.
  • If this is a Squid bug, then fixing it is definitely more important than merging this PR on the fast track.

@rousskov
Copy link
Contributor

@eduard-bagdasaryan, thanks a lot for researching this problem! I have updated PR description to reflect your findings. Please adjust as needed. Once the PR description is agreed upon, we should merge this PR.

@rousskov rousskov marked this pull request as ready for review September 14, 2022 17:32
squid-anubis pushed a commit that referenced this pull request Sep 14, 2022
This warning is a false positive. Introduced in GCC v12, use-after-free
checks have attracted dozens of bug reports, with several reports still
open as of GCC v12.2, including one that looks very similar to our case:
https://gcc.gnu.org/bugzilla//show_bug.cgi?id=106776

Removing (excessive) "inline" works around this GCC bug. We do not have
a better workaround right now. If we see more false positives like this,
we may disable -Wuser-after-free until its support matures.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Sep 14, 2022
@eduard-bagdasaryan
Copy link
Contributor Author

The PR description looks correct now.

@rousskov rousskov added the S-waiting-for-committer privileged action is expected (and usually required) label Sep 14, 2022
@rousskov
Copy link
Contributor

Thank you. I will give Amos a few days to review the updated PR description before clearing this PR for merging. @yadij , please feel free to clear at any time.

squid-anubis pushed a commit that referenced this pull request Sep 14, 2022
This warning is a false positive. Introduced in GCC v12, use-after-free
checks have attracted dozens of bug reports, with several reports still
open as of GCC v12.2, including one that looks very similar to our case:
https://gcc.gnu.org/bugzilla//show_bug.cgi?id=106776

Removing (excessive) "inline" works around this GCC bug. We do not have
a better workaround right now. If we see more false positives like this,
we may disable -Wuse-after-free until its support matures.
@squid-anubis squid-anubis added M-passed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels labels Sep 14, 2022
@yadij yadij added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-committer privileged action is expected (and usually required) labels Sep 15, 2022
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-passed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Sep 15, 2022
kinkie pushed a commit to kinkie/squid that referenced this pull request Dec 28, 2022
This warning is a false positive. Introduced in GCC v12, use-after-free
checks have attracted dozens of bug reports, with several reports still
open as of GCC v12.2, including one that looks very similar to our case:
https://gcc.gnu.org/bugzilla//show_bug.cgi?id=106776

Removing (excessive) "inline" works around this GCC bug. We do not have
a better workaround right now. If we see more false positives like this,
we may disable -Wuse-after-free until its support matures.
kinkie pushed a commit to kinkie/squid that referenced this pull request Jul 2, 2023
This warning is a false positive. Introduced in GCC v12, use-after-free
checks have attracted dozens of bug reports, with several reports still
open as of GCC v12.2, including one that looks very similar to our case:
https://gcc.gnu.org/bugzilla//show_bug.cgi?id=106776

Removing (excessive) "inline" works around this GCC bug. We do not have
a better workaround right now. If we see more false positives like this,
we may disable -Wuse-after-free until its support matures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
4 participants