This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Optimize kills code (does not recognize memcpy) - missing warning

Processor STM32F4 (Cortex M4), uVision 4.54, C++ V4.1.0.894 (in C++ module), Optimization level O1:

The following construction does not recognize the code change in the memcpy function:

...
long long ll= 0;

// copy the upper 6 bytes of ll from the unaligned data source pbData
memcpy( ((BYTE*)&ll)+2, pbData, 6);

if( !ll)
  return;

... the following code will never be reached, as ll is not reloaded in the
preceding "if( !ll)" condition. (The return statement is ALWAYS used - no
code is generated for the "if( !ll)" condition).

It would be very nice, if the compiler in such a case would generate a warning "Condition without use / always true" or so (as definitely any other C/C++ compiler I know would do). (This case can be solved by replacing the "ll=0" by "*(short*)&ll= 0" - but it took me quite a time to recognized the problem - I did not expect this).

Also in this case, I would appreciate very much a warning for missing brackets:

int iStatus;
#define STATUS_Bit1  0x01
#define STATUS_Bit2  0x04
if( iStatus == STATUS_Bit1 | STATUS_Bit2){
... this code is then executed always, as the compiler seems to handle "==" and "|" with identic priority, so it translates the condition as "(iStatus == STATUS_Bit1) | STATUS_Bit2"
}

Here any other C/C++ compiler I use would give two warnings: "Using ambiguous operators without brackets" and "Condition without use / always true".

Please consider to add these two warnings in some future compiler update if possible.

Parents Reply Children
  • I wonder what the warning would be like? "You just used the address operator: be careful to not shoot yourself in the foot"? Come on, this is your responsibility.
    Besides, the language standard appears to prohibit this kind of code:

    An object shall have its stored value accessed only by an lvalue expression that has one of the following types:
    - a type compatible with the effective type of the object,
    - a qualified version of a type compatible with the effective type of the object,
    - a type that is the signed or unsigned type corresponding to the effective type of the object,
    - a type that is the signed or unsigned type corresponding to a qualified version of the effective type of the object,
    - an aggregate or union type that includes one of the aforementioned types among its members (including, recursively, a member of a subaggregate or contained union),
    - or a character type.

    So you should expect undefined behaviour from your code.

  • The "be careful do not shoot yourself in the foot warning" is shown at many in the manual / help file (e. g. in the part you cited).

    I would prefer warnings of the sort "be careful you possibly shot yourself in the foot".

    A small but quite significant difference ... .

  • but Microsoft compiler
    Microsoft sell their compiler in millions and have the revenue from that
    Keil sell their compiler in thousands and have the revenue from that

    There is no way in Hades that Kail can (afford to) add all 'niceties'. I am glad that they spend their limited budget on making the compiler work rather than 'niceties'.

    Oh yes, I do occasionally get frustrated with Keil, but my product compiles correctly, horray

    Erik

  • Yes, of course you are right, Keil compiler is great.

    But anyway it is a bit disturbing that there comes not any warning if somehow code drops out completely or if conditions are completely "empty".

    (I have no overview, how difficult implementation of such warnings would be - in the uVision load window you recognize empty "if statements" very easily because of the missing "code bar" on the left side - so generally I would expect that such functionality somehow already is included in the Keil software, perhaps there would be some easy way to get it done).

    (I cited "Microsoft" only in response to the post before - one other uC compiler I know very well (based on GNU CC I think) definitely also gives warnings for empty "if statements").

  • Keil sell their compiler in thousands and have the revenue from that

    Isn't Keil's compiler simply a repackaged ARM's RealView compiler?

  • ... one further wish (if possible) ...
    the assignment of a 32 bit value to a 16 bit variable should give a warning (... loosing precision ...):

    int l= 0x10000;
    short s= l;
    

    now this just compiles without any warning, which really is not very usual (the other C/C++ compilers I know, gives warnings here, unless I cast excplicitely "s= (short)l").