Xen project Mailing List

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.

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

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

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

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

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

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

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

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

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

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_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

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.