GNU bug report logs - #75451
scratch/igc: Enable CHECK_STRUCTS

Previous Next

Full log

Message #19 received at 75451-done <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Gerd Möllmann <gerd.moellmann <at> gmail.com>
Cc: Gerd Moellmann <gerd <at> gnu.org>, Stefan Kangas <stefankangas <at> gmail.com>,
 75451-done <at> debbugs.gnu.org
Subject: Re: bug#75451: scratch/igc: Enable CHECK_STRUCTS
Date: Thu, 09 Jan 2025 10:09:00 +0000

Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>>
>>> (BTW, the contents of the #if might be out of date, not sure if we
>>> meanwhile fix/scan additional structs.)
>>
>> I've now added checks for all structs that we use in "fix_*" functions
>> (commit 76a0d739024).  I kept notes to make sure I didn't miss anything,
>> but it's not impossible that I did.  I'll double-check things later.
>>
>> I plan to do anything passed to the "finalize_*" ones next, which should
>> cover most of the ones we currently depend on, I think.
>
> Very nice! Thanks for taking this on!

Thanks!

This isn't strictly about the scratch/igc branch, but I personally think
struct hashes should be checked in all builds, mismatches should be
downgraded to #warnings, and --enable-checking=all could include
-Werror=cpp.  (So the warnings would still abort a build with
--enable-checking=all, but they'd *also* show up in regular builds.)

Also, we should include them in the nativecomp ABI hash, as nativecomp
relies on struct layout and the tagging scheme in nontrivial ways
(most-positive-fixnum, for example, is treated as a compile-time
constant by comp.el, but I'm not sure all changes to it would
automatically affect the comp abi hash).

Changes required might include:

* autodetection of gawk vs awk (I think this simply means using $(AWK)
  rather than "awk" in src/Makefile.in; this is important on some
  machines which provide a very different "awk")

* igc.o and comp.o  should depend explicitly on dmpstruct.h (this is
  important for re-builds; in theory, the current scratch/igc branch is
  broken for highly parallel builds because dmpstruct.h might be
  generated after igc.o, but that is not important).

* a hash-of-hashes for nativecomp

Proposed partial patch:

diff --git a/src/Makefile.in b/src/Makefile.in
index 3d249a1abdd..f0ff9203b9d 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -546,10 +546,13 @@ dmpstruct_headers=
 	$(srcdir)/treesit.h $(srcdir)/window.h $(srcdir)/xwidget.h
 ifeq ($(CHECK_STRUCTS),true)
 pdumper.o: dmpstruct.h
+igc.o: dmpstruct.h
+comp.o: dmpstruct.h
 endif
+AWK = @AWK@
 dmpstruct.h: $(srcdir)/dmpstruct.awk
 dmpstruct.h: $(libsrc)/make-fingerprint$(EXEEXT) $(dmpstruct_headers)
-	$(AM_V_GEN)POSIXLY_CORRECT=1 awk -f $(srcdir)/dmpstruct.awk \
+	$(AM_V_GEN)POSIXLY_CORRECT=1 $(AWK) -f $(srcdir)/dmpstruct.awk \
 		$(dmpstruct_headers) > $@
 
 AUTO_DEPEND = @AUTO_DEPEND@
diff --git a/src/comp.c b/src/comp.c
index cf15817c2fc..c029572d612 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -24,6 +24,7 @@
 #include "igc.h"
 #include "comp.h"
 #include "pdumper.h"
+#include "dmpstruct.h"
 
 #ifdef HAVE_NATIVE_COMP
 
@@ -821,9 +822,12 @@ hash_native_abi (void)
 
   Vcomp_abi_hash =
     comp_hash_string (
-      concat3 (build_string (ABI_VERSION),
-	       concat3 (Vemacs_version, Vsystem_configuration,
-			Vsystem_configuration_options),
+      CALLN (Fconcat,
+	       build_string (ABI_VERSION),
+	       Vemacs_version,
+	       Vsystem_configuration,
+	       Vsystem_configuration_options,
+	       build_string (HASH_DMPSTRUCT),
 	       Fmapconcat (intern_c_string ("comp--subr-signature"),
 			   Vcomp_subr_list, build_string (""))));
 
diff --git a/src/dmpstruct.awk b/src/dmpstruct.awk
index e5e359e10b7..ac639082719 100644
--- a/src/dmpstruct.awk
+++ b/src/dmpstruct.awk
@@ -21,6 +21,7 @@ BEGIN {
   print "#define EMACS_DMPSTRUCT_H"
   struct_name = ""
   tmpfile = "dmpstruct.tmp"
+  tmpfile2 = "dmpstruct2.tmp"
 }
 # Match a type followed by optional syntactic whitespace
 /^(enum|struct|union) [a-zA-Z0-9_]+([\t ]|\/\*.*\*\/)*$/ {
@@ -29,6 +30,7 @@ BEGIN {
 }
 /^(enum|struct|union) [a-zA-Z0-9_]+([\t ]|\/\*.*\*\/)*$/, /^(  )?};$/ {
   print $0 > tmpfile
+  print $0 > tmpfile2
 }
 /^(  )?} *(GCALIGNED_STRUCT)? *;$/ {
   if (struct_name != "") {
@@ -41,5 +43,11 @@ BEGIN {
   }
 }
 END {
+  fflush (tmpfile2)
+  cmd = "../lib-src/make-fingerprint -r " tmpfile2
+  cmd | getline hash
+  close (cmd)
+  printf "#define HASH_%s_%.10s\n", "dmpstruct", hash
+  printf "#define HASH_DMPSTRUCT \"%.10s\"\n", hash
   print "#endif /* EMACS_DMPSTRUCT_H */"
 }

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.