flasher_stub GCC 9 warnings/errors

[paravoid]

Building flasher_stub (as of efb55ac) with gcc version 9.2.1 20191130 (9.2.1-21+4) (Debian's), results into the following compiler warnings (that due to the stock Makefile's -Werror are converted into errors):

stub_write_flash.c:53:23: warning: 'STATUS_QIE_BIT' defined but not used [-Wunused-const-variable=]
   53 | static const uint32_t STATUS_QIE_BIT = (1 << 9);  /* Quad Enable */
      |                       ^~~~~~~~~~~~~~
miniz.c: In function 'tinfl_decompress':
miniz.c:1516:9: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
 1516 |         for ( i = 0; i <= 143; ++i) *p++ = 8; for ( ; i <= 255; ++i) *p++ = 9; for ( ; i <= 279; ++i) *p++ = 7; for ( ; i <= 287; ++i) *p++ = 8;
      |         ^~~
miniz.c:1516:47: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'for'
 1516 |         for ( i = 0; i <= 143; ++i) *p++ = 8; for ( ; i <= 255; ++i) *p++ = 9; for ( ; i <= 279; ++i) *p++ = 7; for ( ; i <= 287; ++i) *p++ = 8;
      |                                               ^~~
miniz.c: In function 'tdefl_find_match':
miniz.c:2334:5: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
 2334 |     if (!dist) break; p = s; q = d->m_dict + probe_pos; for (probe_len = 0; probe_len < max_match_len; probe_len++) if (*p++ != *q++) break;
      |     ^~
miniz.c:2334:23: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
 2334 |     if (!dist) break; p = s; q = d->m_dict + probe_pos; for (probe_len = 0; probe_len < max_match_len; probe_len++) if (*p++ != *q++) break;
      |                       ^
  WRAP build/stub_flasher_8266.elf wrap_stub.py -> build/stub_flasher_snippet.py

I'm looking at updating miniz to a newer upstream, but it doesn't seem too trivial for now (string.h/memset conflicts). I'll look into it more, will update if I get to something.

[paravoid]

While the errors can obviously be fixed by either -Wno-misleading-indentation or by actually fixing the indentation, I thought it may be a good idea to update miniz to a newer version.... but failed :)

What I've tried so far is forward-porting the patches from miniz 1.15 r4 to 2.1.0 (the latest as of now), but have been unsuccessful in building this, at least with picolibc. The error I'm getting is around GCC's __ashldi3:

/usr/lib/gcc/xtensa-lx106-elf/9.2.1/../../../xtensa-lx106-elf/bin/ld: /tmp/stub_flasher_8266.elf.wPiww7.ltrans0.ltrans.o:(.text.stub_main+0x100): undefined reference to `__ashldi3'
/usr/lib/gcc/xtensa-lx106-elf/9.2.1/../../../xtensa-lx106-elf/bin/ld: /tmp/stub_flasher_8266.elf.wPiww7.ltrans0.ltrans.o: in function `stub_main':
/home/paravoid/esptool-git/flasher_stub/miniz.c:2729:
/usr/lib/gcc/xtensa-lx106-elf/9.2.1/../../../xtensa-lx106-elf/bin/ld: /home/paravoid/esptool-git/flasher_stub/miniz.c:2797: undefined reference to `__ashldi3'
collect2: error: ld returned 1 exit status

The patch against miniz I'm using is:

diff --git a/miniz.h b/miniz.h
index 3018dae..e5588ca 100644
--- a/miniz.h
+++ b/miniz.h
@@ -118,30 +118,30 @@
    If all macros here are defined the only functionality remaining will be CRC-32, adler-32, tinfl, and tdefl. */
 
 /* Define MINIZ_NO_STDIO to disable all usage and any functions which rely on stdio for file I/O. */
-/*#define MINIZ_NO_STDIO */
+#define MINIZ_NO_STDIO
 
 /* If MINIZ_NO_TIME is specified then the ZIP archive functions will not be able to get the current time, or */
 /* get/set file times, and the C run-time funcs that get/set times won't be called. */
 /* The current downside is the times written to your archives will be from 1979. */
-/*#define MINIZ_NO_TIME */
+#define MINIZ_NO_TIME
 
 /* Define MINIZ_NO_ARCHIVE_APIS to disable all ZIP archive API's. */
-/*#define MINIZ_NO_ARCHIVE_APIS */
+#define MINIZ_NO_ARCHIVE_APIS
 
 /* Define MINIZ_NO_ARCHIVE_WRITING_APIS to disable all writing related ZIP archive API's. */
-/*#define MINIZ_NO_ARCHIVE_WRITING_APIS */
+#define MINIZ_NO_ARCHIVE_WRITING_APIS
 
 /* Define MINIZ_NO_ZLIB_APIS to remove all ZLIB-style compression/decompression API's. */
-/*#define MINIZ_NO_ZLIB_APIS */
+#define MINIZ_NO_ZLIB_APIS
 
 /* Define MINIZ_NO_ZLIB_COMPATIBLE_NAME to disable zlib names, to prevent conflicts against stock zlib. */
-/*#define MINIZ_NO_ZLIB_COMPATIBLE_NAMES */
+#define MINIZ_NO_ZLIB_COMPATIBLE_NAMES
 
 /* Define MINIZ_NO_MALLOC to disable all calls to malloc, free, and realloc. 
    Note if MINIZ_NO_MALLOC is defined then the user must always provide custom user alloc/free/realloc
    callbacks to the zlib and archive API's, and a few stand-alone helper API's which don't provide custom user
    functions (such as tdefl_compress_mem_to_heap() and tinfl_decompress_mem_to_heap()) won't work. */
-/*#define MINIZ_NO_MALLOC */
+#define MINIZ_NO_MALLOC
 
 #if defined(__TINYC__) && (defined(__linux) || defined(__linux__))
 /* TODO: Work around "error: include file 'sys\utime.h' when compiling with tcc on Linux */
diff --git a/miniz_common.h b/miniz_common.h
index 231f5bc..5954364 100644
--- a/miniz_common.h
+++ b/miniz_common.h
@@ -43,7 +43,7 @@ typedef struct mz_dummy_time_t_tag
 #define MZ_TIME_T time_t
 #endif
 
-#define MZ_ASSERT(x) assert(x)
+#define MZ_ASSERT(x)
 
 #ifdef MINIZ_NO_MALLOC
 #define MZ_MALLOC(x) NULL
diff --git a/miniz_tdef.h b/miniz_tdef.h
index 9b3e9d2..fd879ac 100644
--- a/miniz_tdef.h
+++ b/miniz_tdef.h
@@ -7,7 +7,7 @@ extern "C" {
 /* ------------------- Low-level Compression API Definitions */
 
 /* Set TDEFL_LESS_MEMORY to 1 to use less memory (compression will be slightly slower, and raw/dynamic blocks will be output more frequently). */
-#define TDEFL_LESS_MEMORY 0
+#define TDEFL_LESS_MEMORY 1
 
 /* tdefl_init() compression flags logically OR'd together (low 12 bits contain the max. number of probes per dictionary search): */
 /* TDEFL_DEFAULT_MAX_PROBES: The compressor defaults to 128 dictionary probes per dictionary search. 0=Huffman only, 1=Huffman+LZ (fastest/crap compression), 4095=Huffman+LZ (slowest/best compression). */
@@ -79,7 +79,7 @@ enum
     TDEFL_MAX_HUFF_SYMBOLS_0 = 288,
     TDEFL_MAX_HUFF_SYMBOLS_1 = 32,
     TDEFL_MAX_HUFF_SYMBOLS_2 = 19,
-    TDEFL_LZ_DICT_SIZE = 32768,
+    TDEFL_LZ_DICT_SIZE = (8*1024),
     TDEFL_LZ_DICT_SIZE_MASK = TDEFL_LZ_DICT_SIZE - 1,
     TDEFL_MIN_MATCH_LEN = 3,
     TDEFL_MAX_MATCH_LEN = 258
@@ -89,7 +89,7 @@ enum
 #if TDEFL_LESS_MEMORY
 enum
 {
-    TDEFL_LZ_CODE_BUF_SIZE = 24 * 1024,
+    TDEFL_LZ_CODE_BUF_SIZE = 8 * 1024,
     TDEFL_OUT_BUF_SIZE = (TDEFL_LZ_CODE_BUF_SIZE * 13) / 10,
     TDEFL_MAX_HUFF_SYMBOLS = 288,
     TDEFL_LZ_HASH_BITS = 12,

[projectgus]

Hi @paravoid,

Thanks for the detailed report and for pushing forward with newer gcc versions.

I haven't had time to look deeply but I have a short answer and a long answer which may help:

The Makefile in the flasher stub doesn't link libgcc (not on purpose, just that it it wasn't previously needed for anything). If you add -lgcc to CFLAGS in the Makefile, does it link?

(Worth noting that the flasher stub also doesn't link any libc, so choice of libc should only matter for compile-time inclusion of libc headers.)

--

The long answer is that __ashldi3 indicates a long long divided by an integer and, as the ESP8266/ESP32 can't do this natively it needs the libgcc call. I have some concerns (may turn out to be unfounded) about how big the flasher stub might end up if it needs to do a lot of 64-bit math. It would be good if it's possible to constrain it back to mostly 32-bit operations.

Looking up line 2797 in miniz.c to see exactly what operation triggered this doesn't seem to be quite right, may be a binutils/gcc bug? If you remove -flto from the linker args then you might get a more accurate line number (but a bigger final binary).

[paravoid]

I can confirm that current master (which includes the aforementioned 18647f2 and b36b59d) these warnings are now gone. Thanks so much!