On Thu, Apr 8, 2021 at 8:19 AM David Malcolm <dmalc...@redhat.com> wrote:

> On Wed, 2021-04-07 at 01:59 +0530, Saloni Garg wrote:
> > Hi, apologies for the delayed reply. I was having some college
> > commitments.
> > On Wed, Mar 31, 2021 at 11:22 PM David Malcolm <dmalc...@redhat.com>
> > wrote:
> >
> > > On Wed, 2021-03-31 at 21:41 +0530, Saloni Garg wrote:
> > > > On Tue, Mar 30, 2021 at 6:42 PM David Malcolm <
> > > > dmalc...@redhat.com>
> > > > wrote:
> > > >
> > > > > On Tue, 2021-03-30 at 16:06 +0530, Saloni Garg wrote:
> > > > > > On Sun, Mar 28, 2021 at 8:03 PM David Malcolm <
> > > > > > dmalc...@redhat.com>
> > > > > > wrote:
> > >
>
> [...snip...]
>
> > > > > Please do work on the formal proposal - without it we can't
> > > > > accept
> > > > > you
> > > > > as a GSoC student.  The window for submitting proposals opened
> > > > > yesterday, and I believe it closes in a couple of weeks, and
> > > > > you
> > > > > need
> > > > > to do that, so any experimentation you do now should really
> > > > > just be
> > > > > in
> > > > > support of writing a good proposal.  It would be a shame to not
> > > > > have a
> > > > > good prospective candidate because they didn't allow enough
> > > > > time to
> > > > > do
> > > > > the proper GSoC paperwork before the deadline.
> > > > >
> > > > Thanks for understanding. Here is an initial draft (
> > > >
> > > >
> > >
> > >
> https://docs.google.com/document/d/1inkkU5B55s_FOWRzUuf38s7XEet65kc0dW3yFn0yh1M/edit?usp=sharing
> > > > )
> > > > of my GSoC proposal. I am yet to fill in the missing blocks.
> > > > Please, let me know if you have any comments on the document
> > > > itself.
> > >
> > > Caveat: I'm not familiar with the expected format of such
> > > documents.
> > >
> > > Looks like a good first draft.
> >
> > I don't think there is any such expected format(I checked some
> > previous
> > years accepted proposals). I believe if we clearly write the expected
> > goal
> > and the tentative approach to reach it, that would be okay for the
> > proposal.
>
> Looking at:
>   https://gcc.gnu.org/wiki/SummerOfCode#Application
> we don't have a specific format to be followed.
>
> That said, I saw this
>   https://google.github.io/gsocguides/student/writing-a-proposal
> which seems to have useful advice to prospective GSoC students.  In
> particular, the "Elements of a Quality Proposal" lists various things
> that in your current draft are missing, and which would strengthen your
> proposal.  So I'd recommend that you (and other prospective GSoC
> candidates) have a look at that.
>
Added some new sections. Tried to explain them as well. There are some
things I am not clear about, so explicitly mentioned them and will add the
relevant explanations and present them in the later reports. Please let me
know if this sounds good to you and provide feedback as well.

>
> [...snip...]
>
> > > - in Example 2, maybe spell out why it's a leak - when does the
> > > allocated buffer stop being referenceable?
> > >
> > Explained. Please let me know if you feel it has any loose ends.
>
> I think the leak is actually at line 8, when the "catch" clause ends.
> Isn't the buffer passed into the exception-state of the thread, and
> becomes live during the "catch" clause, but stops being live at the end
> of the catch clause?
>
Yes, I understood your point and have added the same in the document as
well. The memory becomes unreferenced after the catch block ends so that is
basically the point where this leak started.

>
> >
> > > - you have a simple example of a false negative; is it possible to
> > > give
> > > a simple example of a false positive?  (I think "new" is meant to
> > > raise
> > > an exception if it fails, so a diagnostics about a NULL-deref on
> > > unchecked new might be a false positive.  I'm not sure)
> > >
> > I tried the following example:
> >
> > #include <new>
> > #include <iostream>
> > using namespace std;
> > int *p;
> > int* alloc() {
> >    return new int;
> > }
> > void free() {
> >    delete p;
> > }
>
> FWIW please don't create a top-level function called "free" that isn't
> the C stdlib's free, it's confusing!
>
Sorry, my bad renamed it to `myfree`.

>
> > int main()
> > {
> >    try {
> >      p = alloc();
> >      free();
> >    } catch(...) {
> >    }
> >    return 0;
> > }
> > Please, have a look here (https://godbolt.org/z/8WvoaP67n). I believe
> > it is
> > a false positive, I am not sure, please confirm.
>
> It looks like one to me.  I had a look at -fdump-analyzer-exploded-
> graph and the false positive seems to be associated with the edge with
> the EH flag (to the catch handler).
>
I have understood the exploded graph but not able to understand the `EH
flag` point you are making, so I will get back to you on this.

>
> [...snip...]
>
> >
> > > - "sample example programs": for "sample" did you mean to write
> > > "simple" here?
> > >
> > By sample examples, I meant the test cases that shall be used to
> > prove the
> > correctness of the patches during the course.
>
> Isn't "sample examples" a tautology, though?  (or, at least, my
> interpretation of "sample" here makes it so, I think).
>
yes, I guess I didn't frame that in words appropriately. But, yes I meant
the same as what you are saying.

>
> >
> > > - as well as understanding code, you'll need to understand data,
> > > specifically getting a feel for the kinds of control flow graphs
> > > that
> > > the analyzer is receiving as inputs i.e. what the analyzer "sees"
> > > when
> > > the user inputs various C++ language constructs; what
> > > interprocedural
> > > vs intraprocedural raise/try/catch situations look like, etc.
> > >
> > I am in the process to understand how the analyzer works. I believe I
> > have
> > just got a gist of the approach being used. The gimple graph has been
> > processed in a Worklist manner. We analyze each statement and add
> > predecessors/successors if there is some new information coming out
> > of the
> > analyzed statement.
>
> When we process a node in the worklist, we only ever add successor
> nodes, recording the next <point, state> pairs.
>
Yes. Right. Since this analysis is similar to dead variable analysis(Read
from the Internet) we only add successors.

>
> > For example, In the memory leak examples discussed
> > here, if at a statement I see a *new *expression, then I need to add
> > all
> > the nodes where this value can flow into, to find out a possible
> > *free
> > *expression
> > to block the possible memory leak.
>
> Right.  Or the first place where the value is no longer live, so that
> we can report a leak there.
>
Yes.

>
> > In the case of interprocedural, I need to incorporate the effects of
> > any
> > function calls in between and see if it changes my analysis result or
> > not.
>
> Currently the analyzer has a "brute force" approach to interprocedural
> analysis, and attempts to simulate the calls and returns in a fairly
> direct way.  It's crude (and has exponential growth), but is reasonably
> simple conceptually (or at least I think so).  The analyzer implements
> setjmp/longjmp in a similar way, and exception-handling could be based
> on that code.
>
Going through that already and your comments at the start of every data
structure defined are really helpful.

>
> There *is* some placeholder code to attempt to summarize the effects of
> a whole function, but it doesn't work well and is disabled by default
> (I hope to fix this in GCC 12).

>
> > Please, let me know if I am wrong somewhere.
> > -Saloni
>
> Hope this is helpful
> Dave
>
> Yes. It was.

- Saloni

Reply via email to