Skip to content

Commit 8c87dba

Browse files
authored
Merge pull request #745 from github/lcartey/rule-10-4-lits-and-consts
`RULE-10-4`: Improve essential type detection
2 parents 7736c34 + 2ae343c commit 8c87dba

File tree

6 files changed

+103
-5
lines changed

6 files changed

+103
-5
lines changed

c/misra/src/codingstandards/c/misra/EssentialTypes.qll

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,17 @@ EssentialTypeCategory getEssentialTypeCategory(Type type) {
130130
essentialType.(IntegralType).isSigned() and
131131
not essentialType instanceof PlainCharType
132132
or
133+
// Anonymous enums are considered to be signed
134+
result = EssentiallySignedType() and
135+
essentialType instanceof AnonymousEnumType and
136+
not essentialType instanceof MisraBoolType
137+
or
133138
result = EssentiallyUnsignedType() and
134139
essentialType.(IntegralType).isUnsigned() and
135140
not essentialType instanceof PlainCharType
136141
or
137142
result = EssentiallyEnumType() and
138-
essentialType instanceof Enum and
143+
essentialType instanceof NamedEnumType and
139144
not essentialType instanceof MisraBoolType
140145
or
141146
result = EssentiallyFloatingType() and
@@ -348,16 +353,51 @@ class EssentialBinaryArithmeticExpr extends EssentialExpr, BinaryArithmeticOpera
348353
}
349354
}
350355

356+
/**
357+
* A named Enum type, as per D.5.
358+
*/
359+
class NamedEnumType extends Enum {
360+
NamedEnumType() {
361+
not isAnonymous()
362+
or
363+
exists(Type useOfEnum | this = useOfEnum.stripType() |
364+
exists(TypedefType t | t.getBaseType() = useOfEnum)
365+
or
366+
exists(Function f | f.getType() = useOfEnum or f.getAParameter().getType() = useOfEnum)
367+
or
368+
exists(Struct s | s.getAField().getType() = useOfEnum)
369+
or
370+
exists(Variable v | v.getType() = useOfEnum)
371+
)
372+
}
373+
}
374+
375+
/**
376+
* An anonymous Enum type, as per D.5.
377+
*/
378+
class AnonymousEnumType extends Enum {
379+
AnonymousEnumType() { not this instanceof NamedEnumType }
380+
}
381+
382+
/**
383+
* The EssentialType of an EnumConstantAccess, which may be essentially enum or essentially signed.
384+
*/
351385
class EssentialEnumConstantAccess extends EssentialExpr, EnumConstantAccess {
352-
override Type getEssentialType() { result = getTarget().getDeclaringEnum() }
386+
override Type getEssentialType() {
387+
exists(Enum e | e = getTarget().getDeclaringEnum() |
388+
if e instanceof NamedEnumType then result = e else result = stlr(this)
389+
)
390+
}
353391
}
354392

355393
class EssentialLiteral extends EssentialExpr, Literal {
356394
override Type getEssentialType() {
357395
if this instanceof BooleanLiteral
358-
then result instanceof MisraBoolType
396+
then
397+
// This returns a multitude of types - not sure if we really want that
398+
result instanceof MisraBoolType
359399
else (
360-
if this.(CharLiteral).getCharacter().length() = 1
400+
if this instanceof CharLiteral
361401
then result instanceof PlainCharType
362402
else
363403
exists(Type underlyingStandardType |

c/misra/src/rules/RULE-10-4/OperandsWithMismatchedEssentialTypeCategory.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ where
3838
// be reported as non-compliant.
3939
leftOpTypeCategory = EssentiallyEnumType() and
4040
rightOpTypeCategory = EssentiallyEnumType() and
41-
not leftOpEssentialType = rightOpEssentialType and
41+
not leftOpEssentialType.getUnspecifiedType() = rightOpEssentialType.getUnspecifiedType() and
4242
message =
4343
"The operands of this operator with usual arithmetic conversions have mismatched essentially Enum types (left operand: "
4444
+ leftOpEssentialType + ", right operand: " + rightOpEssentialType + ")."

c/misra/test/c/misra/EssentialTypes.expected

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
2+
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
3+
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
4+
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
5+
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
6+
| file://:0:0:0:0 | 0 | signed char | signed char | essentially Signed type |
17
| test.c:4:20:4:20 | 1 | signed char | signed char | essentially Signed type |
28
| test.c:4:20:4:20 | (unsigned int)... | unsigned int | unsigned int | essentially Unsigned type |
39
| test.c:5:23:5:23 | 1 | signed char | signed char | essentially Signed type |
@@ -73,3 +79,14 @@
7379
| test.c:54:3:54:5 | ~ ... | int | int | essentially Signed type |
7480
| test.c:54:4:54:5 | (int)... | int | int | essentially Signed type |
7581
| test.c:54:4:54:5 | ss | signed short | signed short | essentially Signed type |
82+
| test.c:63:30:63:32 | ((unnamed enum))... | (unnamed enum) | (unnamed enum) | essentially Enum Type |
83+
| test.c:63:30:63:32 | EC5 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
84+
| test.c:70:3:70:5 | EC1 | signed char | signed char | essentially Signed type |
85+
| test.c:71:3:71:5 | EC2 | E1 | E1 | essentially Enum Type |
86+
| test.c:72:3:72:5 | EC3 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
87+
| test.c:73:3:73:5 | EC4 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
88+
| test.c:74:3:74:5 | EC5 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
89+
| test.c:75:3:75:5 | EC6 | (unnamed enum) | (unnamed enum) | essentially Enum Type |
90+
| test.c:79:3:79:5 | 97 | char | char | essentially Character type |
91+
| test.c:80:3:80:6 | 10 | char | char | essentially Character type |
92+
| test.c:81:3:81:6 | 0 | char | char | essentially Character type |

c/misra/test/c/misra/test.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,31 @@ void testUnary() {
5252
~us; // Should be essentially unsigned
5353
~s; // Should be essentially signed
5454
~ss; // Should be essentially signed
55+
}
56+
57+
enum { EC1 };
58+
enum E1 { EC2 };
59+
typedef enum { EC3 } E2;
60+
61+
enum { EC4 } g;
62+
63+
enum { EC5 } test() { return EC5; }
64+
65+
struct S1 {
66+
enum { EC6 } m;
67+
};
68+
69+
void testEnums() {
70+
EC1; // Should be essentially signed
71+
EC2; // Should be essentially enum
72+
EC3; // Should be essentially enum
73+
EC4; // Should be essentially enum
74+
EC5; // Should be essentially enum
75+
EC6; // Should be essentially enum
76+
}
77+
78+
void testControlChar() {
79+
'a'; // Essentially char
80+
'\n'; // Essentially char
81+
'\0'; // Essentially char
5582
}

c/misra/test/rules/RULE-10-4/test.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,13 @@ void testOps() {
3333
A < A; // COMPLIANT
3434
e1a < e2a; // NON_COMPLIANT
3535
A < D; // NON_COMPLIANT
36+
37+
enum { G };
38+
s32 + G; // COMPLIANT
39+
c == '\n'; // COMPLIANT
40+
41+
typedef enum { H } E3;
42+
43+
E3 e3a = H;
44+
e3a < H; // COMPLIANT
3645
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
- `RULE-10-4` - `OperandswithMismatchedEssentialTypeCategory.ql`:
2+
- Removed false positives where a specified or typedef'd enum type was compared to an enum constant type.
3+
- `EssentialType` - for all queries related to essential types:
4+
- `\n` and other control characters are now correctly deduced as essentially char type, instead of an essentially integer type.
5+
- Enum constants for anonymous enums are now correctly deduced as an essentially signed integer type instead of essentially enum.

0 commit comments

Comments
 (0)