C++ problem

Today, I discovered that if you use:

const unsigned int xxx = value; (which in this case happens to be = 0)

and then in the code try and compare it with a signed int yyy

the compiler ignores your signed int and does an unsigned compare.

Up until now this hadn't been a problem as I've (quite by chance) always looked for equality with the const.

But today I counted yyy down to -1 wishing to stop when yyy < xxx but the sodding compiler decided that I now had a very large positive number and refused to break from my loop.

Took me flipping ages to work out what was wrong :-(

The compiler does give me a warning - that I am comparing a signed with an unsigned but as I was sure that the unsigned int was the zero value const I didn't give it a second thought.

But I did get rid of it by using a c-style cast to make the unsigned int a signed one when comparing, and that suppressed the warning, but it still didn't work. - not until I redefined my const as signed!

I'm not hugely experienced with C++ [1], is it meant to work like this or do I have a broken compiler?

We're using GCC

TIA

Tim

[1] nor are any of my co-workers, finding good ones who will work for the pittance that embedded work now pays in my neck of the woods is tres difficult :-)
Reply to
tim...
Loading thread data ...

1: Yes, it's supposed to work like that. Google "automatic type conversions". 2: I'm pretty sure that C is supposed to work like that, too. If it isn't, then the behavior is implementation defined -- meaning that it won't be predictable at all. 3: "by using a c-style cast to make the unsigned int a signed one when comparing, and that suppressed the warning, but it still didn't work. - not until I redefined my const as signed!"

What? 'splain, please. I thought the const was the only unsigned.

You should use a C++ - style cast:

if (static_cast(xxx) >= yyy) { yadda yadda }

4: Use -Wall. Then, since "all" does not, somehow, mean "all" to the crew at GCC, use -Wextra, too.
--
www.wescottdesign.com
Reply to
Tim Wescott

Post the /exact/ code, then we can see.

When you do an operation (comparison or arithmetic operation), the operands are first promoted towards int, then converted to matching types. If you have an unsigned int and a signed int, the signed int will be converted to unsigned before the operation. That's the rules of C (and C++) - often it can be unexpected, so compilers can warn for such cases.

Reply to
David Brown

I _thought_ it was both, but I wasn't sure.

I vacillate between using unsigned ints to represent whole numbers because -- hey, they're whole numbers, and using ints for everything because of the screwy things that can happen when an unsigned and a signed interact. I still haven't come up with one unified philosophy for it all, though.

--
Tim Wescott 
Wescott Design Services 
 Click to see the full signature
Reply to
Tim Wescott

Am 01.03.2016 um 22:02 schrieb tim...:

That reasoning was just wrong. The thing you were sure about is correct. It doesn't support the conclusion you drew, though.

That attempt most probably failed in some other, unrelated way. Which way it was can only be found out if you show an actual, essentially complete (as in: compilable) example case.

Reply to
Hans-Bernhard Bröker

That's easy. He said he was counting down to -1 which won't work if the value is cast to an unsigned.

The original problem was ignoring warnings. They aren't "errors" unless you don't understand what is happening. They do indicate you are doing something in your code that you should straighten out to make sure you know what is happening. That's way I like strong typing. You either tell the tool just what you intend it to do, or it sits down in the middle of the road until you do.

--

Rick
Reply to
rickman

I pretty much only use unsigned if it's an upcounter that can go past the 0x7FF...F range, unless for some reason is in play.

--
Les Cargill
Reply to
Les Cargill

Yes, C works the same here. The size of "int" is implementation defined, but the other conversions are fixed in the C and C++ standards. You can be sure that gcc follows the rules here - there are bugs in any compiler, but not with something that fundamental.

We need to see the code here to know exactly what is going on (including compiler version and relevant flags).

Well, there is more than one way to view that. C++ offers several ways to cast an unsigned "xxx" into an int:

static_cast(xxx) (int) xxx int(xxx) int xxx2 = xxx; // Not technically a cast, but similar effect

People often say "always use C++ style static_cast", with no justification or reasoning - it's just a rule that "everybody" knows. Sometimes people /do/ try to provide justification. Two common ones are "casts are evil" (which is given as an axiom) and therefore we should use "static_cast" because it stands out, and it is easy to search for. The other reason is that C-style casts and function style casts (which are equivalent) are more powerful - they could also do const_cast or reinterpret_cast, so static_cast is more explicit.

Now, casting with "complicated" types (classes, pointers, references, etc.) should always be done with great care. Using C++ style casts is a good idea there, and can give better compiler checking.

But in a case like this, I would say that int(xxx) is the simplest, clearest, and easiest to include an expression, and it does exactly what it says on the tin. I think int(xxx) is a touch clearer than "(int) x" with a view to precedence and the order of the expression, but for some uses the C compatibility is useful.

In gcc warnings, "all" historically meant "all warnings that are not going to generate warning on a lot of common code", while "extra" (originally just "-W", not "-Wextra") meant "some more warnings including code that many people think is perfectly good". Since then, gcc has acquired many new warnings - a few have been added to -Wall, a few to -Wextra, and many are not part of either set. The gcc team have the attitude that if well-used code has compiled cleanly with -Wall or

-Wall -Wextra before, then it should continue to do so with newer versions of gcc in most cases. Amongst other things, a full rebuild of the debian code base is used to test this.

My makefiles contain a long list of warnings beyond -Wall -Wextra. In many cases, I insist that my own code compiles cleanly there - but a lot of other code does not. For example, gcc has a "-Wpadded" warning that triggers if there is padding added to a struct. Personally, I like to make any padding explicit, and checked with "-Wpadded", but lots of manufacturer-supplied header files will rely on implicit padding. Putting "-Wpadded" in "-Wall" would flood your build outputs with spurious errors.

Reply to
David Brown

Unsigned and signed types have slightly different properties, and they have their advantages and disadvantages. In particular, overflow behaviour of unsigned types is well defined (see below for exceptions) using modular arithmetic, while for signed types it is explicitly undefined. If you want defined overflow behaviour, use unsigned types. But sometimes the undefined nature of signed overflow leads to more efficient code (since the compiler can assume it won't happen, and optimise accordingly).

There are a few cases where unsigned arithmetic can lead to undefined behaviour. In particular, if you have an unsigned type that is of a smaller size than an int, then integer promotion rules say that the values are promoted to signed int, which can have undefined overflow behaviour.

For example, assume we have a 32-bit int, and consider the function:

uint16_t mult16(uint16_t a, uint16_b) { return a * b; }

In practice, the compiler will use whatever is the most convenient unsigned multiply instruction, and truncate the result modulo 16-bit. However, /logically/ (as used in inlining, constant propagation, etc.), the function is this:

uint16_t mult16(uint16_t a, uint16_b) { int32_t a1 = (int32_t) a; int32_t b1 = (int32_t) b; int32_t res1 = a1 * b1; uint16_t res = ((uint32_t) res1) & 0xffff; return res; }

If this is called with a and b equal to 0xffff, the 32-bit signed multiplication will overflow - it is undefined behaviour.

You can also get undefined behaviour with shifts of unsigned types, similar to those for signed types.

As for one unified philosophy, I don't think it is possible to get one. "int" and "unsigned int" (and the types) have overlapping ranges - as long as there are legal int values that cannot be represented cleanly in an unsigned int, and vice versa, there will always be issues. The C conversion and promotion rules are designed so that most things work most of the time without any extra explicit conversions - but sometimes they are simply wrong. Be cautious, be generous with your casts when in doubt, and be generous with your warning flags (like -Wsign-conversion and -Wsign-compare).

This is comp.arch.embedded - should /always/ be in play!

Reply to
David Brown

Since you want only 32bit of the result, this is correct. You get incorrect results only if you use full multiplication (64bit result) with different unsigned/signed operation/result.

When the result of a multiplication has the same size as the operands, the result is correct regardless signed/unsigned of any operands, including the result. This may be counter-intuitive unless you know what you are doing. You must remember that a multiplication always gives a double product, so taking only the low part gives you ... the low part.

Reply to
raimond.dragomir

No, it is not correct - it is undefined behaviour. As signed 32-bit ints, 0xffff * 0xffff would be an overflow, and therefore undefined behaviour. Operations like that are not defined as nice two's complement arithmetic in C (even though in practice, that is what the compiler would probably generate).

It has nothing to do with that (I understand how multiplication works in this context, though I appreciate that not everyone does). The point to be aware of here is that operations on unsigned types smaller than "int" are promoted to /signed/ int before the operation is carried out. That can mean you get unexpected undefined behaviour due to /signed/ overflows, even though /unsigned/ overflow is fully defined in C (and C++).

The point is subtle, and it is unlikely to be an issue in practice. But I think it is worth mentioning these details.

Reply to
David Brown

Sorry, maybe I don't understand your point. I basically said that taking the low part gives correct result. In the very example given, 0xffff (16bit initially)

  • 0xffff (16bit) gives the 0x0001 result (16bit). What 16bit result would you expect? This result is correct in both cases (a 16bit operation (if available) and a converted operation with signed 32bit integers).

Can you give an example where the conversion of an unsigned to the bigger signed type and then the multiplied result of the signed numbers truncated back is wrong?

Reply to
raimond.dragomir

I suppose he means that you do get the correct result by ignoring the overflow, which after all is not the correct result.

$ffff unsigned expands to 32 bit $0000ffff signed, $ffff signed expands to $ffffffff signed. The signed product of these is $ffff0001, which does not fit in 16 bits, it takes 17 bits at least. Hardly a huge issue if one knows what he is doing, but if you rely on a compiler to know these things for you it can't be just dismissed.

Dimiter

------------------------------------------------------ Dimiter Popoff, TGI

formatting link

------------------------------------------------------

formatting link

Reply to
Dimiter_Popoff

0000ffff * 0000ffff = fffe0001 0000ffff * ffffffff = ffff0001 ffffffff * ffffffff = 00000001 Taking the 16bit part: 0001

Which is what I said in the first place: regardless the signed/unsigned operands, you get the same "correct" result. I say "correct" because you DO ignore the overflow. The operands and the result are the SAME SIZE.

Reply to
raimond.dragomir

I don't disagree with your desired results - multiplying 0xffff by

0xffff using uint16_t, one would expect a result of 0x0001 as a uint16_t.

The issue is that the logical process of doing this multiplication involves undefined behaviour in C. As I noted earlier (but you snipped), the logical operation in C, once all integer promotions and arithmetic conversions are made explicit, is this:

uint16_t mult16(uint16_t a, uint16_b) { int32_t a1 = (int32_t) a; int32_t b1 = (int32_t) b; int32_t res1 = a1 * b1; uint16_t res = ((uint32_t) res1) & 0xffff; return res; }

(As before, we are assuming 32-bit int, and nice two's complement representation - no "weird" architectures.)

If you want chapter and verse, the references here are to the C11 standard. The final draft version, N1570, is freely found on the net.

  1. First, the uint16_t objects get integer promotion to "int" (6.3.2). Thus values (uint16_t) 0xffff become (int32_t) 0xffff. (Technically, "int32_t" does not have to be the same as "int" - it could be "long". But I'll call it "int32_t" to make the sizes clear.)

  1. Then the calculation is done as a 32-bit signed integer multiplication. (Note that this is the /logical/ operation, required by the C standards. The compiler can optimise this to a different unsigned operation if it wants as long as the results are the same.)

  2. This 32-bit signed int result "res1" is then converted to a uint16_t by repeatedly adding or subtracting 0x10000 until it is in the range 0 .. 0xffff. (6.3.1.3p2) This is the final value of the multiplication.

It is at stage 2 that we have a problem. The operation is to multiply two 32-bit signed integers, each of value 0xffff. This is impossible within the realm of 32-bit signed integers - the mathematically correct result of 0xffff0001 cannot be represented as an int32_t (that bit-pattern corresponds to -65535, which is a different number). Thus it falls foul of 6.5p5 - "If an exceptional condition occurs during the evaluation of an expression (that is, if the result is not mathematically defined or not in the range of representable values for its type), the behavior is undefined.".

What will the compiler do when it sees such undefined behaviour? Probably nothing - it is likely that multiplying two uint16_t will generate exactly the code you would expect all along. But you /could/ get unexpected behaviour, especially if inlining and/or constant propagation passes mean that the compiler could see the values of "a" and "b" at compile time, and knows that there is going to be undefined behaviour during the calculation.

What is a little more likely with the integer promotion rules is not nasal daemons due to such undefined behaviour, but unexpected behaviour nonetheless. For example, given this code:

uint16_t foo1(void) { uint16_t a = 0xffff; uint16_t b = 0xffff; return a * b; }

A compiler will typically optimise this to "return 1;". It /could/ return any value it wanted, since the behaviour is undefined, but compiler writers go out of their way to produce efficient code, rather than actively annoying code.

On the other hand, this code:

bool foo2(void) { uint16_t a = 0xffff; uint16_t b = 0xffff; return (a * b) > 0; }

will typically optimise to "return false;". So the same expression "a * b" looks like 1 in the first function, but is not greater than 0 in the second function. It all makes sense when you remember that (a * b) is a

32-bit signed integer expression, even though "a" and "b" are unsigned 16-bit types.
Reply to
David Brown

No, the issue is that the signed product 0xffffffff does not fit in a signed 32-bit integer - and that is undefined behaviour. This intermediary value exists logically before the conversion to 16-bit unsigned by modulo arithmetic.

Reply to
David Brown

David, the product is not $ffffffff - how did you arrive at that?

Dimiter

Reply to
Dimiter_Popoff

Hmmmm, but 0001 does not equal ffff0001. The result would be correct it we state it as (-1)*($ffff)!.$ffff (!. being logical and, $ffff being unsigned and -1 being signed,

32 bit all 1. ). IOW, (-1)*(-1) is 1, both input numbers being signed; but

-1*65535 (just the -1 being signed) is -65535 or $10001 .

Like you say it is all about ignoring the overflow, if you are OK with that of course you get the correct result - but it is not just multiplication in that case, there is also the AND above.

Dimiter

Reply to
Dimiter_Popoff

Yup, downcounting loops with unsigned loop counters is a classic bug. The OP will remember next time. ;)

Signed ints don't have the problem, obviously, and this is one reason it's a bad idea to reuse loop counter variables, unless you're writing for an 8-bit MCU.

Since essentially all computers still crunching use twos-complement binary for signed ints, that behaviour is very nearly guaranteed. IIRC you can turn on integer overflow traps so that it behaves like a divide-by-zero, but a whole lot of software relies on this behaviour.

Yup. A copy of PC-Lint is helpful too (assuming they've kept it up to date--my copy doesn't understand anything past C++08.)

Cheers

Phil Hobbs

--
Dr Philip C D Hobbs 
Principal Consultant 
 Click to see the full signature
Reply to
Phil Hobbs

The counter IS (was) signed, it is the const value being compared against that is not

it's the const that being reused (sort of) here, everywhere else they need to be unsigned because we need the range :-(

I now have an inconsistency in my set of definitions for this class

tim

Reply to
tim...

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.