> On 07-Jul-2021, at 4:16 AM, David Malcolm <dmalc...@redhat.com> wrote:
>
> On Sat, 2021-07-03 at 20:07 +0530, Ankur Saini wrote:
>> AIM for today :
>>
>> - update the call_stack to track something else other than supergraph
>> edges
>>
>> —
>>
>> PROGRESS :
>>
>> - After some brainstorming about tracking the callstack, I think one
>> better way to track the call stack is to keep track of source and
>> destination of the call instead of supperedges representing them.
>>
>> - so I branched again and updated the call-string to now capture a pair
>> of supernodes ( one representing callee and other representing caller
>> ), like this I was not only able to easily port the entire code to
>> adapt it without much difficulty, but now can also push calls to
>> functions that doesn’t possess a superedge.
>>
>> - changes done can be seen on the "
>> refs/users/arsenic/heads/call_string_update “ branch. ( currently this
>> branch doesn’t contain other changes I have done till now, as I wanted
>> to test the new call-string representation exclusively and make sure it
>> doesn’t affect the functionality of the base analyser )
>
> I'm not an expert at git, so it took me a while to figure out how to
> access your branch.
>
> It's easier for me if you can also use "git format-patch" to generate a
> patch and "git send-email" to send it to this list (and me, please), so
> that the patch content is going to the list.
>
> The approach in the patch seems reasonable.
>
> I think you may have a memory leak, though: you're changing call_string
> from:
> auto_vec<const return_superedge *> m_return_edges;
> to:
> auto_vec<const std::pair<const supernode *,const supernode *>*> m_supernodes;
>
> and the std:pairs are being dynamically allocated on the heap.
> Ownership gets transferred by call_string::operator=, but if I'm
> reading the patch correctly never get deleted. This is OK for
> prototyping, but will need fixing before the code can be merged.
>
> It's probably simplest to get rid of the indirection and allocation in
> m_supernodes and have the std::pair be stored by value, rather than by
> pointer, i.e.:
> auto_vec<std::pair<const supernode *, const supernode *> > m_supernodes;
>
> Does that work? (or is there a problem I'm not thinking of).
yes, I noticed that while creating, was thinking to empty the vector and delete
the all the memory in the destructor of the call-string ( or make them unique
pointers and let them destroy themselves ) but looks like storing the values of
the pairs would make more sense.
>
> If that's a problem, I think you might be able to get away with
> dropping the "first" from the pair, and simply storing the supernode to
> return to; I think the only places that "first" gets used are in dumps
> and in validation. But "first" is probably helpful for debugging, so
> given that you've got it working with that field, better to keep it.
yes, I see that too, but my idea is to keep it as is for now ( maybe it might
turn out to be helpful in future ). I will change it back if it proves to be
useless and we get time at the end.
>
> Hope this is helpful
> Dave
>
>>
>> - now hopefully all that is left for tomorrow is to update the analyzer
>> to finally see, call and return from the function aclled via the
>> function pointer.
>>
>> STATUS AT THE END OF THE DAY :-
>>
>> - update the call_stack to track something else other than supergraph
>> edges ( done )
>>
>> Thank you
>> - Ankur
———
> On 07-Jul-2021, at 4:20 AM, David Malcolm <dmalc...@redhat.com> wrote:
>
> On Tue, 2021-07-06 at 18:46 -0400, David Malcolm wrote:
>> On Sat, 2021-07-03 at 20:07 +0530, Ankur Saini wrote:
>>> AIM for today :
>>>
>>> - update the call_stack to track something else other than
>>> supergraph
>>> edges
>>>
>>> —
>>>
>>> PROGRESS :
>>>
>>> - After some brainstorming about tracking the callstack, I think
>>> one
>>> better way to track the call stack is to keep track of source and
>>> destination of the call instead of supperedges representing them.
>>>
>>> - so I branched again and updated the call-string to now capture a
>>> pair
>>> of supernodes ( one representing callee and other representing
>>> caller
>>> ), like this I was not only able to easily port the entire code to
>>> adapt it without much difficulty, but now can also push calls to
>>> functions that doesn’t possess a superedge.
>>>
>>> - changes done can be seen on the "
>>> refs/users/arsenic/heads/call_string_update “ branch. ( currently
>>> this
>>> branch doesn’t contain other changes I have done till now, as I
>>> wanted
>>> to test the new call-string representation exclusively and make
>>> sure it
>>> doesn’t affect the functionality of the base analyser )
>>
>> I'm not an expert at git, so it took me a while to figure out how to
>> access your branch.
>>
>> It's easier for me if you can also use "git format-patch" to generate
>> a
>> patch and "git send-email" to send it to this list (and me, please),
>> so
>> that the patch content is going to the list.
>>
>> The approach in the patch seems reasonable.
>>
>> I think you may have a memory leak, though: you're changing
>> call_string
>> from:
>> auto_vec<const return_superedge *> m_return_edges;
>> to:
>> auto_vec<const std::pair<const supernode *,const supernode *>*>
>> m_supernodes;
>
> One other, unrelated idea that just occurred to me: those lines get
> very long, so maybe introduce a typedef e.g.
> typedef std::pair<const supernode *,const supernode *> element_t;
> so that you can refer to the pairs as call_string::element_t, and just
> element_t when you're in call_string scope, and just have a:
>
> auto_vec<element_t> m_supernodes;
>
> or
>
> auto_vec<element_t> m_elements;
>
> within the call_string, if that makes sense. Does that simplify
> things?
Yes, this is a nice idea, I will update the call-stack with next update to the
analyzer, or should I update it and send a patch to the mailing list with this
call_string changes for review first and then work on the other changes ?
also regarding the ChangeLog
how exactly does one update the changelog, does it get updated with the commit
messages or do one have to update it manually ?
I found and read this doc
(https://gcc.gnu.org/codingconventions.html#ChangeLogs
<https://gcc.gnu.org/codingconventions.html#ChangeLogs>) about the same which
says that “ ChangeLog entries are part of git commit messages and are
automatically put into a corresponding ChangeLog file “. But when I pushed the
commits which looks properly formatted to me, I was not able to see the changes
reflecting back in gcc/analyzer/changelog .
>
> Dave
Thanks
- Ankur Saini
