Skip to content

RULE-10-4: Improve essential type detection #745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 44 additions & 4 deletions c/misra/src/codingstandards/c/misra/EssentialTypes.qll
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,17 @@ EssentialTypeCategory getEssentialTypeCategory(Type type) {
essentialType.(IntegralType).isSigned() and
not essentialType instanceof PlainCharType
or
// Anonymous enums are considered to be signed
result = EssentiallySignedType() and
essentialType instanceof AnonymousEnumType and
not essentialType instanceof MisraBoolType
or
result = EssentiallyUnsignedType() and
essentialType.(IntegralType).isUnsigned() and
not essentialType instanceof PlainCharType
or
result = EssentiallyEnumType() and
essentialType instanceof Enum and
essentialType instanceof NamedEnumType and
not essentialType instanceof MisraBoolType
or
result = EssentiallyFloatingType() and
Expand Down Expand Up @@ -348,16 +353,51 @@ class EssentialBinaryArithmeticExpr extends EssentialExpr, BinaryArithmeticOpera
}
}

/**
* A named Enum type, as per D.5.
*/
class NamedEnumType extends Enum {
NamedEnumType() {
not isAnonymous()
or
exists(Type useOfEnum | this = useOfEnum.stripType() |
exists(TypedefType t | t.getBaseType() = useOfEnum)
or
exists(Function f | f.getType() = useOfEnum or f.getAParameter().getType() = useOfEnum)
or
exists(Struct s | s.getAField().getType() = useOfEnum)
or
exists(Variable v | v.getType() = useOfEnum)
)
}
}

/**
* An anonymous Enum type, as per D.5.
*/
class AnonymousEnumType extends Enum {
AnonymousEnumType() { not this instanceof NamedEnumType }
}

/**
* The EssentialType of an EnumConstantAccess, which may be essentially enum or essentially signed.
*/
class EssentialEnumConstantAccess extends EssentialExpr, EnumConstantAccess {
override Type getEssentialType() { result = getTarget().getDeclaringEnum() }
override Type getEssentialType() {
exists(Enum e | e = getTarget().getDeclaringEnum() |
if e instanceof NamedEnumType then result = e else result = stlr(this)
)
}
}

class EssentialLiteral extends EssentialExpr, Literal {
override Type getEssentialType() {
if this instanceof BooleanLiteral
then result instanceof MisraBoolType
then
// This returns a multitude of types - not sure if we really want that
result instanceof MisraBoolType
else (
if this.(CharLiteral).getCharacter().length() = 1
if this instanceof CharLiteral
then result instanceof PlainCharType
else
exists(Type underlyingStandardType |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ where
// be reported as non-compliant.
leftOpTypeCategory = EssentiallyEnumType() and
rightOpTypeCategory = EssentiallyEnumType() and
not leftOpEssentialType = rightOpEssentialType and
not leftOpEssentialType.getUnspecifiedType() = rightOpEssentialType.getUnspecifiedType() and
message =
"The operands of this operator with usual arithmetic conversions have mismatched essentially Enum types (left operand: "
+ leftOpEssentialType + ", right operand: " + rightOpEssentialType + ")."
Expand Down
17 changes: 17 additions & 0 deletions c/misra/test/c/misra/EssentialTypes.expected
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
| test.c:4:20:4:20 | 1 | signed char | signed char | essentially Signed type |
| test.c:4:20:4:20 | (unsigned int)... | unsigned int | unsigned int | essentially Unsigned type |
| test.c:5:23:5:23 | 1 | signed char | signed char | essentially Signed type |
Expand Down Expand Up @@ -73,3 +79,14 @@
| test.c:54:3:54:5 | ~ ... | int | int | essentially Signed type |
| test.c:54:4:54:5 | (int)... | int | int | essentially Signed type |
| test.c:54:4:54:5 | ss | signed short | signed short | essentially Signed type |
| test.c:63:30:63:32 | ((unnamed enum))... | (unnamed enum) | (unnamed enum) | essentially Enum Type |
| test.c:63:30:63:32 | EC5 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
| test.c:70:3:70:5 | EC1 | signed char | signed char | essentially Signed type |
| test.c:71:3:71:5 | EC2 | E1 | E1 | essentially Enum Type |
| test.c:72:3:72:5 | EC3 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
| test.c:73:3:73:5 | EC4 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
| test.c:74:3:74:5 | EC5 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
| test.c:75:3:75:5 | EC6 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
| test.c:79:3:79:5 | 97 | char | char | essentially Character type |
| test.c:80:3:80:6 | 10 | char | char | essentially Character type |
| test.c:81:3:81:6 | 0 | char | char | essentially Character type |
27 changes: 27 additions & 0 deletions c/misra/test/c/misra/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,31 @@ void testUnary() {
~us; // Should be essentially unsigned
~s; // Should be essentially signed
~ss; // Should be essentially signed
}

enum { EC1 };
enum E1 { EC2 };
typedef enum { EC3 } E2;

enum { EC4 } g;

enum { EC5 } test() { return EC5; }

struct S1 {
enum { EC6 } m;
};

void testEnums() {
EC1; // Should be essentially signed
EC2; // Should be essentially enum
EC3; // Should be essentially enum
EC4; // Should be essentially enum
EC5; // Should be essentially enum
EC6; // Should be essentially enum
}

void testControlChar() {
'a'; // Essentially char
'\n'; // Essentially char
'\0'; // Essentially char
}
9 changes: 9 additions & 0 deletions c/misra/test/rules/RULE-10-4/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,13 @@ void testOps() {
A < A; // COMPLIANT
e1a < e2a; // NON_COMPLIANT
A < D; // NON_COMPLIANT

enum { G };
s32 + G; // COMPLIANT
c == '\n'; // COMPLIANT

typedef enum { H } E3;

E3 e3a = H;
e3a < H; // COMPLIANT
}
5 changes: 5 additions & 0 deletions change_notes/2024-10-15-lits-and-constants-10-4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- `RULE-10-4` - `OperandswithMismatchedEssentialTypeCategory.ql`:
- Removed false positives where a specified or typedef'd enum type was compared to an enum constant type.
- `EssentialType` - for all queries related to essential types:
- `\n` and other control characters are now correctly deduced as essentially char type, instead of an essentially integer type.
- Enum constants for anonymous enums are now correctly deduced as an essentially signed integer type instead of essentially enum.
Loading