This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

GCC plugins & GGC & explicit gcc_free


Hello All,

[[I know that this is a sensitive issue, and I also know that I am in
the minority believing that a garbage collection is useful inside the
GCC compiler]]

How should plugins deal with data that is explicitly gcc_free-d outside
of ggc_collect?

In particular, plugins are apparently allowed to have their own GTY-ed
data (which is very good, I certainly don't want that feature to
disappear). I'm particularly thinking of "static analysis" (or
"observing") plugins, which would take advantage of the GCC compiler to
add some extra measures and analysis but which won't change anything
observable in the GCC internal representations (e.g. plugins that won't
add or remove any Gimple inside existing functions, for instance).

I believe that such "observing" plugins should certainly be allowed to
register a pass which would put some data (e.g. tree-s or basic_block-s)
inside some GTY-ed variable and examine it much later, perhaps even at
PLUGIN_FINISH time (and certainly at PLUGIN_FINISH_UNIT time). This
would make sense if such a plugin would like to make some detailed
statistics, serialize *some* of the Gimple or Tree-s in a database, etc.

However, such a plugin could crash, because at several occasions
ggc_free is *forcibly* called *outside* of the garbage collector - and
there is no mechanism (finalization, plugin hooks) to communicate that
deletion to the plugin. So such a plugin will keep a *stale* data
referenced by its GTY-ed roots.

I have a concrete example within my MELT branch -currently close to GCC
4.9-  & plugin (see http://gcc-melt.org/ if you never heard of MELT).
the justcountipa pass I have in file gcc/melt/xtramelt-ana-simple.melt
near lines 840 and following. It stores some global MELT data singleton
of CLASS_JUSTCOUNTIPA_DATA in the MELT pass, and that data stays
available, so is never freed by the GGC. Unfortunately, that data
contains (very indirectly) some tree-s and edge-s which have been
*explicitly* ggc_free-d, e.g. by execute_todo calling indirectly
cleanup_tree_cfg calling indirectly free_edge.

If you are interested here is the offending backtrace (with a watchpoint
triggered when ggc_free is overwriting, by enable-checking code, some
edge still kept in MELT global data thru a GTY-ed variable) 

0  in memset of ../sysdeps/x86_64/memset.S:65
1  in ggc_free of /usr/src/Lang/basile-melt-gcc/gcc/ggc-page.c:1544
2  in free_edge of /usr/src/Lang/basile-melt-gcc/gcc/cfg.c:92
3  in remove_edge_raw of /usr/src/Lang/basile-melt-gcc/gcc/cfg.c:351
4  in remove_edge of /usr/src/Lang/basile-melt-gcc/gcc/cfghooks.c:418
5  in remove_phi_nodes_and_edges_for_unreachable_block
of /usr/src/Lang/basile-melt-gcc/gcc/tree-cfg.c:1934
6  in remove_bb of /usr/src/Lang/basile-melt-gcc/gcc/tree-cfg.c:2016
7  in delete_basic_block
of /usr/src/Lang/basile-melt-gcc/gcc/cfghooks.c:565
8  in remove_forwarder_block
of /usr/src/Lang/basile-melt-gcc/gcc/tree-cfgcleanup.c:529
9  in cleanup_tree_cfg_bb
of /usr/src/Lang/basile-melt-gcc/gcc/tree-cfgcleanup.c:633
10 in cleanup_tree_cfg_1
of /usr/src/Lang/basile-melt-gcc/gcc/tree-cfgcleanup.c:675
11 in cleanup_tree_cfg_noloop
of /usr/src/Lang/basile-melt-gcc/gcc/tree-cfgcleanup.c:731
12 in cleanup_tree_cfg
of /usr/src/Lang/basile-melt-gcc/gcc/tree-cfgcleanup.c:786
13 in execute_function_todo
of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:1811
14 in do_per_function of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:1574
15 in execute_todo of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:1887
16 in execute_one_pass
of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:2247
17 in execute_pass_list
of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:2286
18 in execute_pass_list
of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:2287
19 in do_per_function_toporder
of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:1630
20 in execute_ipa_pass_list
of /usr/src/Lang/basile-melt-gcc/gcc/passes.c:2617
21 in ipa_passes of /usr/src/Lang/basile-melt-gcc/gcc/cgraphunit.c:2043
22 in compile of /usr/src/Lang/basile-melt-gcc/gcc/cgraphunit.c:2174
23 in finalize_compilation_unit
of /usr/src/Lang/basile-melt-gcc/gcc/cgraphunit.c:2329

If I continue the above execution my MELT plugin crashes (in GGC marking
routines) because the stale edge has been free-d and reused later.
 
Of course, a dirty workaround is for me to clear manually that data
before it could be seen by the GGC marking garbage collector. And such a
workaround is working. But I really find it shameful.

Unfortunately, the data which are manually freed outside of ggc_collect
(so which should perhaps not be kept without care in GTY-ed variables
inside plugins) is not exactly documented.

>From my biased point of view, I would much prefer if ggc_free was called
*only* from ggc_collect. Sadly, I believe that few GCC people would
agree with that. Or we could have ggc_free called only from ggc_collect,
and add a new ggc_becomes_freeable routine (and every call to ggc_free
outside of ggc_collect would be replaced by a call to
ggc_becomes_freeable which for example would call ggc_free if not plugin
is loaded, and stays a no-op if some plugins are loaded...).

Perhaps we could improve the situation e.g.


   document more when and which data is cleaned up

   add a finalizer plugin hook for the few static calls to ggc_free, but
that probably would be quite expensive (e.g. we'll run that hook from
free_edge in gcc/cfg.c ...) because ggc_free is probably called a big
lot of times.

   more realistically, add plugin hooks in cleanup routines, notably in
cleanup_cfg file gcc/cfgcleanup.c


Please comment on this. If we agree on something I'll try to propose
some patch to GCC 5.0 during its stage 1. 

Regards.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]