ELKS compilation appears broken

[uityyy]

I built dev86 (ELKS's development environment) and elks from github source yesterday and today tried to build nano-x/microwindows with the config.elks settings.

This resulted in compilation errors beginning with: src/engine/devdraw.c:1124.13: error: duplicate case in switch

This code is switching several constants that have different hex values but are identical in the least-significant 16-bits. This appears to be breaking the 16-bit compilation.

[georgp24]

The devdraw.c code has not been modified for six years. I guess it would need some maintenance and testing for the elks platform.

There is a DOS port but that is based on DJGPP. The real mode DOS parts within the code will not work.

You may download an old version of Microwindows that has been tested with ELKS here: ftp://microwindows.censoft.com/pub/microwindows/

Georg

[ppisa]

Hello Georg and Uityyy,

the data_format type truncation can appear in many places. I have prepared transient patch to find (check by compiler) all possible occurrences where truncation can happen.

See my branch data_format-int32-transient which replaces all format manipulations by structure to let compiler find all occurrences for me

https://github.com/ppisa/microwindows/tree/data_format-int32-transient

I have prepared branch with change of these all occurrences to int32_t there

https://github.com/ppisa/microwindows/tree/data_format-int32

You should try to build that to check if there are other problems for 16-bit targets. I have used long long time ago Microwindows on 16-bit MCUs and pushed some corrections. But I expect that there are more problems anyway.

But I think that data_format correction for 16-bit systems should be pushed to mainline. I think that direct use of int32_t is not big problem in most of Microwindows code. It is used already for many types.

But there is some problem with external API headers

include/nano-X.h include/windows.h nanox/serv.h

The standard X-windows headers do not use directly int32_t and standard Windows GDI/UI API do not use it either. So even Microwindows should strive to not use these data types in these headers directly.

The data_format-int32-transient allows locate all problematic places very easily and preparation of sources version which uses specific type for data_format (for example MWFORMATVAL) means running three sed commands if we agree on this way.

Then

typeded MWFORMATVAL GR_FORMATVAL;

can be defined for X-11 API. For Microwindows windows API version, there is problem with LONG definition which is 64-bit on 64-bit Unix systems. Probably to allow pointers passing. This is different to 64-bit Windows. But using 64-bit type for format is nonsense.

If we agree that format is unsigned (when used in bitwise bool operations, then it should be anyway) then we can use uint32_t and DWORD on Windows API which would be quite reasonable solution.

If ACKed then I can prepare such change and push it to Microwindows master.

[ghaerr]

Microwindows became too large for an ELKS link some time ago, and its recent version won’t work for that and a variety of other reasons. I saved a version known to work on ELKS that is documented on the website and release notes, please refer to that version to build a version known to link and run on ELKS.

More specifically, the error below is relating to 32 bit constants that were used to allow opcode-style drawing instructions along with additional format information within a 32 bit word. The ELKS compiler doesn’t support longs in switch statements. Recoding this would be more work than it’s worth, and it wouldn’t link or run anyways due to the 64k real mode restriction for both the OS and running program in ELKS.

Regards,

Greg

On Dec 27, 2016, at 8:48 PM, uityyy notifications@github.com wrote:

I built dev86 (ELKS's development environment) and elks from github source yesterday and today tried to build nano-x/microwindows with the config.elks settings.

This resulted in compilation errors beginning with: src/engine/devdraw.c:1124.13: error: duplicate case in switch

This code is switching several constants that have different hex values but are identical in the least-significant 16-bits. This appears to be breaking the 16-bit compilation.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ghaerr/microwindows/issues/2, or mute the thread https://github.com/notifications/unsubscribe-auth/ALbi5b6uo1JMth1AEVOg5OwMUY9gUds_ks5rMeoQgaJpZM4LWq8w.

[ghaerr]

Pavel,

In general, I am in disagreement that anything should be done to sort out 16 vs 32 bit issues in microwindows. The decision was made long ago to advance out of 16-bit only targets and known working source trees were made that can be used for those systems (none of which I am aware of still used, other than ELKS). There have been many enhancements during the time microwindows was being actively written to enhance performance on 32 bit systems, and there is little benefit to undo these changes.

Regarding the external API headers, it is very important that those do not change (windows.h and nano-X.h) or there will be major compatibility issues with existing windows and nano-X program source code.

More specifically, the DWORD, LONG and uint_32t types are very specially handled for 32-bit windows compatibility, and changing them will cause all sorts of issues with older programs. That said, the windows API in microwindows is definitely not 64-bit compatible as you have mentioned. For that matter, microwindows has not been developed nor tested on 64 bit systems, and none of the fast blit routines are optimized to take advantage of those systems. It seems most of the modern systems use GPUs to do drawing and don’t need the software drawing approach microwindows uses anyways.

Of course, all this could be figured out for modern Windows API compatibility, but I’m not sure anybody cares, and it risks backwards compatibility problems for anybody using microwindows for 32-bit systems.

Do we need to upgrade mwin to run on 64-bit compilers? If so, I suggest that the nano-X and Windows portions be done entirely seperately, with nano-X done first. That will eliminate the problem of having to worry about 32-bit Windows compatibility and still allow all nano-X programs to compile with little to no changes.

My $0.02 worth,

Greg

On Dec 28, 2016, at 4:40 AM, Pavel Pisa notifications@github.com wrote:

Hello Georg and Uityyy,

the data_format type truncation can appear in many places. I have prepared transient patch to find (check by compiler) all possible occurrences where truncation can happen.

See my branch data_format-int32-transient which replaces all format manipulations by structure to let compiler find all occurrences for me

https://github.com/ppisa/microwindows/tree/data_format-int32-transient https://github.com/ppisa/microwindows/tree/data_format-int32-transient I have prepared branch with change of these all occurrences to int32_t there

https://github.com/ppisa/microwindows/tree/data_format-int32 https://github.com/ppisa/microwindows/tree/data_format-int32 You should try to build that to check if there are other problems for 16-bit targets. I have used long long time ago Microwindows on 16-bit MCUs and pushed some corrections. But I expect that there are more problems anyway.

But I think that data_format correction for 16-bit systems should be pushed to mainline. I think that direct use of int32_t is not big problem in most of Microwindows code. It is used already for many types.

But there is some problem with external API headers

include/nano-X.h include/windows.h nanox/serv.h

The standard X-windows headers do not use directly int32_t and standard Windows GDI/UI API do not use it either. So even Microwindows should strive to not use these data types in these headers directly.

The data_format-int32-transient allows locate all problematic places very easily and preparation of sources version which uses specific type for data_format (for example MWFORMATVAL) means running three sed commands if we agree on this way.

Then

typeded MWFORMATVAL GR_FORMATVAL;

can be defined for X-11 API. For Microwindows windows API version, there is problem with LONG definition which is 64-bit on 64-bit Unix systems. Probably to allow pointers passing. This is different to 64-bit Windows. But using 64-bit type for format is nonsense.

If we agree that format is unsigned (when used in bitwise bool operations, then it should be anyway) then we can use uint32_t and DWORD on Windows API which would be quite reasonable solution.

If ACKed then I can prepare such change and push it to Microwindows master.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ghaerr/microwindows/issues/2#issuecomment-269471972, or mute the thread https://github.com/notifications/unsubscribe-auth/ALbi5e8zJYu5ycrAwK8V9Rq3PphoHMe9ks5rMljVgaJpZM4LWq8w.

[ppisa]

My proposal to change to int32_t is exactly fixing API to keep already used ABI for all target options. So this should be completely safe. If there is no need for negative format numbers then I would prefer uint32_t. Negative numbers would be problem anyway, because the second complement encoding and use of masks for negative values is not guaranteed by current C standard and use on signed types is generally discouraged for bitwise operations.

So change to uint32_t can be required in a long term anyway. Defining MWFORMATVAL would generally clarify encoding of values. So this change would help as well. DWORD in Win API is unit32_t, so again good step to freeze encoding. I am not sure about GRFORMATVAL or alternative. I expect that GR should match X11 definitions, so that can be problem.

I have weak preference/vote for above change to make code better readable and builds results for different environments more predictable. C standard requires only +/-32767 range for int. So direct use of int in this role is not good practice.

As for 64-bit, we do not use 64-bit embedded targets which run Microwindows code. But I build our RTEMS (32 MB RAM) and MCU (NXP LPC 32k RAM) on Debian amd64/x86_64 to test code, use Valgrind on it etc from time to time. And actual Microwindows demos runs in this environment and I have never problem with subset of APIs used in our applications. I have not checked any ABI compatibility with true X11 applications and there would be fundamental problems.

What is real problem on 64-bit Windows is LONG type. It is and should be 32-bit there to match Microsoft LLP64 choice but it has been often used to hold pointers in callbacks and this would break. Unfortunately, Microsoft has invented many own types instead of using standard defined intptr_t and uintptr_t directly. So there is INT_PTR, LONG_PTR, ULONG_PTR and DWORD_PTR which maps to another nonstandard base type __int64. See

https://msdn.microsoft.com/en-us/library/ms810720

The Win API should introduce *_PTR types in long term to support 64-bit Windows. But that is area I do not care much.

There could be problem with LONG and DWORD on 64-bit LP64 (Linux) systems in actual Microwindows/Nano-X code. If LONG is used in any place where fixed size (some image header for example) layout is expected then it breaks on 64-bit Linux. DWORD is mapped/fixed to uint32_t, so it is OK. But copying of pointer to DWORD leads to truncation. Mixing LONG and DWORD in some overlaid data structures could lead to breakage on 64-bit LP64 systems.

I have not encountered crashes which I would account to this problem. But some (ideally tools, sparse, compiller, llvm, driven) analysis to check for such problems could be important in long term.

If you agree with MWFORMATVAL and ideally unsigned one, I would prepare patch to introduce that. It is cheap for me now. It would cost much more time after some other changes to the sources because it would require to start mostly from scratch.

[georgp24]

Pavel,

I agree with Greg that it is hard to see applications for 16 bit code. I assume you have one yourself for which you did this commit. This could be a good objective for the change, but otherwise if this commit does not get ELKS working, as Greg mentioned, I think the commit is not required and just makes the code complicated to read and to extend in the future. At least Uityyy should test if it works with ELKS before this is committed to the main branch. Probably he does not have the time for that.

I looked at your commit but could not find the structure that defines trans_data_format_t and trans_data_format_val. In general I would call these MWint_t and MWint_val instead for better readability before your commit is applied.

I think it makes good sense to replace „int“ with „int32_t“ or „uint32_t“ but I would do that in the code directly and not use redefine. Then you can check if a negative format is required at that position in the code too. Also the header files should not be changed without testing whether FLTK still runs with Microwindows.

For my Nanolinux 64bit version I also compiled Microwindows for 64bit. And got it to run on SUSE 64bit for development and testing. I also compiled it for the Raspberry PI but got stuck modifing the mouse driver for that platform.

I do not expect new applications using the Windows API of Microwindows, so this API should mainly target the existing applications.

Georg

[ppisa]

Hello Greg and Georg,

I do not plan to use Microwindows on any 16-bit target. But there exists bunch of targets with 16-bit int which can be considered. They are not limited to 64kB address space. I have used Microwindows on H8S, 16-bit int but 32-bit pointers long ago. Other such potential targets are dsPIC, MSP430X etc. My actual use of some subset of older Microwindows snapshot on one ARM Cortex-M4 runs on 32+32 kB RAM and takes with complex application 220 kB of Flash. So there is use for Microwindows on memory constrained targets.

But generally I do not expect to use H8S or other 16-bit target with Microwindows anymore and do not care much about 16-bit. On the other hand declaring type right has some reason and when it helps others with 16-bit or other porting then even better.

The transient type is defined only for "data_format-int32-transient" branch.

https://github.com/ppisa/microwindows/commit/a614b9845c0fc160ce297e76d767da9c05e4d419#diff-a1e78a077907ef63a7802fc785e7172cL255

typedef struct {
  int32_t trans_data_format_val;
} trans_data_format_t;

This is dirty hack to ensure that compiler fails with error for each assignment of any kind of integer to the type. But it allows to locate all occurrences of such assignment.

The branch "data_format-int32" does not introduce any such hackery. It uses plain int32_t. I would incline to use type which declares purpose of the value - MWFORMATVAL in this case. I would prefer (over MWint_t) to use plain standard defined types int32_t etc for other cases, when only size is in the question. C99 standard is more than 16 years old so I hope that even such strange nonstandard environments (as Microsoft compilers and systems are) can provide these. I have found information that Visual C++ 2010 includes proper stdint.h.

[georgp24]

Pavel, found your commit now: https://github.com/ppisa/microwindows/commit/b580fd45d546386f2b3ad7bf25631b0bb9147ddd

Replacing" int" with "int32_t" looks good to me and I would do that. I doubt that an image format needs negative values so you could manually do "uint32_t" if it is safe to do so.

I do not understand what you mean with: "I would incline to use type which declares purpose of the value - MWFORMATVAL in this case." When I read "int32_t" in the code I know immediately what is meant, if there is MWFORMATVAL instead I have to look up what that means. So please just replace "int" with "int32_t" or "uint32_t". This is better than just "int" which is platform dependent.

Georg

[uityyy]

What was the latest version tested to work with ELKS? I haven't seen this in the documentation.

I did see 64kb-barrier-related errors with some combination of nano-x and bcc version last night, so I'd imagine (without actual testing) that Greg is right that data types alone will not fix ELKS compilation under the current revision.

I guessed 0.87 would work and spent a couple hours trying to get it to compile with a version of bcc released shortly before it (dev86 0.15.0 and elks headers from the time of nano-x 0.87), but so far I haven't succeeded (comments in inline assembly causing compile errors and variants of variable names used inside inline assembly appearing as undefined symbols -- feels like a different permutation of bcc or some system assembly might fix that if I'm up for more tinkering)

[georgp24]

Uityyy,

for ELKS I would try microwindows-full-0.91.tar.gz or, smaller without fonts, microwindows-src-0.91.tar.gz.

Georg

[uityyy]

Building 0.91 for elks failed with devfont.c...:782.16: error: non-integral selector in switch on what appears to be an unsigned long.

It appears the last version tested with ELKS was 0.86. One Jody Bruchon patched 0.87 last year to build for ELKS but wasn't able to get the demos to work: http://markmail.org/message/nopjbj4cnyzubyu7

I, for one, have now tried some of the old pre-compiled microwindows ELKS demos (ftp://microwindows.censoft.com/pub/microwindows/Historic/bin/ElksExamples/) with ELKS 0.1.0, 0.1.4, and current without seeing anything more exciting than a blank screen (microwin generally produces a blank screen when run alone or after another demo and "cannot initialize screen" when run with &;other-demo. mterm produces nothing or an error when run alone depending on the ELKS version.) I could still be doing something wrong, but I think I'll stop here.

[georgp24]

I am not familiar with DEV86 but in this file https://github.com/lkundrak/dev86/blob/80d485b8014cbb926ce8a00cb87b2ca923c5618c/libc/include/stdint.h there is a typedef for unsigned long.

So I am not aware why this error is caused.

Looking at the change.log there were patches added for ELKS in 0.88pre2. However, my experience is that these versions of Microwindows usually do not work "out of the box" today.

[georgp24]

uityyy,

I looked at the 0.88pre2 code. The screen driver used did write directly to the VGA hardware. This driver will also be used by earlier versions.

Newer PCs will not have VGA compatible graphics any more. So this may be the reason why you got a blank screen when running the examples.

Georg