Re: Fwd: 5.8: LTO exposes some new issues

[Roman Perepelitsa <roman.perepelitsa@xxxxxxxxx>]

On Mon, Jul 27, 2020 at 12:08 PM Tomasz Kłoczko
<kloczko.tomasz@xxxxxxxxx> wrote:
>
> For someone who is maintaining that code, making such a decision should be
> way easier.

This statement is true but it's too general to be of any use here. Of
course making changes is easier for maintainers than for outsiders.
This doesn't mean that all changes (and this change in particular)
must be made by maintainers.

Anyway, I took a look at it. The problem is that `struct hashtable`
has a different set of members in different translation units. This is
undefined behavior whether LTO is used or not. Undefined behavior is
bad, so it would be nice to have this bug fixed. A patch is attached.

Roman.

diff --git a/Src/hashtable.c b/Src/hashtable.c
index e210ddeca..fa9d31052 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -28,23 +28,6 @@
  */
 
 #include "../config.h"
-
-#ifdef ZSH_HASH_DEBUG
-# define HASHTABLE_DEBUG_MEMBERS \
-    /* Members of struct hashtable used for debugging hash tables */ \
-    HashTable next, last;	/* linked list of all hash tables           */ \
-    char *tablename;		/* string containing name of the hash table */ \
-    PrintTableStats printinfo;	/* pointer to function to print table stats */
-#else /* !ZSH_HASH_DEBUG */
-# define HASHTABLE_DEBUG_MEMBERS
-#endif /* !ZSH_HASH_DEBUG */
-
-#define HASHTABLE_INTERNAL_MEMBERS \
-    ScanStatus scan;		/* status of a scan over this hashtable     */ \
-    HASHTABLE_DEBUG_MEMBERS
-
-typedef struct scanstatus *ScanStatus;
-
 #include "zsh.mdh"
 #include "hashtable.pro"
 
diff --git a/Src/zsh.h b/Src/zsh.h
index c48be4ffd..f9caa2f6e 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1182,6 +1182,18 @@ typedef void (*PrintTableStats) _((HashTable));
 
 /* hash table for standard open hashing */
 
+#ifdef ZSH_HASH_DEBUG
+# define HASHTABLE_DEBUG_MEMBERS \
+    /* Members of struct hashtable used for debugging hash tables */ \
+    HashTable next, last;	/* linked list of all hash tables           */ \
+    char *tablename;		/* string containing name of the hash table */ \
+    PrintTableStats printinfo;	/* pointer to function to print table stats */
+#else /* !ZSH_HASH_DEBUG */
+# define HASHTABLE_DEBUG_MEMBERS
+#endif /* !ZSH_HASH_DEBUG */
+
+typedef struct scanstatus *ScanStatus;
+
 struct hashtable {
     /* HASHTABLE DATA */
     int hsize;			/* size of nodes[]  (number of hash values)   */
@@ -1205,9 +1217,9 @@ struct hashtable {
     ScanFunc printnode;		/* pointer to function to print a node        */
     ScanTabFunc scantab;	/* pointer to function to scan table          */
 
-#ifdef HASHTABLE_INTERNAL_MEMBERS
-    HASHTABLE_INTERNAL_MEMBERS	/* internal use in hashtable.c                */
-#endif
+    /* HASHTABLE INTERNAL MEMBERS */
+    ScanStatus scan;		/* status of a scan over this hashtable       */
+    HASHTABLE_DEBUG_MEMBERS
 };
 
 /* generic hash table node */