Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open Watcom 16-bit BSD API build warnings #112

Closed
Lethja opened this issue Feb 26, 2024 · 7 comments · Fixed by #113
Closed

Open Watcom 16-bit BSD API build warnings #112

Lethja opened this issue Feb 26, 2024 · 7 comments · Fixed by #113

Comments

@Lethja
Copy link

Lethja commented Feb 26, 2024

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
Copy link
Owner

gvanem commented Feb 27, 2024

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
Copy link
Author

Lethja commented Feb 27, 2024

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
Copy link
Owner

gvanem commented Feb 29, 2024

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():

  • f == file
  • I == interface
  • s == socket

the macro IOCGROUP() is used. Ref. ioctlsocket():

int W32_CALL ioctlsocket (int s, long cmd, void *argp)
{
  Socket *socket = _socklist_find (s);

  SOCK_PROLOGUE (socket, "\nioctlsocket:%d", s);
  SOCK_DEBUGF ((", %s", get_ioctl_cmd(cmd)));
  switch (IOCGROUP(cmd))
  ...

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

@Lethja
Copy link
Author

Lethja commented Feb 29, 2024

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 #112 (comment)

@gvanem
Copy link
Owner

gvanem commented Feb 29, 2024

Good idea. I assume you mean:

typedef union _iocGroup {
   long  l;
   short s [2];
   char  c [4];
 } IocGroup;

@jmalak
Copy link

jmalak commented Mar 26, 2024

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
Copy link
Author

Lethja commented Mar 27, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants