Opearation may be undefined in C

Question

I have macros like:

#define INCREMENT(ip) *ip++
#define RETAIN(ip) *ip

and in code, I use them like:

*op |= INCREMENT(ip)<<15;
*op |= INCREMENT(ip)<<20 | RETAIN(ip);

My issue is, I do not get any warning for first line where as I get a warning:

"operation on ‘*ip’ may be undefined [-Wsequence-point]"

for second line.

What is it going wrong?

How should I change my macro to avoid that warning?

Note: I have several statements in code which use macros. I can't change all of them(tidious task). So, please tell me the perfect way of writing macro to avaoid that warning.

Potentially relevant: http://stackoverflow.com/questions/4176328/undefined-behavior-and-sequence-points — Chad, Jul 20 '16 at 16:35
Undefined behaviour: which value of `*ip` will be the operand of `RETAIN(ip)`? — Weather Vane, Jul 20 '16 at 16:37

Eugene Sh. · Answer 1 · 2016-07-20T16:51:21.167

4

The line:

*op |= INCREMENT(ip)<<20 | RETAIN(ip);

is preprocessed into

*op |= *ip++ << 20 | *ip;

As there is no sequence point between the *ip++ << 20and *ip, the postincrement of ip can be sequenced either before or after evaluation of *ip. So the operation has an undefined behavior.

And as a side note: To avoid other(unrelated) troubles (Macro Pitfalls), replace the macro definitions with these:

#define INCREMENT(ip) (*(ip)++)
#define RETAIN(ip) (*(ip))

edited Jul 20 '16 at 16:51

answered Jul 20 '16 at 16:40

Eugene Sh.

17,802
8
40
61

Got it! Can you suggest the way of writing macro for this so as to avoid the warning? – mit23 Jul 20 '16 at 16:42
1

@user2582701 It's not just a warning. It is actual error. And..you have to answer yourself - do you want `ip` to get incremented before or after `RETAIN`? – Eugene Sh. Jul 20 '16 at 16:43
I have changed the macros as suggested, but still the warnings persist!! – mit23 Jul 20 '16 at 16:46
3

You cannot fix this by changing the macro. You need to introduce a sequence point to make the behavior defined. Either `tmp = RETAIN(ip); *op |= INCREMENT(ip)<<20 | tmp` or `tmp = INCREMENT(ip); *op |= tmp<<20 | RETAIN(ip)` will work depending on which thing you want to happen first – trent Jul 20 '16 at 16:46
1

@user2582701 As I said, the last suggestion is to avoid *other unrelated* problems. – Eugene Sh. Jul 20 '16 at 16:47

score 2 · Answer 2 · answered Jul 20 '16 at 16:37

2

When second example is expanded it becomes

*op |= *ip++ << 20 | *ip;

Here it is undefined whether value of second *ip is read before or after ìp is incremented.

answered Jul 20 '16 at 16:37

Markus Laire

2,837
2
17
23

Yeah!! Can you suggest the correct way of writing macro so as to avoid the warning? – mit23 Jul 20 '16 at 16:43

Opearation may be undefined in C

2 Answers2