GEvent srcflags out of range

[TriZet]

Hello!

When I have use virtual keyboard I see compiller warning in gwin_keyboard.c line psl->srcflags |= GKEYSTATE_MISSED_EVENT; about out of range.

static void SendKeyboardEventToListener(GSourceListener	*psl, GKeyboardObject *gk) {
	GEventKeyboard		*pe;
	const GVSpecialKey	*skey;
	unsigned			i;

	// If there is no event buffer just mark a missed event
	if (!(pe = (GEventKeyboard *)geventGetEventBuffer(psl))) {
		// This listener is missing - save the meta events that have happened
		psl->srcflags |= GKEYSTATE_MISSED_EVENT;
		return;
	}

[Joel Bodenmann]

Thank you for bringing this to our attention.

For completeness, can you please also add the actual compiler output and information about the compiler you are using?

~ Tectu

[TriZet]

Strange thing...

I see what GKEYSTATE_MISSED_EVENT = 1<<31.

srcflags is 32 bit.

But warning.

..\ugfx\src\gwin/gwin_keyboard.c(131): warning:  #61-D: integer operation result is out of range
..\ugfx\src\gwin/gwin_keyboard.c(131): warning:  #68-D: integer conversion resulted in a change of sign

But if I change GKEYSTATE_MISSED_EVENT to 0x80000000 warning is gone... :?

IDE is Keil 5.

[TriZet]

I think this is stupid compiller bug...

I see in debug what GKEYSTATE_MISSED_EVENT also = 0x80000000. As it should be.

[Joel Bodenmann]

We were able to reproduce the issue.

The problem is that the compiler doesn't seem to recognize that this is an unsigned number and therefore it thinks that an overflow happened. You can supress that warning by casing the flag explicitely:

psl->srcflags |= (unsigned)GKEYSTATE_MISSED_EVENT;

We will see if there's a better solution to this. After all we shouldn't be using unsigned but uint32_t instead for the flag types in the GEVENT struct(s) (not that this would change anything in your case).

For now, I'd recommend you not to worry about this warning. We'll let you know when we changed anything.

Edit: We just pushed a commit to the repository that replaces unsigned with uint32_t. But as mentioned before, this of course doesn't make the warning go away.

I'd just prefer not to explicitly cast flags each time...

~ Tectu

[inmarket]

This is a bug caused by a misinterpretation of the standard with respect to the C constants or alternatively a bug caused by incorrect preprocessor mathematics. Which is happenning is not clear as the expression (1 << 32) could be being evaluated by the compiler or the preprocessor - the standard doesnt specify which. There are simple tests to find out which but that is not important for this discussion.

Possibility 1 - Compiler evaluation...

The compiler is supposed to do all rhs (right hand side) mathematics in the type that matches the lhs expression except where promoted to a different type by a sub expression on the rhs. For this expression the mathematics should be occuring in the same type as the srcflags as there is no explicit sub expression type conversion. In this compiler it is not doing that. Instead it is doing the constant mathematics (1 << 32) in the signed version of that type and then complaining about a sign bit overflow. This is clearly a bug.

Possibility 2 - Preprocessor evaluation

The preprocessor is supposed to evaluate in the "largest available numeric space". For a compiler that has any 64 bit support (and all do these days) that should be in atleast 64 bit mathematics. As such to generate the warning/error is obviously incorrect. Also, even for a compiler/preprocessor with only 32 bit support, the larhest available numeric space for non negative expression is a 32 bit unsigned integer so again the warning/error is a preprocessor bug.

I suspect in this case the expression is being evaluated by the compiler. The solution is not to typecast the assignment but to typecast the macro definition ie #define GKEYSTATE_MISSED_EVENT (((uint32_t)1)<<32).

The other way to work around this is to use the hex representation of the same value. For some reason this seems to trick some of these faulty compilers into treating it as an unsigned value (but not always).

Note this compiler bug workaround is only required for the top bit definition.

[Joel Bodenmann]

Well, I guess the most proper thing that we can do from our side is properly/explicitly typecast the macro definitions. However, for the sake of consistency I'd like to add that to ALL flag definitions, and not just the top bit one.

Any thoughts?

~ Tectu

[TriZet]

Interesting. Other compilers also show this warning? If not, possible only Keil bug.

[Joel Bodenmann]

So far we only witnessed this warning with ARMCC (the compiler used by default by Keil MDK-ARM).

~ Tectu

[TriZet]

Keil still show the same warnings. No matter "unsigned" or "uint32" :?

I have version 5.14 for now. Some time later I will try in 5.17. And will write about results.

[Joel Bodenmann]

Yes, as written in a previous post the uint32_t modification is not meant to fix the warning that you're getting. It's just to ensure that we always get 32-bits worth of flags because 'unsigned' it not 32-bits on all platforms.

We'll push the real fix (casting the statement in the macro explicitly to uint32_t) in the following days.

~ Tectu