On Mon, 2026-06-15 at 20:26 +0100, Egas Ribeiro wrote:
> On Mon Jun 15, 2026 at 5:10 PM WEST, David Malcolm wrote:
> > On Sun, 2026-06-14 at 19:45 +0100, Egas Ribeiro wrote:
> > > The following test (exception-subclass-2.C) xfails
> >
> > When you say it "xfails", do you mean that it continues to fail, or
> > that it is now unexpectedly succeeding?
> >
> It continues to fail because there doesn't seem to be any output
> coming
> from the line of the caller function, so there is no output to catch
> from the testsuite.
That's weird. Is there any output coming from the
__analyzer_dump_path?
>
> > > What is the intended behavior of this test?
> >
> > I'm not sure, but I think my idea was to verify that the analysis
> > does
> > in fact return from the call to __analyzer_inner.
> >
> > BTW, have a look at exploded_graph::build_initial_worklist and
> > toplevel_function_p. By default, the analyzer tries to explore
> > execution paths starting at every externally-callable function, so
> > explores execution paths starting at "test" (with the
> > interprocedurally
> > call into __analyzer_inner). It would also explore paths starting
> > at
> > __analyzer_inner, but toplevel_function_p specifically ignores
> > functions with names starting with "__analyzer_".
> >
>
> That makes sense. I did notice that __analyzer_inner was not evaluted
> other than when it was called.
>
> > > I'm not sure why the
> > > dg-message directive is on the call to __analyzer_inner,
> >
> > Prior to r16-264-g7a39e0ca0652ff84a31efa3c7d4c7a78d9bb95ae the
> > analyzer
> > was very confused by CFGs containing exception-handling, so my
> > guess is
> > that we weren't even reaching that line, and perhaps the dump-path
> > was
> > reported in the wrong place. But looking at it now, I'm not sure
> > what
> > I was thinking.
> >
> > > should I add a
> > > __analyzer_dump_path in the `catch (exception &exc)` path?
> >
> > Sounds good.
> >
> If we do this, the test will be basically mirroring
> exception-subclass-1.C:
> ```cpp
> #include "../../gcc.dg/analyzer/analyzer-decls.h"
>
> class exception
> {
> };
>
> class io_error : public exception
> {
> };
>
> int test ()
> {
> try {
> throw io_error();
> } catch (exception &exc) {
> __analyzer_dump_path (); // { dg-message "path" "PR
> analyzer/119697" }
> return -1;
> }
> __analyzer_dump_path (); // { dg-bogus "path" }
> return 0;
> }
> ```
> but only now we do the calls interprocedurally. if thats OK then I'll
> include the added __analyzer_dump_path in a new version of the patch,
> otherwise maybe the test is redundant.
I think the fact that it's interprocedural is probably enough for it to
be worth keeping the test.
>
> > >
> > >
> > >
> > > Separately from the question above, I found some typos in the
> > > internal
> > > docs for the analyzer:
> > > In the Region Model section, it seems in this section:
> > > ```
> > > Consider this example C code:
> > >
> > > @smallexample
> > > void *
> > > calls_malloc (size_t n)
> > > @{
> > > void *result = malloc (1024);
> > > return result; /* HERE */
> > > @}
> > >
> > > void test (size_t n)
> > > @{
> > > void *ptr = calls_malloc (n * 4);
> > > /* etc. */
> > > @}
> > > @end smallexample
> > > ```
> > >
> > > the docs write `void *result = malloc (1024);`, but the dumps
> > > after
> > > write:
> > > ```
> > > │ │ ╰─ frame: 'calls_malloc'@2
> > > │ │ ├─ result_4: &HEAP_ALLOCATED_REGION(27)
> > > │ │ ╰─ _5: &HEAP_ALLOCATED_REGION(27)
> > > │ ╰─ Dynamic Extents
> > > │ ╰─ HEAP_ALLOCATED_REGION(27): (INIT_VAL(n_2(D))*(size_t)4)
> > > ```
> > > which seems to assume we did `void *result = malloc (n);`
> > > instead.
> >
> > You're right, I think the doc needs patching so that the example
> > does a
> > "malloc (n)".
> >
> > Looks like in the dump you quoted you're seeing the exploration of
> > "calls_malloc" as called from "test", as opposed to being explored
> > directly (both test and calls_malloc will have toplevel_function_p
> > return true, so I'd expect the initial worklist to contain both
> > entrypoints).
> >
>
> Should I submit a patch later with the fixed docs or do you want to
> do
> that? I don't have commit access so someone would have to push it for
> me.
Please submit a patch.
>
> > >
> > > Also, the dumps say `HEAP_ALLOCATED_REGION(27)` but later we
> > > write
> > > ```
> > > within test the whole cluster for result_4 is bound to a
> > > region_svalue pointing at
> > > HEAP_ALLOCATED_REGION(12)
> > > ```
> > > should be ` HEAP_ALLOCATED_REGION(27)` not `
> > > HEAP_ALLOCATED_REGION(12)`.
> >
> > The precise numbering of heap-allocated regions will change from
> > run to
> > run depending on what other code is being explored and in what
> > order.
> > Maybe the docs should say that?
> >
> Intuitively, I noticed that the numbering changed from run to run,
> but
> thought that the docs maybe should (or maybe should not if its
> documented) keep the same numbering for consistency given both parts
> of
> the docs refer to the same "thing".
In theory the numbering ought to be deterministic, so if the code and
options are identical, and the analyzer is "seeing" the same gimple,
the numbering ought to be the same. But minor changes could give you
different numberings.
Dave
>
> > Hope this all makes sense
> >
> > BTW, I'm working on a major rewrite of how store.{cc,h} works,
> > which
> > (as well as fixing a bunch of bugs) will greatly change the way
> > dumps
> > look - but I don't expect it to be ready to merge until late July
> > at
> > the earliest.
>
> Will keep this in mind.
>
> Thanks,
> Egas
>
