[Tree][Static Analyzer] Tree representing types and svalues type

2024-01-16 Thread Pierrick Philippe
Hi David, hi all,

I was playing along with APIs from the Static Analyzer and encountered a
segfault in gcc/tree.cc:5068 (i.e. in function build2 and failure is due
to a
gcc_assert call), after a call to
ana::region_model::get_representative_tree.

>From my debugging of the problem, I realized that the svalue which I
built with
the SA API as the following from an element_region (C sample):

    const ana::region *base = elm_reg->get_base_region();
    // base_ptr is the svalue passed to get_representative_tree
    const ana::svalue *base_ptr = base->get_ptr_svalue(
                                                                  
TYPE_POINTER_TO(base->get_type),
                                                                   base);

I realized that the SA resulting in the call to get_ptr_svalue has
NULL_TREE as
type (return by get_type on base_ptr).  And this is because TYPE_POINTER_TO
(base->get_type) is returning a NULL_TREE.  I found my way around by calling
build_pointer_type(base->get_type()) which is currently building the
expecting
type.

But from this, two questions raised in my mind:

1. Is it coherent for the ana::region_model_manager::get_ptr_svalue to
return a sval with a null type?

2. Can a tree view as a tree_type_common member have an uninitialized
pointer_to member?  I think, but definitely not sure) that the member is not
initialized here by some part of the compiler, because I think that the
inner
type of regions and svalues are build by the SA by using the TREE_TYPE macro
directly from the GIMPLE.  I ask this question to know if this is a bug
and I
should try to do a minimal example out of it, or if it is a intended
behavior
for any reason from the compiler.

Thank you for reading me,

Pierrick



[PATCH] libstdc++: hashtable: No need to update before begin node in _M_remove_bucket_begin

2024-01-16 Thread Huanghui Nie via Gcc
Hi.

When I implemented a hash table with reference to the C++ STL, I found that
when the hash table in the C++ STL deletes elements, if the first element
deleted is the begin element, the before begin node is repeatedly assigned.
This creates unnecessary performance overhead.


First, let’s see the code implementation:

In _M_remove_bucket_begin, _M_before_begin._M_nxt is assigned when
&_M_before_begin == _M_buckets[__bkt]. That also means
_M_buckets[__bkt]->_M_nxt is assigned under some conditions.

_M_remove_bucket_begin is called by _M_erase and _M_extract_node:

   1. Case _M_erase a range: _M_remove_bucket_begin is called in a for loop
   when __is_bucket_begin is true. And if __is_bucket_begin is true and
   &_M_before_begin == _M_buckets[__bkt], __prev_n must be &_M_before_begin.
   __prev_n->_M_nxt is always assigned in _M_erase. That means
   _M_before_begin._M_nxt is always assigned, if _M_remove_bucket_begin is
   called and &_M_before_begin == _M_buckets[__bkt]. So there’s no need to
   assign _M_before_begin._M_nxt in _M_remove_bucket_begin.
   2. Other cases: _M_remove_bucket_begin is called when __prev_n ==
   _M_buckets[__bkt]. And __prev_n->_M_nxt is always assigned in _M_erase and
   _M_before_begin. That means _M_buckets[__bkt]->_M_nxt is always assigned.
   So there's no need to assign _M_buckets[__bkt]->_M_nxt in
   _M_remove_bucket_begin.

In summary, there’s no need to check &_M_before_begin == _M_buckets[__bkt]
and assign _M_before_begin._M_nxt in _M_remove_bucket_begin.


Then let’s see the responsibility of each method:

The hash table in the C++ STL is composed of hash buckets and a node list.
The update of the node list is responsible for _M_erase and _M_extract_node
method. _M_remove_bucket_begin method only needs to update the hash
buckets. The update of _M_before_begin belongs to the update of the node
list. So _M_remove_bucket_begin doesn’t need to update _M_before_begin.


Existing tests listed below cover this change:

23_containers/unordered_set/allocator/copy.cc

23_containers/unordered_set/allocator/copy_assign.cc

23_containers/unordered_set/allocator/move.cc

23_containers/unordered_set/allocator/move_assign.cc

23_containers/unordered_set/allocator/swap.cc

23_containers/unordered_set/erase/1.cc

23_containers/unordered_set/erase/24061-set.cc

23_containers/unordered_set/modifiers/extract.cc

23_containers/unordered_set/operations/count.cc

23_containers/unordered_set/requirements/exception/basic.cc

23_containers/unordered_map/allocator/copy.cc

23_containers/unordered_map/allocator/copy_assign.cc

23_containers/unordered_map/allocator/move.cc

23_containers/unordered_map/allocator/move_assign.cc

23_containers/unordered_map/allocator/swap.cc

23_containers/unordered_map/erase/1.cc

23_containers/unordered_map/erase/24061-map.cc

23_containers/unordered_map/modifiers/extract.cc

23_containers/unordered_map/modifiers/move_assign.cc

23_containers/unordered_map/operations/count.cc

23_containers/unordered_map/requirements/exception/basic.cc


Regression tested on x86_64-pc-linux-gnu. Is it OK to commit?


---


diff --git a/libstdc++-v3/include/bits/hashtable.h
b/libstdc++-v3/include/bits/hashtable.h

index b48610036fa..6056639e663 100644

--- a/libstdc++-v3/include/bits/hashtable.h

+++ b/libstdc++-v3/include/bits/hashtable.h

@@ -872,13 +872,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION

if (!__next_n || __next_bkt != __bkt)

  {

// Bucket is now empty

-   // First update next bucket if any

+   // Update next bucket if any

if (__next_n)

  _M_buckets[__next_bkt] = _M_buckets[__bkt];



-   // Second update before begin node if necessary

-   if (&_M_before_begin == _M_buckets[__bkt])

- _M_before_begin._M_nxt = __next_n;

_M_buckets[__bkt] = nullptr;

  }

   }


no-need-to-update-before-begin-node.diff
Description: Binary data


Re: [PATCH] libstdc++: hashtable: No need to update before begin node in _M_remove_bucket_begin

2024-01-16 Thread Sam James via Gcc


Huanghui Nie  writes:

> Hi.

Please CC the libstdc++ LM for libstdc++ patches, per
https://gcc.gnu.org/onlinedocs/libstdc++/manual/appendix_contributing.html#list.patches.

> [...]