Patchwork Go patch committed: Don't use memcmp for struct == if trailing padding

login
register
mail settings
Submitter Ian Taylor
Date Nov. 25, 2012, 12:56 a.m.
Message ID <mcr7gpaqz4t.fsf@google.com>
Download mbox | patch
Permalink /patch/201502/
State New
Headers show

Comments

Ian Taylor - Nov. 25, 2012, 12:56 a.m.
The Go frontend was using memcmp for struct equality if there was no
internal padding but there was trailing padding.  This doesn't work when
the struct values are on the stack, because there is nothing that
explicitly zeroes out the trailing padding.  This patch fixes the bug.
The patch is larger than expected because to implement this I had to
change a Type method to be non-const.  This had no real effect as it was
never called with a const pointer anyhow.  Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu.  Committed to mainline and 4.7
branch.

Ian

Patch

diff -r 12661f3c99d9 go/types.cc
--- a/go/types.cc	Sat Nov 24 12:20:02 2012 -0800
+++ b/go/types.cc	Sat Nov 24 16:37:16 2012 -0800
@@ -2382,7 +2382,7 @@ 
 
  protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   Btype*
@@ -2420,7 +2420,7 @@ 
 
  protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   Btype*
@@ -2458,7 +2458,7 @@ 
 
  protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return true; }
 
   Btype*
@@ -3085,7 +3085,7 @@ 
 
  protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   Btype*
@@ -3963,7 +3963,7 @@ 
 
  protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   Btype*
@@ -4014,7 +4014,7 @@ 
   }
 
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   Btype*
@@ -4291,7 +4291,7 @@ 
 // comparisons.
 
 bool
-Struct_type::do_compare_is_identity(Gogo* gogo) const
+Struct_type::do_compare_is_identity(Gogo* gogo)
 {
   const Struct_field_list* fields = this->fields_;
   if (fields == NULL)
@@ -4323,6 +4323,16 @@ 
 	return false;
       offset += field_size;
     }
+
+  unsigned int struct_size;
+  if (!this->backend_type_size(gogo, &struct_size))
+    return false;
+  if (offset != struct_size)
+    {
+      // Trailing padding may not be zero when on the stack.
+      return false;
+    }
+
   return true;
 }
 
@@ -5267,7 +5277,7 @@ 
 // Whether we can use memcmp to compare this array.
 
 bool
-Array_type::do_compare_is_identity(Gogo* gogo) const
+Array_type::do_compare_is_identity(Gogo* gogo)
 {
   if (this->length_ == NULL)
     return false;
@@ -7968,7 +7978,7 @@ 
 // function.
 
 bool
-Named_type::do_compare_is_identity(Gogo* gogo) const
+Named_type::do_compare_is_identity(Gogo* gogo)
 {
   // We don't use this->seen_ here because compare_is_identity may
   // call base() later, and that will mess up if seen_ is set here.
diff -r 12661f3c99d9 go/types.h
--- a/go/types.h	Sat Nov 24 12:20:02 2012 -0800
+++ b/go/types.h	Sat Nov 24 16:37:16 2012 -0800
@@ -576,7 +576,7 @@ 
   // identity function which gets nothing but a pointer to the value
   // and a size.
   bool
-  compare_is_identity(Gogo* gogo) const
+  compare_is_identity(Gogo* gogo)
   { return this->do_compare_is_identity(gogo); }
 
   // Return a hash code for this type for the method hash table.
@@ -950,7 +950,7 @@ 
   { return false; }
 
   virtual bool
-  do_compare_is_identity(Gogo*) const = 0;
+  do_compare_is_identity(Gogo*) = 0;
 
   virtual unsigned int
   do_hash_for_method(Gogo*) const;
@@ -1458,7 +1458,7 @@ 
 
 protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return true; }
 
   unsigned int
@@ -1535,7 +1535,7 @@ 
 
  protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   unsigned int
@@ -1604,7 +1604,7 @@ 
 
  protected:
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   unsigned int
@@ -1664,7 +1664,7 @@ 
   { return true; }
 
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   Btype*
@@ -1778,7 +1778,7 @@ 
   { return true; }
 
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   unsigned int
@@ -1853,7 +1853,7 @@ 
   { return true; }
 
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return true; }
 
   unsigned int
@@ -2139,7 +2139,7 @@ 
   do_has_pointer() const;
 
   bool
-  do_compare_is_identity(Gogo*) const;
+  do_compare_is_identity(Gogo*);
 
   unsigned int
   do_hash_for_method(Gogo*) const;
@@ -2272,7 +2272,7 @@ 
   }
 
   bool
-  do_compare_is_identity(Gogo*) const;
+  do_compare_is_identity(Gogo*);
 
   unsigned int
   do_hash_for_method(Gogo*) const;
@@ -2365,7 +2365,7 @@ 
   { return true; }
 
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   unsigned int
@@ -2451,7 +2451,7 @@ 
   { return true; }
 
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return true; }
 
   unsigned int
@@ -2582,7 +2582,7 @@ 
   { return true; }
 
   bool
-  do_compare_is_identity(Gogo*) const
+  do_compare_is_identity(Gogo*)
   { return false; }
 
   unsigned int
@@ -2865,7 +2865,7 @@ 
   do_has_pointer() const;
 
   bool
-  do_compare_is_identity(Gogo*) const;
+  do_compare_is_identity(Gogo*);
 
   unsigned int
   do_hash_for_method(Gogo*) const;
@@ -2949,7 +2949,7 @@ 
   // function exits.
   mutable bool seen_;
   // Like seen_, but used only by do_compare_is_identity.
-  mutable bool seen_in_compare_is_identity_;
+  bool seen_in_compare_is_identity_;
   // Like seen_, but used only by do_get_backend.
   bool seen_in_get_backend_;
 };
@@ -3004,7 +3004,7 @@ 
   { return this->real_type()->has_pointer(); }
 
   bool
-  do_compare_is_identity(Gogo* gogo) const
+  do_compare_is_identity(Gogo* gogo)
   { return this->real_type()->compare_is_identity(gogo); }
 
   unsigned int