Skip to content
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

scripts: coccinelle #3991

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,14 @@ cscope:
${q}find $(PWD) -name "*.[chSs]" | grep -v export-ta_ > cscope.files
${q}cscope -b -q -k

.PHONY: checkpatch checkpatch-staging checkpatch-working
.PHONY: checkpatch checkpatch-staging checkpatch-working coccicheck
checkpatch: checkpatch-staging checkpatch-working

checkpatch-working:
${q}./scripts/checkpatch.sh

checkpatch-staging:
${q}./scripts/checkpatch.sh --cached

coccicheck:
${q} ./scripts/coccicheck
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: remove space, for consistency with the makefile.

141 changes: 141 additions & 0 deletions scripts/coccicheck
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
#!/bin/bash

# SPDX-License-Identifier: BSD-2-Clause
#

DIR="`pwd`"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DIR not used. can remove.

OPTIONS=" --dir . "
TIMESTAMP="`date -R | sed 's/^.\{5\}//' | sed 's/.\{6\}$//' | tr ' ' '_'`"

###########################################################################################
# Check if spatch is installed

SPATCH="`which ${SPATCH:=spatch}`"
if [ ! -x "$SPATCH" ]; then
echo 'spatch is required to run this script, exiting.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest "Error: " as prefix trace message.

exit 1
fi

###########################################################################################
# Help section

if [[ $H -ge 1 ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is $H?

echo "coccicheck help section"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe: "Help section for optee_os makefile coccicheck target"
Looks strange that this shell script shows debug info related to makefile usage.

echo "This target allows the execution of *.cocci scripts using the coccinelle semantic patch engine"
echo "More information about semantic patching is available at"
echo "http://coccinelle.lip6.fr/"
echo -e "\n Options \n"
echo -e "\t COCCIFILE=<path/to/script.cocci> : specify a single .cocci script"
echo -e "\t D=1 : Outputs a debug log, default is /tmp/debug_cocci_\$TIMESTAMP"
echo -e "\t DEBUG_FILE=<path/to/file>: specify a different debug file"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/debug file/debug log file/

add space before : for consistency.

Note: script reverts to default file path when specified log file already exist (line 78). Maybe this should be stated in this help section.

echo -e "\t EXTERNAL_COCCI=<path/to/external/patchesdir/.> : specify a dir containing .cocci scripts"
echo -e "\t MODALITY=<name> : only for external .cocci scripts, specify the virtual mode (-D option)"
echo -e "\t V=1 : Verbose mode"

echo -e "\n Usage: \n"
echo -e "\t- Execute all local .cocci scripts on optee-os"
echo -e "\t\t make coccicheck [V=1] [USE_SUBTREE=<path/to/subdir>]"
echo -e "\t- Execute a single .cocci script "
echo -e "\t\t make coccicheck COCCIFILE=<relative/path/to/script.cocci> [V=1] [USE_SUBTREE=<path/to/subdir>]"
echo -e "\t- Execute a subset of external .cocci scripts"
echo -e "\t\t make coccicheck EXTERNAL_COCCI=<path/to/external/patchesdir/.> [MODALITY=<name>] [V=1] [USE_SUBTREE=<path/to/subdir>]"

exit 0
fi

###########################################################################################
# Variable init + checks

if [ -z $MODALITY ] ; then
MODALITY=report
fi
VIRTUAL="-D $MODALITY"

# if USE_SUBSET == 1, an external set of .cocci scripts is executed
if [[ -v EXTERNAL_COCCI && -d $EXTERNAL_COCCI ]] ; then
USE_SUBSET=1

elif [[ -f $EXTERNAL_COCCI ]] ; then
USE_SUBSET=1
fi

# if USE_COCCIFILE == 1, only the specified .cocci script is executed
if [[ -v COCCIFILE && -f $COCCIFILE && $USE_SUBSET -ne 1 ]]; then
USE_COCCIFILE=1

elif [[ -v COCCIFILE && -f $COCCIFILE && $USE_SUBSET -eq 1 ]]; then
echo "Warining: both EXTERNAL_COCCI and COCCIFILE have been specified."
echo "Only .cocci files in $EXTERNAL_COCCI will be executed"
fi

# if USE_SUBTREE != "", run the semantic patches on the specified subdir, instead from the whole tree
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

USE_SUBTREE is a file path whereas USE_* are boolean flags. Maybe rename USE_SUBTREE to COCCI_SUBTREE for consistency.

if [[ -d USE_SUBTREE ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/USE_SUBTREE/$USE_SUBTREE/

OPTIONS=" --dir ./$USE_SUBTREE "
echo "Running coccicheck on $USE_SUBTREE"
fi

if [ -e $DEBUG_FILE ] ; then
DEBUG_FILE="/tmp/debug_cocci_"$TIMESTAMP
fi

if [[ $D -eq 1 ]] ; then
echo "Debug mode is active, debug file is:"
echo $DEBUG_FILE
echo -e "\n"
else
DEBUG_FILE="/dev/null"
fi

if [[ $V -eq "" ]]; then
Copy link
Contributor

@etienne-lms etienne-lms Aug 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$V -eq "" fails when set with V="" or V=.
Prefer [[ "$V" -eq ""]], [[ -z $V ]] or [[ -v V ]].

V=0
fi

###########################################################################################
# Functions

run_cmd() {
if [ $V -ne 0 ] ; then
echo "Running semantic patch: $@"
fi
echo $@ >>$DEBUG_FILE
$@ 2>>$DEBUG_FILE

err=$?
if [[ $err -ne 0 ]]; then
echo "coccicheck failed"
exit $err
fi
}

coccinelle () {
COCCI="$1"
run_cmd $SPATCH $VIRTUAL $FLAGS --cocci-file $COCCI $OPTIONS || exit 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FLAGS used here is not documented. It is related to coccinelle legacy env variables?

}

###########################################################################################
# Execution possibilities

if [[ $USE_SUBSET -eq 1 ]] ; then
# external patches: make <target name> EXTERNAL_COCCI=path/to/external/patchesdir/. [V=1]

echo "Using specified subset of semantic pathces"
for f in `find ${EXTERNAL_COCCI} -name '*.cocci' -type f | sort`; do
coccinelle $f
done

elif [[ $USE_COCCIFILE -eq 1 ]] ; then
# to launch a single spatch: make <target name> COCCIFILE=relative/path/to/script.cocci [V=1]
VIRTUAL=""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why dropping modality argument here and in below case?

echo "Using specified semantic patch"
coccinelle ${COCCIFILE}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: remove {} for consistency in the script file.


elif [ "$COCCI" = "" ] ; then
# to launch the full subset of op-tee semantic patches: make <target name> [V=1]
VIRTUAL=""
echo "Searching for local semantic patches"
for f in `find scripts/coccinelle/ -name '*.cocci' -type f | sort`; do
coccinelle $f
done
else
coccinelle $COCCI
fi
51 changes: 51 additions & 0 deletions scripts/coccinelle/bad_null.cocci
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* SPDX-License-Identifier: BSD-2-Clause
*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can drop this empty line.
ditto for other .cocci files.

*/

/************************************************************************************
* This semantic patch is responsible of replacing "foo *= NULL" expressions, into
* the equivalent contracted form.
*
*/
@rule1@
expression E;
@@

(
- E == NULL
+ !E
|
- NULL == E
+ !E
|
- E != NULL
+ E
|
- NULL != E
+ E
)


@rule2@
expression E;
identifier f;
@@

E = f(...)
<...
(
- E == NULL
+ !E
|
- E != NULL
+ E
|
- NULL == E
+ !E
|
- NULL != E
+ E
)
...>

59 changes: 59 additions & 0 deletions scripts/coccinelle/initialize_variables.cocci
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* SPDX-License-Identifier: BSD-2-Clause
*
*/

/************************************************************************************
* This semantic patch matches all occurrencies of variable declaration without
* initialization and then initializes them accordingly to their type.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/accordingly/according/

*
* e.g.
* - int a;
* + int a = 0;
*
*/

@variable_declaration@
identifier v, n;
type t;
type numeric = {
short, short int, signed short, signed short int, unsigned short, unsigned short int,
int, signed, signed int, unsigned, unsigned int, long, long int, signed long, signed long int,
unsigned long, unsigned long int, long long, long long int, signed long long,
signed long long int, unsigned long long, unsigned long long int,
float, double, long double, size_t
};
@@

(
// generic char
char v
+ = NULL
;
|
// numeric
numeric v
+ = 0
;
|
// size_t array
size_t v[]
+ = {0}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer { 0 } over {0}.

;
|
// generic array
t v[]
+ = {NULL}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

;
|
// pointer type
t* v
+ = NULL
;
|
// every other type
t v
+ = NULL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really applicable? NULL likely not applies to structure, enums, ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just thinking of a placeholder value to assign to every other type, but you're right. Maybe for now i can simply remove these lines, and let others specify a type when necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer = { } for anonymous type as per Optee OS coding rules.

;
)

29 changes: 29 additions & 0 deletions scripts/coccinelle/warning_overflow.cocci
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* SPDX-License-Identifier: BSD-2-Clause
*
*/

/************************************************************************************
* This semantic patch signals possible cases of overflow.
* Currently we are only looking for the following construct:
* if(a + b > c)
*
*/

@r1@
identifier A, B, C;
binary operator add = {+};
binary operator gt = {>};
position p1;
@@

(
if@p1 (A add B gt C) {...}
)

@script:python@
p1 << r1.p1;
@@

coccilib.report.print_report(p1[0], "Warning, Possible overflow")