On Thu, 2026-08-13 at 19:19 -0400, David Malcolm wrote: > On Sat, 2026-08-01 at 13:14 +0100, Egas Ribeiro wrote: > > Bootstrapped/regtested on x86_64-pc-linux-gnu. > > > > I cc'd Jakub and Jason so I could get input on my test coverage and > > on > > the implementation of evaluate_dyncast. Everything is done on > > GIMPLE > > other than some calls in impl_call_post, so its effectively just an > > implementation of static evaluation of dynamic_cast but relying on > > the > > analyzer region/store model to get some information. > > > > As I mentioned to Jakub on IRC, I found that on some test cases > > cxx_eval_dynamic_cast wasn't agreeing with the runtime > > dynamic_cast, > > so > > I also opened a BZ for that: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126510 > > > > My implementation with the analyzer matches the runtime for that > > case. > > > > There was no particular way for me to reuse the methods that > > cxx_eval_dynamic_cast uses because not only are they in the > > frontend > > (so > > I can't access them) but also they consider more c++ semantics than > > I > > have to for what I am doing, so my solution was to reimplement the > > rules > > as per the c++ standard. All things considered the implementation > > isn't > > too extensive. Would like some feedback from David whether this is > > a > > good approach here. > > Sorry about the delay in reviewing this. > > Could you please split this patch up into two parts: > > (a) an initial part that just has these bits: > > gcc/analyzer/ChangeLog: > > * analyzer.cc (is_fndecl_in_toplevel_namespace_p): New > function, > factored out of... > (is_std_function_p): ...here. Call it. > (is_cxxabi_function_p): New function. > * common.h (is_cxxabi_function_p): New decl. > * known-function-manager.cc > (known_function_manager::get_match): > Also match functions declared in namespace __cxxabiv1. > > (b) everything else > > I think (a) is pretty-much ready to go. I'm attempting to review > (b), > trying to compare it to cxx_eval_dynamic_cast_fn, but I'm not very > familiar with C++'s constexpr code, sorry. > > I'll continue to look at (b).
Some review of the (b) parts of the patch inline below: > > -- >8 -- > > > > The analyzer doesn't currently understand dynamic_cast calls. This > > means > > that the returned object from a call is always opaque, and thus we > > miss > > a few classes of bugs such as dereferencing the result of a failed > > cast. > > > > This patch implements a new known_function for dynamic_cast, and > > sets > > the result of the callbased on the rules of [expr.dynamic.cast]. > > > > The implementation effectively evaluates the results of a > > dynamic_cast > > cast call statically, similarly to what cxx_eval_dynamic_cast does Nit: is this cxx_eval_dynamic_cast_fn? (with a "_fn" suffix) > > in > > the frontend, but without using frontend methods and trees. > > > > It tries to derive the target object from the argumments passed to > > __dynamic_cast calls by traversing BINFOs based on the rules of > > [expr.dynamic.cast] and then calculating the relative offsets of > > the > > resulting object from the BINFOs we found. > > > > get_vtable_from_obj was also added so we can reuse the logic for > > deriving the vtable from objects added with virtual function > > support. > > > > gcc/analyzer/ChangeLog: > > > > * analyzer.cc (is_fndecl_in_toplevel_namespace_p): New > > function, > > factored out of... > > (is_std_function_p): ...here. Call it. > > (is_cxxabi_function_p): New function. > > * common.h (is_cxxabi_function_p): New decl. > > * kf-lang-cp.cc: Include "cgraph.h" and "ipa-utils.h". > > (struct dyncast_subobject): New struct. > > (get_type_from_tinfo_arg): New function. > > (lookup_binfo_at_same_offset): New function. > > (lookup_subobject_matches): New function. > > (evaluate_dyncast): New function. > > (class kf_dynamic_cast): New class. > > (register_known_functions_lang_cp): Register > > "__dynamic_cast". > > * known-function-manager.cc > > (known_function_manager::get_match): > > Also match functions declared in namespace __cxxabiv1. > > * region-model.cc (region_model::get_vtable_from_obj): New > > function, > > factored out of... > > (region_model::get_fndecl_for_virtual_call): ...here. > > Call > > it. > > Update comment. > > * region-model.h (region_model::get_vtable_from_obj): New > > decl. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/analyzer/dyncast-1.C: Rewrite to cover > > [expr.dynamic.cast]/9.1 and /9.2 over public, non-virtual > > inheritance. > > * g++.dg/analyzer/dyncast-2.C: New test. > > * g++.dg/analyzer/dyncast-3.C: New test. > > * g++.dg/analyzer/dyncast-4.C: New test. > > * g++.dg/analyzer/dyncast-5.C: New test. > > > > Signed-off-by: Egas Ribeiro <[email protected]> > > --- > > gcc/analyzer/analyzer.cc | 26 +- > > gcc/analyzer/common.h | 1 + > > gcc/analyzer/kf-lang-cp.cc | 286 > > ++++++++++++++++++++++ > > gcc/analyzer/known-function-manager.cc | 10 +- > > gcc/analyzer/region-model.cc | 79 ++++-- > > gcc/analyzer/region-model.h | 4 + > > gcc/testsuite/g++.dg/analyzer/dyncast-1.C | 78 ++++-- > > gcc/testsuite/g++.dg/analyzer/dyncast-2.C | 57 +++++ > > gcc/testsuite/g++.dg/analyzer/dyncast-3.C | 50 ++++ > > gcc/testsuite/g++.dg/analyzer/dyncast-4.C | 83 +++++++ > > gcc/testsuite/g++.dg/analyzer/dyncast-5.C | 30 +++ > > 11 files changed, 657 insertions(+), 47 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/analyzer/dyncast-2.C > > create mode 100644 gcc/testsuite/g++.dg/analyzer/dyncast-3.C > > create mode 100644 gcc/testsuite/g++.dg/analyzer/dyncast-4.C > > create mode 100644 gcc/testsuite/g++.dg/analyzer/dyncast-5.C > > [...snip...] > > --- a/gcc/analyzer/kf-lang-cp.cc > > +++ b/gcc/analyzer/kf-lang-cp.cc > > @@ -19,6 +19,8 @@ along with GCC; see the file COPYING3. If not > > see > > <http://www.gnu.org/licenses/>. */ > > > > #include "analyzer/common.h" > > +#include "cgraph.h" > > +#include "ipa-utils.h" > > > > #include "diagnostic.h" > > > > @@ -63,6 +65,287 @@ namespace ana { > > > > /* Implementations of specific functions. */ > > > > +/* Handler for __dynamic_cast. */ > > + > > +/* A candidate TYPE subobject found on one inheritance path. */ > > + > > +struct dyncast_subobject > > +{ > > + tree binfo; /* the BINFO that represents our subobject. > > + NULL_TREE if failed. */ > > + bool accessible; /* Every edge from the root was public. */ > > +}; > > + > > +/* Recover the class type from a type_info argument of > > __dynamic_cast, expected > > + to be &_ZTIxxx. The C++ FE sets TREE_TYPE on the tinfo decl's > > DECL_NAME > > + identifier. __dynamic_cast is callable directly, so a runtime > > tinfo pointer > > + (an SSA name) can exist. Return NULL_TREE on any shape > > mismatch. */ > > + > > +static tree > > +get_type_from_tinfo_arg (tree arg) > > +{ > > + if (!arg || TREE_CODE (arg) != ADDR_EXPR) > > + return NULL_TREE; > > + tree tinfo_decl = TREE_OPERAND (arg, 0); > > + if (!DECL_P (tinfo_decl) || !DECL_NAME (tinfo_decl)) > > + return NULL_TREE; > > + tree type = TREE_TYPE (DECL_NAME (tinfo_decl)); > > + if (!type || !RECORD_OR_UNION_TYPE_P (type)) > > + return NULL_TREE; > > + return TYPE_MAIN_VARIANT (type); > > +} Does this correspond to something in the C++ frontend, or in ipa- devirt.cc? > > + > > +/* Find the sub-BINFO of BINFO that has type TARGET_TYPE and sits > > at > > the same > > + address as BINFO itself (i.e. at relative offset 0) by > > descending > > through > > + every base at that same (absolute) offset. */ > > + > > +static tree > > +lookup_binfo_at_same_offset (tree binfo, tree target_type) > > +{ > > + if (types_same_for_odr (BINFO_TYPE (binfo), target_type)) > > + return binfo; > > + > > + tree offset = BINFO_OFFSET (binfo); > > + tree base_binfo; > > + for (unsigned i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); > > i++) > > + if (tree_int_cst_equal (BINFO_OFFSET (base_binfo), offset)) > > + if (tree found = lookup_binfo_at_same_offset (base_binfo, > > target_type)) > > + return found; > > + return NULL_TREE; > > +} Likewise: does this correspond to something in the C++ frontend, or in ipa-devirt.cc? > > + > > +/* Look recursively for a BINFO that matches our DST_TYPE. This > > method might > > + find multiple matches. If none is found, matches won't be > > changed. > > + > > + Even if one path is private, it is still ambiguous according to > > the > > + definition, so that case still counts as having multiple > > matches. That > > + means we ignore access specifiers when searching bases. > > + > > + Morally virtual matches of the same type under different > > virtual > > ancestors > > + are distinct subobjects, i.e: > > + > > + class B0 {}; > > + class V1 : B0 {}; class V2 : B0 {}; > > + class B2 : virtual V1 {}; class B4 : virtual V2 {}; > > + class MD : B2, B3 {}; > > + > > + Here, each B0 is a different subobject, so we must account for > > this case > > + when checking virtual inheritance. We compare the BINFO_OFFSET > > of all the > > + BINFOs that match (the offset is relative to our most-derived > > object) to > > + decide if they belong to the same suboject. Note that if it is > > at the same > > + BINFO_OFFSET and has the same TREE_TYPE, it must necessarily be > > the same > > + subobject. */ > > + > > +static void > > +lookup_subobject_matches (const_tree target_type, const > > dyncast_subobject match, > > + auto_vec<dyncast_subobject> &matches) > > +{ > > + if (types_same_for_odr (BINFO_TYPE (match.binfo), target_type)) > > + { > > + /* Check if we had already found this particular subobject. > > */ > > + tree match_offset = BINFO_OFFSET (match.binfo); > > + for (auto &subobject : matches) > > + if (tree_int_cst_equal (BINFO_OFFSET (subobject.binfo), > > match_offset)) > > + { > > + subobject.accessible |= match.accessible; > > + return; /* We are adding the same subobject, so skip > > it. */ > > + } > > + matches.safe_push (match); > > + return; > > + } > > + tree parent_binfo = match.binfo; > > + tree base_binfo; > > + for (unsigned i = 0; BINFO_BASE_ITERATE (parent_binfo, i, > > base_binfo); i++) > > + { > > + dyncast_subobject child = match; > > + child.binfo = base_binfo; > > + /* If BINFO_BASE_ACCESSES is not present, public access is > > implied. */ > > + child.accessible > > + &= !BINFO_BASE_ACCESSES (parent_binfo) > > + || BINFO_BASE_ACCESS (parent_binfo, i) == > > access_public_node; > > + /* Check if the next binfo might be our DST_TYPE binfo > > recursively. */ > > + lookup_subobject_matches (target_type, child, matches); > > + } > > +} > > + > > +/* We implement the runtime check rules as per > > [expr.dynamic.cast]9. > > + As a general overview, those rules state: > > + [expr.dynamic.cast]/9.1: Does SRC_OBJ point to a public base > > subobject of > > + a DST_TYPE object? And is there only one DST_TYPE object > > derived from > > + SRC_OBJ? > > + We expect the hierarchy to be something like: > > + SRC_TYPE -> ... -> DST_TYPE -> ... -> MD_TYPE. > > + > > + [expr.dynamic.cast]/9.2: Otherwise, does SRC_OBJ point to a > > public base > > + subobject of a MDTYPE object? And is DST_TYPE an > > unambiguous > > and public > > + base of MDTYPE? > > + We expect the hierarchy to be something like: > > + MD_TYPE -> ... -> DST_TYPE > > + > > + [expr.dynamic.cast]/9.3: Otherwise, the runtime check fails. > > */ > > + > > +static dyncast_subobject > > +evaluate_dyncast (tree dst_type, tree md_binfo, tree src_binfo) > > +{ > > + dyncast_subobject no_base_match = {.binfo = NULL_TREE, > > .accessible > > = false}; > > + > > + /* Start by assuming the path will be public. */ > > + auto_vec<dyncast_subobject> dst_matches; > > + dyncast_subobject md_subobject = {.binfo = md_binfo, .accessible > > = > > true}; Am I right in thinking that this is using the "designated initializer" syntax here? I believe this was added in C++20, but we're stuck using C++14. Would it make sense to add a default ctor to the struct? (not sure) > > + lookup_subobject_matches (dst_type, md_subobject, dst_matches); > > + > > + /* Per [expr.dynamic.cast]/9.1: > > + Only one object of DST_TYPE can be derived from this SRC_OBJ > > and > > + The path from DST -> SRC must be public. */ > > + tree src_offset = BINFO_OFFSET (src_binfo); > > + tree src_type = BINFO_TYPE (src_binfo); > > + > > + auto_vec<dyncast_subobject> clause1_matches; > > + for (const auto &dst_subobj : dst_matches) > > + { > > + auto_vec<dyncast_subobject> src_matches; > > + dyncast_subobject from_dst = {dst_subobj.binfo, /* > > accessible > > = */ true}; Likewise here. > > + lookup_subobject_matches (src_type, from_dst, src_matches); > > + /* Only keep matches that derive from this src subobject. > > */ > > + for (const auto &src_subobj : src_matches) > > + if (tree_int_cst_equal (BINFO_OFFSET (src_subobj.binfo), > > src_offset)) > > + /* Keep dst BINFO but save whether SRC is a public base > > of > > DST. */ > > + clause1_matches.safe_push ({dst_subobj.binfo, > > src_subobj.accessible}); > > + } > > + if (clause1_matches.length () == 1 && > > clause1_matches[0].accessible) > > + return clause1_matches[0]; /* No ambiguity, only one public > > match. */ > > + > > + /* No match or the match we found was private. Try clause 2. > > */ > > + > > + /* Otherwise, per [expr.dynamic.cast]/9.2: > > + Require a public path from MD_OBJ -> SRC_OBJ and > > + Require that DST_TYPE is an unambiguous and public base of > > MD_TYPE. */ > > + if (dst_matches.length () == 1 && dst_matches[0].accessible) > > + { > > + auto_vec<dyncast_subobject> src_matches; > > + lookup_subobject_matches (src_type, md_subobject, > > src_matches); > > + /* Find any public path from MD_OBJ -> SRC_OBJ. */ > > + for (const auto &src_subobj : src_matches) > > + if (src_subobj.accessible) > > + return dst_matches[0]; /* Found a public match. */ > > + } > > + return no_base_match; /* No match or the match we found was > > private. */ > > +} > > + > > +class kf_dynamic_cast : public known_function > > +{ > > +public: > > + bool matches_call_types_p (const call_details &cd) const final > > override > > + { > > + /* A call will look something like: > > + Derived *d; > > + d = __dynamic_cast ((Base*) b, &_ZTI1Base, &_ZTI1Derived, > > 8); */ > > + return (cd.num_args () == 4 && POINTER_TYPE_P (cd.get_arg_type > > (0)) > > + && POINTER_TYPE_P (cd.get_arg_type (1)) > > + && POINTER_TYPE_P (cd.get_arg_type (2)) > > + && INTEGRAL_TYPE_P (cd.get_arg_type (3))); > > + } > > + void impl_call_post (const call_details &cd) const final > > override > > + { > > + region_model *model = cd.get_model (); > > + region_model_manager *mgr = cd.get_manager (); > > + > > + cd.set_any_lhs_with_defaults (); > > + > > + tree dst_ptr_type = cd.get_lhs_type (); > > + if (!dst_ptr_type) > > + return; > > + > > + /* Recover the class types from the tinfo args. */ > > + tree src_type = get_type_from_tinfo_arg (cd.get_arg_tree (1)); > > + tree dst_type = get_type_from_tinfo_arg (cd.get_arg_tree (2)); > > + if (!src_type || !dst_type) > > + return; > > + > > + /* Read the vptr binding of the object; VPTR_OFF selects the > > + sub-vtable within the vtable decl, so it identifies which > > subobject's > > + vptr we read. */ > > + tree src_obj = cd.get_arg_tree (0); > > + unsigned HOST_WIDE_INT vptr_off; > > + tree vtable > > + = model->get_vtable_from_obj (src_obj, src_type, mgr, > > nullptr, > > &vptr_off); > > + /* The class the vtable belongs to is the dynamic (most- > > derived) > > type of > > + the object. VTABLE is whatever decl the vptr slot happened > > to point at, > > + so check it really is a vtable. */ > > + if (!vtable || !VAR_P (vtable) || !DECL_VIRTUAL_P (vtable)) > > + return; > > + tree mdtype = DECL_CONTEXT (vtable); > > + if (!mdtype || !RECORD_OR_UNION_TYPE_P (mdtype)) > > + return; > > + tree md_binfo = TYPE_BINFO (mdtype); > > + if (!md_binfo) > > + return; > > + > > + /* Given the value stored to SRC_OBJ's vtpr field (&_ZTV* + > > offset), find > > + which subobject of this hierarchy would have this value > > written into its > > + vptr. */ > > + tree vtable_binfo > > + = subbinfo_with_vtable_at_offset (md_binfo, vptr_off, > > vtable); > > + if (!vtable_binfo) > > + return; > > + /* With a shared primary-base vtable the owning binfo may be > > an > > enclosing > > + type. Consider: > > + class A {}; > > + class B {}; > > + class C : B {}; > > + class D : A, C {}; > > + Here the vptr value stored in the B-subobject's slot is > > owned > > by the C > > + binfo (C's sub-vtable group), and a lookup with B's vptr > > value returns > > + the C binfo, not B's (BINFO_VTABLE is only set on the > > owner, > > cf. > > + ipa-devirt.cc:61). In this case, the src subobject sits on > > its primary > > + chain (relative offset 0), which > > lookup_binfo_at_same_offset > > finds. */ > > + tree src_binfo = lookup_binfo_at_same_offset (vtable_binfo, > > src_type); > > + if (!src_binfo) > > + return; > > + > > + dyncast_subobject dst_match > > + = evaluate_dyncast (dst_type, md_binfo, src_binfo); > > + > > + if (!dst_match.binfo) > > + { /* [expr.dynamic.cast]/9.3: Otherwise, the runtime check > > failed. */ > > + cd.maybe_set_lhs (mgr->get_or_create_null_ptr > > (dst_ptr_type)); > > + return; > > + } > > + > > + /* Build a pointer to the dst subobject. Work in byte offsets > > relative to > > + SRC_REG's base region; we never need a region for the > > mdtype > > object > > + itself, only its start offset, recovered from where the src > > subobject > > + sits within MDTYPE. */ > > + const region *src_reg = cd.deref_ptr_arg (0); > > + region_offset off = src_reg->get_offset (mgr); > > + if (!off.concrete_p ()) > > + return; /* Bail, leave lhs conjured. */ > > + byte_offset_t src_obj_start; > > + if (!off.get_concrete_byte_offset (&src_obj_start)) > > + return; > > + > > + HOST_WIDE_INT src_off_in_md = tree_to_shwi (BINFO_OFFSET > > (src_binfo)); > > + HOST_WIDE_INT dst_off_in_md = tree_to_shwi (BINFO_OFFSET > > (dst_match.binfo)); > > + HOST_WIDE_INT md_start_in_base = src_obj_start.to_shwi () - > > src_off_in_md; > > + if (md_start_in_base < 0) > > + return; /* Layout disagreement between the store and the > > binfo > > data; > > + bail rather than build a negative-offset region. > > */ > > + HOST_WIDE_INT dst_off_in_base = md_start_in_base + > > dst_off_in_md; > > + > > + /* BASE_REG is the outermost region, not necessarily the > > mdtype > > + object (it might sit at a nonzero offset inside BASE_REG, > > e.g. as an > > + array element or a member subobject). The store binds > > values > > by byte > > + ranges within a base region, so a concrete offset_region > > aliases the > > + FE's field-path accesses to the same bytes. */ > > + const region *base_reg = off.get_base_region (); > > + const svalue *dst_off_sval > > + = mgr->get_or_create_int_cst (size_type_node, > > dst_off_in_base); > > + const region *dst_reg > > + = mgr->get_offset_region (base_reg, dst_type, dst_off_sval); > > + cd.maybe_set_lhs (mgr->get_ptr_svalue (dst_ptr_type, > > dst_reg)); > > + } > > +}; > > + > > /* Handler for "operator new" and "operator new []". */ > > > > class kf_operator_new : public known_function > > @@ -371,6 +654,9 @@ register_known_functions_lang_cp > > (known_function_manager &kfm) > > kfm.add ("__cxa_end_catch", std::make_unique<kf_cxa_end_catch> > > ()); > > kfm.add ("__cxa_call_unexpected", > > std::make_unique<kf_cxa_call_unexpected> ()); > > + > > + /* TODO: [add Itanium C++ ABI mention of __dynamic_cast here] > > */ > > + kfm.add ("__dynamic_cast", std::make_unique<kf_dynamic_cast> > > ()); > > } > > > > } // namespace ana > > diff --git a/gcc/analyzer/known-function-manager.cc > > b/gcc/analyzer/known-function-manager.cc > > index fcc9a618d15..679fe1a7015 100644 > > --- a/gcc/analyzer/known-function-manager.cc > > +++ b/gcc/analyzer/known-function-manager.cc > > @@ -120,9 +120,13 @@ known_function_manager::get_match (tree > > fndecl, > > const call_details &cd) const > > return nullptr; > > } > > > > - if (DECL_CONTEXT (fndecl) > > - && TREE_CODE (DECL_CONTEXT (fndecl)) != > > TRANSLATION_UNIT_DECL) > > - return nullptr; > > + /* Only match functions declared at global scope, or within > > namespace > > + __cxxabiv1 (e.g. __dynamic_cast). */ > > + if (!is_cxxabi_function_p (fndecl)) > > + if (DECL_CONTEXT (fndecl) > > + && TREE_CODE (DECL_CONTEXT (fndecl)) != > > TRANSLATION_UNIT_DECL) > > + return nullptr; > > + > > if (tree identifier = DECL_NAME (fndecl)) > > if (const known_function *candidate = get_by_identifier > > (identifier)) > > if (candidate->matches_call_types_p (cd)) > > diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- > > model.cc > > index 8addf1d9a07..caa166497df 100644 > > --- a/gcc/analyzer/region-model.cc > > +++ b/gcc/analyzer/region-model.cc > > @@ -6490,25 +6490,23 @@ region_model::can_merge_with_p (const > > region_model &other_model, > > return true; > > } > > > > -/* Attempt to get the fndecl for a virtual call via OBJ_TYPE_REF, > > or > > - NULL_TREE if it can't be resolved. > > +/* Recover the vtable OBJ's vptr (for OBJ_TYPE) actually points > > to, > > plus the > > + byte offset into it, so callers can recover the most-derived > > type > > from the > > + vtable's DECL_CONTEXT and BINFO. > > > > - Reads the value bound to the object's vptr field > > (OBJ_TYPE_REF_OBJECT's > > - vfield). > > - If that value has the form "&vtable_decl + constant" (a > > region_svalue for a > > - _ZTV* decl plus a byte offset), recover the vtable decl and > > offset and use > > - gimple_get_virt_method_for_vtable, together with > > OBJ_TYPE_REF_TOKEN, to look > > - up the concrete fndecl in the vtable's initializer. > > - > > - Relies on the store having bound the vptr field to the _ZTV* > > instance, so no > > - separate modeling of the object's dynamic type is needed. */ > > + Only recognizes the shape we model for a vptr store, > > "vptr_field > > = > > + &vtable_decl + offset", i.e. a POINTER_PLUS_EXPR binop_svalue > > of > > a > > + region_svalue for the _ZTV* decl and a constant offset. The > > offset is > > + nonzero when this vptr slot belongs to a non-primary base's own > > sub-vtable > > + group within the same decl; we return it via OUT for callers > > that > > need to > > + index into the vtable (e.g. > > gimple_get_virt_method_for_vtable). > > */ > > > > tree > > -region_model::get_fndecl_for_virtual_call (const_tree > > obj_type_ref, > > - region_model_context > > *ctxt) > > +region_model::get_vtable_from_obj (tree obj, tree obj_type, > > + region_model_manager *mgr, > > + region_model_context *ctxt, > > + unsigned HOST_WIDE_INT *out) > > const > > { > > - tree obj = OBJ_TYPE_REF_OBJECT (obj_type_ref); > > - tree obj_type = obj_type_ref_class (obj_type_ref); > > if (!obj_type) > > return NULL_TREE; > > tree vfield = TYPE_VFIELD (obj_type); > > @@ -6517,7 +6515,7 @@ region_model::get_fndecl_for_virtual_call > > (const_tree obj_type_ref, > > > > const svalue *obj_sval = get_rvalue (obj, ctxt); > > const region *obj_reg = deref_rvalue (obj_sval, obj, ctxt); > > - const region *vptr_reg = m_mgr->get_field_region (obj_reg, > > vfield); > > + const region *vptr_reg = mgr->get_field_region (obj_reg, > > vfield); > > > > const svalue *vptr_sval = get_store_value (vptr_reg, ctxt); > > while (const svalue *cast = vptr_sval->maybe_undo_cast ()) > > @@ -6527,20 +6525,55 @@ region_model::get_fndecl_for_virtual_call > > (const_tree obj_type_ref, > > if (!b || b->get_op () != POINTER_PLUS_EXPR) > > return NULL_TREE; > > > > - vptr_sval = b->get_arg0 (); > > - const svalue *offset_sval = b->get_arg1 (); > > - > > - tree offset_const = offset_sval->maybe_get_constant (); > > - if (!offset_const || TREE_CODE (offset_const) != INTEGER_CST) > > - return NULL_TREE; > > - unsigned HOST_WIDE_INT offset = tree_to_uhwi (offset_const); > > + if (out) > > + { > > + const svalue *offset_sval = b->get_arg1 (); > > + tree offset_const = offset_sval->maybe_get_constant (); > > + if (!offset_const || TREE_CODE (offset_const) != > > INTEGER_CST) > > + return NULL_TREE; > > + *out = tree_to_uhwi (offset_const); > > + } I notice that the failure path here is now only checked/triggered when "out" is non-null. Is that intentional? > > > > + vptr_sval = b->get_arg0 (); > > const region_svalue *vptr = vptr_sval->dyn_cast_region_svalue > > (); > > /* Give up if we have a conjured vptr. */ > > if (!vptr) > > return NULL_TREE; > > > > tree vtable = vptr->get_pointee ()->maybe_get_decl (); > > + return vtable; > > +} > > + > > +/* Attempt to get the fndecl for a virtual call via OBJ_TYPE_REF, > > or > > + NULL_TREE if it can't be resolved. > > + > > + A virtual call's callee is a GIMPLE OBJ_TYPE_REF: > > + OBJ_TYPE_REF(EXPR; (TYPE)OBJECT->TOKEN) > > + EXPR is the function pointer actually loaded and called; > > OBJECT, > > TYPE and > > + TOKEN are devirtualization metadata, not needed to perform the > > call itself. > > + OBJECT is the "this" pointer, TYPE its static type, TOKEN the > > vtable slot > > + index. We ignore EXPR and instead resolve TOKEN against > > OBJECT's > > modeled > > + dynamic type rather than its static TYPE. > > + > > + Reads the value bound to the object's vptr field > > (OBJ_TYPE_REF_OBJECT's > > + vfield). > > + If that value has the form "&vtable_decl + constant" (a > > region_svalue for a > > + _ZTV* decl plus a byte offset), recover the vtable decl and > > offset and use > > + gimple_get_virt_method_for_vtable, together with > > OBJ_TYPE_REF_TOKEN, to look > > + up the concrete fndecl in the vtable's initializer. > > + > > + Relies on the store having bound the vptr field to the _ZTV* > > instance, so no > > + separate modeling of the object's dynamic type is needed. */ > > + > > +tree > > +region_model::get_fndecl_for_virtual_call (const_tree > > obj_type_ref, > > + region_model_context > > *ctxt) > > +{ > > + tree obj = OBJ_TYPE_REF_OBJECT (obj_type_ref); > > + tree obj_type = obj_type_ref_class (obj_type_ref); > > + > > + unsigned HOST_WIDE_INT offset; > > + tree vtable = get_vtable_from_obj (obj, obj_type, m_mgr, ctxt, > > &offset); Rather than passing &offset to get_vtable_from_obj and discarding the result, could we simply pass nullptr? > > if (!vtable) > > return NULL_TREE; > > [...snip...] Thanks Dave
