gvanem / Watt-32

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

DJGPP warnings #120

Open Lethja opened 1 month ago

Lethja commented 1 month ago

Since #119 made it possible to build DJGPP on DOS naively again. GCC reports several warnings.

Native compiler

Reports as gcc version 4.7.1

Unused function

gettod.c:363:13: warning: 'adjust_cpu_clock' defined but not used [-Wunused-function]

Implicit Function Declaration

cpuid.c: In function 'cpu_get_model':
cpuid.c:277:3: warning: implicit declaration of function 'snprintf' [-Wimplicit-function-declaration]
cpuid.c:277:3: warning: incompatible implicit declaration of built-in function 'snprintf' [enabled by default]
cpuid.c: In function 'cpu_get_freq_info1':
cpuid.c:326:3: warning: incompatible implicit declaration of built-in function 'snprintf' [enabled by default]
cpuid.c: In function 'cpu_get_freq_info2':
cpuid.c:344:3: warning: incompatible implicit declaration of built-in function 'snprintf' [enabled by default]
cpuid.c: In function 'cpu_get_brand_info':
cpuid.c:365:3: warning: incompatible implicit declaration of built-in function 'snprintf' [enabled by default]
pcdbug.c: In function 'dbug_dump':
pcdbug.c:2246:8: warning: incompatible implicit declaration of built-in function 'snprintf' [enabled by default]

This warning manifests itself as a build error later when libwatt.a included in an application

Pragmas

pctcp.c:2213:3: warning: unknown option after '#pragma GCC diagnostic' kind [-Wpragmas]

Type Limits

misc_str.c: In function 'strltrim':
misc_str.c:254:3: warning: comparison is always true due to limited range of data type [-Wtype-limits]

Cross Compiler

Reports as gcc version 12.2.0

Unused Function

gettod.c:363:13: warning: 'adjust_cpu_clock' defined but not used [-Wunused-function]
  363 | static void adjust_cpu_clock (const struct timeval *tv)
      |             ^~~~~~~~~~~~~~~~

Array Bounds

In file included from ../inc/sys/werrno.h:13,
                 from pcdns.c:37:
In function 'send_query',
    inlined from 'lookup_domain' at pcdns.c:575:10:
../inc/sys/w32api.h:37:29: warning: array subscript 'union sock_type[0]' is partly outside array bounds of '_udp_Socket[1]' {aka 'struct udp_Socket[1]'} [-Warray-bounds]
   37 |   #define W32_NAMESPACE(x)  _w32_ ## x
../inc/sys/whide.h:82:37: note: in expansion of macro 'W32_NAMESPACE'
   82 | #define outsnl                      W32_NAMESPACE (outsnl)
      |                                     ^~~~~~~~~~~~~
pcdns.c:209:7: note: in expansion of macro 'outsnl'
  209 |       outsnl (sock->udp.err_msg);
      |       ^~~~~~
pcdns.c: In function 'lookup_domain':
pcdns.c:496:15: note: object 'sock' of size 1736
  496 |   _udp_Socket sock;
      |               ^~~~
udp_rev.c: In function 'reverse_lookup':
udp_rev.c:156:15: warning: array subscript 'union sock_type[0]' is partly outside array bounds of '_udp_Socket[1]' {aka 'struct udp_Socket[1]'} [-Warray-bounds]
  156 |       if (sock->udp.usr_yield)
      |               ^~
udp_rev.c:108:15: note: object 'dom_sock' of size 1736
  108 |   _udp_Socket dom_sock;
      |               ^~~~~~~~
udp_rev.c:156:15: warning: array subscript 'union sock_type[0]' is partly outside array bounds of '_udp_Socket[1]' {aka 'struct udp_Socket[1]'} [-Warray-bounds]
  156 |       if (sock->udp.usr_yield)
      |               ^~
udp_rev.c:108:15: note: object 'dom_sock' of size 1736
  108 |   _udp_Socket dom_sock;
      |               ^~~~~~~~
Lethja commented 1 month ago

Note: configur.sh does not currently build dj_err.exe nor does it use the existing binary to generate build/djgpp/syserr.c or inc/sys/djgpp.err so #114 (5debd9c81a1cb56e082d2ffa66279f2d3ccfae72) was used to generate these files. No significant changes where found between the configur.bat and configur.lua versions of the files and the chances any warnings coming from this change is unlikely.

--- syserr.c    2024-08-11 12:34:46.459171889 +1000
+++ SYSERR.C    2024-08-11 12:37:44.000000000 +1000
@@ -1,6 +1,6 @@
 /*
- * THIS FILE WAS GENERATED BY %WATT_ROOT%\e:/home/mario/git/projects/watt-32/util/dj_err.exe
- * at Sun Aug 11 12:06:01 2024.
+ * THIS FILE WAS GENERATED BY %WATT_ROOT%\c:/watt-32/util/dj_err.exe
+ * at Sun Aug 11 12:37:45 2024.
  * DO NOT EDIT.
  *
  * The Watt-32 'sys_errlist[]' that replaces vendor's 'sys_errlist[]'
@@ -51,7 +51,7 @@
 char __syserr038[] = "No more files (ENMFILE)";
 char __syserr039[] = "Too many levels of symbolic links (ELOOP)";
 char __syserr040[] = "Value too large (EOVERFLOW)";
-char __syserr041[] = "Invalid or incomplete multibyte or wide character (EILSEQ)";
+char __syserr041[] = "Multibyte/wide character encoding error (EILSEQ)";
 char __syserr042[] = "Operation would block (EWOULDBLOCK)";
 char __syserr043[] = "Operation now in progress (EINPROGRESS)";
 char __syserr044[] = "Operation already in progress (EALREADY)";
--- djgpp.err   2024-08-11 12:35:23.528344479 +1000
+++ DJGPP.ERR   2024-08-11 12:37:44.000000000 +1000
@@ -2,18 +2,18 @@
 #define __SYS_WERRNO_ERR

 /*
- * THIS FILE WAS GENERATED BY %WATT_ROOT%\e:/home/mario/git/projects/watt-32/util/dj_err.exe
- * at Sun Aug 11 12:06:01 2024.
+ * THIS FILE WAS GENERATED BY %WATT_ROOT%\c:/watt-32/util/dj_err.exe
+ * at Sun Aug 11 12:37:45 2024.
  * DO NOT EDIT.
  *
- * Watt-32 errnos are after vendor's errnos (1 - 41)
+ * Watt-32 errnos are after vendor's errnos (1 - 38)
  */

 #ifndef __DJGPP__
 #error This file is only for use by "__DJGPP__"
 #endif

-#define ERRNO_VENDOR_VERSION  "2.05"
+#define ERRNO_VENDOR_VERSION  "2.03"

 #define EDOM              1
 #define ERANGE            2
Lethja commented 1 month ago

Looks like the pragma warning comes from that very code trying to cover up another warning https://github.com/gvanem/Watt-32/blob/748284844c0b3c24f30839be9072a7730f590c62/src/pctcp.c#L2206-L2230 Not exactly sure what tcp_opt_md5_sign() is writing into the opt pointer but it should be possible to write as an array to remove stringop-overflow warning to begin with

gvanem commented 1 month ago

Ok, but gcc 4.7.1 is very old (June 2012 according to this). Yet, it warns on snprint() being a built-in function. So does it mean djgpp 2.03 doesn't have a prototype for it? (long time since I've had that installed).

On the gcc 12.2.0 (August 2022) warning; I've seen these array-bounds warning elsewhere. But fail to see their importance. Perhaps it's a false positive?

Lethja commented 1 month ago

So does it mean djgpp 2.03 doesn't have a prototype for it? (long time since I've had that installed).

The compiler claims to have __builtin_snprintf() and using it will remove the warning however the more serious problem is that linker will error out later when an application is building against libwatt.a with undefined reference to 'snprintf'. It's worth noting that snprintf() is not part of the C89 standard and there might be other older compilers/linkers/toolchains etc effected by this problem.

My temporary workaround for this build problem was replacing each instance of snprintf() with sprintf() in my DJGPP library build. I haven't tested these specific code paths but looking at each instance the format string seems to always contain a fixed length output. If we assume every number could be up to an unsigned 64bit max then allowing up to 20 characters for each %(l)u makes a buffer overflow with sprintf() impossible: https://github.com/gvanem/Watt-32/blob/748284844c0b3c24f30839be9072a7730f590c62/src/cpuid.c#L275-L282 "%.4s%.4s%.4s" <= 13 https://github.com/gvanem/Watt-32/blob/748284844c0b3c24f30839be9072a7730f590c62/src/cpuid.c#L324-L328 "Core base: %lu, Core max: %lu, Core bus: %lu (MHz)" <= 102 https://github.com/gvanem/Watt-32/blob/748284844c0b3c24f30839be9072a7730f590c62/src/cpuid.c#L343-L346 "EDX: 0x%08lX, FID: %lu, EffFreqRO: %lu, ProcFeedback: %lu" <= 112 https://github.com/gvanem/Watt-32/blob/748284844c0b3c24f30839be9072a7730f590c62/src/cpuid.c#L363-L370 "%.4s%.12s" "%.4s%.12s" "%.4s%.12s" <= 49 https://github.com/gvanem/Watt-32/blob/748284844c0b3c24f30839be9072a7730f590c62/src/pcdbug.c#L2245-L2248 ", size %u" <= 28

sezero commented 1 month ago

So does it mean djgpp 2.03 doesn't have a prototype for it?

djgpp < 2.04 simply doesn't have [v]snprintf(), and naturally doesn't have prototypes of them. That's simply what it's about.

gvanem commented 1 month ago

I haven't tested these specific code paths but looking at each instance the format string seems to always contain a fixed length output.

I'm okay with assuming that. And replace with sprintf() instead.

Lethja commented 2 weeks ago

I'm not sure whats trying to be achieved with these two variables. sock is declared but doesn't seem to ever be assigned https://github.com/gvanem/Watt-32/blob/dca78e7800c0d523c5b67a2a66afdd1556179632/src/pcdns.c#L495-L498 https://github.com/gvanem/Watt-32/blob/dca78e7800c0d523c5b67a2a66afdd1556179632/src/pcdns.c#L573-L580