Re: daily report on extending static analyzer project [GSoC]

[Ankur Saini via Gcc]

AIM for today : 

- get "state_purge_per_ssa_name::process_point () “ to  go from the “return" 
supernode to the “call” supernode.
- fix bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546 
 in the process 
- test and observe the effect of changes done so far on vfunc calls

—

PROGRESS :

- In order to go from “return” supernode to “call” supernode, I used the fact 
that return supernode will have GIMPLE call statement which when passed to 
“get_supernode_for_stmt ()”  returns pointer to “call” supernode. 

now that part of the function look something like this 

File: {SCR_DIR}/gcc/analyzer/state-purge.cc 

347:/* Add any intraprocedually edge for a call.  */
348:if (snode->m_returning_call)
349:  {
350:cgraph_edge *cedge
351:  = supergraph_call_edge (snode->m_fun,
352:  snode->m_returning_call);
353:if(!cedge)
354:{
355:supernode* callernode = map.get_sg 
().get_supernode_for_stmt(snode->m_returning_call);
356:gcc_assert (callernode);
357:add_to_worklist
358:  (function_point::after_supernode (callernode),
359:   worklist, logger);
360:}
361:else
362:{
363:gcc_assert (cedge);
364:superedge *sedge
365:  = map.get_sg 
().get_intraprocedural_edge_for_call (cedge);
366:gcc_assert (sedge);
367:add_to_worklist
368:  (function_point::after_supernode 
(sedge->m_src),
369:   worklist, logger);
370:}
371:  }

- now the patch also fixes bug #100546 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546 
) and doesn’t give out a 
false report about dereferencing a null pointer which will never happen.

- now I tested it with vfuncs to see what happens in that case, the results 
were as expected where the analyzer detects the call to virtual function and 
split call and returning supernodes, but did not understand which function to 
calll, making nodes after it unreachable. 

- Now If we somehow able to update the regional model to understand which 
function is called ( or may be called ) then the analyzer can now easily call 
and analyze that virtual function call.


STATUS AT THE END OF THE DAY :- 

- get "state_purge_per_ssa_name::process_point () “ to  go from the “return" 
supernode to the “call” supernode. ( done )
- fix bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546 
 in the process. ( done )
- test and observe the effect of changes done so far on vfunc calls ( done )

—
P.S. 
regarding the patch I sent to mailing list yesterday. I found it, apparently 
the mail was detected as a "spam mail” by my system and was redirected  to my 
spam inbox. 
Btw I am also attaching that patch file with this mail for the records.




Thank you 
- Ankur

Re: daily report on extending static analyzer project [GSoC]

[David Malcolm via Gcc]

On Sat, 2021-07-10 at 21:27 +0530, Ankur Saini wrote:
> AIM for today : 
> 
> - update the call_string to store a vector of pair of supernode*
> instead of pointer to it 
> - create a typdef to give a meaning full name to these " pair of
> supernode* “
> - send the patch list to gcc-patches mailing list
> - add the example program I created to the GCC tests
> 
> —
> 
> PROGRESS :
> 
> - I successfully changed the entire call string representation again to
> keep track of "auto_vec m_elements;” from "auto_vec std::pair*> m_supernodes;” 
> 
> - while doing so, one hurdle I found was to change "hashval_t hash ()
> const;”, function of which I quite didn’t understood properly, but
> looking at source, it looked like it just needed some value ( integer
> or pointer ) to add to ‘hstate’ and ' m_elements’ was neither a pointer
> nor an integer so I instead added pointer to callee supernode (
> “second” of the m_elements ) to the ‘hstate’ for now. 
> 
> - for the callstring patch, I created a patch file ( using git format-
> patch ) and sent it to patches mailing list (via git send email ) and
> CCed my mentor.
> Although I received a positive reply from the command log (git send
> email) saying the mail was sent , I didn’t received that mail ( being
> subscribed to the patches list ) regarding the same ( I sent that just
> before sending this mail ).
> The mail should be sent from arse...@sourceware.org  arse...@sourceware.org> 

Thanks.

I see the patch email in the list archives here:
  https://gcc.gnu.org/pipermail/gcc-patches/2021-July/574888.html
but for some reason it's not showing up in my inbox.  I'm not sure why;
I recently got migrated to a new email server and my filters are
currently a mess so it could be a problem at my end; sorry if that's
the case.

Given that neither you nor I seem to have received the patch I wonder
if anyone else received it?

Given that, I'm going to reply to that patch email inline here (by
copying and pasting it from the archive):

> [PATCH 1/2] analyzer: refactor callstring to work with pairs of supernodes 
> [GSoC]
> 
> 2021-07-3  Ankur Saini  

There are some formatting rules that we follow with ChangeLog entries.
We have a script:

  ./contrib/gcc-changelog/git_check_commit.py --help

that you can run to check the formatting.

> * gcc/analyzer/call-string.cc: refactor callstring to work with pair 
> of supernodes instead of super superedges
> * gcc/analyzer/call-string.h: make callstring work with pairs of 
> supernodes
> * gcc/analyzer/program-point.cc: refactor program point to work with 
> new call-string format

The "gcc/analyzer" directory has its own ChangeLog file, and filenames
should be expressed relative to it, so these entries should read
something like:

gcc/analyzer/ChangeLog:
* call-string.cc: ...etc
* call-string.h: ...etc
* program-point.cc: ...etc

The entries should be sentences (i.e. initial capital letter and
trailing full-stop).

They should be line-wrapped (I do it at 74 columns), giving this:

gcc/analyzer/ChangeLog:
* call-string.cc: Refactor callstring to work with pair of
supernodes instead of superedges.
* call-string.h: Make callstring work with pairs of supernodes.
* program-point.cc: Refactor program point to work with new
call-string format.

Your text editor might have a mode for working with ChangeLog files.

[...snip...]

> @@ -152,22 +152,40 @@ call_string::push_call (const supergraph &sg,
>gcc_assert (call_sedge);
>const return_superedge *return_sedge = call_sedge->get_edge_for_return 
> (sg);
>gcc_assert (return_sedge);
> -  m_return_edges.safe_push (return_sedge);
> +  const std::pair *e = new 
> (std::pair)

We don't want lines wider than 80 columns unless it can't be helped. 
Does your text editor have a feature to warn you about overlong lines?

Changing from:
  std::pair
to:
  element_t
should make it much easier to avoid overlong lines.

[...snip...]

> diff --git a/gcc/analyzer/call-string.h b/gcc/analyzer/call-string.h
> index 7721571ed60..0134d185b90 100644
> --- a/gcc/analyzer/call-string.h
> +++ b/gcc/analyzer/call-string.h

[...snip...]

> +
> +  void push_call (const supernode *src, 
> +const supernode *dest);

There's some odd indentation here.  Does your text editor have option
to
(a) show visible whitespace (distinguish between tabs vs spaces)
(b) enforce a coding standard?

If your editor supports it, it's easy to comply with a project's coding
standards, otherwise it can be a pain.

[...snip...]

>  private:
> -  auto_vec m_return_edges;
> +  //auto_vec m_return_edges;
> +  auto_vec*> 
> m_supernodes;
>  };

Commenting out lines is OK during prototyping.  Obviously as the patch
gets closer to be being ready we want to simply delete them instead.

[...]

> >From 95572742f1aaad1975aa35a663e8b26e671d4323 Mon Sep 17 00:00:00 2001
> From: Ankur Saini 
> Date: Sat, 10 Jul 2021 19:28:49 +0530
> Subject: [PATCH 2/2]

Re: daily report on extending static analyzer project [GSoC]

[David Malcolm via Gcc]

On Sun, 2021-07-11 at 22:31 +0530, Ankur Saini wrote:
> AIM for today : 
> 
> - get "state_purge_per_ssa_name::process_point () “ to  go from the
> “return" supernode to the “call” supernode.
> - fix bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546 <
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546> in the process 
> - test and observe the effect of changes done so far on vfunc calls
> 
> —
> 
> PROGRESS :
> 
> - In order to go from “return” supernode to “call” supernode, I used
> the fact that return supernode will have GIMPLE call statement which
> when passed to “get_supernode_for_stmt ()”  returns pointer to “call”
> supernode. 
> 
> now that part of the function look something like this 
> 
> File: {SCR_DIR}/gcc/analyzer/state-purge.cc 
> 
> 347:    /* Add any intraprocedually edge for a call.  */
> 348:    if (snode->m_returning_call)
> 349:  {
> 350:cgraph_edge *cedge
> 351:  = supergraph_call_edge (snode->m_fun,
> 352:  snode-
> >m_returning_call);
> 353:if(!cedge)
> 354:{
> 355:supernode* callernode = map.get_sg
> ().get_supernode_for_stmt(snode->m_returning_call);
> 356:gcc_assert (callernode);
> 357:add_to_worklist
> 358:  (function_point::after_supernode
> (callernode),
> 359:   worklist, logger);
> 360:}
> 361:else
> 362:{
> 363:gcc_assert (cedge);
> 364:superedge *sedge
> 365:  = map.get_sg
> ().get_intraprocedural_edge_for_call (cedge);
> 366:gcc_assert (sedge);
> 367:add_to_worklist
> 368:  (function_point::after_supernode
> (sedge->m_src),
> 369:   worklist, logger);
> 370:}
> 371:  }
> 
> - now the patch also fixes bug #100546 (
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546 <
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546>) and doesn’t give
> out a false report about dereferencing a null pointer which will never
> happen.

Excellent.  You should add a testcase for that bug to the test suite.

> 
> - now I tested it with vfuncs to see what happens in that case, the
> results were as expected where the analyzer detects the call to virtual
> function and split call and returning supernodes, but did not
> understand which function to calll, making nodes after it unreachable. 
> 
> - Now If we somehow able to update the regional model to understand
> which function is called ( or may be called ) then the analyzer can now
> easily call and analyze that virtual function call.

I had some ideas about how to do this here:
  https://gcc.gnu.org/pipermail/gcc/2021-April/235335.html
which might work for simple cases where we have a code path through a
ctor of a known subclass

...but I haven't looked in detail at ipa-devirt.c yet, so I might be
wrong.

> 
> 
> STATUS AT THE END OF THE DAY :- 
> 
> - get "state_purge_per_ssa_name::process_point () “ to  go from the
> “return" supernode to the “call” supernode. ( done )
> - fix bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546 <  
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100546> in the process. (
> done )
> - test and observe the effect of changes done so far on vfunc calls (
> done )
> 
> —
> P.S. 
> regarding the patch I sent to mailing list yesterday. I found it,
> apparently the mail was detected as a "spam mail” by my system and was
> redirected  to my spam inbox. 

Strange.  I didn't see it in my spam folder.

> Btw I am also attaching that patch file with this mail for the records.

Thanks.  I've replied to it in another email here:
  https://gcc.gnu.org/pipermail/gcc/2021-July/236726.html

Dave

gcc-12-20210711 is now available

[GCC Administrator via Gcc]

Snapshot gcc-12-20210711 is now available on
  https://gcc.gnu.org/pub/gcc/snapshots/12-20210711/
and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 12 git branch
with the following options: git://gcc.gnu.org/git/gcc.git branch master 
revision 269256f33c51222167ad461f775d5468bb5ecaf5

You'll find:

 gcc-12-20210711.tar.xz   Complete GCC

  SHA256=aac38f1f2cdb3972a6b636b4954dcfe9621af41595e39f2234907b9c1b0d7066
  SHA1=e068bfbbdc1c9e1eb63fc436b314edd5d5c29afd

Diffs from 12-20210704 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-12
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.