On 3/18/20 6:04 PM, Martin Sebor wrote:
On 3/16/20 3:10 PM, Jason Merrill wrote:
On 3/16/20 4:41 PM, Martin Sebor wrote:
The recent fix to avoid modifying in place the type argument in
handle_access_attribute (PR 92721) was incomplete and didn't fully
resolve the problem (an ICE in the C++ front-end). The attached
patch removes the remaining modification that causes the ICE. In
addition, the change adjusts checking calls to functions declared
with the attribute to scan all its instances.
The attached patch was tested on x86_64-linux.
I'm puzzled that the ICE only triggers in the C++ front-end and not
also in C. The code that issues the internal error is in comptypes()
in cp/typeck.c and has this comment:
/* The two types are structurally equivalent, but their
canonical types were different. This is a failure of the
canonical type propagation code.*/
internal_error
("canonical types differ for identical types %qT and %qT",
t1, t2);
What is "the canonical type propagation code" it refers to?
Generally, code that makes sure that TYPE_CANONICAL equality is
equivalent to type identity, not any one specific place.
Is it valid to modify the type in an attribute handler
Only if (flags & ATTR_FLAG_TYPE_IN_PLACE).
If not, and if modifying a type in place is not valid I'd expect
decl_attributes to enforce it. I looked for other attribute handlers
to see if any also modify the type argument in place (by adding or
removing attributes) but couldn't really find any. So if it is
invalid I'd like to add such an assertion (probably for GCC 11) but
before I do I want to make sure I'm not missing something.
Generally messing with _ATTRIBUTES happens in decl_attributes:
changing it directly if it's a DECL or ATTR_FLAG_TYPE_IN_PLACE,
otherwise using build_type_attribute_variant. If you need to do it in
your handler, you should follow the same pattern.
That's the conclusion I came to as well, but thanks for confirming
it. With the patch I don't need to make the change but since it's
not obvious that it's a no-no and since it's apparently only detected
under very special conditions I'm wondering is if it's possible to
detect it more reliably than only in C++ comptypes. The trouble is
that I don't exactly what is allowed and what isn't and what to look
for to tell if the handler did something that's not allowed.
The C++ ICE triggered because the redeclared function's type is
considered the same as the original (structural_comptypes()
returns true) but the declarations' canonical types are different.
structural_comptypes() is C++-specific and I don't know what
alternative to call in the middle-end to compare the types and
get the equivalent result.
I think type_cache_hasher::equal is the closest, but it looks like it
doesn't check everything.
PS As a data point, I found just two attribute handlers in
c-attribs.c that modify a type in place: one for attribute const
and the other noreturn. They both do it for function pointers
to set the 'const' and 'noreturn' bits on the pointed to types,
and by calling build_type_variant.
Hmm, yes, that does sound like a bug.
Jason
