Brodes/nested sizeof or operation in sizeof audit fixes #310
Conversation
| // Apparently derived types, eg., functoin pointers, aren't PointerType | ||
| // according to codeql, so special casing them out here. | ||
| other.getUnspecifiedType() instanceof DerivedType | ||
| or | ||
| op instanceof SizeofOperator and tmpMsg = "sizeof" | ||
| ) and | ||
| if sizeofExpr.isInMacroExpansion() | ||
| then message = tmpMsg + "(in a macro expansion)" | ||
| else message = tmpMsg | ||
| other.getUnspecifiedType() instanceof PointerType |
There was a problem hiding this comment.
Did you really mean for other.getUnspecifiedType() instanceof DerivedType? It's true that funmction pointers aren't PointerTypes, but then I would have assumed that you would do:
other.getUnspecifiedType() instanceof FunctionPointerType(or maybe FunctionPointerIshType.), but you use DerivedType which includes lots of other things such as reference types, and const/volatile types (but you are calling getUnspecifiedType so const/volatiles are removed. All in all, this is very confusing 😂 Also, since PointerTypes are also DerivedTypes what you wrote is equivalent to:
other.getUnspecifiedType() instanceof DerivedTypeThere was a problem hiding this comment.
I missed they shared a common ancestor of derived. I should just make it instanceof Derived Type like you mentioned. I'll make that change. I don't mind that derived type includes other things, but I if Derived type does include specifiers for some reason... that is confusing. Basically I wanted to make the pattern as generic as possible. (TYPE*)0 == (other). I suppose I could maybe ignore "other's" type might be the most general solution.
There was a problem hiding this comment.
Diving more, yea it is confusing that derived type includes the const/volatile specifiers. That's unfortunate. I'm going to special case to just function pointer as you suggest and see if it changes my results. If not, I'll submit a new PR with this fix.
Fixes false positives as a result of a new audit.