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

Remove std::copy from ETW exporter #756

Closed
maxgolov opened this issue May 14, 2021 · 0 comments · Fixed by #840
Closed

Remove std::copy from ETW exporter #756

maxgolov opened this issue May 14, 2021 · 0 comments · Fixed by #840
Assignees
Labels
area:exporter bug Something isn't working

Comments

@maxgolov
Copy link
Contributor

maxgolov commented May 14, 2021

This is related to controversial topic of Annex K bound checkers added as an optional extension in C11. This thread here provides some background detail, with reasonably written community feedback about the usefulness of Annex K.

ETW exporter uses std::copy here:

std::copy(spanId, spanId + 8, reinterpret_cast<uint8_t *>(&outGuid));

This triggers a false-positive with some strict code analysis tooling when Secure Development Lifecycle checks are turned on. I already recommended the customer to temporarily suppress the warning just around this compilation unit. Since it is false-positive in this context.

My goal is to fix this for ETW exporter. Unfortunately fixing it one thing at a time won't necessarily bring us anyhow closer to totally making code std::copy-free, since the same pattern is actively employed by nlohmann/json.hpp and some parts of Google Abseil library.

Perhaps part of the fix should be to establish a CI/CD pipeline for Windows, to ensure that the entire codebase (core parts of API and SDK) remain compliant with secure coding practices on Windows. It is progressively harder for many customers to keep suppressing these (perhaps unnecessarily strict) warnings in their own builds.

@maxgolov maxgolov added the bug Something isn't working label May 14, 2021
@maxgolov maxgolov self-assigned this May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:exporter bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant