Hi,
Pinging that regression fix.
Is everything OK for trunk ?

Thanks,
Benjamin

On Thu, Jun 22, 2023 at 9:57 PM <priour...@gmail.com> wrote:

   From: benjamin priour <priour...@gmail.com>

   Resend with proper subject line ...

   Hi,

   Below is the fix to regression bug
   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110198
   Was bootstrapped and regtested successfully on x86_64-linux-gnu
   Considering mishap from last patch, I would appreciate if you could
   also regtest it, to be sure :)

   Thanks,
   Benjamin.


   g++.dg/analyzer/pr100244.C was failing after a patch of PR109439.
   The reason was a spurious preemptive return of get_store_value upon
   out-of-bounds read that
   was preventing further checks. Now instead, a boolean value
   check_poisoned goes to false when
   a OOB is detected, and is later on given to get_or_create_initial_value.

   gcc/analyzer/ChangeLog:

            * region-model-manager.cc
   (region_model_manager::get_or_create_initial_value): Take an
                    optional boolean value to bypass poisoning checks
            * region-model-manager.h: Update declaration of the above
   function.
            * region-model.cc (region_model::get_store_value): No longer
                    returns on OOB, but rather gives a boolean to
   get_or_create_initial_value.
            (region_model::check_region_access): Update docstring.
            (region_model::check_region_for_write): Update docstring.

   Signed-off-by: benjamin priour <priour...@gmail.com>
   ---
     gcc/analyzer/region-model-manager.cc |  5 +++--
     gcc/analyzer/region-model-manager.h  |  3 ++-
     gcc/analyzer/region-model.cc         | 15 ++++++++-------
     3 files changed, 13 insertions(+), 10 deletions(-)

   diff --git a/gcc/analyzer/region-model-manager.cc
   b/gcc/analyzer/region-model-manager.cc
   index 1453acf7bc9..4f11ef4bd29 100644
   --- a/gcc/analyzer/region-model-manager.cc
   +++ b/gcc/analyzer/region-model-manager.cc
   @@ -293,9 +293,10 @@ region_model_manager::create_unique_svalue
   (tree type)
        necessary.  */

     const svalue *
   -region_model_manager::get_or_create_initial_value (const region *reg)
   +region_model_manager::get_or_create_initial_value (const region *reg,
   +                                                  bool check_poisoned)
     {
   -  if (!reg->can_have_initial_svalue_p ())
   +  if (!reg->can_have_initial_svalue_p () && check_poisoned)
         return get_or_create_poisoned_svalue (POISON_KIND_UNINIT,
                                              reg->get_type ());

   diff --git a/gcc/analyzer/region-model-manager.h
   b/gcc/analyzer/region-model-manager.h
   index 3340c3ebd1e..ff5333bf07c 100644
   --- a/gcc/analyzer/region-model-manager.h
   +++ b/gcc/analyzer/region-model-manager.h
   @@ -49,7 +49,8 @@ public:
                                                 tree type);
       const svalue *get_or_create_poisoned_svalue (enum poison_kind kind,
                                                   tree type);
   -  const svalue *get_or_create_initial_value (const region *reg);
   +  const svalue *get_or_create_initial_value (const region *reg,
   +                                            bool check_poisoned =
   true);
       const svalue *get_ptr_svalue (tree ptr_type, const region *pointee);
       const svalue *get_or_create_unaryop (tree type, enum tree_code op,
                                           const svalue *arg);
   diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
   index 6bc60f89f3d..187013a37cc 100644
   --- a/gcc/analyzer/region-model.cc
   +++ b/gcc/analyzer/region-model.cc
   @@ -2373,8 +2373,9 @@ region_model::get_store_value (const region *reg,
       if (reg->empty_p ())
         return m_mgr->get_or_create_unknown_svalue (reg->get_type ());

   +  bool check_poisoned = true;
       if (check_region_for_read (reg, ctxt))
   -    return m_mgr->get_or_create_unknown_svalue(reg->get_type());
   +    check_poisoned = false;

       /* Special-case: handle var_decls in the constant pool.  */
       if (const decl_region *decl_reg = reg->dyn_cast_decl_region ())
   @@ -2427,7 +2428,7 @@ region_model::get_store_value (const region *reg,
           == RK_GLOBALS)
         return get_initial_value_for_global (reg);

   -  return m_mgr->get_or_create_initial_value (reg);
   +  return m_mgr->get_or_create_initial_value (reg, check_poisoned);
     }

     /* Return false if REG does not exist, true if it may do.
   @@ -2790,7 +2791,7 @@ region_model::get_string_size (const region
   *reg) const

     /* If CTXT is non-NULL, use it to warn about any problems
   accessing REG,
        using DIR to determine if this access is a read or write.
   -   Return TRUE if an UNKNOWN_SVALUE needs be created.
   +   Return TRUE if an OOB access was detected.
        If SVAL_HINT is non-NULL, use it as a hint in diagnostics
        about the value that would be written to REG.  */

   @@ -2804,10 +2805,10 @@ region_model::check_region_access (const
   region *reg,
       if (!ctxt)
         return false;

   -  bool need_unknown_sval = false;
   +  bool oob_access_detected = false;
       check_region_for_taint (reg, dir, ctxt);
       if (!check_region_bounds (reg, dir, sval_hint, ctxt))
   -    need_unknown_sval = true;
   +    oob_access_detected = true;

       switch (dir)
         {
   @@ -2820,7 +2821,7 @@ region_model::check_region_access (const
   region *reg,
           check_for_writable_region (reg, ctxt);
           break;
         }
   -  return need_unknown_sval;
   +  return oob_access_detected;
     }

     /* If CTXT is non-NULL, use it to warn about any problems writing
   to REG.  */
   @@ -2834,7 +2835,7 @@ region_model::check_region_for_write (const
   region *dest_reg,
     }

     /* If CTXT is non-NULL, use it to warn about any problems reading
   from REG.
   -  Returns TRUE if an unknown svalue needs be created.  */
   +  Returns TRUE if an OOB read was detected.  */

     bool
     region_model::check_region_for_read (const region *src_reg,
-- 2.34.1

Reply via email to