[lnkForumImage]
TotalShareware - Download Free Software
Confronta i prezzi di migliaia di prodotti.
Asp Forum
 Home | Login | Register | Search 


 

Forums >

comp.lang.c

compiler optimizing wrong?

Dirk Zabel

6/10/2011 12:08:00 PM

Hi,
I get unexpected behaviour from some code which was part of a
manufacturer-supplied library for hardware-related functions
(programming internal flash memory). I would like to get your opinion if
compiler or the code is the culprit.

The compiler is gcc-arm 4.5.1. When no optimization is selected, the
generated code looks as expected (surprise!). If size optimization is
selected with argument "-Os", the getstatus() call inside the loop is
removed.
I stripped down the code as far as possible, so it is more or less
abstract now.

--------------- snip --------------------------------
typedef enum {
st_busy, st_complete, st_timeout
} status_t;

typedef struct
{
volatile uint32_t CR;
volatile uint32_t SR;
} hw_reg_t;


extern hw_reg_t statusreg;
/*
* Get status (busy or complete)
* from inspection of hardware status register
*/
status_t getstatus(void)
{
status_t rval;

if ((statusreg.SR & 1) == 1) {
rval = st_busy;
} else {
rval = st_complete;
}
return rval;
}

/*
* wait until hardware operation is complete or
* timeout occurred
*/
status_t wait_for_operation(unsigned long Timeout)
{
status_t status;

status = getstatus();
while((status == st_busy) && (Timeout != 0)) {
status = getstatus(); /* <- optimized out! */
Timeout--;
}
if(Timeout == 0 && status == st_busy ) {
status = st_timeout;
}
return status;
}
--------------- snip --------------------------------

If I remove the volatile qualifiers from the struct members and instead
put volatile in front of the variable declaration, the problem vanishes:

--------------- snip --------------------------------
typedef struct
{
uint32_t CR;
uint32_t SR;
} hw_reg_t;

extern volatile hw_reg_t statusreg;
--------------- snip --------------------------------

Btw, it also does not occur if I access the status register inline in
wait_for_operation() instead of calling getstatus().

Is this a compiler error or are the volatile declarations inside the
typedef unsufficient?

Regards
Dirk
21 Answers

Barry Briggs

6/10/2011 12:19:00 PM

0

Dirk Zabel wrote:

> I get unexpected behaviour from some code which was part of a
> manufacturer-supplied library for hardware-related functions
> (programming internal flash memory). I would like to get your opinion if
> compiler or the code is the culprit.
>
> The compiler is gcc-arm 4.5.1. When no optimization is selected, the
> generated code looks as expected (surprise!). If size optimization is
> selected with argument "-Os", the getstatus() call inside the loop is
> removed.
> I stripped down the code as far as possible, so it is more or less
> abstract now.

On a related note, you might be interested in the following
article by John Regehr.

Volatile Bugs, Three Years Later
http://blog.regehr.org/ar...

Regards.

Leo Havmøller

6/10/2011 12:30:00 PM

0

extern volatile hw_reg_t statusreg;

Leo Havmøller.

Thad Smith

6/11/2011 3:53:00 AM

0

On 6/10/2011 5:08 AM, Dirk Zabel wrote:
> Hi,
> I get unexpected behaviour from some code which was part of a
> manufacturer-supplied library for hardware-related functions (programming
> internal flash memory). I would like to get your opinion if compiler or the code
> is the culprit.
....
> status = getstatus();
> while((status == st_busy) && (Timeout != 0)) {
> status = getstatus(); /* <- optimized out! */
> Timeout--;

This is a compiler bug. Since getstatus() accesses a volatile object, it must
be done each time through the loop.

--
Thad

Dirk Zabel

6/14/2011 11:25:00 AM

0

Am 10.06.2011 14:08, schrieb Dirk Zabel:
Thanks for the answers.

jstrap42

6/14/2011 2:42:00 PM

0

On Jun 10, 1:19 pm, Noob <r...@127.0.0.1> wrote:
>
> On a related note, you might be interested in the following
> article by John Regehr.
>
> Volatile Bugs, Three Years Laterhttp://blog.regehr.org/ar...
>
> Regards.

The example given in section 2.1 of that paper is this (I've had to
retype it so there could be mistakes):

volatile int buffer_ready;
char buffer[BUF_SIZE];

void buffer_init() {
int i;
for(i=0; i<BUF_SIZE; i++)
buffer[i]=0;
buffer_ready=1;
}

Can anyone point out exactly where the sequence points are in that
code snippet?

Chris H

6/14/2011 2:54:00 PM

0

In message <ist23m$l4r$1@dont-email.me>, Noob <root@127.0.0.1> writes
>Dirk Zabel wrote:
>
>> I get unexpected behaviour from some code which was part of a
>> manufacturer-supplied library for hardware-related functions
>> (programming internal flash memory). I would like to get your opinion if
>> compiler or the code is the culprit.
>>
>> The compiler is gcc-arm 4.5.1. When no optimization is selected, the
>> generated code looks as expected (surprise!). If size optimization is
>> selected with argument "-Os", the getstatus() call inside the loop is
>> removed.
>> I stripped down the code as far as possible, so it is more or less
>> abstract now.
>
>On a related note, you might be interested in the following
>article by John Regehr.
>
>Volatile Bugs, Three Years Later
>http://blog.regehr.org/ar...

The article is misleading as it talks about "embedded compilers" by
which they mean GCC. 11 of the 14 were GCC and two of the other three
were hardly mainstream embedded.

On the other hand I know a lot of embedded compilers they did not look
at that work correctly. (I know about two of them myself we tested for
safety critical use)




--
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
\/\/\/\/\ Chris Hills Staffs England /\/\/\/\/
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/

Barry Briggs

6/14/2011 3:30:00 PM

0

Chris H wrote:

> In message <ist23m$l4r$1@dont-email.me>, Noob <root@127.0.0.1> writes
>> Dirk Zabel wrote:
>>
>>> I get unexpected behaviour from some code which was part of a
>>> manufacturer-supplied library for hardware-related functions
>>> (programming internal flash memory). I would like to get your opinion if
>>> compiler or the code is the culprit.
>>>
>>> The compiler is gcc-arm 4.5.1. When no optimization is selected, the
>>> generated code looks as expected (surprise!). If size optimization is
>>> selected with argument "-Os", the getstatus() call inside the loop is
>>> removed.
>>> I stripped down the code as far as possible, so it is more or less
>>> abstract now.
>>
>> On a related note, you might be interested in the following
>> article by John Regehr.
>>
>> Volatile Bugs, Three Years Later
>> http://blog.regehr.org/ar...
>
> The article is misleading as it talks about "embedded compilers" by
> which they mean GCC.

You're confusing the article and the paper.
I counted 0 occurence of the string "embedded compiler"
in Regehr's article.

> 11 of the 14 were GCC and two of the other three
> were hardly mainstream embedded.

IIUC, you dispute the conclusion of their 2008 paper, because
their study didn't include several widely-used compilers?

Can you name these compilers? Are they available at no cost
for academic research?

> On the other hand I know a lot of embedded compilers they did not look
> at that work correctly. (I know about two of them myself we tested for
> safety critical use)

What point are you're trying to make.

Shao Miller

6/14/2011 3:32:00 PM

0

On 6/14/2011 9:41 AM, jstrap42@yahoo.co.uk wrote:
> On Jun 10, 1:19 pm, Noob<r...@127.0.0.1> wrote:
>>
>> On a related note, you might be interested in the following
>> article by John Regehr.
>>
>> Volatile Bugs, Three Years Laterhttp://blog.regehr.org/ar...
>>
>> Regards.
>
> The example given in section 2.1 of that paper is this (I've had to
> retype it so there could be mistakes):
>
> volatile int buffer_ready;
> char buffer[BUF_SIZE];
>
> void buffer_init() {
> int i;
> for(i=0; i<BUF_SIZE; i++)
> buffer[i]=0;
> buffer_ready=1;
> }
>
> Can anyone point out exactly where the sequence points are in that
> code snippet?
>

I think at each semi-colon, except for the first three. Also in the
evaluation of the size specification in the declaration of 'buffer'.
Also after 'i++'.

Chris H

6/14/2011 5:11:00 PM

0

In message <it7upn$tqq$1@dont-email.me>, Noob <root@127.0.0.1> writes
>>
>> The article is misleading as it talks about "embedded compilers" by
>> which they mean GCC.
>
>You're confusing the article and the paper.
>I counted 0 occurence of the string "embedded compiler"
>in Regehr's article.

The introduction says
//////////////////////////
We tested thirteen production-quality C compilers
and, for each, found situations in which the compiler generated
incorrect code for accessing volatile variables. This result is
disturbing because it implies that embedded software and operating
systems both typically coded in C, both being bases for many
mission-critical and safety-critical applications, and both relying
on the correct translation of volatilesâ??may be being miscompiled.
///////////////

and the targets were IA32, Coldfire, MSP430 and AVR

Though you are not likely to be using GCC for safety critical or mission
critical code. Certainly not on the higher SIL levels.

But I do continually hear, due to this report, that "embedded compilers
don't handle volatile correctly" normally when referring to compilers
that were not referenced in the report and do handle volatile correctly.

>IIUC, you dispute the conclusion of their 2008 paper, because
>their study didn't include several widely-used compilers?

Well.... To be blunt it only really tested one. Variants of GCC.

>Can you name these compilers?

No I was/am under NDA when we did the testing but it was for safety
critical use.

>Are they available at no cost
>for academic research?

So it is research on compilers they "can get for free?" Though I do
take your point that it is difficult to test compilers when you have to
buy them as they are not cheap.

On the other hand as most of the commercial compilers are heavily and
rigorously tested internally what is the advantage in letting a bunch of
academics have them?

In which case the report should have been entitled "GCC Volatiles Are
Miscompiled, and What to Do about It"


--
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
\/\/\/\/\ Chris Hills Staffs England /\/\/\/\/
\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/

Tim Rentsch

6/15/2011 9:16:00 AM

0

Please excuse my following up in the wrong place in the thread
and with no attributions. Newsreader glitch.

> The example given in section 2.1 of that paper is this (I've had to
> retype it so there could be mistakes):
>
> volatile int buffer_ready;
> char buffer[BUF_SIZE];
>
> void buffer_init() {
> int i;
> for(i=0; i<BUF_SIZE; i++)
> buffer[i]=0;
> buffer_ready=1;
> }
>
> Can anyone point out exactly where the sequence points are in that
> code snippet?

After each full declarator:

buffer_ready in 'volatile int buffer_ready;'
buffer[BUF_SIZE] in 'char buffer[BUF_SIZE];'
i in 'int i;'

After each full expression:

i=0
i<BUF_SIZE
i++
buffer[i]=0
buffer_ready=1

(Of course the middle three of these potentially occur multiple
times because of the 'for' loop.)


It doesn't make much sense to talk about sequence points
in (or after) declarators at file scope, but these points
are what's specified in the Standard.