Merge pull request #703 from github/lcartey/language-extensions-improvements

Rule 1.2: Address false positives for variable length arrays, improve reporting

Author: nicolaswill

c/common/src/codingstandards/c/Extensions.qll

@@ -4,21 +4,28 @@ import codingstandards.cpp.Extensions
 /**
  * Common base class for modeling compiler extensions.
  */
-abstract class CCompilerExtension extends CompilerExtension { }
+abstract class CCompilerExtension extends CompilerExtension {
+  abstract string getMessage();
+}
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#Other-Builtins
 abstract class CConditionalDefineExtension extends CCompilerExtension, PreprocessorIfdef {
+  string feature;
+
   CConditionalDefineExtension() {
-    exists(toString().indexOf("__has_builtin")) or
-    exists(toString().indexOf("__has_constexpr_builtin")) or
-    exists(toString().indexOf("__has_feature")) or
-    exists(toString().indexOf("__has_extension")) or
-    exists(toString().indexOf("__has_attribute")) or
-    exists(toString().indexOf("__has_declspec_attribute")) or
-    exists(toString().indexOf("__is_identifier")) or
-    exists(toString().indexOf("__has_include")) or
-    exists(toString().indexOf("__has_include_next")) or
-    exists(toString().indexOf("__has_warning"))
+    feature =
+      [
+        "__has_builtin", "__has_constexpr_builtin", "__has_feature", "__has_extension",
+        "__has_attribute", "__has_declspec_attribute", "__is_identifier", "__has_include",
+        "__has_include_next", "__has_warning"
+      ] and
+    exists(toString().indexOf(feature))
+  }
+
+  override string getMessage() {
+    result =
+      "Call to builtin function '" + feature +
+        "' is a compiler extension and is not portable to other compilers."
   }
 }
 
@@ -31,6 +38,12 @@ class CMacroBasedExtension extends CCompilerExtension, Macro {
         "__clang_version__", "__clang_literal_encoding__", "__clang_wide_literal_encoding__"
       ]
   }
+
+  override string getMessage() {
+    result =
+      "Use of builtin macro '" + getBody() +
+        "' is a compiler extension and is not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Variable-Attributes.html#Variable-Attributes
@@ -41,6 +54,12 @@ class CAttributeExtension extends CCompilerExtension, Attribute {
         "fallthrough", "read_only", "alias"
       ]
   }
+
+  override string getMessage() {
+    result =
+      "Use of attribute  '" + getName() +
+        "' is a compiler extension and is not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#g_t_005f_005fsync-Builtins
@@ -61,21 +80,41 @@ class CFunctionExtension extends CCompilerExtension, FunctionCall {
     // the built-in extensions
     getTarget().getName().indexOf("__builtin_") = 0
   }
+
+  override string getMessage() {
+    result =
+      "Call to builtin function '" + getTarget().getName() +
+        "' is a compiler extension and is not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Alignment.html#Alignment
 class CFunctionLikeExtension extends CCompilerExtension, AlignofExprOperator {
   CFunctionLikeExtension() { exists(getValueText().indexOf("__alignof__")) }
+
+  override string getMessage() {
+    result = "'__alignof__' is a compiler extension and is not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html#Statement-Exprs
-class CStmtExprExtension extends CCompilerExtension, StmtExpr { }
+class CStmtExprExtension extends CCompilerExtension, StmtExpr {
+  override string getMessage() {
+    result =
+      "Statement expressions are a compiler extension and are not portable to other compilers."
+  }
+}
 
 // Use of ternary like the following: `int a = 0 ?: 0;` where the
 // one of the branches is omitted
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html#Conditionals
 class CTerseTernaryExtension extends CCompilerExtension, ConditionalExpr {
   CTerseTernaryExtension() { getCondition() = getElse() or getCondition() = getThen() }
+
+  override string getMessage() {
+    result =
+      "Ternaries with omitted middle operands are a compiler extension and is not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fint128.html#g_t_005f_005fint128
@@ -87,31 +126,63 @@ class CRealTypeExtensionExtension extends CCompilerExtension, DeclarationEntry {
     getType() instanceof Decimal64Type or
     getType() instanceof Float128Type
   }
+
+  override string getMessage() {
+    result = "Decimal floats are a compiler extension and are not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fint128.html#g_t_005f_005fint128
 class CIntegerTypeExtension extends CCompilerExtension, DeclarationEntry {
   CIntegerTypeExtension() { getType() instanceof Int128Type }
+
+  override string getMessage() {
+    result = "128-bit integers are a compiler extension and are not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Long-Long.html#Long-Long
 class CLongLongType extends CCompilerExtension, DeclarationEntry {
   CLongLongType() { getType() instanceof LongLongType }
+
+  override string getMessage() {
+    result =
+      "Double-Word integers are a compiler extension and are not portable to other compilers."
+  }
 }
 
 class CZeroLengthArraysExtension extends CCompilerExtension, DeclarationEntry {
   CZeroLengthArraysExtension() { getType().(ArrayType).getArraySize() = 0 }
+
+  override string getMessage() {
+    result = "Zero length arrays are a compiler extension and are not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Empty-Structures.html#Empty-Structures
 class CEmptyStructExtension extends CCompilerExtension, Struct {
   CEmptyStructExtension() { not exists(getAMember(_)) }
+
+  override string getMessage() {
+    result = "Empty structures are a compiler extension and are not portable to other compilers."
+  }
 }
 
 // Reference: https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html#Variable-Length
-class CVariableLengthArraysExtension extends CCompilerExtension, DeclarationEntry {
+class CVariableLengthArraysExtension extends CCompilerExtension, Field {
   CVariableLengthArraysExtension() {
     getType() instanceof ArrayType and
-    not getType().(ArrayType).hasArraySize()
+    not getType().(ArrayType).hasArraySize() and
+    // Not the final member of the struct, which is allowed to be variably sized
+    not exists(int lastIndex, Class declaringStruct |
+      declaringStruct = getDeclaringType() and
+      lastIndex = count(declaringStruct.getACanonicalMember()) - 1 and
+      this = declaringStruct.getCanonicalMember(lastIndex)
+    )
+  }
+
+  override string getMessage() {
+    result =
+      "Variable length arrays are a compiler extension and are not portable to other compilers."
   }
 }

c/misra/src/rules/RULE-1-2/LanguageExtensionsShouldNotBeUsed.ql

@@ -13,8 +13,9 @@
 
 import cpp
 import codingstandards.c.misra
+import codingstandards.cpp.AlertReporting
 import codingstandards.c.Extensions
 
 from CCompilerExtension e
 where not isExcluded(e, Language3Package::languageExtensionsShouldNotBeUsedQuery())
-select e, "Is a compiler extension and is not portable to other compilers."
+select MacroUnwrapper<CCompilerExtension>::unwrapElement(e), e.getMessage()