engineering ethics (it was [SOLVED] funny CC)

[legacy]

yesterday I was hammering my head with a piece of C code (not written by me, & bare metal profiled for MIPS32) which continuously crashes when compiled with SierraC while it works when compiled with gcc

Code: [Select]

        while (p_btree->context.queue isNotEqualTo NULL)
        {
            p_node = queue_de(p_btree);
            if ((p_node->parent isNotEqualTo NULL)logicalAnd(p_node isEqualTo p_node->parent->pointer[0]))
            {
                rank_new = tree_get_path_lengh_to_root(p_root, p_node);
                if (rank_new isNotEqualTo rank)
                {
                    rank = rank_new;
                }
            }
        }
working with gcc, not working with sierraC

Code: [Select]

        is_done=(p_btree->context.queue isEqualTo NULL);
        while (is_done isNotEqualTo True)
        {
            p_node = queue_de(p_btree);           
            is_ok1=(p_node->parent isNotEqualTo NULL);
            if (is_ok1 isEqualTo True)
            {
                is_ok2=(p_node isEqualTo p_node->parent->pointer[0]);
                if (is_ok2 isEqualTo True)
                {
                   rank2 = tree_get_path_lengh_to_root(p_root, p_node);
                   if (rank2 isNotEqualTo rank1)
                   {
                       rank1 = rank2;
                   }
                }
            }
            is_done=(p_btree->context.queue isEqualTo NULL);
        }
working with both sierraC and gcc


of course the "fixed" version is too pedantic, but just a proof of concept, in order to understand WTF is wrong in the original code
and as far as I understand WTF happened, my debug TAP has found that the problem is around the following line

Code: [Select]

if ((p_node->parent isNotEqualTo NULL)logicalAnd(p_node isEqualTo p_node->parent->pointer[0]))

more specifically (analyzing the assembly code) it seems that it depends by the CC implementation

SierraC seems to implement the IF-STATEMENT as

• step1: eval (p_node->parent isNotEqualTo NULL)
• step2: eval (p_node isEqualTo p_node->parent->pointer[0])
• step3: if they are both True then take the IF branch, else take the ELSE branch

GCC seems to implement the IF-STATEMENT as

• step1: eval (p_node->parent isNotEqualTo NULL)
• step2: if the previous eval is True then eval (p_node isEqualTo p_node->parent->pointer[0])
• step3: if (is_ok2 isEqualTo True) then take the IF branch, else take the ELSE branch

so the problem with SierraC is … with "p_node->parent->pointer" which crashes when "p_node->parent" is NULL


if so, it means that I have 4K lines of C to be fixed

[Kalvin]

You may want to check for the compiler switch for the "short-circuit" evaluation.

[0xdeadbeef]

Sure the "logicalAnd" is defined the same in both versions?
I'm pretty sure that "&&" should always be short circuit according to the standard while "&" is not.
If both versions use "&&", there must be a compiler switch to ignore the standard.
Well, or the compiler is crap.

[grumpydoc]

isNotEqualTo, logicalAnd etc - just use the *&^%!!! language FFS 

These sort of shenanigans do not, in general, help write bug free code.

[newbrain]

If logicalAnd is defined as && (this is really horrible, what's the purpose?), then Sierra C is not ANSI standard compliant.

From C89 standard (well, the draft...):

3.3.13 Logical AND operator
[...]
   Unlike the bitwise binary & operator, the && operator guarantees
left-to-right evaluation; there is a sequence point after the
evaluation of the first operand.  If the first operand compares equal
to 0, the second operand is not evaluated.

The sequence point and definition guarantee that the second operand:

• will only be evaluated if the first one is non-zero
• its (possible) evaluation will happen only when all the side effects of the first one are completed

K&R C, if my memory helps, did not mandate short-circuit evaluation.
I'm not acquainted with Sierra C, there are almost no traces of it on the web, but it might have a switch to make it ANSI compliant.

[Howardlong]

isNotEqualTo, logicalAnd etc - just use the *&^%!!! language FFS 

These sort of shenanigans do not, in general, help write bug free code.

Pretty much my thoughts entirely, at the very least the code is much less readable.

I guess it's to save people from themselves on = vs == and the difference between bitwises and logicals. Any compiler worth its salt would offer a warning, assuming the dev hadn't tried to disable warnings that is.

[grumpydoc]

K&R C, if my memory helps, did not mandate short-circuit evaluation.

My copy of K&R clearly states that && and || stop evaluation as soon as the value of the whole expression is known.

[legacy]

Sure the "logicalAnd" is defined the same in both versions?

it's the alias for "&&"

Code: [Select]

#define logicalAnd     &&

I had to use the alias for an internal reason (my boss wants this way)

Well, or the compiler is crap.

 

[legacy]

If logicalAnd is defined as &&

yes it is

this is really horrible, what's the purpose?

it's more horrible to see "&&", which can be confused with "&"

then Sierra C is not ANSI standard compliant.

I am really afraid they are not C89

If the first operand compares equal to 0, the second operand is not evaluated.

exactly what happens 

K&R C, if my memory helps, did not mandate short-circuit evaluation.

So it might be a pre-ANSI-C compiler, or probably they have give me the wrong compiler version
or it's not configured in the right way (missing FLAGS? missing external profile that "enables/disables" features? who knows)

[newbrain]

K&R C, if my memory helps, did not mandate short-circuit evaluation.

My copy of K&R clearly states that && and || stop evaluation as soon as the value of the whole expression is known.

I stand corrected , unless that copy is the "Second Edition" (that is, already ANSI). My original one is long lost

it's more horrible to see "&&", which can be confused with "&"

To each their own. I do not agree in the least, but if you are fine with that

Code: [Select]

#define isAssignedTheFollowingExpression =
The latest thing I could find about SierraC is a 1993 reference...so, well in the C89 domain (but still a number of non-compliant compiler might still be around, and you might have an older version...). Check the documentation for C89/ANSI compliance, or at least short-circuit evaluation.

[Godzil]

sierraC, are you building for 68000 target? This was used by TI for there 68K based calculator, and the resulted assembly was just absolutely horrible..

[legacy]

I guess it's to save people from themselves on = vs == and the difference between bitwises and logicals.
Any compiler worth its salt would offer a warning, assuming the dev hadn't tried to disable warnings that is.

emmm, SierraC does not offer a warning when you write "if (a=0) {}" instead of "if (a==0) {}"
I can agree or disagree, however it's imposed by our customers (more specifically by their QA dudes)

[legacy]

sierraC, are you building for 68000 target?

yes, but it's not the SierraC Compiler that was bought by TI and used for their calculators
I can say that because it supports CPU32 (683xx) which was not supported by the TI version

here I have a few 683xx cores, and a DOS16 CC (it works under WinXP, or inside a virtual machine)
the full toolchain has been provided by a few customers who want to support their old equipment
I have suggested them a full port to PIC32, but they said that they can't change the baseline until their EOL
(avionics products, they are crazy)

[Godzil]

Avionics that use that crap compiler?!?!? O_O

[legacy]

* * * solution found * * *

there is a profile file (.bat?) in the bin path which has the following line

Code: [Select]

SET  ev_shortcircuit 0

I have changed into

Code: [Select]

SET  ev_shortcircuit 1

and everything is now working as expected



so, it seems that it's really possible to "disable" that feature
too amazing 

[legacy]

emmm, SierraC does not offer a warning when

when you have this set

Code: [Select]

SET  ev_wall 0

I have changed into

Code: [Select]

SET  ev_wall 1

and now it offers a lot of useful warnings 


conclusion: the environment was not configured in the proper way!

[Jeroen3]

I had to use the alias for an internal reason (my boss wants this way)
 

Apparently your boss does not know C, but still wants to read it.
Send him on C-bootcamp or something. 

[tggzzz]

I had to use the alias for an internal reason (my boss wants this way)

Why don't you suggest also using
#define begin {
#define end }

Of course, if your boss cannot expain why they want something, then you shold get another boss.

[Ian.M]

Lets go the whole hog!

Code: [Select]

#define if if(
#define then )


[legacy]

Apparently your boss does not know C

he is a pro ADA dude, probably too Pascal addicted 

[richardman]

You have a broken compiler and a broken boss. This could not be good for your career.

[legacy]

cannot expain why

and can you explain why MISRA-C must be applied ?

e.g.
why on the why do I have to do a lot of "code refactoring" (which is extremely boring) in order to
1) replace multiple "return" with just one "return" at the end of the function body ?
2) remove "break" and "continue" ?

my boss, our customers, fanny people in avionics
but yes, if you can employ me, I will change my job 

[grumpydoc]

I stand corrected , unless that copy is the "Second Edition" (that is, already ANSI). My original one is long lost

1st edition, bought 1986 according to the note I pot on the fly sheet. I had an earlier printing but mislaid it.

emmm, SierraC does not offer a warning when you write "if (a=0) {}" instead of "if (a==0) {}"

You can always use the "if (0==a){}" trick.

I can agree or disagree, however it's imposed by our customers (more specifically by their QA dudes)

Well, whoever pays the piper calls the tune I suppose.

[djacobow]

I don't know why I feel the need to chime in, but not only is

        while (p_btree->context.queue isNotEqualTo NULL) { ... }

barfalicious, but even

        while(p_btree->context.queue != NULL) { ... } is awful

This is more idiomatic c:

        while(p_btree->context.queue) { ... }

In fact, I never compare against NULL or 0, and even when assigning a null pointer, I prefer to just use a 0, which is what NULL is defined as, anyway, unless a maniac did something else.

In c++11, nullptr solves the annoying problem of overloaded functions that might take a pointer or an integer, so I'd use that in that circumstance.

[Godzil]

You are all thinking about 'normal" engineering, here it's related to avionic, where EVERYTHING need to be proved and done explicitely.

That's doesn't surprise me that the coding standard ask to use keyword instead of "==" or "&&" just to prevent any possible error, that does not surprise me that you HAVE to explicitly check for "while(xxx notEqual NULL)" instead of "while(xxx)"

As pointed, look at the MISRA-C reference, and find people that work on avionic software development, they are really strict on the coding standard.