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

Silence CodeQL false positive warnings #4942

Merged

Conversation

StephanTLavavej
Copy link
Member

  • "Unclear validation of array index" silenced by // lgtm [cpp/unclear-array-index-validation]
    • __msvc_string_view.hpp 668: We have bool _Matches[256] and the index is cast to unsigned char.
    • __msvc_string_view.hpp 1334: basic_string_view::operator[] has optional validation controlled by _CONTAINER_DEBUG_LEVEL and intentionally doesn't validate by default.
    • charconv: _Digit_from_byte has 256 elements (enforced by _STL_INTERNAL_STATIC_ASSERT) and the index is cast to unsigned char.
  • "Cast from char* to wchar_t*" silenced by // lgtm [cpp/incorrect-string-type-conversion]
    • xutility: _Copy_memmove() is correctly bypassing pointer arithmetic for performance.
    • StlLCMapStringA.cpp: This function has LCMapStringA's narrow interface, but implements LCMAP_SORTKEY with wide LCMapStringEx. "The sort key is stored in the buffer and treated as an opaque array of bytes" so there is no correctness issue.

* "Unclear validation of array index" silenced by `// lgtm [cpp/unclear-array-index-validation]`
  + `__msvc_string_view.hpp` 668: We have `bool _Matches[256]` and the index is cast to `unsigned char`.
  + `__msvc_string_view.hpp` 1334: `basic_string_view::operator[]` has optional validation controlled by `_CONTAINER_DEBUG_LEVEL` and intentionally doesn't validate by default.
  + `charconv`: `_Digit_from_byte` has 256 elements (enforced by `_STL_INTERNAL_STATIC_ASSERT`) and the index is cast to `unsigned char`.
* "Cast from `char*` to `wchar_t*`" silenced by `// lgtm [cpp/incorrect-string-type-conversion]`
  + `xutility`: `_Copy_memmove()` is correctly bypassing pointer arithmetic for performance.
  + `StlLCMapStringA.cpp`: This function has [`LCMapStringA`][]'s narrow interface, but implements `LCMAP_SORTKEY` with wide [`LCMapStringEx`][]. "The sort key is stored in the buffer and treated as an opaque array of bytes" so there is no correctness issue.

[`LCMapStringA`]: https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-lcmapstringa
[`LCMapStringEx`]: https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-lcmapstringex
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Sep 7, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner September 7, 2024 18:27
@AlexGuteniev
Copy link
Contributor

  • __msvc_string_view.hpp 668: We have bool _Matches[256] and the index is cast to unsigned char.

This looks like clear false positive that might be reported

@StephanTLavavej
Copy link
Member Author

We're trying to get in touch with the maintainers to report severe usability issues - if we get traction on those, then we can report specific false positives like these.

@CaseyCarter CaseyCarter removed their assignment Sep 8, 2024
@StephanTLavavej StephanTLavavej self-assigned this Sep 9, 2024
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit e34645f into microsoft:main Sep 9, 2024
39 checks passed
@StephanTLavavej StephanTLavavej deleted the wisdom-like-silence branch September 9, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants