On Thu, 2021-07-22 at 22:40 +0530, Ankur Saini wrote: > AIM FOR TODAY: > > - Add custom edge info to the eedges created for dynamically > discovered calls > - Add the custom events to be showing in diagnostics > - update call_event and return_event to also work for the cases where > there is no underlying superedge representing the call > > --- > PROGRESS : > > - I created "dynamic_call_info_t" subclass reprsenting custom info on > the edge representing the dynamically discovered calls > > - I overloaded it's "add_events_to_path ()" function to add call and > return event to checkers path > > - Now call_event and return_event subclasses mostly make use of the > underlying interprocedural superedge representing the call to work > properly. To tackle this problem, I used the same method I used for > callstring patch earlier working with src and dest supernodes instead > of superedge ) > > - The call_event subclass (and same applies to return_event subclass > also) now have 2 additional pointers to source and destination > supernodes representing the call in absense of a superedge. > > - I have also tweeked a few more things to make it work, I think the > best way to show them all is to attach a patch ( it should be > attached with this mail ) for just the changes I did today for better > understanding on what exactly have I changed since last update. ( > this patch would be squashed in previous one before the final review > ).
It's much easier to understand via the patch :) > > - After all the changes done, now the analyzer emmits the following > error message for the test program ( godbolt link > https://godbolt.org/z/Td8n4c9a6 <https://godbolt.org/z/Td8n4c9a6> ), > which I think now emmits all the events it was missing before. > > ``` > test.c: In function ‘fun’: > test.c:6:9: warning: double-‘free’ of ‘int_ptr’ [CWE-415] [- > Wanalyzer-double-free] > 6 | free(int_ptr); > | ^~~~~~~~~~~~~ > ‘double_call’: events 1-3 > | > | 16 | void double_call() > | | ^~~~~~~~~~~ > | | | > | | (1) entry to ‘double_call’ > | 17 | { > | 18 | int *int_ptr = (int*)malloc(sizeof(int)); > | | ~~~~~~~~~~~~~~~~~~~ > | | | > | | (2) allocated here > | 19 | void (*fun_ptr)(int *) = &fun; > | 20 | (*fun_ptr)(int_ptr); > | | ~~~~~~~~~~~~~~~~~~~ > | | | > | | (3) calling ‘fun’ from ‘double_call’ > | > +--> ‘fun’: events 4-5 > | > | 4 | void fun(int *int_ptr) > | | ^~~ > | | | > | | (4) entry to ‘fun’ > | 5 | { > | 6 | free(int_ptr); > | | ~~~~~~~~~~~~~ > | | | > | | (5) first ‘free’ here > | > <------+ > | > ‘double_call’: events 6-7 > | > | 20 | (*fun_ptr)(int_ptr); > | | ~^~~~~~~~~~~~~~~~~~ > | | | > | | (6) returning to ‘double_call’ from ‘fun’ > | 21 | (*fun_ptr)(int_ptr); > | | ~~~~~~~~~~~~~~~~~~~ > | | | > | | (7) calling ‘fun’ from ‘double_call’ > | > +--> ‘fun’: events 8-9 > | > | 4 | void fun(int *int_ptr) > | | ^~~ > | | | > | | (8) entry to ‘fun’ > | 5 | { > | 6 | free(int_ptr); > | | ~~~~~~~~~~~~~ > | | | > | | (9) second ‘free’ here; first ‘free’ was > at (5) > | > ``` Looks great. > > --- > STATUS AT THE END OF THE DAY :- > > - Add custom edge info to the eedges created for dynamically > discovered calls (done ) > - Add the custom events to be showing in diagnostics (done) > - update call_event and return_event to also work for the cases where > there is no underlying superedge representing the call (done) > > --- > Question / doubt :- > > - In "case EK_RETURN_EDGE” of > "diagnostic_manager::prune_for_sm_diagnostic ()” function. > > File:{source_dir}/gcc/analyzer/diagnostic-manager.cc > 2105: log ("event %i:" > 2106: " recording critical state for %qs at > return" > 2107: " from %qE in caller to %qE in callee", > 2108: idx, sval_desc.m_buffer, callee_var, > callee_var); > > shouldn’t it be > > 2107: " from %qE in caller to %qE in callee", > 2108: idx, sval_desc.m_buffer, caller_var, > callee_var); Good catch: I think it should be the latter version. (posting it as a unified diff would make it easier for me to read, but maybe your email client makes that hard?) > > and get value of caller_var before ? will they always be same ? IIRC this is for if you have something like: int * foo () { int *p = malloc (sizeof (int)); set_int (p); return p; } void set_int (int *q) { *q = 42; } We want to complain about the malloc result being unchecked, and thus a possible deref of NULL at "*q = 42;" The "critical state" is that "q" is unchecked in set_int, but as we walk backwards through the events we see that it was passed to set_int as "p", and that in "foo" it is "p" that is unchecked. Here the callee_var is "q" (in set_int) and the caller_var is "p" (in foo). This affects the wording of the events (see the "precision of wording" hooks in pending-diagnostic.h). > --- > Patch representing changes done today :- The patch looks reasonable; does it preserve the wording of the various events? You've made a lot of progress in handling jumps through dynamically- discovered function pointers. I think you should now look at simple cases of virtual function calls (building on your work so far). Hope this is helpful Dave
