Xen project Mailing List

Hi Lars,

Gcc does much more and complex tasks than just simple doing -Wmisleading-indentation.

So if you qualify gcc as e.g. TCL2 (mainly by means of very extensive validation testing and to some extent increased confidence from use), then you can also easily do, as part of it, the validation that -Wmisleading-indentation works correctly.

If you do not qualify gcc (it is then TCL1), then you need to argue that that this is detected by your process, e.g. by a reasonable mix of following

You have another tool that does the same check (e.g. a commercial static code analyser, or you write e.g. a coccinelle script that does the same check – you could consider that two TCL1 tools (e.g. gcc + coccinelle) are fine if they are independent)
Maybe you can argue an extensive development process with thorough reviews (no such error ever occurred in Xen)
You have extensive/complete set of tests that cover full set of requirements and you have a 100% branch or even mc/dc coverage, so the faulty location of a line would trigger a false test result.

But maybe it is still easier for you to apply this fix along with all other MISRA fixes in one shot, then you don’t have to take care of all the problems/activities above.

As we see, software is not so soft.

Regards

Piotr

From: Fusa-sig <fusa-sig-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Lars Kurth
Sent: Wednesday, 16 October 2019 23:43
To: fusa-sig@xxxxxxxxxxxxxxxxxxxx
Subject: [FuSa SIG] Minutes for Oct 8th meeting, call for agenda items for next weeks meeting

Hi all,

can you please let me know of any agenda items you want to discuss at next week’s meeting?

Also, I think it would be beneficial if we had an assessor at next week’s meeting

Possible agenda items I am aware of, feel free to add additional items

Some progress updates on multiple actions (Artem, possibly Francesco)

See minutes

How to approach CERT EXP19-C/MISRA C:2012, 15.6 violations, which make up the bulk of the violations identified by Resiltech
See https://wiki.sei.cmu.edu/confluence/display/c/EXP19-C.+Use+braces+for+the+body+of+an+if%2C+for%2C+or+while+statement

Arguments for just fixing the violations

This is a valid issue which as caused security issues in other projects before (however not in Xen Project)

Arguments against just fixing the violations:

Change cannot be isolated to the scope in question, as we would violate the current Xen Coding standard
We would have to change the current Xen Coding Standard and implement the change for the entire code base
This is very disruptive due to the sheer number of issues (on average one issue every 15 lines of code). The impact would be

o Our capability to backport fixes to older but supported code lines would be curtailed: changes would not cleanly apply
This could be worked around by backporting fixes to CERT EXP19-C/MISRA C:2012, 15.6 violations to older supported code lines

o The capability of down streams (consumers of Xen) to build products and services
Many of these products consist of Xen + sometimes a significant set of modifications that are usually applied automatically
By implementing CERT EXP19-C/MISRA C:2012, 15.6 we would break and inflict significant pain
Every single time we imposed major change on down streams there were significant consequences for the project, e.g. when we switched from xm to xl the consequence was that AWS was for many years stuck on Xen 4.3. This may have played a significant role in AWS supporting KVM based instances alongside Xen based instances.

A number of community members would very likely complain about the readability of the code and would be likely to block implementing CERT EXP19-C/MISRA C:2012, 15.6: but this is minor compared to the impact on downstreams
Such a change carries the risk of introducing new bugs

A possible alternative safety argumentation:

According to ELISA, MISRA C:2012, 15.6 is not strictly required
If we were to find an alternative means to satisfy the rationale of 15.6 aka.

It is possible for a developer to mistakenly believe that a sequence of statements forms the body of an iteration -statement or selection-statement by virtue of their indentation. The accidental inclusion of a semi-colon after the controlling _expression_ is a particular danger, leading to a null control statement. Using a compound-statement clearly defines which statements actually form the body. Additionally, it is possible that indentation may lead a developer to associate an else statement with the wrong if.

It should be possible to argue that an alternative approach will deliver the same effect as complying with MISRA C:2012, 15.6

One possibility would be to use -Wmisleading-indentation in GCC 6 and treating warnings as errors
See https://developers.redhat.com/blog/2016/02/26/gcc-6-wmisleading-indentation-vs-goto-fail/

This would warn/error for code such as

for (i = 0; i < n; i++)

sum[i] = a[i] + b[i];

prod[i] = a[i] * b[i];

and similar cases, which essentially addresses the rationale of MISRA C:2012, 15.6. However, GCC 6 is not a qualified tool, which is likely a problem.

There may be qualified/certified tools/compilers which have a similar option or which could be extended if need be.

We may have to treat C macros differently and require compliance with MISRA C:2012, 15.6 in Macros as I don’t believe -Wmisleading-indentation could handle this case correctly. This would likely be uncontroversial and have a much more localised impact, in particular as patch queues of down streams rarely touch macros. Even, if they did, the impact for down streams would be much smaller and there would be a definite benefit in addressing these.

That’s the core of my thinking, and I was wondering whether we can test this somehow in next week’s meeting

Best Regards

Lars

P.S.: Note that I am on PTO on Friday and Monday

Re: [FuSa SIG] Minutes for Oct 8th meeting, call for agenda items for next weeks meeting