On 3/12/20 2:10 PM, Jeff Law wrote:
On Mon, 2020-03-09 at 14:33 -0600, Martin Sebor wrote:
On 3/5/20 5:26 PM, Jeff Law wrote:
On Fri, 2020-02-14 at 15:41 -0700, Martin Sebor wrote:
Because attribute weakref introduces a kind of a definition, it can
only be applied to declarations of symbols that are not defined. GCC
normally issues a warning when the attribute is applied to a defined
symbol, but PR 92799 shows that it misses some cases on which it then
leads to an ICE.
The ICE was introduced in GCC 4.5. Prior to then, GCC accepted such
invalid definitions and silently dropped the weakref attribute.
The attached patch avoids the ICE while again dropping the invalid
attribute from the definition, except with the (now) usual warning.
Tested on x86_64-linux.
I also looked for code bases that make use of attribute weakref to
rebuild them as another test but couldn't find any. (There are
a couple of instances in the Linux kernel but they look #ifdef'd
out). Does anyone know of any that do use it that I could try to
build on Linux?
So you added this check
... || DECL_INITIAL (decl) != error_mark_node
Do you need to check that DECL_INITIAL is not NULL? IIUC DECL_INITIAL in
this
context is a tri-state.
NULL -- DECL is not a function definition
error_mark_node -- it was a function definition, but the body was free'd
everything else -- the function definition
I've only seen two values come up for a function declared weakref in
the test suite: error_mark_node and something with the TREE_CODE of
BLOCK (the block where the weakref function is used when it's also
explicitly defined in the code, and when the attribute is subsequently
diagnosed by the warning).
So when it's a BLOCK it's giving you the outermost block scope for the function
which we usually use to generate debug info.
The key point is that we use ERROR_MARK_NODE because a non-NULL DECL_INITIAL
denotes an actual function definition, so we can't set it to NULL blindly. See
cgraph_node::expand, cgraph_node:release_body, rest_of_handle_final and perhaps
other places. Another example would be setting up the ifunc resolver. It's a
real function so DECL_INITIAL must be non-NULL, but we don't really have a block
structure we can point to, so instead we set it to error_mark_node
(make_dispatcher_decl).
I wonder if the earlier node->definition check ultimately prevents DECL_INITIAL
from being NULL at this point. According to cgraph.h that field is true when
the
symbol corresponds to a definition in the current unit. That would seem to
indicate yes.
So I think we can go forward with your patch as-is.
Thanks. I committed it in r10-7267. I'll give it a few days and then
backport it to release branches unless there's any concern with it.
Martin
