Looking at this further,
my initial analysis on item 4 (rule 5.8) is less clear than I thought. Part of the problem is that messages such as “Non unique external identifier p conflicts with entity p in file inflate.c on line 243” make it hard to identify the file
which has the conflict as some file names are ambiguous (e.g. there are multiple mm.c’s, etc). Am wondering whether the reporting can be configured to include the path
Looking at the item below does not explain the issue
|
ID_MISRA_VIOL_198
|
xsm\flask\ss\services.c
|
Non unique external identifier p conflicts with entity p in file inflate.c on line 243
|
1169
|
45
|
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/xsm/flask/ss/services.c
1164
/*
1165
* Verify that each kernel class that is defined in the
1166
* policy is correct
1167
*/
1168
static int validate_classes(struct policydb *p)
1169
{
1170
int i;
1171
struct class_datum *cladatum;
1172
struct perm_datum *perdatum;
1173
u32 nprim, perm_val, pol_val;
1174
u16 class_val;
1175
const struct selinux_class_perm *kdefs = &selinux_class_perm;
1176
const char *def_class, *def_perm, *pol_class;
1177
struct symtab *perms;
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/common/inflate.c (as far as I know there is only one inflate.c)
238
static unsigned long INITDATA malloc_ptr;
239
static int INITDATA malloc_count;
240
241
static void *INIT malloc(int size)
242
{
243
void *p;
|
ID_MISRA_VIOL_964
|
xsm\flask\ss\policydb.c
|
Non unique external identifier p conflicts with entity p in file inflate.c on line 243
|
169
|
39
|
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/xsm/flask/ss/policydb.c
169
static int roles_init(struct policydb *p)
170
{
171
char *key = NULL;
172
int rc;
173
struct role_datum *role;
174
175
role = xzalloc(struct role_datum);
In particular I can’t match what I see it to the example
/* file1.c */
int32_t
count; /* "count" has external linkage */
void
foo ( void ) /* "foo" has external linkage */
{
…
}
/* file2.c */
static void foo ( void ) /* Non-compliant - "foo" is not unique
{
int16_t count; /* Non-compliant - "count" has no linkage
* but clashes with an identifier with
* external linkage */
…
}
Both files have not changed in years
@Franceso: can your guys have a quick look and point me in the right direction?
Regards
Lars
From: Fusa-sig <fusa-sig-bounces@xxxxxxxxxxxxxxxxxxxx> on behalf of Lars Kurth <lars.kurth@xxxxxxxxxx>
Date: Monday, 25 November 2019 at 14:45
To: Francesco Brancati <francesco.brancati@xxxxxxxxxxxxx>, "fusa-sig@xxxxxxxxxxxxxxxxxxxx" <fusa-sig@xxxxxxxxxxxxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>
Subject: Re: [FuSa SIG] Call for Agenda items for next week's meeting (Nov 19)
Hi all,
I did a little bit of a cross-check on the top 11 classes of issues. If we fixed these we would have fixed more than 90% of issues
@Franceso: can you look at lines 8 & 9 of the table. There is either an issue with the tool or for item 9, I don’t understand what is wrong
@Franceso:: is there a way to configure rule 5.1 (line 5 in the table below) in the MISRA checking tool? In fact, I would generally want to receive some advice about that rule.
How do users generally choose the limits?
Is argumentation for the choice of limit required?
@George: did you just do a simple cross-check or are you doing some more analysis?
It seems to be me that addressing 90% of the issues should mostly be straightforward. We have a lot of issues with declarations or lack thereof.
|
1
|
MISRA12_15.6
|
The body of an iteration-statement or a selection-statement shall be a compound-statement
We discussed this before and have two options: fix or use GCC6 options to identify. For safety certification we would then need an equivalent option in a qualified compiler.
|
4030
|
|
2
|
MISRA12_8.4
|
A compatible declaration shall be visible when an object or function with external linkage is defined
At least 951 instances are external function definitions without declarations (see below). Fixing these would be fairly mechanical and would primarily touch header files. The rest are internal symbols. To maintain
this going forward, we would need to update the coding standard.
|
1144
|
|
3
|
MISRA12_8.6
|
An identifier with external linkage shall have exactly one external definition
All of these appear to be duplicates of 8.4 violations: in other words, fixing rule 8.4 would fix all these.
|
951
|
|
4
|
MISRA12_5.8
|
Identifiers that define objects or functions with external linkage shall be unique
This is an interesting rule: in a nutshell you MUST NEVER re-use a variable with external linkage anywhere else. The root cause of this are the following 26 externally defined variables/functions
|
Non unique external identifier size conflicts with entity size in file mm.c on line 558
|
|
Non unique external identifier s conflicts with entity s in file boot.c on line 298
|
|
Non unique external identifier p conflicts with entity p in file inflate.c on line 243
|
|
Non unique external identifier i conflicts with entity i in file setup.c on line 254
|
|
Non unique external identifier end conflicts with entity end in file boot.c on line 711
|
|
Non unique external identifier slen conflicts with entity slen in file boot.c on line 712
|
|
Non unique external identifier r conflicts with entity r in file boot.c on line 258
|
|
Non unique external identifier ptr conflicts with entity ptr in file boot.c on line 300
|
|
Non unique external identifier tail conflicts with entity tail in file boot.c on line 573
|
|
Non unique external identifier cmd conflicts with entity cmd in file setup.c on line 297
|
|
Non unique external identifier safe_copy_string_from_guest conflicts with entity safe_copy_string_from_guest in file guestcopy.c on line 9
|
|
Non unique external identifier do_flask_op conflicts with entity do_flask_op in file flask_op.c on line 635
|
|
Non unique external identifier np conflicts with entity np in file device_tree.c on line 2027
|
|
Non unique external identifier offset conflicts with entity offset in file mm.c on line 556
|
|
Non unique external identifier __read_mostly conflicts with entity __read_mostly in file flask_op.c on line 28
|
|
Non unique external identifier wait conflicts with entity wait in file schedule.c on line 2014
|
|
Non unique external identifier sched_id conflicts with entity sched_id in file schedule.c on line 1388
|
|
Non unique external identifier burn_credits conflicts with entity burn_credits in file sched_credit2.c on line 1321
|
|
Non unique external identifier __cacheline_aligned conflicts with entity __cacheline_aligned in file timer.c on line 37
|
|
Non unique external identifier tainted conflicts with entity tainted in file kernel.c on line 323
|
|
Non unique external identifier w conflicts with entity w in file boot.c on line 299
|
|
Non unique external identifier __init conflicts with entity __init in file setup.c on line 250
|
|
Non unique external identifier match conflicts with entity match in file boot.c on line 713
|
|
Non unique external identifier mods conflicts with entity mods in file setup.c on line 252
|
|
Non unique external identifier mod conflicts with entity mod in file setup.c on line 253
|
|
Non unique external identifier cmds conflicts with entity cmds in file setup.c on line 296
|
This then leads to MISRA violations when variables of name i, s, p, etc. are used in local functions. Assuming the identifiers above can be renamed without breaking compatibility (aka they are not
part of an ABI), these should be easy to fix. Ideally this should be part of the coding standard, but this may be hard to express and even harder to check without an easy way to check.
|
896
|
|
5
|
MISRA12_5.1
|
External identifiers shall be distinct
The definition of distinct depends on the implementation and on the version of the C language that is being used:
• In C90 the minimum requirement is that the first 6 characters of external identifiers are significant but their case is not required to be significant;
• In C99 the minimum requirement is that the first 31 characters of external identifiers are significant, with each universal character or corresponding extended source character occupying between
6 and 10 characters.
In practice, many implementations provide greater limits. For example, it is common for external identifiers in C90 to be case-sensitive and for at least the first 31 characters to be significant.
I believe this would need to be configured in the MISRA checking tool implementing the limits which all supported compilation toolchains that we may want to use on Xen support. The rationale is not
readability, but avoiding linker ambiguity.
|
562
|
|
6
|
MISRA12_8.8
|
The static storage class specifier shall be used in all declarations of objects and functions that have internal linkage
Requires that all symbols that are not external, are declared as static. This would be another one for the coding standard. I believe that most of these are a consequence of rules 8.4 & 8.6 not being implemented
in our code and the MISRA checking tool making assumptions on what should be static based on the function call tree. In other words, most of these should go away if rule 8.4 was fixed.
|
558
|
|
7
|
MISRA12_16.3
|
An unconditional break statement shall terminate every switch-case
These seems to be primarily a cosmetic issue, which should be easy to fix by requiring a break statement in a default block. We have an unwritten convention that the default statement is at the end of a switch
statement. We should update coding standards.
|
543
|
|
8
|
MISRA12_8.2
|
Function types shall be in prototype form with named parameters
NOT SURE WHAT IS WRONG HERE: the CodecheckResultsXen column only shows 4 instances of issues, whereas Results by Rule shows 277
|
277
|
|
9
|
MISRA12_21.2
|
A reserved identifier or macro name shall not be declared
This I had problems investigating and I am not sure why these are highlighted, see
the first few examples of CodecheckResultsXen filtered on MISRA12_21.2
http://xenbits.xen.org/gitweb/?p=xen.git&a=search&h=HEAD&st=grep&s=_DEBUG_HASHES
http://xenbits.xen.org/gitweb/?p=xen.git&a=search&h=HEAD&st=grep&s=policydb_class_isvalid
…
|
223
|
|
10
|
MISRA12_8.3
|
All declarations of an object or function shall use the same names and type qualifiers
This rule requires that macro or function declarations are globally unique (aka they should not rely on which headers to include). The following macros/functions have different incompatible definitions
|
boolean_param
|
|
burn_credit
|
|
create_mappings
|
|
custom_param
|
|
custom_runtime_param
|
|
DECLARE_SOFTIRQ_TASKLET
|
|
DECLARE_TASKLET
|
|
DEFINE SPINLOCK
|
|
DEFINE_PER_CPU
|
|
DEFINE_PER_CPU_READ_MOSTLY
|
|
DEFINE_PER_CPU_READ_MOSTLY
|
|
DEFINE_RCU_READ_LOCK
|
|
do_debug_key
|
|
do_toggle_alt_key
|
|
dump_hwdom_registers
|
|
dump_registers
|
|
efi_rs_leave
|
|
error
|
|
EXPORT_SYMBOL
|
|
integer_param
|
|
integer_runtime_param
|
|
LIST_HEAD
|
|
PAGE_LIST_HEAD
|
|
PLATFORM_START
|
|
presmp_initcall
|
|
read_clocks
|
|
reboot_machine
|
|
REGISTER_SCHEDULER
|
|
show_handlers
|
|
size_param
|
|
string_param
|
|
177
|
|
11
|
MISRA12_17.7
|
The value returned by a function having non-void return type shall be used
You can’t implicitly throw away the return value of a function. There is clearly value to that rule, but this one could be controversial.
In a nutshell
uint16_t func ( uin t16_t para1 )
{
return para1;
}
…
func ( para2 ); /* Non-compliant - value discarded */
( void ) func ( para2 ); /* Compliant */
x = func ( para2 ); /* Compliant */
|
153
|
Best Regards
Lars
From: Fusa-sig <fusa-sig-bounces@xxxxxxxxxxxxxxxxxxxx> on behalf of Francesco Brancati <francesco.brancati@xxxxxxxxxxxxx>
Date: Thursday, 21 November 2019 at 10:11
To: "fusa-sig@xxxxxxxxxxxxxxxxxxxx" <fusa-sig@xxxxxxxxxxxxxxxxxxxx>
Subject: Re: [FuSa SIG] Call for Agenda items for next week's meeting (Nov 19)
Hi George,
the violations refers to the absence of { } parentheses in the IF statements.
Cheers,
Francesco.
Il 21/11/2019 16:22, Francesco Brancati ha scritto:
Hi George,
I just asked the team....
I will back to you ASAP.
cheers,
Francesco.
Il 21/11/2019 16:10, George Dunlap ha scritto:
On Nov 21, 2019, at 10:01 AM, Francesco Brancati <francesco.brancati@xxxxxxxxxxxxx> wrote:
Hi All,
please find attached the resuts of the Misra 2012 checks on 4.12.0 release.
Hmm, under the CodecheckResultXEN tab, I looked up the top couple from xen/drivers/iommu.c. It says "Iteration-statement or selection-statement not a compound-statement” at lines 496, 484, 479, and 468. But when I `git checkout RELEASE-4.12.0`, I don’t see anything like an “iteration statement or selection statement” at those lines. Any idea what’s going on?
-George
--
Francesco Brancati
Innovation Manager and SW Solutions Expert
Email: francesco.brancati@xxxxxxxxxxxxx
Phone: +39 0587 21 24 65 (internal number: 104)
Mobile: +39 333 48 52 041
Skype: francesco.brancati
www.resiltech.com
This e-mail and related attachments are property of ResilTech S.r.l. and may also be privileged. If you are not the intended recipient please delete it
from your system and notify the sender.
You shouldn't copy it or use it for any purpose nor disclose or distribute its contents to any other person.
Questa e-mail e tutti i suoi allegati sono proprietà di ResilTech S.r.l. e possono essere soggetti a restrizioni legali. Se non siete l'effettivo destinatario o avete ricevuto il messaggio per errore siete pregati di cancellarlo dal vostro sistema e di avvisare
il mittente. E' vietata la duplicazione, l'uso a qualsiasi titolo, la divulgazione o la distribuzione dei contenuti di questa e-mail a qualunque altro soggetto.
_______________________________________________
Fusa-sig mailing list
Fusa-sig@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/fusa-sig
--
Francesco Brancati
Innovation Manager and SW Solutions Expert
Email: francesco.brancati@xxxxxxxxxxxxx
Phone: +39 0587 21 24 65 (internal number: 104)
Mobile: +39 333 48 52 041
Skype: francesco.brancati
www.resiltech.com
This e-mail and related attachments are property of ResilTech S.r.l. and may also be privileged. If you are not the intended recipient please delete it
from your system and notify the sender.
You shouldn't copy it or use it for any purpose nor disclose or distribute its contents to any other person.
Questa e-mail e tutti i suoi allegati sono proprietà di ResilTech S.r.l. e possono essere soggetti a restrizioni legali. Se non siete l'effettivo destinatario o avete ricevuto il messaggio per errore siete pregati di cancellarlo dal vostro sistema e di avvisare
il mittente. E' vietata la duplicazione, l'uso a qualsiasi titolo, la divulgazione o la distribuzione dei contenuti di questa e-mail a qualunque altro soggetto.
