This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] extend bugprone-signed-char-misuse check with array subscript case.
ClosedPublic

Authored by ztamas on Apr 27 2020, 1:43 AM.

Details

Summary

To cover STR34-C rule's second use case, where `signed char` is
used for array subscript after an integer conversion. In the case
of non-ASCII character this conversion will result in a value
in excess of UCHAR_MAX.

There is another clang-tidy check which catches these cases.
cppcoreguidelines-pro-bounds-constant-array-index catches any
indexing which is not integer constant. I think this check is
very strict about the index (e.g. constant), so it's still useful
to cover the `signed char` use case in this check, so we
can provide a way to catch the SEI cert rule's use cases on a
codebase, where this CPP guideline is not used.

Diff Detail

Event Timeline

ztamas created this revision.Apr 27 2020, 1:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2020, 1:43 AM

I run the check both on LibreOffice and LLVM codebase but did not find any catch. I expect that it's not a frequent use case.
I added this extension only to fully cover the CERT rule. If this patch is accepted I can add a cert alias for this check.

ztamas updated this revision to Diff 260243.Apr 27 2020, 1:52 AM

Fix typo

Harbormaster completed remote builds in B54761: Diff 260243.
Eugene.Zelenko added a project: Restricted Project.
ztamas edited the summary of this revision. (Show Details)Apr 28 2020, 1:36 AM
aaron.ballman accepted this revision.Apr 28 2020, 11:15 AM

LGTM aside from some minor nits.

clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
106

singed -> signed

clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
36

unequality -> inequality

37

to integer before array subscript -> to an integer in the array subscript

This revision is now accepted and ready to land.Apr 28 2020, 11:15 AM
ztamas updated this revision to Diff 261634.May 2 2020, 5:03 AM

Spelling / grammar fixes.

ztamas marked 3 inline comments as done.May 2 2020, 5:03 AM
This revision was automatically updated to reflect the committed changes.