Re: Fix uninitialized memory use in ipa-modref

2020-11-06 Thread Martin Liška

On 11/5/20 6:37 PM, Jan Hubicka wrote:

We can't because our vec does not accept non-pods and this needs to be
GGC safe since it points to trees.


Ah, that's new to me!

Thanks,
Martin


Re: Fix uninitialized memory use in ipa-modref

2020-11-06 Thread Martin Liška

On 11/5/20 6:54 PM, Jan Hubicka wrote:

On 11/5/20 3:27 PM, Jan Hubicka wrote:

 poly_int64 offset;
 struct modref_parm_map parm_map;
+  parm_map.parm_offset_known = false;
+  parm_map.parm_offset = 0;
+


I'm curious, can't we use a proper C++ class construction.
The IPA pass is new and so we can make it more C++-ish? Similarly
for all newly introduced structs in mod ref.


We can't because our vec does not accept non-pods and this needs to be
GGC safe since it points to trees.


We could probably add construction of writes_errno even though in corret
run it should be never used (in analysis we need to be able to
reinitialize and during stream in we will always stream it in).


It may be error prone approach to initialize it to NULL.


What else do you think can be more ++-ish? The pass even has two
templates :).


Heh, all right :)



diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index b40f3da3ba2..e80f6de09f2 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -124,7 +124,7 @@ static GTY(()) fast_function_summary 
  /* Summary for a single function which this pass produces.  */
  
  modref_summary::modref_summary ()

-  : loads (NULL), stores (NULL)
+  : loads (NULL), stores (NULL), writes_errno (NULL)
  {
  }
  





Re: Fix uninitialized memory use in ipa-modref

2020-11-05 Thread Jan Hubicka
> > On 11/5/20 3:27 PM, Jan Hubicka wrote:
> > > poly_int64 offset;
> > > struct modref_parm_map parm_map;
> > > +  parm_map.parm_offset_known = false;
> > > +  parm_map.parm_offset = 0;
> > > +
> > 
> > I'm curious, can't we use a proper C++ class construction.
> > The IPA pass is new and so we can make it more C++-ish? Similarly
> > for all newly introduced structs in mod ref.
> 
> We can't because our vec does not accept non-pods and this needs to be
> GGC safe since it points to trees.

We could probably add construction of writes_errno even though in corret
run it should be never used (in analysis we need to be able to
reinitialize and during stream in we will always stream it in).
What else do you think can be more ++-ish? The pass even has two
templates :).

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index b40f3da3ba2..e80f6de09f2 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -124,7 +124,7 @@ static GTY(()) fast_function_summary 
 /* Summary for a single function which this pass produces.  */
 
 modref_summary::modref_summary ()
-  : loads (NULL), stores (NULL)
+  : loads (NULL), stores (NULL), writes_errno (NULL)
 {
 }
 


Re: Fix uninitialized memory use in ipa-modref

2020-11-05 Thread Jan Hubicka
> On 11/5/20 3:27 PM, Jan Hubicka wrote:
> > poly_int64 offset;
> > struct modref_parm_map parm_map;
> > +  parm_map.parm_offset_known = false;
> > +  parm_map.parm_offset = 0;
> > +
> 
> I'm curious, can't we use a proper C++ class construction.
> The IPA pass is new and so we can make it more C++-ish? Similarly
> for all newly introduced structs in mod ref.

We can't because our vec does not accept non-pods and this needs to be
GGC safe since it points to trees.

Honza
> 
> Thanks,
> Martin


Re: Fix uninitialized memory use in ipa-modref

2020-11-05 Thread Martin Liška

On 11/5/20 3:27 PM, Jan Hubicka wrote:

poly_int64 offset;
struct modref_parm_map parm_map;
  
+  parm_map.parm_offset_known = false;

+  parm_map.parm_offset = 0;
+


I'm curious, can't we use a proper C++ class construction.
The IPA pass is new and so we can make it more C++-ish? Similarly
for all newly introduced structs in mod ref.

Thanks,
Martin


Fix uninitialized memory use in ipa-modref

2020-11-05 Thread Jan Hubicka
Hi,
this patch fixes two uninitialized memory uses in ipa-modref.  First is
harmless because the values are never used, but they will make valgrind
unhapy.
Second is an actual bug: while breaking the patch in half I forgot to
initialize errno at stream in time.

Bootstrapped/regtested x86_64-linux, comitted.

* ipa-modref.c (parm_map_for_arg): Initialize parm_offset and
parm_offset_knonw.
(read_section): Set writes_errno to false.
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index b40f3da3ba2..9df3d2bcf2d 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -525,6 +525,9 @@ parm_map_for_arg (gimple *stmt, int i)
   poly_int64 offset;
   struct modref_parm_map parm_map;
 
+  parm_map.parm_offset_known = false;
+  parm_map.parm_offset = 0;
+
   offset_known = unadjusted_ptr_and_unit_offset (op, , );
   if (TREE_CODE (op) == SSA_NAME
   && SSA_NAME_IS_DEFAULT_DEF (op)
@@ -1533,10 +1536,12 @@ read_section (struct lto_file_decl_data *file_data, 
const char *data,
   modref_summary_lto *modref_sum_lto = summaries_lto
   ? summaries_lto->get_create (node)
   : NULL;
-
   if (optimization_summaries)
modref_sum = optimization_summaries->get_create (node);
 
+  if (modref_sum)
+   modref_sum->writes_errno = false;
+
   gcc_assert (!modref_sum || (!modref_sum->loads
  && !modref_sum->stores));
   gcc_assert (!modref_sum_lto || (!modref_sum_lto->loads