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

memory safety: audit use of static variables #11130

Open
junr03 opened this issue May 10, 2020 · 5 comments
Open

memory safety: audit use of static variables #11130

junr03 opened this issue May 10, 2020 · 5 comments
Assignees
Labels

Comments

@junr03
Copy link
Member

junr03 commented May 10, 2020

We should audit the use of static variables in the codebase to prevent static initialization/deinitialization order problems internally, and for dependent projects.

The codebase already defines a construct on first use macro. We should audit other static variables that should be migrated, and enforce the pattern via code linting.

@junr03 junr03 added tech debt help wanted Needs help! no stalebot Disables stalebot from closing an issue labels May 10, 2020
@mattklein123
Copy link
Member

There is a clang-tidy rule that can check this. We tried it but then turned it off for some reason. @lizan knows the history here.

@jmarantz
Copy link
Contributor

/sub

@lizan
Copy link
Member

lizan commented May 11, 2020

There is a check in clang-tidy: https://clang.llvm.org/extra/clang-tidy/checks/fuchsia-statically-constructed-objects.html
We added and reverted in #5733

The problem was tests are registered via static registration mechanism which will be flagged by clang-tidy.

If we wrap those macro, i.e. ENVOY_TEST_F, ENVOY_TEST_P, then we can add NOLINT in the macro definition to workaround that.

@mattklein123
Copy link
Member

This has caused enough problems lately that I think we should turn this check on and deal with the macros. @lizan wdyt?

@mattklein123 mattklein123 added this to the 1.15.0 milestone May 11, 2020
@mattklein123 mattklein123 removed the no stalebot Disables stalebot from closing an issue label May 11, 2020
@lizan
Copy link
Member

lizan commented May 11, 2020

sounds reasonable.

@mattklein123 mattklein123 modified the milestones: 1.15.0, 1.16.0 Jun 17, 2020
@mattklein123 mattklein123 removed this from the 1.16.0 milestone Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants