What is the rationale for this undefined behavior?

Question

Warning[...]: undefined behavior: the order of volatile accesses is undefined in this statement x.cpp xxx

Why this line is undefined behavior?

 case 2: Vdda = 3.3 * (*VREFINT_CAL) / ADC_DR->DATA;

Where the declarations/initializations are:

volatile short const *VREFINT_CAL = (short *) 0x1FFFF7BA;

and

volatile STRUCT_ADC_DR *ADC_DR = (STRUCT_ADC_DR*) 0x40012440;

defined by:

typedef struct { unsigned DATA : 16; unsigned : 16; } STRUCT_ADC_DR;

Is it because the compiler isn't sure about the volatile elements could act diferent in the order they get accessed? (What is the case)

But shouldn't it be ensured that the calculation gets performed from left to right as the operators have the same priority?

I think this should be rephrased as "why does the standard explicitly mark this as undefined behaviour / what's the rationale for it ...", otherwise you'll just get "the standard says so" from a lot of people. — OMGtechy
– OMGtechy, Commented Jul 7, 2015 at 12:54
@deviantfan: At least in C I do and I would bet it is just inherited from it by c++. And why there are hard coded addresses? because it is an embedded system. — dhein
– dhein, Commented Jul 7, 2015 at 12:56
The compiler is right at issuing that warning, though the text is not entirely correct: the order of evaluation of the operands (in this case) is unspecified not undefined. — edmz
– edmz, Commented Jul 7, 2015 at 13:20

ComicSansMS · Accepted Answer · 2015-07-07 13:03:09Z

volatile implies to the compiler that you are reading something that is not an ordinary memory address, like an I/O port. For two such reads, it is very likely that you will want those reads to happen in a certain order.

In both C and C++, the evaluation order of operands is not defined. If it helps you, think of the division as a function call:

Vdda = 3.3 * divide(*VREFINT_CAL, ADC_DR->DATA);

The point is now, that for volatile, where it's likely that the order is important, you might not want to leave this decision to the compiler. So it warns about it.

To get rid of the warning, simply make the order explicit by introducing additional sequence points to your code. For instance:

short const x = *VREFINT_CAL; unsigned const y = ADC_DR->DATA; Vdda = 3.3 * x / y;

You are so right. I could get big trouble when DATA gets accessed before VREFINT_CAL as it causes to reset the value.
@Zaibis Glad to hear that the warning actually helps you catch a bug in this case.
@Zaibis Indeed, the link Alf reported tells you exactly that, when replying to the question "Is it a problem?": it depends on your data and how your applications handles it. The compiler is saying that it may cause problems as its job is far from anything beyond that.
@black I have for my self set a 0 warning tollerance so it at least would be a problem for my conscience. But in this case it was even more. So thanks for help and also thanks Alf for the source.

rodrigo · Accepted Answer · 2015-07-07 13:04:38Z

To understand this you have to know the difference between order of evaluation and precedence.

Take your expression, for example:

Vdda = 3.3 * (*VREFINT_CAL) / ADC_DR->DATA;

Precedence (and parentheses) determines how the abstract syntax tree (AST) is built. The result would be something like this:

= Vdda * 3.3 / * VREFINT_CAL -> ADC_DR DATA

The order of evaluation is determined by the existence of sequence points. And your code has only one sequence point, at the end of the expression (;).

So the order of evaluation of any subexpression is unspecified. That is, the compiler can make any intermediate calculation and memory access in any order it sees fit. Some people like to think that subexpressions are evaluated from left to right, but that's not how the language works.

Normally it will not make any difference, but two of your subexpressions are volatile (*VREFINT_CAL and ADC_DR->DATA) so the order matters. Maybe it does not matter to you, but it certainly matters to the compiler.

To solve the issue use some temporary, just to add a intermediate sequence point:

short a = *VREFINT_CAL; unsigned b = ADC_DR->DATA; Vdda = 3.3 * a / b;

@Zaibis: Well, yes. But you still need a local variable: short a; a = *VREFINT_CAL, b = ADC_DR->DATA, Vdda = 3.3 * a / ...;. If you try to do: Vdda = 3.3 * (0, *VREFINT_CAL) / ...; this won't work.

Collectives™ on Stack Overflow

What is the rationale for this undefined behavior?

2 Answers 2

4 Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

1 Comment

Related