Page MenuHomePhabricator

[analyzer] Fix in self assignment checker
Needs RevisionPublic

Authored by bruntib on Feb 7 2019, 6:07 AM.

Details

Summary

For self assignment checker it was necessary to force checking of assignment operators even if those are not called. The reason of this is to check whether "this" is equal to the address of the assignee object.

The buffer overlap checker checks if the intervals of the arguments of a memcpy() call are disjoint. If a class has an array member then the compiler generated assignment operator copies it with memcpy() function without checking self assignment at the beginning. Since the analyzer forces the check of assignment operators, the buffer overflow checker reported a false positive on classes with compiler generated assignment operator and array member.

This commit prevents the forced check of compiler generated assignment operators.

Diff Detail

Event Timeline

bruntib created this revision.Feb 7 2019, 6:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 6:07 AM
xazax.hun accepted this revision.Feb 11 2019, 3:20 AM

LG!

Just wanted to make sure I get it right. You did not add a test since it is only reproducible with an internal (non-upstreamed) checker. Since the change is trivial, I think it is ok to commit this without a test.

This revision is now accepted and ready to land.Feb 11 2019, 3:20 AM
NoQ added a comment.Feb 11 2019, 2:02 PM

Sounds reasonable, but it also sounds like something that should be reproducible on the upstream clang. Do you have a code snippet that causes the problematic AST to appear? Even if we don't have the false positive up here in upstream, Is it something we can test via -analyzer-display-progress | FileCheck or with the help of the analysis order checker or something like that?

Szelethus requested changes to this revision.Mar 12 2019, 2:59 AM

Yup, I agree.

This revision now requires changes to proceed.Mar 12 2019, 2:59 AM