This is an archive of the discontinued LLVM Phabricator instance.

[dfsan] Fix a bug in strcasecmp() and strncasecmp(): Compare the lowercase versions of the characters when choosing a return value.
ClosedPublic

Authored by skerner on Apr 20 2020, 6:45 AM.

Details

Summary

Resolves this bug:

https://bugs.llvm.org/show_bug.cgi?id=38369

Event Timeline

skerner created this revision.Apr 20 2020, 6:45 AM
Herald added a subscriber: Restricted Project. · View Herald TranscriptApr 20 2020, 6:45 AM
morehouse accepted this revision.Apr 20 2020, 8:48 AM
morehouse added inline comments.
compiler-rt/test/dfsan/custom.cpp
217

IIUC, this test would still pass with the buggy code. I think the case we want is:

s1 = "ABCZ";
s2 = "abcy";
...
assert(rv > 0);

Since 'y' - 'Z' > 0 but 'y' - 'z' < 0.

264

Same as above.

This revision is now accepted and ready to land.Apr 20 2020, 8:48 AM
skerner updated this revision to Diff 258811.Apr 20 2020, 12:30 PM
skerner marked 2 inline comments as done.

Address review comments.

compiler-rt/test/dfsan/custom.cpp
217

Since 'y' - 'Z' > 0 but 'y' - 'z' < 0.

You are correct. It has been a few decades since I looked at an ASCII-code chart, and I was under the impression that lowercase chars had smaller values than uppercase.

Fixed.

morehouse requested changes to this revision.Apr 20 2020, 2:51 PM

Looks like the diff lost the actual fix. Please add it back and I'll merge.

This revision now requires changes to proceed.Apr 20 2020, 2:51 PM
skerner updated this revision to Diff 258850.Apr 20 2020, 3:18 PM
skerner edited the summary of this revision. (Show Details)

Address review comments.

skerner updated this revision to Diff 258852.Apr 20 2020, 3:24 PM

Address review comments.

skerner updated this revision to Diff 258854.Apr 20 2020, 3:26 PM

Diff issues...

skerner updated this revision to Diff 258855.Apr 20 2020, 3:27 PM

All changes in one commit.

Harbormaster completed remote builds in B54013: Diff 258852.
Harbormaster completed remote builds in B54015: Diff 258854.
Harbormaster completed remote builds in B54016: Diff 258855.

Looks like the diff lost the actual fix. Please add it back and I'll merge.

Done. PTAL.

This revision is now accepted and ready to land.Apr 20 2020, 5:12 PM
This revision was automatically updated to reflect the committed changes.