This is the mail archive of the gcc-patches@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]

Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)


On Thu, Jun 29, 2017 at 10:23 PM, Martin Sebor <msebor@gmail.com> wrote:
> On 06/29/2017 10:15 AM, Jan Hubicka wrote:
>>
>> Hello,
>>>
>>> diff --git a/gcc/hash-table.h b/gcc/hash-table.h
>>> index 0f7e21a..443d16c 100644
>>> --- a/gcc/hash-table.h
>>> +++ b/gcc/hash-table.h
>>> @@ -803,7 +803,10 @@ hash_table<Descriptor, Allocator>::empty_slow ()
>>>        m_size_prime_index = nindex;
>>>      }
>>>    else
>>> -    memset (entries, 0, size * sizeof (value_type));
>>> +    {
>>> +      for ( ; size; ++entries, --size)
>>> +       *entries = value_type ();
>>> +    }
>>>    m_n_deleted = 0;
>>>    m_n_elements = 0;
>>>  }
>>
>>
>> This change sends our periodic testers into an infinite loop.  It is fault
>> of gcc 4.2 being used
>> as bootstrap compiler, but perhaps that can be worked around?
>
>
> The warning in the original code could have been suppressed (by
> casting the pointer to char*), but it was valid so I opted not
> to.  I'd expect it to be possible to work around the bug but
> I don't have easy access to GCC 4.2 to reproduce it or verify
> the fix.
>
> FWIW, after looking at the function again, I wondered if zeroing
> out the elements (either way) was the right thing to do and if
> they shouldn't be cleared by calling Descriptor::mark_empty()
> instead, like in alloc_entries(), but making that change broke
> a bunch of ipa/ipa-pta-*.c tests.  It's not really clear to me
> what this code is supposed to do.
>
> Martin
>
> PS Does this help at all?
>
> @@ -804,8 +804,8 @@ hash_table<Descriptor, Allocator>::empty_slow ()
>      }
>    else
>      {
> -      for ( ; size; ++entries, --size)
> -       *entries = value_type ();
> +      for (size_t i = 0; i != size; ++i)
> +       entries[i] = value_type ();
>
>      }
>    m_n_deleted = 0;
>    m_n_elements = 0;

alloc_entries uses mark_empty.  untested:

Index: gcc/hash-table.h
===================================================================
--- gcc/hash-table.h    (revision 249780)
+++ gcc/hash-table.h    (working copy)
@@ -804,8 +804,8 @@ hash_table<Descriptor, Allocator>::empty
     }
   else
     {
-      for ( ; size; ++entries, --size)
-       *entries = value_type ();
+      for (size_t i = 0; i < size; ++i)
+       mark_empty (entries[i]);
     }
   m_n_deleted = 0;
   m_n_elements = 0;


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