Patchwork Save checksum of compiler options in LTO files

login
register
mail settings
Submitter Andi Kleen
Date Oct. 2, 2010, 9:16 p.m.
Message ID <1286054198-19617-1-git-send-email-andi@firstfloor.org>
Download mbox | patch
Permalink /patch/66583/
State New
Headers show

Comments

Andi Kleen - Oct. 2, 2010, 9:16 p.m.
From: Andi Kleen <ak@linux.intel.com>

I was finally tired of constant ICEs when I updated to a newer svn version
because there were some stale LTO files lying around somewhere. The usual
reason were added gcc options, which then change the option offsets
and confuse the LTO options save/restore code.

There is already a version check, but it only checks major/minor and
does not handle smaller scale changes.

This patch computes an checksum over all supported options at build time
and then saves/compares that in the LTO object file.

I used a simple 8bit fletcher checksum implemented in awk. This
is not very strong, but should be good enough for this purpose.

This fixes PR lto/44965

Passes bootstrap and full testsuite on x86_64-linux.

Ok to commit?

2010-10-02  Andi Kleen <ak@linux.intel.com>

	PR lto/44965
	* lto-opts.c (lto_opts_header): Add.
	(lto_write_options): Write out options checksum.
	(lto_read_file_options): Read and check options checksum.
	* opt-functions.awk (csum, csum_combine): Add.
	* optc-gen.awk (END): Compute and dump options checksum.
	* opth-gen.awk (END): Print declaration for opts_checksum.
---
 gcc/lto-opts.c        |   35 ++++++++++++++++++++++-------------
 gcc/opt-functions.awk |   27 +++++++++++++++++++++++++++
 gcc/optc-gen.awk      |    4 ++++
 gcc/opth-gen.awk      |    3 +++
 4 files changed, 56 insertions(+), 13 deletions(-)
Andi Kleen - Oct. 17, 2010, 6:30 p.m.
Andi Kleen <andi@firstfloor.org> writes:

> From: Andi Kleen <ak@linux.intel.com>
>
> I was finally tired of constant ICEs when I updated to a newer svn version
> because there were some stale LTO files lying around somewhere. The usual
> reason were added gcc options, which then change the option offsets
> and confuse the LTO options save/restore code.
>
> There is already a version check, but it only checks major/minor and
> does not handle smaller scale changes.

Could someone please review that patch?

Thanks,

-And
Richard Guenther - Oct. 18, 2010, 9:17 a.m.
On Sun, Oct 17, 2010 at 8:30 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Andi Kleen <andi@firstfloor.org> writes:
>
>> From: Andi Kleen <ak@linux.intel.com>
>>
>> I was finally tired of constant ICEs when I updated to a newer svn version
>> because there were some stale LTO files lying around somewhere. The usual
>> reason were added gcc options, which then change the option offsets
>> and confuse the LTO options save/restore code.
>>
>> There is already a version check, but it only checks major/minor and
>> does not handle smaller scale changes.
>
> Could someone please review that patch?

There are many more issues that affect LTO bytecode compatibility
(such as added tree codes for example).  So I think as developers
we should just live with the fact that this happens on the trunk.

Richard.

> Thanks,
>
> -And
>
> --
> ak@linux.intel.com -- Speaking for myself only.
>
Andi Kleen - Oct. 18, 2010, 9:34 a.m.
> There are many more issues that affect LTO bytecode compatibility
> (such as added tree codes for example).  So I think as developers
> we should just live with the fact that this happens on the trunk.

It's not only trunk -- the problem can easily happen for minor 
releases if someone changes/adds an option (and I believe that 
happens)

Also I think the code is relatively clean and not a big 
maintenance burden (but then I'm somewhat biased)

-Andi
Richard Guenther - Oct. 18, 2010, 9:37 a.m.
On Mon, Oct 18, 2010 at 11:34 AM, Andi Kleen <andi@firstfloor.org> wrote:
>> There are many more issues that affect LTO bytecode compatibility
>> (such as added tree codes for example).  So I think as developers
>> we should just live with the fact that this happens on the trunk.
>
> It's not only trunk -- the problem can easily happen for minor
> releases if someone changes/adds an option (and I believe that
> happens)

It usually doesn't, and we could easily include the minor version number
in the LTO bytecode revision (now I have no idea where that code actually
is nor what it does currently ;)).

Richard.

> Also I think the code is relatively clean and not a big
> maintenance burden (but then I'm somewhat biased)
>
> -Andi
> --
> ak@linux.intel.com -- Speaking for myself only.
>

Patch

diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index bbe2138..4c73c07 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -79,6 +79,11 @@  typedef struct GTY(()) opt_d
 DEF_VEC_O (opt_t);
 DEF_VEC_ALLOC_O (opt_t, heap);
 
+struct lto_opts_header 
+{
+  struct lto_simple_header simple;
+  unsigned checksum;
+};
 
 /* Options are held in two vectors, one for those registered by
    command line handling code, and the other for those read in from
@@ -296,7 +301,7 @@  lto_write_options (void)
 {
   char *const section_name = lto_get_section_name (LTO_section_opts, NULL, NULL);
   struct lto_output_stream stream;
-  struct lto_simple_header header;
+  struct lto_opts_header header;
   struct lto_output_stream *header_stream;
 
   lto_begin_section (section_name, !flag_wpa);
@@ -306,12 +311,13 @@  lto_write_options (void)
   output_options (&stream);
 
   memset (&header, 0, sizeof (header));
-  header.lto_header.major_version = LTO_major_version;
-  header.lto_header.minor_version = LTO_minor_version;
-  header.lto_header.section_type = LTO_section_opts;
+  header.simple.lto_header.major_version = LTO_major_version;
+  header.simple.lto_header.minor_version = LTO_minor_version;
+  header.simple.lto_header.section_type = LTO_section_opts;
 
-  header.compressed_size = 0;
-  header.main_size = stream.total_size;
+  header.simple.compressed_size = 0;
+  header.simple.main_size = stream.total_size;
+  header.checksum = opts_checksum;
 
   header_stream = ((struct lto_output_stream *)
 		   xcalloc (1, sizeof (*header_stream)));
@@ -351,7 +357,7 @@  lto_read_file_options (struct lto_file_decl_data *file_data)
 {
   size_t len, l, skip;
   const char *data, *p;
-  const struct lto_simple_header *header;
+  const struct lto_opts_header *header;
   int32_t opts_offset;
   struct lto_input_block ib;
 
@@ -368,16 +374,19 @@  lto_read_file_options (struct lto_file_decl_data *file_data)
   p = data;
   do 
     { 
-      header = (const struct lto_simple_header *) p;
+      header = (const struct lto_opts_header *) p;
       opts_offset = sizeof (*header);
 
-      lto_check_version (header->lto_header.major_version,
-			 header->lto_header.minor_version);
-      
-      LTO_INIT_INPUT_BLOCK (ib, p + opts_offset, 0, header->main_size);
+      lto_check_version (header->simple.lto_header.major_version,
+			 header->simple.lto_header.minor_version);
+
+      if (header->checksum != opts_checksum)
+        fatal_error ("LTO input file options checksum does not match mine"); 
+
+      LTO_INIT_INPUT_BLOCK (ib, p + opts_offset, 0, header->simple.main_size);
       input_options (&ib);
       
-      skip = header->main_size + opts_offset;
+      skip = header->simple.main_size + opts_offset;
       l -= skip;
       p += skip;
     } 
diff --git a/gcc/opt-functions.awk b/gcc/opt-functions.awk
index 863c478..75a7f33 100644
--- a/gcc/opt-functions.awk
+++ b/gcc/opt-functions.awk
@@ -215,3 +215,30 @@  function opt_enum(name)
 {
 	return "OPT_" opt_sanitized_name(name)
 }
+
+# 8bit fletcher checksum. dumb, but simple version
+function csum(array)
+{
+	# yes this is the official recommended GNU awk method :-(
+	for (i = 0; i < 255; i++) {
+		t = sprintf("%c", i)
+		ord[t] = i
+	}	
+
+	a = 0
+	b = 0
+	for (s in array) {
+		for (k = 0; k < length("" s); k++) {
+			a = a + ord[substr(s, k, 1)]
+			b = b + a
+		}
+		a = a % 0xff
+		b = b % 0xff
+	}	
+	return or(lshift(a,8),b)
+}
+
+function csum_combine(a,b)
+{
+	return a + lshift(b, 16)
+}
diff --git a/gcc/optc-gen.awk b/gcc/optc-gen.awk
index bad055f..86e0c96 100644
--- a/gcc/optc-gen.awk
+++ b/gcc/optc-gen.awk
@@ -596,6 +596,10 @@  print "  if (targetm.target_option.print)";
 print "    targetm.target_option.print (file, indent, ptr);";
 
 print "}";
+
+sum = csum_combine(csum(opts), csum(indices));
+printf("const unsigned opts_checksum = 0x%x;\n", sum);
+
 print "#endif";
 
 }
diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk
index d27dfc0..dc3b6d8 100644
--- a/gcc/opth-gen.awk
+++ b/gcc/opth-gen.awk
@@ -283,6 +283,9 @@  print "extern void cl_target_option_restore (struct gcc_options *, struct cl_tar
 print "";
 print "/* Print target option variables from a structure.  */";
 print "extern void cl_target_option_print (FILE *, int, struct cl_target_option *);";
+print "";
+print "/* Checksum of all options */";
+print "extern const unsigned opts_checksum;";
 print "#endif";
 print "";