On Thu, Jan 12, 2023 at 10:32 PM Alexandre Oliva via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> Existing hash_table traits that use the same representation for empty
> and deleted slots reject marking slots as deleted, and to not pass
> is_deleted for slots that pass is_empty.
>
> Nevertheless, nearly everywhere, we only test for is_deleted after
> checking that !is_empty first. The one exception was the copy
> constructor, that would fail if traits recognized is_empty slots as
> is_deleted, but then refused to mark_deleted.
>
> This asymmetry is neither necessary nor desirable, and there is a
> theoretical risk that traits might not only fail to refuse to
> mark_deleted, but also return is_deleted for is_empty slots.
>
> This patch introduces checks that detect these potentially problematic
> situations, and reorders the tests in the copy constructor so as to
> use the conventional testing order and thus avoid them.
>
> Regstrapped on x86_64-linux-gnu. Ok to install?
OK.
>
> for gcc/ChangeLog
>
> * hash-table.h (is_deleted): Precheck !is_empty.
> (mark_deleted): Postcheck !is_empty.
> (copy constructor): Test is_empty before is_deleted.
> ---
> gcc/hash-table.h | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/hash-table.h b/gcc/hash-table.h
> index 1d3166504c38e..e37625dc315bf 100644
> --- a/gcc/hash-table.h
> +++ b/gcc/hash-table.h
> @@ -534,6 +534,11 @@ private:
> void expand ();
> static bool is_deleted (value_type &v)
> {
> + /* Traits are supposed to avoid recognizing elements as both empty
> + and deleted, but to fail safe in case custom traits fail to do
> + that, make sure we never test for is_deleted without having
> + first ruled out is_empty. */
> + gcc_checking_assert (!Descriptor::is_empty (v));
> return Descriptor::is_deleted (v);
> }
>
> @@ -545,6 +550,11 @@ private:
> static void mark_deleted (value_type &v)
> {
> Descriptor::mark_deleted (v);
> + /* Traits are supposed to refuse to set elements as deleted if
> + those would be indistinguishable from empty, but to fail safe
> + in case custom traits fail to do that, check that the
> + just-deleted element does not look empty. */
> + gcc_checking_assert (!Descriptor::is_empty (v));
> }
>
> static void mark_empty (value_type &v)
> @@ -700,9 +710,11 @@ hash_table<Descriptor, Lazy, Allocator>::hash_table
> (const hash_table &h,
> for (size_t i = 0; i < size; ++i)
> {
> value_type &entry = h.m_entries[i];
> - if (is_deleted (entry))
> + if (is_empty (entry))
> + continue;
> + else if (is_deleted (entry))
> mark_deleted (nentries[i]);
> - else if (!is_empty (entry))
> + else
> new ((void*) (nentries + i)) value_type (entry);
> }
> m_entries = nentries;
>
> --
> Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
> Free Software Activist GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts. Ask me about <https://stallmansupport.org>
