On Sun, Mar 28, 2021 at 8:03 PM David Malcolm <dmalc...@redhat.com> wrote:

> On Sun, 2021-03-28 at 18:06 +0530, Saloni Garg wrote:
> > Hi, I have tried the following examples with the fanalyzer option in
> > g++.
> >
> > 1 (a)
> > void myFunction()
> > {
> >     char *p =new char;
> > }
> > int main()
> > {
> >    func();
> >    return 0;
> > }
>
> BTW, are you familiar with Compiler Explorer (godbolt.org)?  It's very
> handy for testing small snippets of code on different compilers and
> different compiler versions.  Though I don't know how long the URLs are
> good for (in terms of how long code is cached for)
>
> Fixing up the name of the called function to "func":
>   https://godbolt.org/z/TnM6n4xGc
> I get the leak report, as per RFE 94355.  This warning looks correct,
> in that p does indeed leak.
>
> Hi, thanks for the effort, sorry for the typo. I now know about the
godbolt.org and it is certainly useful.

> I should mention that the analyzer has some special-casing for "main",
> in that the user might not care about one-time leaks that occur within
> "main", or something only called directly by it; this doesn't seem to
> be the case here.  If I remove the implementation to main, the analyzer
> still correctly complains about the leak:
>   https://godbolt.org/z/zhK4vW6G8
>
> That's something new. I also didn't know that. I believe we can shift our
minimal example to just func() and remove main().

> <source>: In function 'void func()':
> <source>:4:1: warning: leak of 'p' [CWE-401] [-Wanalyzer-malloc-leak]
>     4 | }
>       | ^
>   'void func()': events 1-2
>     |
>     |    3 |     char *p =new char;
>     |      |                  ^~~~
>     |      |                  |
>     |      |                  (1) allocated here
>     |    4 | }
>     |      | ~
>     |      | |
>     |      | (2) 'p' leaks here; was allocated at (1)
>     |
>
>
> > 1(b)
> > void myFunction()
> > {
> >     try {
> >          char *p = new char;
> >          throw p;
> >     }
> >     catch(...) {
> >     }
> > }
> > int main()
> > {
> >    myFunction();
> >    return 0;
> > }
> > In 1(a), there is no exception handling. When I ran `cc1plus`, a
> > memory
> > leak was reported as shown in bug #94355.
> > In 1(b), there is a use of exception handling. When I ran cc1plus`,
> > no
> > memory leaks were detected. I believe there should be one. Can you
> > please
> > confirm from your side as well?
>
> I too am seeing no diagnostics on 1(b).
>
Thanks for confirming.

>
> > As you said all the calls to try, catch and
> > throw got converted to _cxa prefixed functions.
>
> -fdump-ipa-analyzer=stderr shows the _cxa-prefixed functions:
>   https://godbolt.org/z/YMa9dE6aM
>
> > I am trying to find the
> > places where the corresponding checks can be placed for the analysis
> > of
> > exception handling in gimple IR.
>
> Have a look at exploded_node::on_stmt in engine.cc; in particular, see
> the GIMPLE_CALL case in the switch statement.  Most of the the
> analyzer's "knowledge" of the behaviors of specific functions is here,
> or called from here.
>
> The simpler cases are handled in the call to
>   m_region_model->on_call_pre
> for functions which merely update state, which are implemented in
> region-model-impl-calls.cc
>
> Cases involving state machines (e.g. allocation) are handled in the:
>   sm.on_stmt
> call torwards the bottom of the function.
>
> But exception-handling is a special case, in that it affects control
> flow.  The closest thing to compare it to currently within the analyzer
> is setjmp/longjmp, so it's worth stepping through how that is handled.
> In particular, the real implementation of longjmp involves directly
> updating the program counter, registers and stack, potentially popping
> multiple stack frames.  This is similar to what throwing an exception
> does.
>
> So I'd recommend looking at the analyzer's implementation of
> setjmp/longjmp, the custom classes that I added to handle them, and
> stepping through how exploded_node::on_stmt handles setjmp and longjmp
> calls, and what the resulting exploded_graph looks like (-fdump-
> analyzer-exploded-graph and -fdump-analyzer-supergraph), in that
> special-cased edges have to be created that weren't in the original
> CFGs or callgraph (for the interprocedural case).
>
> I think an implementation of exception-handling would look somewhat
> similar.
>
> Thanks, for all the references to the code. I am new to GCC, so apologies
if I am a bit slow in understanding. I am trying to run and go through all
the references that you gave me.

> > Please, let me know your thoughts on this.
>
> Looks like you're making a great start.
>
Thanks for the feedback.  In parallel, can I start working on the Gsoc
proposal as well? I hope, we can get suggestions from the gcc community as
well once the things are written properly in a document.

- Saloni

>
> Hope this is helpful
> Dave
>
>
> > On Fri, Mar 26, 2021 at 12:48 AM David Malcolm <dmalc...@redhat.com>
> > wrote:
> >
> > > On Thu, 2021-03-25 at 14:52 +0530, Saloni Garg via Gcc wrote:
> > > > Hi all,
> > > > I am an undergraduate student in AMU, Aligarh. I am interested in
> > > > the
> > > > project* `Extend the static analysis pass`. *I have followed
> > > > this(
> > > > https://gcc.gnu.org/pipermail/gcc/2021-March/234941.html) and
> > > > been
> > > > able to
> > > > successfully build and successfully ran and pass the test suite
> > > > for C
> > > > and
> > > > C++.
> > > >
> > > > I found this sub-project `C++ support (new/delete checking,
> > > > exceptions,
> > > > etc)` interesting and may be the conservative code for this can
> > > > be
> > > > made
> > > > along the lines of malloc/free implementation in C. I found here(
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355) that some
> > > > part of
> > > > it
> > > > has already been implemented . I would like to expand it further
> > > > and
> > > > learn
> > > > about it, maybe start with writing some test cases, please let me
> > > > know.
> > > >
> > > > Further, I am inclined on this(
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97111). Let me know
> > > > if
> > > > it is
> > > > still available.
> > > >
> > > > Looking forward to hearing from you guys.
> > > > Thanks,
> > > > Saloni Garg
> > >
> > > Hi!
> > >
> > > I'm the author/maintainer of the static analysis pass, and would be
> > > the
> > > mentor for any GSoC project(s) involving it.
> > >
> > > I've already implemented most of the new/delete checking in GCC 11;
> > > the
> > > big missing component there is exception-handling.
> > >
> > > Implementing exception-handling in the analyzer could make a good
> > > GSoC
> > > project: it's non-trivial, but hopefully doable in one summer.  I
> > > see
> > > you've already seen bug 97111, and there are some links in that bug
> > > to
> > > resources.  Given that the analyzer runs on the gimple-ssa
> > > representation, by the time it sees the code, much of the
> > > exception-
> > > handling has already been translated into calls to various __cxa_-
> > > prefixed functions in the C++ runtime, so part of the work would
> > > involve "teaching" the analyzer about those functions.  One way to
> > > make
> > > a start on this would be to create a collection of trivial C++
> > > examples
> > > that use exceptions, and then look at analyzer dumps to see what IR
> > > is
> > > being "seen" by the analyzer for the various constructs.   (I
> > > actually
> > > started this a long time ago and have a very crude barely-working
> > > prototype, but it was just the start, and I've forgotten almost all
> > > of
> > > it...)
> > >
> > > Hope this is helpful
> > > Dave
> > >
> > >
> > >
>
>
>

Reply via email to