If a Go struct or array has any padding, then we can't use memcmp to
compare the values, because if a value is allocated on the stack then
the padding bytes may not be zeroed out. This patch corrects the Go
compiler to check for this possibility, and to use a generated function
if it occurs. This required moving the generation of hash/equality
functions to after the lowering pass, as otherwise a constant expression
used for an array index might not be resolvable. Bootstrapped and ran
Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r 569599d42f46 go/expressions.cc
--- a/go/expressions.cc Wed Jan 11 08:24:08 2012 -0800
+++ b/go/expressions.cc Wed Jan 11 13:09:30 2012 -0800
@@ -5844,7 +5844,7 @@
// See if we can compare using memcmp. As a heuristic, we use
// memcmp rather than field references and comparisons if there are
// more than two fields.
- if (st->compare_is_identity() && st->total_field_count() > 2)
+ if (st->compare_is_identity(gogo) && st->total_field_count() > 2)
return this->lower_compare_to_memcmp(gogo, inserter);
Location loc = this->location();
@@ -5919,7 +5919,7 @@
// Call memcmp directly if possible. This may let the middle-end
// optimize the call.
- if (at->compare_is_identity())
+ if (at->compare_is_identity(gogo))
return this->lower_compare_to_memcmp(gogo, inserter);
// Call the array comparison function.
@@ -12966,10 +12966,10 @@
lower_struct(Gogo*, Type*);
Expression*
- lower_array(Gogo*, Type*);
+ lower_array(Type*);
Expression*
- make_array(Gogo*, Type*, Expression_list*);
+ make_array(Type*, Expression_list*);
Expression*
lower_map(Gogo*, Named_object*, Statement_inserter*, Type*);
@@ -13036,7 +13036,7 @@
else if (type->struct_type() != NULL)
ret = this->lower_struct(gogo, type);
else if (type->array_type() != NULL)
- ret = this->lower_array(gogo, type);
+ ret = this->lower_array(type);
else if (type->map_type() != NULL)
ret = this->lower_map(gogo, function, inserter, type);
else
@@ -13249,11 +13249,11 @@
// Lower an array composite literal.
Expression*
-Composite_literal_expression::lower_array(Gogo* gogo, Type* type)
+Composite_literal_expression::lower_array(Type* type)
{
Location location = this->location();
if (this->vals_ == NULL || !this->has_keys_)
- return this->make_array(gogo, type, this->vals_);
+ return this->make_array(type, this->vals_);
std::vector<Expression*> vals;
vals.reserve(this->vals_->size());
@@ -13353,15 +13353,14 @@
for (size_t i = 0; i < size; ++i)
list->push_back(vals[i]);
- return this->make_array(gogo, type, list);
+ return this->make_array(type, list);
}
// Actually build the array composite literal. This handles
// [...]{...}.
Expression*
-Composite_literal_expression::make_array(Gogo* gogo, Type* type,
- Expression_list* vals)
+Composite_literal_expression::make_array(Type* type, Expression_list* vals)
{
Location location = this->location();
Array_type* at = type->array_type();
@@ -13373,10 +13372,6 @@
Expression* elen = Expression::make_integer(&vlen, NULL, location);
mpz_clear(vlen);
at = Type::make_array_type(at->element_type(), elen);
-
- // This is after the finalize_methods pass, so run that now.
- at->finalize_methods(gogo);
-
type = at;
}
if (at->length() != NULL)
diff -r 569599d42f46 go/gogo.cc
--- a/go/gogo.cc Wed Jan 11 08:24:08 2012 -0800
+++ b/go/gogo.cc Wed Jan 11 13:09:30 2012 -0800
@@ -1149,11 +1149,74 @@
this->specific_type_functions_.push_back(tsf);
}
+// Look for types which need specific hash or equality functions.
+
+class Specific_type_functions : public Traverse
+{
+ public:
+ Specific_type_functions(Gogo* gogo)
+ : Traverse(traverse_types),
+ gogo_(gogo)
+ { }
+
+ int
+ type(Type*);
+
+ private:
+ Gogo* gogo_;
+};
+
+int
+Specific_type_functions::type(Type* t)
+{
+ Named_object* hash_fn;
+ Named_object* equal_fn;
+ switch (t->classification())
+ {
+ case Type::TYPE_NAMED:
+ {
+ if (!t->compare_is_identity(this->gogo_) && t->is_comparable())
+ t->type_functions(this->gogo_, t->named_type(), NULL, NULL, &hash_fn,
+ &equal_fn);
+
+ // If this is a struct type, we don't want to make functions
+ // for the unnamed struct.
+ Type* rt = t->named_type()->real_type();
+ if (rt->struct_type() == NULL)
+ {
+ if (Type::traverse(rt, this) == TRAVERSE_EXIT)
+ return TRAVERSE_EXIT;
+ }
+ else
+ {
+ if (rt->struct_type()->traverse_field_types(this) == TRAVERSE_EXIT)
+ return TRAVERSE_EXIT;
+ }
+
+ return TRAVERSE_SKIP_COMPONENTS;
+ }
+
+ case Type::TYPE_STRUCT:
+ case Type::TYPE_ARRAY:
+ if (!t->compare_is_identity(this->gogo_) && t->is_comparable())
+ t->type_functions(this->gogo_, NULL, NULL, NULL, &hash_fn, &equal_fn);
+ break;
+
+ default:
+ break;
+ }
+
+ return TRAVERSE_CONTINUE;
+}
+
// Write out type specific functions.
void
Gogo::write_specific_type_functions()
{
+ Specific_type_functions stf(this);
+ this->traverse(&stf);
+
while (!this->specific_type_functions_.empty())
{
Specific_type_function* tsf = this->specific_type_functions_.back();
@@ -1520,10 +1583,6 @@
t->struct_type()->finalize_methods(this->gogo_);
break;
- case Type::TYPE_ARRAY:
- t->array_type()->finalize_methods(this->gogo_);
- break;
-
default:
break;
}
diff -r 569599d42f46 go/types.cc
--- a/go/types.cc Wed Jan 11 08:24:08 2012 -0800
+++ b/go/types.cc Wed Jan 11 13:09:30 2012 -0800
@@ -571,7 +571,8 @@
}
else if (t1->array_type() != NULL)
{
- if (!t1->array_type()->element_type()->is_comparable())
+ if (t1->array_type()->length()->is_nil_expression()
+ || !t1->array_type()->element_type()->is_comparable())
{
if (reason != NULL)
*reason = _("invalid comparison of non-comparable array");
@@ -1372,7 +1373,7 @@
const char* hash_fnname;
const char* equal_fnname;
- if (this->compare_is_identity())
+ if (this->compare_is_identity(gogo))
{
hash_fnname = "__go_type_hash_identity";
equal_fnname = "__go_type_equal_identity";
@@ -1963,7 +1964,7 @@
// Return whether the backend size of the type is known.
bool
-Type::is_backend_type_size_known(Gogo* gogo) const
+Type::is_backend_type_size_known(Gogo* gogo)
{
switch (this->classification_)
{
@@ -2014,11 +2015,14 @@
}
case TYPE_NAMED:
+ // Begin converting this type to the backend representation.
+ // This will create a placeholder if necessary.
+ this->get_backend(gogo);
return this->named_type()->is_named_backend_type_size_known();
case TYPE_FORWARD:
{
- const Forward_declaration_type* fdt = this->forward_declaration_type();
+ Forward_declaration_type* fdt = this->forward_declaration_type();
return fdt->real_type()->is_backend_type_size_known(gogo);
}
@@ -2038,10 +2042,9 @@
bool
Type::backend_type_size(Gogo* gogo, unsigned int *psize)
{
- Btype* btype = this->get_backend(gogo);
if (!this->is_backend_type_size_known(gogo))
return false;
- size_t size = gogo->backend()->type_size(btype);
+ size_t size = gogo->backend()->type_size(this->get_backend(gogo));
*psize = static_cast<unsigned int>(size);
if (*psize != size)
return false;
@@ -2054,10 +2057,9 @@
bool
Type::backend_type_align(Gogo* gogo, unsigned int *palign)
{
- Btype* btype = this->get_backend(gogo);
if (!this->is_backend_type_size_known(gogo))
return false;
- size_t align = gogo->backend()->type_alignment(btype);
+ size_t align = gogo->backend()->type_alignment(this->get_backend(gogo));
*palign = static_cast<unsigned int>(align);
if (*palign != align)
return false;
@@ -2070,10 +2072,9 @@
bool
Type::backend_type_field_align(Gogo* gogo, unsigned int *palign)
{
- Btype* btype = this->get_backend(gogo);
if (!this->is_backend_type_size_known(gogo))
return false;
- size_t a = gogo->backend()->type_field_alignment(btype);
+ size_t a = gogo->backend()->type_field_alignment(this->get_backend(gogo));
*palign = static_cast<unsigned int>(a);
if (*palign != a)
return false;
@@ -2126,7 +2127,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
Btype*
@@ -2164,7 +2165,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
Btype*
@@ -2202,7 +2203,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return true; }
Btype*
@@ -2793,7 +2794,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
Btype*
@@ -3705,7 +3706,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
Btype*
@@ -3756,7 +3757,7 @@
}
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
Btype*
@@ -4037,16 +4038,35 @@
// comparisons.
bool
-Struct_type::do_compare_is_identity() const
+Struct_type::do_compare_is_identity(Gogo* gogo) const
{
const Struct_field_list* fields = this->fields_;
if (fields == NULL)
return true;
+ unsigned int offset = 0;
for (Struct_field_list::const_iterator pf = fields->begin();
pf != fields->end();
++pf)
- if (!pf->type()->compare_is_identity())
- return false;
+ {
+ if (!pf->type()->compare_is_identity(gogo))
+ return false;
+
+ unsigned int field_align;
+ if (!pf->type()->backend_type_align(gogo, &field_align))
+ return false;
+ if ((offset & (field_align - 1)) != 0)
+ {
+ // This struct has padding. We don't guarantee that that
+ // padding is zero-initialized for a stack variable, so we
+ // can't use memcmp to compare struct values.
+ return false;
+ }
+
+ unsigned int field_size;
+ if (!pf->type()->backend_type_size(gogo, &field_size))
+ return false;
+ offset += field_size;
+ }
return true;
}
@@ -4231,7 +4251,7 @@
pf != this->fields_->end();
++pf)
{
- if (!pf->is_anonymous() || pf->type()->deref()->struct_type() == NULL)
+ if (!pf->is_anonymous() || pf->type()->struct_type() == NULL)
++ret;
else
ret += pf->type()->struct_type()->total_field_count();
@@ -4267,17 +4287,6 @@
void
Struct_type::finalize_methods(Gogo* gogo)
{
- // If this type needs explicit comparison and hash functions, create
- // them now. It would be a bit better to do this only if the
- // functions are needed, but they will be static so the backend can
- // discard them if they are not used.
- if (!this->compare_is_identity() && this->is_comparable())
- {
- Named_object* hash_fn;
- Named_object* equal_fn;
- this->type_functions(gogo, NULL, NULL, NULL, &hash_fn, &equal_fn);
- }
-
if (this->all_methods_ != NULL)
return;
Type::finalize_methods(gogo, this, this->location_, &this->all_methods_);
@@ -4727,10 +4736,10 @@
Struct_type::backend_field_offset(Gogo* gogo, unsigned int index,
unsigned int* poffset)
{
- Btype* btype = this->get_backend(gogo);
if (!this->is_backend_type_size_known(gogo))
return false;
- size_t offset = gogo->backend()->type_field_offset(btype, index);
+ size_t offset = gogo->backend()->type_field_offset(this->get_backend(gogo),
+ index);
*poffset = static_cast<unsigned int>(offset);
if (*poffset != offset)
return false;
@@ -4871,25 +4880,6 @@
return false;
}
-// If this type needs explicit comparison and hash functions, create
-// them now. It would be a bit better to do this only if the
-// functions are needed, but they will be static so the backend can
-// discard them if they are not used.
-
-void
-Array_type::finalize_methods(Gogo* gogo)
-{
- if (this->length_ != NULL
- && !this->length_->is_nil_expression()
- && !this->compare_is_identity()
- && this->is_comparable())
- {
- Named_object* hash_fn;
- Named_object* equal_fn;
- this->type_functions(gogo, NULL, NULL, NULL, &hash_fn, &equal_fn);
- }
-}
-
// Traversal.
int
@@ -4988,6 +4978,33 @@
return true;
}
+// Whether we can use memcmp to compare this array.
+
+bool
+Array_type::do_compare_is_identity(Gogo* gogo) const
+{
+ if (this->length_ == NULL)
+ return false;
+
+ // Check for [...], which indicates that this is not a real type.
+ if (this->length_->is_nil_expression())
+ return false;
+
+ if (!this->element_type_->compare_is_identity(gogo))
+ return false;
+
+ // If there is any padding, then we can't use memcmp.
+ unsigned int size;
+ unsigned int align;
+ if (!this->element_type_->backend_type_size(gogo, &size)
+ || !this->element_type_->backend_type_align(gogo, &align))
+ return false;
+ if ((size & (align - 1)) != 0)
+ return false;
+
+ return true;
+}
+
// Array type hash code.
unsigned int
@@ -7272,20 +7289,6 @@
void
Named_type::finalize_methods(Gogo* gogo)
{
- // If this type needs explicit comparison and hash functions, create
- // them now. It would be a bit better to do this only if the
- // functions are needed, but they will be static so the backend can
- // discard them if they are not used.
- if ((this->struct_type() != NULL
- || (this->array_type() != NULL && !this->is_slice_type()))
- && !this->compare_is_identity()
- && this->is_comparable())
- {
- Named_object* hash_fn;
- Named_object* equal_fn;
- this->type_functions(gogo, this, NULL, NULL, &hash_fn, &equal_fn);
- }
-
if (this->all_methods_ != NULL)
return;
@@ -7539,13 +7542,15 @@
// function.
bool
-Named_type::do_compare_is_identity() const
-{
- if (this->seen_)
+Named_type::do_compare_is_identity(Gogo* gogo) const
+{
+ // 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.
+ if (this->seen_in_compare_is_identity_)
return false;
- this->seen_ = true;
- bool ret = this->type_->compare_is_identity();
- this->seen_ = false;
+ this->seen_in_compare_is_identity_ = true;
+ bool ret = this->type_->compare_is_identity(gogo);
+ this->seen_in_compare_is_identity_ = false;
return ret;
}
diff -r 569599d42f46 go/types.h
--- a/go/types.h Wed Jan 11 08:24:08 2012 -0800
+++ b/go/types.h Wed Jan 11 13:09:30 2012 -0800
@@ -568,8 +568,8 @@
// identity function which gets nothing but a pointer to the value
// and a size.
bool
- compare_is_identity() const
- { return this->do_compare_is_identity(); }
+ compare_is_identity(Gogo* gogo) const
+ { return this->do_compare_is_identity(gogo); }
// Return a hash code for this type for the method hash table.
// Types which are equivalent according to are_identical will have
@@ -880,7 +880,7 @@
// Whether the backend size is known.
bool
- is_backend_type_size_known(Gogo*) const;
+ is_backend_type_size_known(Gogo*);
// Get the hash and equality functions for a type.
void
@@ -924,7 +924,7 @@
{ return false; }
virtual bool
- do_compare_is_identity() const = 0;
+ do_compare_is_identity(Gogo*) const = 0;
virtual unsigned int
do_hash_for_method(Gogo*) const;
@@ -1388,7 +1388,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return true; }
unsigned int
@@ -1461,7 +1461,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
unsigned int
@@ -1530,7 +1530,7 @@
protected:
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
unsigned int
@@ -1590,7 +1590,7 @@
{ return true; }
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
Btype*
@@ -1708,7 +1708,7 @@
{ return true; }
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
unsigned int
@@ -1793,7 +1793,7 @@
{ return true; }
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return true; }
unsigned int
@@ -1985,7 +1985,7 @@
Location) const;
// Return the total number of fields, including embedded fields.
- // This is the number of values which can appear in a conversion to
+ // This is the number of values that can appear in a conversion to
// this type.
unsigned int
total_field_count() const;
@@ -2066,7 +2066,7 @@
do_has_pointer() const;
bool
- do_compare_is_identity() const;
+ do_compare_is_identity(Gogo*) const;
unsigned int
do_hash_for_method(Gogo*) const;
@@ -2136,10 +2136,6 @@
array_has_hidden_fields(const Named_type* within, std::string* reason) const
{ return this->element_type_->has_hidden_fields(within, reason); }
- // Build the hash and equality functions if necessary.
- void
- finalize_methods(Gogo*);
-
// Return a tree for the pointer to the values in an array.
tree
value_pointer_tree(Gogo*, tree array) const;
@@ -2192,11 +2188,7 @@
}
bool
- do_compare_is_identity() const
- {
- return (this->length_ != NULL
- && this->element_type_->compare_is_identity());
- }
+ do_compare_is_identity(Gogo*) const;
unsigned int
do_hash_for_method(Gogo*) const;
@@ -2289,7 +2281,7 @@
{ return true; }
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
unsigned int
@@ -2375,7 +2367,7 @@
{ return true; }
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return true; }
unsigned int
@@ -2490,7 +2482,7 @@
{ return true; }
bool
- do_compare_is_identity() const
+ do_compare_is_identity(Gogo*) const
{ return false; }
unsigned int
@@ -2536,7 +2528,7 @@
location_(location), named_btype_(NULL), dependencies_(),
is_visible_(true), is_error_(false), is_placeholder_(false),
is_converted_(false), is_circular_(false), seen_(false),
- seen_in_get_backend_(false)
+ seen_in_compare_is_identity_(false), seen_in_get_backend_(false)
{ }
// Return the associated Named_object. This holds the actual name.
@@ -2731,7 +2723,7 @@
do_has_pointer() const;
bool
- do_compare_is_identity() const;
+ do_compare_is_identity(Gogo*) const;
unsigned int
do_hash_for_method(Gogo*) const;
@@ -2815,6 +2807,8 @@
// This is mutable because it is always reset to false when the
// function exits.
mutable bool seen_;
+ // Like seen_, but used only by do_compare_is_identity.
+ mutable bool seen_in_compare_is_identity_;
// Like seen_, but used only by do_get_backend.
bool seen_in_get_backend_;
};
@@ -2869,8 +2863,8 @@
{ return this->real_type()->has_pointer(); }
bool
- do_compare_is_identity() const
- { return this->real_type()->compare_is_identity(); }
+ do_compare_is_identity(Gogo* gogo) const
+ { return this->real_type()->compare_is_identity(gogo); }
unsigned int
do_hash_for_method(Gogo* gogo) const
