MISRA Compliance Update (#121)

* Updated the MISRA.md and misra.config files after meeting with senior
SDE.
Put inline supression for a comparison related to the SIZE_MAX macro.
Want to get clarification about the line before putting a change in.

* Fixing some whitespace/formatting issues

* Changing MISRA.md file to reflect new format, modified inline supression in source file to match new formatting

* Adding words to lexicon, and fixing links

* Minor update to MISRA.md file to use an actual violation as the example, and expanding on a message

* Update source/core_json.c

Remove extra set of square brackets

Co-authored-by: Aniruddha Kanhere <[email protected]>

* Changes to the way we determine the end in skipOneHexEscape()

* Removed a redundant check of a variable

* Formatting fix and adding a test in to get more line coverage

* Trying to reach 100% branch coverage

* Adding the removal of debug for coverity target, and then removing two rule exceptions from the misra.config due to the change

* skipOneHexEscape had a line that was flagged as a MISRA 14.3 rule
violation. It was flagged because it believed that the if statement
comparison was invariant. This could be proven as a bug by assigning the
variable a value larger than the comparison, and then still receiving
the violation. A logic change has been made to get around this, but it
now fails CBMC proofs. Trying a different if statement to see if this
passes checks.

* Forgot to add inital assign back in

* After a lot of attempts to create a MISRA and CBMC compliant version of
skipOneHexEscape() I believe proof was found that shows the MISRA
violation is a false flag. Due to this I believe that this should simply
receive a suppression and the focus should be on the CBMC proofs.

Co-authored-by: Soren Ptak <[email protected]>
Co-authored-by: Aniruddha Kanhere <[email protected]>

MISRA.md

@@ -2,25 +2,30 @@
 
 The coreJSON library files conform to the [MISRA C:2012](https://www.misra.org.uk)
 guidelines, with some noted exceptions. Compliance is checked with Coverity static analysis.
-Deviations from the MISRA standard are listed below:
+The specific deviations, suppressed inline, are listed below.
 
-### Ignored by [Coverity Configuration](tools/coverity/misra.config)
-| Deviation | Category | Justification |
-| :-: | :-: | :-: |
-| Directive 4.9 | Advisory | Allow inclusion of function like macros. |
-| Rule 3.1 | Required | Allow nested comments. C++ style `//` comments are used in example code within Doxygen documentation blocks. |
-| Rule 8.13 | Advisory | Allow one function to have a char * argument without const qualifier. |
-| Rule 15.4 | Advisory | Allow more then one `break` statement to terminate a loop. |
-| Rule 19.2 | Advisory | Allow a `union` of a signed and unsigned type of identical sizes. |
-| Rule 20.12 | Required | Allow use of `assert()`, which uses a parameter in both expanded and raw forms. |
-
-### Flagged by Coverity
-| Deviation | Category | Justification |
-| :-: | :-: | :-: |
-| Rule 2.5 | Advisory | A macro is not used by the library; however, it exists to be used by an application. |
-| Rule 8.7 | Advisory | API functions are not used by the library; however, they must be externally visible in order to be used by an application. |
+Additionally, [MISRA configuration file](https://github.com/FreeRTOS/coreJSON/blob/main/tools/coverity/misra.config) contains the project wide deviations.
 
 ### Suppressed with Coverity Comments
-| Deviation | Category | Justification |
-| :-: | :-: | :-: |
-| Rule 11.3 | Required | False positive - the rule permits type qualifiers to be added. |
+To find the violation references in the source files run grep on the source code
+with ( Assuming rule 11.3 violation; with justification in point 1 ):
+```
+grep 'MISRA Ref 11.3.1' . -rI
+```
+
+#### Rule 11.3
+_Ref 11.3.1_
+
+- MISRA C-2012 Rule 11.3 prohibits casting a pointer to a different type.
+        This instance is a false positive, as the rule permits the
+        addition of a const type qualifier.
+
+#### Rule 14.3
+_Ref 14.3.1_
+
+- MISRA C-2012 Rule 14.3 False positive as the static analysis tool believes
+        i can never be larger than SIZE_MAX - HEX_ESCAPE_LENGTH. This can be proven as
+        a bug by setting i to be 18446744073709551615UL at initial assignment, then require
+        start != NULL before assigning the vaue of i to start. This creates a case
+        where i should be large enough to hit the else statement, but the tool still flags
+        this as invariant.

lexicon.txt

@@ -32,8 +32,10 @@ fd
 fe
 ff
 ffff
+freertos
 foo
 gcc
+github
 html
 https
 ifndef

source/core_json.c

@@ -335,6 +335,10 @@ static bool skipOneHexEscape( const char * buf,
 
     i = *start;
 #define HEX_ESCAPE_LENGTH    ( 6U )   /* e.g., \u1234 */
+
+    /* MISRA Ref 14.3.1 [Configuration dependent invariant] */
+    /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-143 */
+    /* coverity[misra_c_2012_rule_14_3_violation] */
     end = ( i <= ( SIZE_MAX - HEX_ESCAPE_LENGTH ) ) ? ( i + HEX_ESCAPE_LENGTH ) : SIZE_MAX;
 
     if( ( end < max ) && ( buf[ i ] == '\\' ) && ( buf[ i + 1U ] == 'u' ) )
@@ -1677,9 +1681,8 @@ JSONStatus_t JSON_SearchT( char * buf,
                            size_t * outValueLength,
                            JSONTypes_t * outType )
 {
-    /* MISRA Rule 11.3 prohibits casting a pointer to a different type.
-     * This instance is a false positive, as the rule permits the
-     * addition of a type qualifier. */
+    /* MISRA Ref 11.3.1 [Misaligned access] */
+    /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-113 */
     /* coverity[misra_c_2012_rule_11_3_violation] */
     return JSON_SearchConst( ( const char * ) buf, max, query, queryLength,
                              ( const char ** ) outValue, outValueLength, outType );

test/CMakeLists.txt

@@ -41,6 +41,9 @@ add_library( coverity_analysis
 # JSON public include path.
 target_include_directories( coverity_analysis PUBLIC ${JSON_INCLUDE_PUBLIC_DIRS} )
 
+# When building the coverity analysis target we disable debug
+target_compile_options(coverity_analysis PUBLIC -DNDEBUG )
+
 #  ====================================  Test Configuration ========================================
 
 # Include Unity build configuration.

tools/coverity/misra.config

@@ -10,6 +10,19 @@
             category: "Advisory",
             reason: "Allow inclusion of function like macros."
         },
+        {
+            deviation: "Rule 2.5",
+            reason: "Allow unused macros. Library headers may define macros intended for the application's use, but not used by a specific file."
+        },
+        {
+            deviation: "Rule 3.1",
+            category: "Required",
+            reason: "Allow nested comments. Documentation blocks contain comments for example code."
+        },
+        {
+            deviation: "Rule 8.7",
+            reason: "API functions are not used by library. They must be externally visible in order to be used by the application."
+        },
         {
             deviation: "Rule 8.13",
             category: "Advisory",
@@ -25,15 +38,5 @@
             category: "Advisory",
             reason: "Allow a union of a signed and unsigned type of identical sizes."
         },
-        {
-            deviation: "Rule 3.1",
-            category: "Required",
-            reason: "Allow nested comments. Documentation blocks contain comments for example code."
-        },
-        {
-            deviation: "Rule 20.12",
-            category: "Required",
-            reason: "Allow use of assert(), which uses a parameter in both expanded and raw forms."
-        },
     ]
 }