This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Fix region cast between the same types with different qualifiers.
ClosedPublic

Authored by ASDenysPetrov on Nov 9 2021, 4:42 AM.

Details

Summary

Specifically, this fixes the case when we get an access to array element through the pointer to element. This covers several FIXME's. in D111654.
Example:

const int arr[4][2];
const int *ptr = arr[1]; // Fixes this.

The issue is that arr[1] is int* (&Element{Element{glob_arr5,1 S64b,int[2]},0 S64b,int}), and ptr is const int*. We don't take qualifiers into account. Consequently, we doesn't match the types as the same ones.

Diff Detail

Event Timeline

ASDenysPetrov created this revision.Nov 9 2021, 4:42 AM
ASDenysPetrov requested review of this revision.Nov 9 2021, 4:42 AM

Very solid patch!

clang/lib/StaticAnalyzer/Core/Store.cpp
98–99

You only use CanonPointeeTy.getLocalUnqualifiedType() all the places. Spell it once and reuse it everywhere.

105

For me at least, a name like IsSameRegionType() would imply a function with two parameters.

115

BTW do you know what isBoundable() is? I'm just curious.

182–183
ASDenysPetrov added inline comments.Nov 10 2021, 9:47 AM
clang/lib/StaticAnalyzer/Core/Store.cpp
98–99

Make sense!

105

I'll carry CanonPointeeTy out.

115

I've looked for information about it. It's poor, I'd say there is nothing of it. I'm curious as well.

182–183

+1

Updated according to comments.

ASDenysPetrov marked 4 inline comments as done.Nov 10 2021, 10:27 AM
steakhal accepted this revision.Nov 10 2021, 11:10 AM

Please clang-format the patch to reflow the touched comment. Given that's done, you are good to go.
Also, wait a couple days before committing to give chance to the rest of the reviewers to have a look.

This revision is now accepted and ready to land.Nov 10 2021, 11:10 AM

@steakhal Thank you!

Please clang-format the patch to reflow the touched comment. Given that's done, you are good to go.

The comment is OK. clang-format doesn't complain on it.

Also, wait a couple days before committing to give chance to the rest of the reviewers to have a look.

Sure.