gvanem / Watt-32

Watt-32 TCP/IP library and samples.
https://www.watt-32.net/
18 stars 8 forks source link

Open Watcom 16-bit BSD API build warnings #112

Closed Lethja closed 5 months ago

Lethja commented 6 months ago

When USE_BSD_API is defined for a 16-bit DOS with Open Watcom build (wmake -h -f watcom_l.mak). The following warnings appear:

ioctl.c(99): Warning! W135: Shift amount too large
ioctl.c(128): Warning! W135: Shift amount too large
ioctl.c(285): Warning! W135: Shift amount too large
ioctl.c(286): Warning! W135: Shift amount too large
ioctl.c(287): Warning! W135: Shift amount too large
ioctl.c(290): Warning! W135: Shift amount too large
ioctl.c(291): Warning! W135: Shift amount too large
ioctl.c(316): Warning! W135: Shift amount too large
ioctl.c(325): Warning! W135: Shift amount too large
ioctl.c(348): Warning! W135: Shift amount too large
ioctl.c(349): Warning! W135: Shift amount too large
ioctl.c(353): Warning! W135: Shift amount too large
ioctl.c(357): Warning! W135: Shift amount too large
ioctl.c(377): Warning! W135: Shift amount too large
ioctl.c(402): Warning! W135: Shift amount too large
ioctl.c(403): Warning! W135: Shift amount too large
ioctl.c(410): Warning! W135: Shift amount too large
ioctl.c(413): Warning! W135: Shift amount too large
ioctl.c(418): Warning! W135: Shift amount too large
ioctl.c(422): Warning! W135: Shift amount too large
ioctl.c(439): Warning! W135: Shift amount too large
ioctl.c(443): Warning! W135: Shift amount too large
ioctl.c(444): Warning! W135: Shift amount too large
ioctl.c(451): Warning! W135: Shift amount too large
ioctl.c(456): Warning! W135: Shift amount too large
ioctl.c(457): Warning! W135: Shift amount too large
ioctl.c(472): Warning! W135: Shift amount too large
ioctl.c(519): Warning! W135: Shift amount too large
ioctl.c(523): Warning! W135: Shift amount too large
ioctl.c(527): Warning! W135: Shift amount too large
ioctl.c(531): Warning! W135: Shift amount too large
ioctl.c(535): Warning! W135: Shift amount too large
ioctl.c(561): Warning! W135: Shift amount too large
ioctl.c(571): Warning! W135: Shift amount too large
ioctl.c(572): Warning! W135: Shift amount too large
ioctl.c(591): Warning! W135: Shift amount too large
ioctl.c(616): Warning! W135: Shift amount too large
ioctl.c(617): Warning! W135: Shift amount too large
ioctl.c(618): Warning! W135: Shift amount too large
ioctl.c(619): Warning! W135: Shift amount too large
ioctl.c(620): Warning! W135: Shift amount too large
ioctl.c(621): Warning! W135: Shift amount too large
ioctl.c(622): Warning! W135: Shift amount too large
ioctl.c(623): Warning! W135: Shift amount too large
ioctl.c(624): Warning! W135: Shift amount too large
ioctl.c(625): Warning! W135: Shift amount too large
ioctl.c(626): Warning! W135: Shift amount too large
ioctl.c(627): Warning! W135: Shift amount too large
ioctl.c(628): Warning! W135: Shift amount too large
ioctl.c(629): Warning! W135: Shift amount too large
ioctl.c(630): Warning! W135: Shift amount too large
ioctl.c(631): Warning! W135: Shift amount too large
ioctl.c(632): Warning! W135: Shift amount too large
ioctl.c(633): Warning! W135: Shift amount too large
ioctl.c(634): Warning! W135: Shift amount too large
ioctl.c(635): Warning! W135: Shift amount too large
ioctl.c(636): Warning! W135: Shift amount too large
ioctl.c(637): Warning! W135: Shift amount too large
ioctl.c(638): Warning! W135: Shift amount too large
ioctl.c(639): Warning! W135: Shift amount too large
ioctl.c(640): Warning! W135: Shift amount too large
ioctl.c(641): Warning! W135: Shift amount too large
ioctl.c(642): Warning! W135: Shift amount too large
ioctl.c(643): Warning! W135: Shift amount too large
ioctl.c(644): Warning! W135: Shift amount too large
ioctl.c(645): Warning! W135: Shift amount too large
ioctl.c(646): Warning! W135: Shift amount too large
ioctl.c(647): Warning! W135: Shift amount too large
ioctl.c(648): Warning! W135: Shift amount too large
ioctl.c(649): Warning! W135: Shift amount too large
ioctl.c(650): Warning! W135: Shift amount too large
ioctl.c(651): Warning! W135: Shift amount too large
ioctl.c(652): Warning! W135: Shift amount too large
ioctl.c(653): Warning! W135: Shift amount too large
ioctl.c(654): Warning! W135: Shift amount too large
ioctl.c(655): Warning! W135: Shift amount too large

These warning haven't caused any trouble in my projects. Creating this issue just to document it in case it's the cause of a bug found in the future

gvanem commented 6 months ago

That would be the expansion of the VERIFY_RW() macro. Only important for protected-mode.

When TARGET_IS_32BIT is not defined (as for 16-bit Watcom), I fail to see what it's complaining about. Care to show the expanded and preprocessed output of line 99 etc.?

Edit: Oh wait. It's FIONREAD etc. A make -f cpp_filter.mak ioctl.i shows: case (0x40000000 | ( (sizeof (int) &0x7f) <<16) | ('f'<<8) |127) : for line 99. So the FIO* values could perhaps be written differently. Ref. inc/sys/ioctl.h.

Lethja commented 6 months ago

I'm not sure what's going on in those switches but nature of the warning is that a 16-bit CPU can only shift up to 15 bits.

/* Compile with wcc bit.c or wcc386 bit.c */

#include <stdio.h>

int main(void) {
        volatile int test;

        test = test << 15; /* Maximum bit-shift for a 8086 */
        test = test << 16; /* Too large of a bit-shift for a 8086 */
        test = test << 32; /* Too large of a bit-shift for a 80386 */
        test = test << 64; /* Too large of a bit-shift for a x86_64 */

        return 0;
}

I have no idea if Watcoms warning means the compiler doesn't know what to do or if it's merely telling us it's poor form. As I said earlier I've had no issue when running projects that link to the large model library

gvanem commented 6 months ago

I have no idea if Watcoms warning means the compiler doesn't know what to do

The warnings are correct since the definition of e.g. FIONREAD uses x <<16. I.e. those definitions assumes 32-bit targets or greater. The inc/sys/ioctl.h was once copied from the BSD header. fcntl.h. The gist of it is in the comment:

 * Ioctl's have the command encoded in the lower word,
 * and the size of any in or out parameters in the upper
 * word.  The high 2 bits of the upper word are used
 * to encode the in/out status of the parameter; for now
 * we restrict parameters to at most 128 bytes.

(word meaning 16-bit).

But very few of these FIO* definitions are needed. But to decode the command-groups for ioctlsocket():

Could do all this sys/ioctl.h stuff lot easier.

Lethja commented 6 months ago

Maybe a union and two switches could solve this without dramatically altering the code outside of ioctlsocket()?

As a rough example:

typedef union _iocGroup {
   long l;
   short s[2];
   char c[4];
} IocGroup;
int W32_CALL ioctlsocket (int s, long cmd, void *argp)
IocGroup group;
group.l = cmd;
swtich(IOCGROUP(cmd.s[0]) {
...
}

switch(IOCGROUP(cmd.s[1]) {
...
case (0x40000000 | ( (sizeof (short) &0x7f) <<1) | ('f'<<8) |127) :
...
}

I think that'd work but maybe there's a better way that would still allow a compiler to optimize a single switch for 32-bit builds

Edit: code correction from https://github.com/gvanem/Watt-32/issues/112#issuecomment-1970724493

gvanem commented 6 months ago

Good idea. I assume you mean:

typedef union _iocGroup {
   long  l;
   short s [2];
   char  c [4];
 } IocGroup;
jmalak commented 5 months ago

Take into account following bit-size for type in dependency on compiler target architecture

C type 16-bit arch 32-bit arch
short 16-bit 16-bit
int 16-bit 32-bit
long 32-bit 32-bit

If you want 32-bit type for both architectures then long type is OK If you want 16-bit type for both architectures then short type is OK int type is natural type that size is different on both architectures. take into account C standard compliance for enum and switch variable which is defined as int type by C standard, but Open Watcom use extension that both type can be 32-bit on both architectures if you apply shift operator to int then on 16-bit architecture it means 16-bit value, therefore int << 16 and more is zero and it is reported by this warning

Lethja commented 5 months ago

If you want 32-bit type for both architectures then long type is OK

Tested this: the following code compiles on wcl without any errors or warnings

#include <stdio.h>

int main(void) {
    volatile long test;

    test = test << 31;

    return 0;
}

Fantastic work!