Ericsson / clang

Cross Translation Unit analysis capability for Clang Static Analyzer. (Fork of official clang at http://llvm.org/git/clang)
http://clang.llvm.org/
Other
15 stars 10 forks source link

Fix in self assignment checker #573

Closed bruntib closed 5 years ago

bruntib commented 5 years ago

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

The buffer overflow 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.

dkrupp commented 5 years ago

Can one of the admins verify this patch?

Xazax-hun commented 5 years ago

Can you add a test? This should also upstreamed very soon. Before branching clang 8.

baloghadamsoftware commented 5 years ago

Please add a test. Please also submit this path on the Phabricator.

bruntib commented 5 years ago

May I get some help here, how could this fix be tested? My problem is the following:

When the buffer overlap checker is enabled then it reports on the following class:

class MyClass {
public:
  int arr[10];
};

int main() {
  MyClass c1, c2;
  c1 = c2;
}

The compiler generated assignment operator copies the array with memcpy(). Even if it is not a self assignment in main() function, the analyzer forced the top-level checking of the assignment operator. This current fix is about not forcing to check the compiler generated assignment operators in order to eliminate this false positive case. But this is observable only if buffer overlap checker is enabled, which is an internal alpha checker.

martong commented 5 years ago

Mr Jenkins, please run tests