View Dror Gluska's profile |
| Source |
Writing code and testing it with unit and integration tests can provide high quality executable, however, it does not indicate if its a maintainable code, moreover code can have side effects when used outside its designed use, so in essence, tests pass, QA approved but then it gets in the field and crashes.
One of the ways to make sure code is maintainable, understandable and quick to get into is code reviews, however, using style checks and static code analysis can help automate some of it before a human can review it.
Applications are like Swiss cheese, one tool can verify certain aspects of it but many tools can discover more possible issues.
Code Style
Code style defines how a code looks like to the developer, it will most likely not affect how the code is run but it can affect how quickly a developer can understand what its doing.
clang-format
pip install clang-format
clang-format -style=LLVM -dump-config > .clang-format
pio run -t format
cpplint
cpplint is a tool that checks Google's C++ Style Guide, it can report the issues to command line or you can use the VSCode Extension and see the issues while writing code.
While the style guide is very strict, you can control how cpplint behaves by adjusting the linelength and which rule to apply or ignore in cpplint.cfg
For PlatformIO specific integration you can use my runner and execute it as follows:
pio run -t lint
By combining clang-format and cpplint you can avoid ever styling your code manually
Static Code Analysis
While coding style can really help for readability of code and preventing confusion, styling by itself does not contribute to the quality of code executed. Lets explore what is availableCompiler Warnings
A great source of warnings and issues that originate from mistakes or lack of understanding of C/C++ languages is compiler warnings, the default warning level attempts to balance between certain mistakes and an attempt not to overwhelm the developer.
These flags can help you to switch between simple mistakes and pedantic development style, if you find yourself in a pickle or would like to avoid the pickle all together it might be beneficial to use the more restrictive warning levels but like everything in software, the tool does not make the software and you'll need to understand why you need to fix what the compiler tells you to fix.
Chris Coleman wrote about The Best and Worst GCC Compiler Flags For Embedded other than the GCC warnings documentation but in summary:
-Wall - enables warnings about questionable practices that are easy to avoid
-Wextra - more warnings
-Wshadow - shadowing is a readability issue that can also lead to bugs since the developer might get confused about which variable is actually in use.
-Wdouble-promotion - some MCUs have FPU that supports floats only, whenever a floating point gets promoted to double for any reason this warning will tell you about it since you might lose performance over it.
-Wformat=2 - checks scanf and printf mistakes
-Wformat-truncation - checks snprintf has enough room, heuristics based.
-Wundef - warning if undefined identifier is evaluated in the preprocessor.
-Weffc++ - Warn about violations of style guidelines from Scott Meyers’ Effective C++ series of books
In any case, you can always view which warnings are enabled by:
gcc -Q --help=warnings
If you'd like to see which ones are enabled with using a certain warning level:
gcc -Wall -Wextra -Q --help=warnings
PlatformIO Check
check_tool = cppcheck, clangtidy
pio check
Checking native > cppcheck (platform: native) ---------------------------------------------------------------------------------------------- src\port_arduino.h:6: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] ... ================================ [PASSED] Took 3.53 seconds ==================================
Checking native > clangtidy (platform: native) ---------------------------------------------------------------------------------------------- src\main.cpp:6: [medium:warning] system include stdio.h not allowed [llvmlibc-restrict-system-libc-headers] ... ================================ [PASSED] Took 1.34 seconds ================================== Component HIGH MEDIUM LOW ------------------ ------ -------- ----- lib\circularbuffer 0 0 32 ... Total 2 17 60 Environment Tool Status Duration ------------- --------- -------- ------------ native cppcheck PASSED 00:00:03.533 native clangtidy PASSED 00:00:01.341 ================================= 2 succeeded in 00:00:04.874 ================================
cppcheck
cppcheck is a free static code analyzer, it detects common mistakes and also supports a subset of MISRA standard, you can find examples and explanations in the Zephyr documentation.
To support MISRA checks, you'll need to add a few things to the default PlatformIO configuration.
1. in platformio.ini section:
check_flags = cppcheck: --enable=all --addon=./scripts/misra.json --addon=cert --addon=threadsafety --addon=y2038
2. download misra.py, misra_9.py and cppcheckdata.py from cppcheck repository and place it in scripts folder.
3. add misra.json to the scripts folder, this is the configuration for the MISRA addon, I've disabled rule 17.7 in this example.
{
"script": "scripts/misra.py",
"args": ["--rule-texts=scripts/misra.txt","--suppress-rules 17.7"]
}
4. download misra.txt and place it in the scripts folder for the addon to pick up and use as messages.
This should get you results similar to this when running the checks:
Checking native > cppcheck (platform: native) -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- src\port_arduino.h:6: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] src\port_arduino.h:7: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] src\port_arduino.h:10: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] lib\examplelib\ProductionCode.h:3: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] src\main.cpp:6: [low:style] The Standard Library input/output functions shall not be used [misra-c2012-21.6] src\main.cpp:22: [low:style] Do not use the rand() function for generating pseudorandom numbers [cert-MSC30-c] lib\circularbuffer\CircularBuffer.cpp:32: [low:style] There should be no unused parameters in functions [misra-c2012-2.7] lib\circularbuffer\CircularBuffer.cpp:88: [low:style] A string literal shall not be assigned to an object unless the object's type is pointer to const-qualified char [misra-c2012-7.4] lib\circularbuffer\CircularBuffer.cpp:102: [low:style] A string literal shall not be assigned to an object unless the object's type is pointer to const-qualified char [misra-c2012-7.4] lib\circularbuffer\CircularBuffer.cpp:105: [low:style] A string literal shall not be assigned to an object unless the object's type is pointer to const-qualified char [misra-c2012-7.4] lib\circularbuffer\CircularBuffer.h:50: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] lib\circularbuffer\CircularBuffer.h:52: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] lib\circularbuffer\CircularBuffer.h:53: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2] lib\circularbuffer\CircularBuffer.h:54: [low:style] Function types shall be in prototype form with named parameters [misra-c2012-8.2]
clang-tidy
pio check --flags "clangtidy: --fix"
Checking native > clangtidy (platform: native) -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- src\main.cpp:6: [medium:warning] system include stdio.h not allowed [llvmlibc-restrict-system-libc-headers] src\main.cpp:6: [medium:warning] inclusion of deprecated C++ header 'stdio.h'; consider using 'cstdio' instead [hicpp-deprecated-headers,modernize-deprecated-headers] src\main.cpp:9: [medium:warning] system include stdlib.h not allowed [llvmlibc-restrict-system-libc-headers] src\main.cpp:9: [medium:warning] inclusion of deprecated C++ header 'stdlib.h'; consider using 'cstdlib' instead [hicpp-deprecated-headers,modernize-deprecated-headers] src\main.cpp:16: [medium:warning] declaration must be declared within the '__llvm_libc' namespace [llvmlibc-implementation-in-namespace]
Flawfinder
pio run -t flawfinder
This should get you results similar to this when running flawfinder:
flawfinder -C -c -D -i -S -Q include src lib\arduino-printf lib\circularbuffer lib\defectedLib lib\examplelib lib\runner
src\main.cpp:21:2: [0] (format) printf:If format strings can be influenced by an attacker, they can be exploited (CWE-134). Use a constant for the format specification. Constant format string, so not considered risky.
printf("test broken %d\r\n", FindFunction_WhichIsBroken(78));
src\main.cpp:24:2: [0] (format) printf:If format strings can be influenced by an attacker, they can be exploited (CWE-134). Use a constant for the format specification. Constant format string, so not considered risky.
printf("displaying float %.6f", c);
lib\circularbuffer\CircularBuffer.cpp:90:5: [2] (buffer) char:Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120). Perform bounds checking, use functions
that limit length, or ensure that the size is larger than the maximum possible length.
char sval[10];
lib\circularbuffer\CircularBuffer.cpp:98:9: [0] (format) snprintf:If format strings can be influenced by an attacker, they can be exploited, and note that sprintf variations do not always \0-terminate (CWE-134). Use a constant for the format specification. Constant format string, so not considered risky.
snprintf(sval, sizeof(sval), "%d", buffer[printIndex]);
lib\circularbuffer\CircularBuffer.cpp:90:5: [2] (buffer) char:Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120). Perform bounds checking, use functions
that limit length, or ensure that the size is larger than the maximum possible length.
char sval[10];doxygen
while doxygen is not a static code analyzer per-se, it can help to generate documentation, call graphs and help other developers to understand the code in shorter time.
To ease with function documentation, you can use the Doxygen VSCode Extension.
Install doxygen and graphviz which is used to generate the graphs.
generate a basic configuration file:
doxygen -g .doxygen
As a quick start override the following settings:
OUTPUT_DIRECTORY = docs INPUT = lib src BUILTIN_STL_SUPPORT = YES EXTRACT_ALL = YES EXTRACT_STATIC = YES WARN_NO_PARAMDOC = YES RECURSIVE = YES STRIP_CODE_COMMENTS = NO REFERENCED_BY_RELATION = YES REFERENCES_RELATION = YES GENERATE_LATEX = NO MACRO_EXPANSION = YES HAVE_DOT = YES UML_LOOK = YES CALL_GRAPH = YES CALLER_GRAPH = YES INTERACTIVE_SVG = YES
Generate the documentation:
doxygen .doxygen
doxygen .doxygen
And finally, you should be able to see the documentation by opening the docs/html/index.html, search for functions and browse the documentation, you'll see that just after a few minutes you already know the basic structure, which function references which and you'll get a feeling of the general flow of the application.
Code Metrics
Code metrics are used to find hot spots that go against clean code's most prominent rule, “The first rule of functions is that they should be small.”With code metrics we can find long functions, long files, complex functions and the most basic thing we can do to our team members to save them from this.
Lizard
cyclomatic_complexity_analyzer = --CCN 15 --length 100 --arguments 1 --warning-msvs
pio run -t lizard
lib\examplelib\ProductionCode2.c(4): warning: ThisFunctionHasNotBeenTested has 6 NLOC, 1 CCN, 28 token, 2 PARAM, 8 length
lizard -Eduplicate include src lib\arduino-printf lib\circularbuffer lib\defectedLib lib\examplelib lib\runner
*** Error 1
================================================
NLOC CCN token PARAM length location
------------------------------------------------
8 2 48 0 10 setup@16-25@src\main.cpp
3 1 5 0 4 loop@27-30@src\main.cpp
7 2 19 0 10 main@10-19@src\port_arduino.h
5 1 44 1 5 CircularBuffer::CircularBuffer@32-36@lib\circularbuffer\CircularBuffer.cpp
4 1 12 0 4 CircularBuffer::~CircularBuffer@38-41@lib\circularbuffer\CircularBuffer.cpp
4 1 10 0 4 CircularBuffer::IsEmpty@43-46@lib\circularbuffer\CircularBuffer.cpp
4 1 10 0 4 CircularBuffer::IsFull@48-51@lib\circularbuffer\CircularBuffer.cpp
8 3 49 1 8 CircularBuffer::Put@53-60@lib\circularbuffer\CircularBuffer.cpp
11 3 51 0 12 CircularBuffer::Get@62-73@lib\circularbuffer\CircularBuffer.cpp
4 1 10 0 4 CircularBuffer::Capacity@75-78@lib\circularbuffer\CircularBuffer.cpp
5 2 23 1 5 CircularBuffer::Next@80-84@lib\circularbuffer\CircularBuffer.cpp
18 5 116 0 22 CircularBuffer::Print@86-107@lib\circularbuffer\CircularBuffer.cpp
11 2 69 0 11 dynamic_buffer_overrun_018@5-15@lib\defectedLib\bufferLibrary.c
4 1 14 0 4 memory_leak_001@17-20@lib\defectedLib\bufferLibrary.c
9 3 36 1 9 FindFunction_WhichIsBroken@11-19@lib\examplelib\ProductionCode.c
4 1 9 1 4 FunctionWhichReturnsLocalVariable@21-24@lib\examplelib\ProductionCode.c
6 1 28 2 8 ThisFunctionHasNotBeenTested@4-11@lib\examplelib\ProductionCode2.c
3 1 11 0 3 setup@30-32@lib\runner\runner.h
13 file analyzed.
==============================================================
NLOC Avg.NLOC AvgCCN Avg.token function_cnt file
--------------------------------------------------------------
19 5.5 1.5 26.5 2 src\main.cpp
9 7.0 2.0 19.0 1 src\port_arduino.h
0 0.0 0.0 0.0 0 src\sdkconfig.h
1 0.0 0.0 0.0 0 lib\arduino-printf\arduino-printf.h
65 7.0 2.0 36.1 9 lib\circularbuffer\CircularBuffer.cpp
29 0.0 0.0 0.0 0 lib\circularbuffer\CircularBuffer.h
18 7.5 1.5 41.5 2 lib\defectedLib\bufferLibrary.c
2 0.0 0.0 0.0 0 lib\defectedLib\bufferLibrary.h
16 6.5 2.0 22.5 2 lib\examplelib\ProductionCode.c
2 0.0 0.0 0.0 0 lib\examplelib\ProductionCode.h
7 6.0 1.0 28.0 1 lib\examplelib\ProductionCode2.c
1 0.0 0.0 0.0 0 lib\examplelib\ProductionCode2.h
12 3.0 1.0 11.0 1 lib\runner\runner.h
===============================================================================================================
No thresholds exceeded (cyclomatic_complexity > 15 or length > 1000 or nloc > 1000000 or parameter_count > 100)
==========================================================================================
Total nloc Avg.NLOC AvgCCN Avg.token Fun Cnt Warning cnt Fun Rt nloc Rt
------------------------------------------------------------------------------------------
181 6.6 1.8 31.3 18 0 0.00 0.00
Duplicates
===================================
Total duplicate rate: 0.00%
Total unique rate: 100.00%Coding Standards
In addition to MISRA, there are other interesting standards that you should be aware of.
SEI CERT C Coding Standard (PDF), what I really like about this standard is the explanation each rule have and why its in the standard.
Lastly but not less important is the AUTOSAR Guidelines for the use of the C++14 language in critical and safety-related systems (PDF), like the others, one of the more important sections is the rational for including the guidelines can provide an important insight into the "why" and is always a good reading material.
Test Sample
References:
Evaluation of Open Source Static Analysis Security Testing (SAST) Tools for C
FOSS Static Analysis Tools for Embedded Systems and How to Use Them
0 comments:
Post a Comment