EmbeddedRelated.com
Forums
Memfault State of IoT Report

C and MISRA, blues... (arithmetic shifts)

Started by Nils April 24, 2008
In message <%_iQj.112101$5i5.67274@newsfe6-gui.ntli.net>, Wilco Dijkstra 
<Wilco_dot_Dijkstra@ntlworld.com> writes
>> Is the rounding of shifts like this well defined in C? I can't >>remember the exact rules off-hand - perhaps that's why >> MISRA doesn't like right shifts of signed integers! > >No it's not well defined, just like most other things in C, but who >cares anyway?!? > >Compiler writers agree on most of these things, so in reality these things are >extremely well defined. Basically nobody cares about what the C standard says
This is true in practice.
>exactly as long as their code compiles.
Yes. Actually as long as the behaviour of the compiler is known documented and repeatable the standard does not matter in reality. Most embedded compilers are not ISO-C99 compliant but mostly ISO-C90/95 with compiler and target extensions.
>If it doesn't work on a particular compiler, >it is that compiler's fault. I know, I have dealt with angry customers >with millions of >lines of code that didn't compile on our expensive but strict ANSI C >compiler...
I assume you mean ISO-C compiler. Which standard? C90, C95, C99? I do have an email where a user says: "We have moved from abc compiler to XYZ compiler. We used to compile with abc and get no errors. We now using xyz compiler we get lots of errors. We think this is because your xyz compiler is more strict than the abc compiler. Please can you supply us a version of the xyz compiler with a less strict parser." Yes I do have all the names. -- \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/ \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/
In article <67ccn9F2na9h5U1@mid.uni-berlin.de>, Nils says...
> I'm currently rewriting some numerical code for MISRA compliance. > > Signed shifts are not defined by the C-standard, and the code-checker > complaints. Well - no big surprise here. I knew that and did it > nevertheless. Now I have to rewrite. > > > But do you do if you need them anyway? I need *lots* of them, so in > despair I've just created this monster of unreadable code:
Echoing the question. What do you need them for that's not covered by a divide by a power of 2? What about int asr( int x, unsigned int shift) { unsigned int temp; temp=(unsigned int)x; if( x < 0) { temp = ~((~temp) >> shift); } else { temp >>= shift; } return (int)temp; } Does that cover all the cases? For a negative value it converts the sign extend to a zero extend and the positive case works as usual. Commenting left to the user. It's small enough you could probably make a macro (with some loss of clarity) and for constant shifts a good compiler might end up optimizing it back to a ASR. Maybe? Or, take the simple way out and declare an exception (whatever the term is MISRA uses for a formal waiver of a rule). As I recall they can apply at indivual case, project level or as company policy as an exception. They do need to be documented though. Robert ** Posted from http://www.teranews.com **
In message <UmjQj.14994$yD2.11517@text.news.virginmedia.com>, 
FreeRTOS.org <noemail@given.com> writes
> >IMHO the point is being missed here. MISRA is intended to assist in making >your code safer. Obfuscation is never safer - from any point of view >(review, error #, maintenance, etc.). MISRA is a "guide", so let it be >just that. Rigorous compliance at all cost in an embedded system will >result in junk code.
*Personally* I would not disagree with that. Strict blind adherence to any set of (arbitrary) rules should avoided. There is AFAIK in MISRA-C room for deviations. If you can document a reason (that will stand up in court in 3 years time) for doing something different then do that. * All comments here are purely personal and not representing any company, organisation or body other than mine.
>Our approach to this would be to run a check on the code, and let all the >none compliances be documented. Most of these non compliances can then be >cleared up with the minimum of effort, but there will be some for which it >makes no sense to change the code.
Sounds very sensible to me.
> Therefore only a handful of >non-compliances make it through to the code review phase. During the code >review actions will be raised to - for example - check exactly what the >compiler does given the input. The result of this check (test) is then also >documented.
You do need to know exactly what your compiler will do. It is amazing how many people make blind assumptions about C compilers.
>Once this is done the senior developer can deviate the >remaining non-compliances with a sound rational argument, safe in the >knowledge that (in combination with other testing) the code is going to >behave as intended.
Sounds good to me. You have documented the behaviour of the tools and reasons for doing "unsafe" things because in this particular instance it is the best solution and is not "unsafe" bea.
> MISRA has done its job - it has highlighted a possible >flaw in your code. The engineers have done their job to ensure that the >highlighted items are removed or guaranteed to do what is expected.
Precisely...... The point is that behaviour is EXACTLY as expected and fully documented. Ie reliable, repeatable and safe as reasonably practicable.
>Which is safer - to have a single line that shifts code in an easily >understandable, obvious way, that is reviewed, tested and checked, or to >replace this with what by your own admission is more confusing.
Rule is rules guv... more than my jobs worff to.... :-) -- \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/ \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/
Richard Phillips wrote:
>
... snip ...
> > The problem is there is some misunderstanding here. MISRA lists > it's rules as "recommended" and "required" (or something like > that), which makes it sound like "required" rules MUST be obeyed, > which doesn't appear to be the case from what I now understand.
The only 'required' rules are those of the C standard. -- [mail]: Chuck F (cbfalconer at maineline dot net) [page]: <http://cbfalconer.home.att.net> Try the download section. ** Posted from http://www.teranews.com **
In message <kukQj.59491$h65.48326@newsfe2-gui.ntli.net>, Richard 
Phillips <raphillips@ntlworld.com> writes
> >This is IMO exactly how MISRA should be used. >I've been obeying MISRA in my coding at work for years (well, sort of), >while there are some things I think are very good (e.g. it makes you think >very carefully about casting and potential overflows, it doesn't like magic >numbers, etc etc), there are some things I get annoyed at, such as: > >* You're not supposed to use "goto". Avoiding this when you're learning to >write code is good practise, but it's very useful in a limited set of >circumstances, if used properly.
This is true.
>I remember a manager of mine telling a >work colleague once that using "goto" was bad practise, I then showed my >colleague my copy of K+R which basically put him right.
Maybe. IT was AFAIK Dykstra who said goto was bad and no one has challenged that with a convincing strategy for using goto. Mainly because the majority who do use it tend to miss use it. So it should not normally be used but...
>* You're not supposed to exit from a controlled loop prematurely. > >* ... Hmm, actually I could go on for a while. Suffice to say, there are a >few things...!
All of the MISRA rules are good *some* of the time. Where they are not it is up to you to come up with a project specific reason why not.
>I've seen some tidy well-written code turned inside out and made >semi-unreadable by trying to make it MISRA compliant.
I know what you mean it is the "tick the box" brigade who want to say we are MISRA compliant without actually using any Engineering thought on it.
> If it makes code more >complicated (and more error-prone) then it's doing the opposite of what's >intended.
Hence MIStRAy-C A spoof on MISRA-C http://www.phaedsys.demon.co.uk/chris/mistrayc/
> I had a chat with a manager of mine about this once and he said >that MISRA was basically a guide, obey it if you can, but if there is a >problem with doing this then all that was needed was to document the >non-compliances. For me, this is a far more sensible approach.
I personally believe this is the way it is meant to be used.
> Sort out >the obvious clangers, but don't turn good neat code into jumbled unreadable >rubbish just to satisfy MISRA!!
Any suggestions on how the MISRA team can stop people doing this All methods must be legal under the UN human rights and Geneva conventions. Water-boarding or stringing up by their thumbs for recalcitrant programmers and managers may be deserved and satisfying but we can't writ it in to a coding standard :-)
>The problem is there is some misunderstanding here. MISRA lists it's rules >as "recommended" and "required" (or something like that), which makes it >sound like "required" rules MUST be obeyed, which doesn't appear to be the >case from what I now understand.
It was required and advisory. IE those that should normally be used and those which are useful a lot of the time. Any suggestions for a different nomenclature? -- \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ \/\/\/\/\ Chris Hills Staffs England /\/\/\/\/ \/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/
"Chris H" <chris@phaedsys.org> wrote in message news:punstsIZjeEIFAMn@phaedsys.demon.co.uk...
> In message <%_iQj.112101$5i5.67274@newsfe6-gui.ntli.net>, Wilco Dijkstra <Wilco_dot_Dijkstra@ntlworld.com> writes >>> Is the rounding of shifts like this well defined in C? I can't remember the exact rules off-hand - perhaps that's >>> why >>> MISRA doesn't like right shifts of signed integers! >> >>No it's not well defined, just like most other things in C, but who cares anyway?!? >> >>Compiler writers agree on most of these things, so in reality these things are >>extremely well defined. Basically nobody cares about what the C standard says > > This is true in practice. > >>exactly as long as their code compiles. > > Yes. Actually as long as the behaviour of the compiler is known documented and repeatable the standard does not > matter in reality. Most embedded compilers are not ISO-C99 compliant but mostly ISO-C90/95 with compiler and target > extensions.
Much of this has to be documented to be conformant. Most compilers also document details of structure layout, language extensions, ABIs etc. I have seen very little C99-only code, although most compilers do support various C99 features in C90 mode. Long long is a good example, we did it years before C99.
>>If it doesn't work on a particular compiler, >>it is that compiler's fault. I know, I have dealt with angry customers with millions of >>lines of code that didn't compile on our expensive but strict ANSI C compiler... > > I assume you mean ISO-C compiler. Which standard? C90, C95, C99?
This was C89, the first ANSI C standard. We used -ansi and -c89 options for a long time, even after ISO C99.
> I do have an email where a user says: > > "We have moved from abc compiler to XYZ compiler. We used to compile with abc and get no errors. We now using xyz > compiler we get lots of errors. We think this is because your xyz compiler is more strict than the abc compiler. > > Please can you supply us a version of the xyz compiler with a less strict parser."
Sounds fairly typical of a customer indeed. It's impossible to even try to talk them into fixing their code, as it works on any other compiler they tried. So you've got to fix the compiler or go out of business. Not a hard choice... Wilco
Nils wrote:
> But do you do if you need them anyway? I need *lots* of them, so in > despair I've just created this monster of unreadable code: > > int ArithmeticShiftRight (int value, unsigned int shift) > {
/* Not sure whether that passes your MISRA checker: */ return (int) ((unsigned long long) value >> shift);
> }
That aside, MISRA usually allows you to deviate from a rule if you have a good reason. This is to make you think about whether you really need it. Arithmetic shifts would be a good thing to answer "yes" to that question. The function could finally even look like this: int ArithmeticShiftRight (int value, unsigned int shift) { #if ((-1) >> 1) == -1 return value >> shift; #else # error Port me! #endif } which would make your code portable under all practical definitions known to me. Stefan
Nils wrote:

> However, it does not work if value is negative because (-7/2) != > (-7>>1).
You can't know that. You can't know if two things are unequal without a reliable source defining the value for either of them. -7>>1 causes undefined behaviour in a C program. So every assumption about its result is wrong by default. And guess what: the result of (-7/2) is unkown, too! Technically, at least as of C90, which still is the effectively valid definition for many of us, it's implementation-defined whether signed division rounds towards negative infinity or zero. So it could be -4 as easily as it could be -3. C99 changed this. Now it's required to be -3.
David Brown wrote:

> Is the rounding of shifts like this well defined in C?
_Nothing_ about shifts of negative values is defined _at_all_ in C. For all you know, the machine could catch fire if ever you try it on a Tuesday between 12:34:56 h and 12:36:54h (Hawaiian time). Rounding as a concept is inapplicable to non-arithmetic operators. There is no full-precision intermediate result, thus nothing to be rounded.
> I can't remember the exact rules off-hand -
Not surprisingly --- nothing there to be remembered.
> perhaps that's why MISRA doesn't like right shifts of signed > integers!
You betcha.
Wilco Dijkstra wrote:

> There is no compiler that doesn't correctly implement signed arithmetic.
That's beside the point --- shifting is not an arithmetic operation. Nor is there even a definition of "correct" in place for shifting negative numbers. If there were, there would never been a need to invent the terms "arithmetic shift" and "logical shift".
> Nobody could get away with creating a compiler that can't compile existing > code.
The problem is not with compiling the code --- it's what the compiled code will actually do.

Memfault State of IoT Report