[RFC] Fix for pr64706 testcase faulre
Richard Biener
rguenther@suse.de
Thu Mar 26 15:08:03 GMT 2020
On Thu, 26 Mar 2020, Jan Hubicka wrote:
> Hi,
> this patch started as an attempt to fix pr64706 that kept me walking in loops
> for some time (I failed to make a hack that would make the testcase work in all
> basic settings of -O0/2 -flto set on the both files independently. All GCC
> releases crashes on some).
>
> The testcase has ODR violation that breaks comdat groups and kind of works with
> non-lto by accident, but it triggers longer problem that we do not handle
> aliases correctly.
>
> In ELF an alias is just additional symbol at a given position in code segment,
> while for GCC we do not have code segment. Function bodies are assigned to
> delcarations and aliases are linked to those declarations. This makes difference
> in the following testcase:
>
> ===== b.c:
>
> __attribute__((weak))
> __attribute__((noinline))
>
> int a()
> {
> return 1;
> }
>
> __attribute__((noinline))
> static int b() __attribute__((alias("a")));
> void
> test()
> {
> if (b()!=1)
> __builtin_abort ();
> }
>
> ===== b.c:
>
> __attribute__((noinline))
> int a();
>
> int a()
> {
> return 2;
> }
> extern void test ();
> int
> main()
> {
> if (a() != 2)
> __builtin_abort ();
> test();
> return 0;
> }
>
> Here LTO linking will replace weak symbol a() by definition from b.c and
> rediect static alias b in a.c to this definition which is wrong.
Is it? What does a non-weak local alias to a weak function mean?
> This patch works towards fixing it by making lto-symtab to notice that there is
> previaling alias (a.c:b in my testcase) of a prevailed symbol (a.c:a in my
> testcase that got previaled by b.c:a). In this case the body of prevailed
> symbol (a.c:a) must be saved because it is still reachable by calling b.c:a.
> This is done by turning the prevailed symbol to a static symbol.
>
> This has some fallout in streaming because we need to stream in summaries of
> such prevailed symbols which is not hard to do and is purpose of
> stream_in_summary_p.
>
> Now however we have a problem if tree sharing decided to share delcaration of
> a.c:a and b.c:a (which it does in my testcase) because we have one decl
> and two bodies. I use the code form profile merging to patch around but this
> code is not fully correct (it works in profile merging only because the
> body is discarded immediatly after).
>
> The code creates new static decl, say a.static and moves the body of a.c to it.
> This however breaks because when streaming in body of a.static we will prevail
> all references to a.c:a to b.c:a. This is OK, for example when streaming in
> statement calling a(). It is wrong when streaming in _CONTEXT pointers.
>
> So in turn my patch mostly works, but sometimes I get errors like
> /aux/hubicka/trunk-git/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c:9:5: error: label context is not the current function declaration
>
> I do not see way around this with our current infrastructure. One way would be
> to read body of a.c:a and clone it to a.static. This is bit wasteful but it is
> also problem in case body of b.c:a is already read to memory.
Why don't we represent the alias at the cgraph level? That is,
why do decls come into play at all here? b prevails and it has a
reference to a.c:a so we need to keep (and emit) that. The only issue
would be that we'd end up with two 'a's so we have to notice that
and fixup somehow.
Isn't this in the end a similar issue as with extern inline bodies
vs. the offline copy body?
> I would like to do this w/o need to stream in the body, because such situation
> happens quite commonly local aliases: Until we are able to prove that all
> referencs to a local aliase of a symbol are gone we need to assume that body
> needs to be preserved and those are potentially quite frequent.
>
> So basically only solution I think of is to make lto-stream-in live with the
> fact that function decl was changed. So we can stream original decl to the
> beggining of the function section and during stream in do rewritting of
> DECL_CONTEXT similar way as tree-inline would do. We need to be careful about
> keeping debug info in shape too (we are not producing inline copy, we are
> merely unsharing the main decl).
>
> What do you think?
I think the not prevailed but still needed cgraph node for 'a' should
have its decl cleared ;)
> Note that I was considering case of simply not sharing decls with
> aliases at stream time. This does not work because by localizing the
> decl one will not redirect references to the symbol correctly.
But all remaining references should be via 'b', not via 'a'.
> Other option would be to always produce a static symbol for every
> symbol with an alias. This again can be IMO only done by cloning it and
> would be relatively expensive because at the stream out time we do have
> considerable quantity of aliases especially with -fpic.
>
> Honza
>
> gcc/ChangeLog:
>
> 2020-03-26 Jan Hubicka <hubicka@ucw.cz>
>
> * cgraph.c (cgraph_node::make_local): Add force parameter.
> * cgraph.h (symtab_node::clone_referring_skip_aliases): New
> member function.
> (symtab_node::stream_in_summary_p): Likewise.
> * ipa-fnsummary.c (inline_read_section): Use
> symtab_node::stream_in_summary_p.
> * ipa-prop.c (ipa_read_node_info): Likewise.
> * symtab.c (symtab_node::clone_referring): Fix formating.
> (symtab_node::clone_referring_skip_aliases): New member function.
> (symtab_node::dump_referring): Fix formating
> (symtab_node::stream_in_summary_p): New member function.
>
> gcc/lto/ChangeLog:
>
> 2020-03-26 Jan Hubicka <hubicka@ucw.cz>
>
> * lto-symtab.c (lto_symtab_symbol_p): New function.
> (has_prevailing_alias_p): New function.
> (lto_symtab_replace_node): Break out from ...; support alias
> interposition
> (lto_cgraph_replace_node): ... here.
> (lto_varpool_replace_node): Use the common code.
>
> gcc/testsuite/ChangeLog:
>
> 2020-03-26 Jan Hubicka <hubicka@ucw.cz>
>
> * gcc.dg/lto/alias-resolution-1_0.c: New test.
> * gcc.dg/lto/alias-resolution-1_1.c: New test.
>
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 6b780f80eb3..1f1319ed5fe 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -2465,9 +2465,10 @@ cgraph_node::call_for_symbol_thunks_and_aliases (bool (*callback)
> /* Worker to bring NODE local. */
>
> bool
> -cgraph_node::make_local (cgraph_node *node, void *)
> +cgraph_node::make_local (cgraph_node *node, void *force)
> {
> - gcc_checking_assert (node->can_be_local_p ());
> + if (!force)
> + gcc_checking_assert (node->can_be_local_p ());
> if (DECL_COMDAT (node->decl) || DECL_EXTERNAL (node->decl))
> {
> node->make_decl_local ();
> @@ -2486,12 +2487,14 @@ cgraph_node::make_local (cgraph_node *node, void *)
> return false;
> }
>
> -/* Bring cgraph node local. */
> +/* Bring cgraph node local. If FORCE is true skip checking if symbol can
> + become local. */
>
> void
> -cgraph_node::make_local (void)
> +cgraph_node::make_local (bool force)
> {
> - call_for_symbol_thunks_and_aliases (cgraph_node::make_local, NULL, true);
> + call_for_symbol_thunks_and_aliases (cgraph_node::make_local,
> + (void *)(size_t)force, true);
> }
>
> /* Worker to set nothrow flag. */
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index 43de3b4a8ac..983cbfe8cd8 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -189,6 +189,9 @@ public:
> with callgraph edges associated with them. */
> void clone_referring (symtab_node *node);
>
> + /* Same as clone_referring but skip aliases. */
> + void clone_referring_skip_aliases (symtab_node *node);
> +
> /* Clone reference REF to this symtab_node and set its stmt to STMT. */
> ipa_ref *clone_reference (ipa_ref *ref, gimple *stmt);
>
> @@ -340,6 +343,9 @@ public:
> in question prevails in the linking to save some memory usage. */
> bool prevailing_p (void);
>
> + /* Return true if the summary should be streamed in. */
> + bool stream_in_summary_p (void);
> +
> /* Return true if NODE binds to current definition in final executable
> when referenced from REF. If REF is NULL return conservative value
> for any reference. */
> @@ -1163,8 +1169,9 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node
> /* cgraph_node is no longer nested function; update cgraph accordingly. */
> void unnest (void);
>
> - /* Bring cgraph node local. */
> - void make_local (void);
> + /* Bring cgraph node local. If FORCE is true skip checking if symbol can
> + be local. */
> + void make_local (bool force = false);
>
> /* Likewise indicate that a node is having address taken. */
> void mark_address_taken (void);
> diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
> index b411bc4d660..a79e75dd001 100644
> --- a/gcc/ipa-fnsummary.c
> +++ b/gcc/ipa-fnsummary.c
> @@ -4176,10 +4176,10 @@ inline_read_section (struct lto_file_decl_data *file_data, const char *data,
> encoder = file_data->symtab_node_encoder;
> node = dyn_cast<cgraph_node *> (lto_symtab_encoder_deref (encoder,
> index));
> - info = node->prevailing_p () ? ipa_fn_summaries->get_create (node) : NULL;
> - params_summary = node->prevailing_p () ? IPA_NODE_REF (node) : NULL;
> - size_info = node->prevailing_p ()
> - ? ipa_size_summaries->get_create (node) : NULL;
> + bool stream_in = node->stream_in_summary_p ();
> + info = stream_in ? ipa_fn_summaries->get_create (node) : NULL;
> + params_summary = stream_in ? IPA_NODE_REF (node) : NULL;
> + size_info = stream_in ? ipa_size_summaries->get_create (node) : NULL;
>
> int stack_size = streamer_read_uhwi (&ib);
> int size = streamer_read_uhwi (&ib);
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index a77130ded39..4035d903d33 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -4937,7 +4937,7 @@ ipa_read_node_info (class lto_input_block *ib, struct cgraph_node *node,
> int k;
> struct cgraph_edge *e;
> struct bitpack_d bp;
> - bool prevails = node->prevailing_p ();
> + bool prevails = node->stream_in_summary_p ();
> class ipa_node_params *info = prevails
> ? IPA_NODE_REF_GET_CREATE (node) : NULL;
>
> diff --git a/gcc/lto/lto-symtab.c b/gcc/lto/lto-symtab.c
> index d5e38beb4b6..3f4ece360d7 100644
> --- a/gcc/lto/lto-symtab.c
> +++ b/gcc/lto/lto-symtab.c
> @@ -36,6 +36,150 @@ along with GCC; see the file COPYING3. If not see
> #include "stringpool.h"
> #include "attribs.h"
>
> +/* Return true, if the symbol E should be resolved by lto-symtab.
> + Those are all external symbols and all real symbols that are not static (we
> + handle renaming of static later in partitioning). */
> +
> +static bool
> +lto_symtab_symbol_p (symtab_node *e)
> +{
> + if (!TREE_PUBLIC (e->decl) && !DECL_EXTERNAL (e->decl))
> + return false;
> + return e->real_symbol_p ();
> +}
> +
> +/* Return ture if NODE is an prevailing alias or has alias that prevalis. */
> +
> +static bool
> +has_prevailing_alias_p (struct symtab_node *node)
> +{
> + ipa_ref *ref;
> +
> + FOR_EACH_ALIAS (node, ref)
> + {
> + symtab_node *alias = ref->referring;
> +
> + if (lto_symtab_symbol_p (alias))
> + return alias->prevailing_p ();
> + else
> + return true;
> + if (has_prevailing_alias_p (alias))
> + return true;
> + }
> + return false;
> +}
> +
> +/* Common part of lto_cgraph_replace_node and lto_varpool_replace_node
> + Retun true if NODE can be removed. */
> +
> +static bool
> +lto_symtab_replace_node (struct symtab_node *node,
> + struct symtab_node *prevailing_node)
> +{
> + /* Merge node flags. */
> + if (node->force_output)
> + prevailing_node->force_output = true;
> + if (node->forced_by_abi)
> + prevailing_node->forced_by_abi = true;
> + if (node->address_taken)
> + {
> + prevailing_node->address_taken = true;
> + prevailing_node->ultimate_alias_target ()->address_taken = true;
> + }
> + /* Redirect incomming references. */
> + prevailing_node->clone_referring_skip_aliases (node);
> +
> + node->set_comdat_group (NULL);
> + /* Dissolve same comdat group. In most cases this is not necessary because
> + all symbols in the group will be previaled by the symbols in prevailing
> + group, but in the case of ODR violations we do not want to end up with
> + multiple comdat groups of same name. */
> + if (node->same_comdat_group)
> + {
> + if (dyn_cast <cgraph_node *> (node))
> + dyn_cast <cgraph_node *> (node)->calls_comdat_local = false;
> + for (symtab_node *next = node->same_comdat_group;
> + next != node; next = next->same_comdat_group)
> + {
> + if (dyn_cast <cgraph_node *> (next))
> + dyn_cast <cgraph_node *> (next)->calls_comdat_local = false;
> + next->set_comdat_group (NULL);
> + }
> + node->dissolve_same_comdat_group_list ();
> + }
> +
> + /* If there is alias that prevails the definition remains reachable.
> + Turn it to a static symbol. */
> + if (has_prevailing_alias_p (node))
> + {
> + bool copy = false;
> +
> + /* See if decl is shared with another symbol definition. In this case
> + we must copy it. */
> + for (symtab_node *e = node->previous_sharing_asm_name;
> + e && copy; e = e->previous_sharing_asm_name)
> + if (e->decl == node->decl
> + && e->stream_in_summary_p ())
> + copy = true;
> + for (symtab_node *e = node->next_sharing_asm_name;
> + e && copy; e = e->next_sharing_asm_name)
> + if (e->decl == node->decl
> + && e->stream_in_summary_p ())
> + copy = true;
> +
> + if (copy)
> + {
> + struct lto_in_decl_state temp;
> + struct lto_in_decl_state *state;
> +
> + /* We are going to move the decl, we want to remove its file decl
> + data and link these with the new decl. */
> + temp.fn_decl = node->decl;
> + lto_in_decl_state **slot
> + = node->lto_file_data->function_decl_states->find_slot (&temp,
> + NO_INSERT);
> + state = *slot;
> + node->lto_file_data->function_decl_states->clear_slot (slot);
> + gcc_assert (state);
> +
> + /* Duplicate the decl and be sure it does not link into body
> + of DST. */
> + node->decl = copy_node (node->decl);
> + node->decl->decl_with_vis.symtab_node = node;
> + DECL_STRUCT_FUNCTION (node->decl) = NULL;
> + DECL_ARGUMENTS (node->decl) = NULL;
> + DECL_INITIAL (node->decl) = NULL;
> + DECL_RESULT (node->decl) = NULL;
> +
> + /* Associate the decl state with new declaration, so LTO streamer
> + can look it up. */
> + state->fn_decl = node->decl;
> + slot
> + = node->lto_file_data->function_decl_states->find_slot (state,
> + INSERT);
> + gcc_assert (!*slot);
> + *slot = state;
> + }
> + node->make_decl_local ();
> + return false;
> + }
> + else
> + {
> + ipa_ref *ref;
> +
> + /* All aliases are going to be removed, but we need to do that only
> + after all replacements are completed. To keep verifier happy turn
> + them into forward declarations. */
> + FOR_EACH_ALIAS (node, ref)
> + {
> + ref->referring->definition = false;
> + ref->referring->alias = false;
> + ref->referring->analyzed = false;
> + }
> + }
> + return true;
> +}
> +
> /* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
> all edges and removing the old node. */
>
> @@ -56,16 +200,8 @@ lto_cgraph_replace_node (struct cgraph_node *node,
> (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (node->decl)))));
> }
>
> - /* Merge node flags. */
> - if (node->force_output)
> - prevailing_node->mark_force_output ();
> - if (node->forced_by_abi)
> - prevailing_node->forced_by_abi = true;
> - if (node->address_taken)
> - {
> - gcc_assert (!prevailing_node->inlined_to);
> - prevailing_node->mark_address_taken ();
> - }
> + bool remove_p = lto_symtab_replace_node (node, prevailing_node);
> +
> if (node->definition && prevailing_node->definition
> && DECL_COMDAT (node->decl) && DECL_COMDAT (prevailing_node->decl))
> prevailing_node->merged_comdat = true;
> @@ -95,15 +231,16 @@ lto_cgraph_replace_node (struct cgraph_node *node,
> e->call_stmt_cannot_inline_p = 1;
> }
> }
> - /* Redirect incomming references. */
> - prevailing_node->clone_referring (node);
> - lto_free_function_in_decl_state_for_node (node);
> + if (remove_p)
> + {
> + lto_free_function_in_decl_state_for_node (node);
>
> - if (node->decl != prevailing_node->decl)
> - node->release_body ();
> + if (node->decl != prevailing_node->decl)
> + node->release_body ();
>
> - /* Finally remove the replaced node. */
> - node->remove ();
> + /* Finally remove the replaced node. */
> + node->remove ();
> + }
> }
>
> /* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
> @@ -116,11 +253,7 @@ lto_varpool_replace_node (varpool_node *vnode,
> gcc_assert (!vnode->definition || prevailing_node->definition);
> gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
>
> - prevailing_node->clone_referring (vnode);
> - if (vnode->force_output)
> - prevailing_node->force_output = true;
> - if (vnode->forced_by_abi)
> - prevailing_node->forced_by_abi = true;
> + bool remove_p = lto_symtab_replace_node (vnode, prevailing_node);
>
> /* Be sure we can garbage collect the initializer. */
> if (DECL_INITIAL (vnode->decl)
> @@ -142,7 +275,7 @@ lto_varpool_replace_node (varpool_node *vnode,
> || vnode->tls_model == TLS_MODEL_NONE
> || vnode->tls_model == TLS_MODEL_EMULATED)
> error = true;
> - /* Linked is silently supporting transitions
> + /* Linker is silently supporting transitions
> GD -> IE, GD -> LE, LD -> LE, IE -> LE, LD -> IE.
> Do the same transitions and error out on others. */
> else if ((prevailing_node->tls_model == TLS_MODEL_REAL
> @@ -166,14 +299,16 @@ lto_varpool_replace_node (varpool_node *vnode,
> if (error)
> {
> error_at (DECL_SOURCE_LOCATION (vnode->decl),
> - "%qD is defined with tls model %s", vnode->decl, tls_model_names [vnode->tls_model]);
> + "%qD is defined with tls model %s", vnode->decl,
> + tls_model_names[vnode->tls_model]);
> inform (DECL_SOURCE_LOCATION (prevailing_node->decl),
> "previously defined here as %s",
> - tls_model_names [prevailing_node->tls_model]);
> + tls_model_names[prevailing_node->tls_model]);
> }
> }
> /* Finally remove the replaced node. */
> - vnode->remove ();
> + if (remove_p)
> + vnode->remove ();
> }
>
> /* Return non-zero if we want to output waring about T1 and T2.
> @@ -408,18 +543,6 @@ lto_symtab_resolve_replaceable_p (symtab_node *e)
> return false;
> }
>
> -/* Return true, if the symbol E should be resolved by lto-symtab.
> - Those are all external symbols and all real symbols that are not static (we
> - handle renaming of static later in partitioning). */
> -
> -static bool
> -lto_symtab_symbol_p (symtab_node *e)
> -{
> - if (!TREE_PUBLIC (e->decl) && !DECL_EXTERNAL (e->decl))
> - return false;
> - return e->real_symbol_p ();
> -}
> -
> /* Return true if the symtab entry E can be the prevailing one. */
>
> static bool
> diff --git a/gcc/symtab.c b/gcc/symtab.c
> index 3022acfc1ea..4745c2c94ec 100644
> --- a/gcc/symtab.c
> +++ b/gcc/symtab.c
> @@ -685,7 +685,7 @@ symtab_node::clone_referring (symtab_node *node)
> {
> ipa_ref *ref = NULL, *ref2 = NULL;
> int i;
> - for (i = 0; node->iterate_referring(i, ref); i++)
> + for (i = 0; node->iterate_referring (i, ref); i++)
> {
> bool speculative = ref->speculative;
> unsigned int stmt_uid = ref->lto_stmt_uid;
> @@ -697,6 +697,27 @@ symtab_node::clone_referring (symtab_node *node)
> }
> }
>
> +/* Clone all referring from symtab NODE to this symtab_node
> + Skip aliases. */
> +
> +void
> +symtab_node::clone_referring_skip_aliases (symtab_node *node)
> +{
> + ipa_ref *ref = NULL, *ref2 = NULL;
> + int i;
> + for (i = 0; node->iterate_referring (i, ref); i++)
> + if (ref->use != IPA_REF_ALIAS)
> + {
> + bool speculative = ref->speculative;
> + unsigned int stmt_uid = ref->lto_stmt_uid;
> +
> + ref2 = ref->referring->create_reference (this, ref->use, ref->stmt);
> + ref2->speculative = speculative;
> + ref2->lto_stmt_uid = stmt_uid;
> + ref2->speculative_id = ref->speculative_id;
> + }
> +}
> +
> /* Clone reference REF to this symtab_node and set its stmt to STMT. */
>
> ipa_ref *
> @@ -813,7 +834,7 @@ symtab_node::dump_referring (FILE *file)
> {
> ipa_ref *ref = NULL;
> int i;
> - for (i = 0; iterate_referring(i, ref); i++)
> + for (i = 0; iterate_referring (i, ref); i++)
> {
> fprintf (file, "%s (%s)",
> ref->referring->dump_asm_name (),
> @@ -2490,3 +2511,21 @@ symtab_node::output_to_lto_symbol_table_p (void)
> }
> return true;
> }
> +
> +/* Return true if summary should be streamed in. */
> +bool
> +symtab_node::stream_in_summary_p ()
> +{
> + ipa_ref *ref;
> + if (prevailing_p ())
> + return true;
> + FOR_EACH_ALIAS (this, ref)
> + {
> + symtab_node *alias = ref->referring;
> +
> + if (!alias->externally_visible
> + || alias->stream_in_summary_p ())
> + return true;
> + }
> + return false;
> +}
> diff --git a/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c
> new file mode 100644
> index 00000000000..4a8471244f6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_0.c
> @@ -0,0 +1,20 @@
> +/* { dg-require-alias "" } */
> +#define ASMNAME2(prefix, cname) STRING (prefix) cname
> +#define STRING(x) #x
> +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname)
> +__attribute__((weak))
> +__attribute__((noinline))
> +int a() __asm__ (ASMNAME ("a"));
> +
> +int a()
> +{
> + return 1;
> +}
> +__attribute__((noinline))
> +static int b() __attribute__((alias("a")));
> +void
> +test()
> +{
> + if (b()!=1)
> + __builtin_abort ();
> +}
> diff --git a/gcc/testsuite/gcc.dg/lto/alias-resolution-1_1.c b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_1.c
> new file mode 100644
> index 00000000000..f25cfa27bc7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/alias-resolution-1_1.c
> @@ -0,0 +1,19 @@
> +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname)
> +#define ASMNAME2(prefix, cname) STRING (prefix) cname
> +#define STRING(x) #x
> +__attribute__((noinline))
> +int a() __asm__ (ASMNAME ("a"));
> +
> +int a()
> +{
> + return 2;
> +}
> +extern void test ();
> +int
> +main()
> +{
> + if (a() != 2)
> + __builtin_abort ();
> + test();
> + return 0;
> +}
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
More information about the Gcc-patches
mailing list