analyzer: New state machine should be C++ only

analyzer: New state machine should be C++ only

[Benjamin Priour]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

Hi David,


Lately I've been working on adding a new state machine to keep track of
ownership transfers

and misuses, e.g. to warn about use-after-move, partial or shallow
copy/move.

I'm trying to stay abstracted from heap allocated regions, and to rather
work with "resources",

so that the state machine could be easily further extended.

However, the whole concern of ownership is really C++-like, and most of the
checks would require

things unheard of in vanilla C, such as copy/move operators, ctors & dtors
...


Using those constructs, it is really doable to guess ownership of
resources, whereas without them it becomes

much more hazardous.

So, should we make this new sm -adroitly called sm-ownership- C++-only ?


Doing so would allow the sm to reuse code from under cp/*, thus it'd reduce
duplicating code and would

likely lead to less false positives in C++ -more precise function checks-,
though it would make any future C-support more tedious.

It's also going against the current flow of porting what's already done for
C to C++.


Best,

Benjamin.

Re: analyzer: New state machine should be C++ only

[David Malcolm]

(apologies for top-posting; I'm on vacation and don't have my usual email setup)

Sounds interesting, but I'm having difficulty imagining exactly what
you have in mind.

Can you post one or more concrete examples of buggy code that would be
caught by such a warning?

Why wouldn't it be caught by C++'s syntax-based ownership support?

Is there a PR in bugzilla for this idea?

Thanks
Dave

On Wed, Jul 12, 2023 at 9:24 AM Benjamin Priour <priour.be@gmail.com> wrote:
>
> Hi David,
>
>
> Lately I've been working on adding a new state machine to keep track of ownership transfers
>
> and misuses, e.g. to warn about use-after-move, partial or shallow copy/move.
>
> I'm trying to stay abstracted from heap allocated regions, and to rather work with "resources",
>
> so that the state machine could be easily further extended.
>
> However, the whole concern of ownership is really C++-like, and most of the checks would require
>
> things unheard of in vanilla C, such as copy/move operators, ctors & dtors ...
>
>
> Using those constructs, it is really doable to guess ownership of resources, whereas without them it becomes
>
> much more hazardous.
>
> So, should we make this new sm -adroitly called sm-ownership- C++-only ?
>
>
> Doing so would allow the sm to reuse code from under cp/*, thus it'd reduce duplicating code and would
>
> likely lead to less false positives in C++ -more precise function checks-, though it would make any future C-support more tedious.
>
> It's also going against the current flow of porting what's already done for C to C++.
>
>
> Best,
>
> Benjamin.

Re: analyzer: New state machine should be C++ only

[Benjamin Priour]

[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]

Hi,
On 13/07/2023 10:48, David Malcolm wrote:

(apologies for top-posting; I'm on vacation and don't have my usual email setup)

Sounds interesting, but I'm having difficulty imagining exactly what
you have in mind.

Can you post one or more concrete examples of buggy code that would be
caught by such a warning?

I'm still writing test cases as I come up with more buggy code, and should
post them as comments

under the existing PR, or open new ones. As for now, you can find below
some of them.


Shallow copy/move operations, when such operation is done on an object
owning a resource,

but no user-defined operation was provided (see PR107534
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107534>)

// from PR107534

struct S {
    S() { p = new int(); }
    ~S() { delete p; }
    int* p = nullptr;
};

int main() {
    S s;
    S ss = s; /* { dg-warning "shallow copy of instance of 'S' lacking
user-defined copy constructor" "-Wanalyzer-shallow-ownership-transfer"  {
xfail *-*-* } }  */
}

User-defined move and copy operations, but that are not moving everything
properly.

struct S
{
  std::string s1;
  std::string s2;

  S (S &&other)
  {
    s1 = std::move (other.s1);
    // no mention of s2
  } /* { dg-warning "partial move of instance of 'S' missing true move of
's2'" "-Wanalyzer-shallow-ownership-transfer"  { xfail *-*-* } }  */
};

Deletion of resource we do not own (with introduction of [[gnu::owner]] as
in PR106390 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106390>)

[[gnu::owner]]
extern void deallocator (int *ptr);

void test1 ()
{
  int *p = new int;
  deallocator (p);
  delete p; /* { dg-warning "'p' released resource owned by 'deallocator'"
"-Wanalyzer-ownership" { xfail *-*-* } } */
}

This would somewhat overlap with double-delete warning. So probably this
one would only be useful for the case above,

i.e. for a function we don't have the body of. Although using this warning
could give hints as to which 'delete' among the two is injurious.

Note that we would not flag a double resource release from the resource's
owner, at it is the job of sm-malloc at the moment.

The above attribute [[gnu::owner]] would help in supporting unique_ptr.


Since we keep track of who the owner of a resource is, we can also flag
use-after-move PR106388
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106388>

struct S {
  std::string s;
  S (S && oth) : s (std::move(oth.s)) {}

};
void test_move ()
{
  S ss1;
  S ss2 = std::move (ss1);
  ss1.s.append('c'); /* { dg-warning "use-after-move of 'ss1.s'"
"-Wanalyzer-use-after-move" { xfail *-*-* } }  */
}


Why wouldn't it be caught by C++'s syntax-based ownership support?

Sorry I'm not sure I understand what you meant here. Are you talking
about the c++ guidelines about

pointers vs reference to represent ownership semantics ?

  Is there a PR in bugzilla for this idea?


None yet, but more like a collection of bugs (cited above) pointing
toward that idea.

Best,

Benjamin.

Re: analyzer: New state machine should be C++ only

[Martin Uecker]

Am Mittwoch, dem 12.07.2023 um 15:23 +0200 schrieb Benjamin Priour via
Gcc:
> Hi David,
> 
> 
> Lately I've been working on adding a new state machine to keep track
> of
> ownership transfers
> 
> and misuses, e.g. to warn about use-after-move, partial or shallow
> copy/move.
> 
> I'm trying to stay abstracted from heap allocated regions, and to
> rather
> work with "resources",
> 
> so that the state machine could be easily further extended.
> 
> However, the whole concern of ownership is really C++-like, and most
> of the
> checks would require
> 
> things unheard of in vanilla C, such as copy/move operators, ctors &
> dtors
> ...
> 
> 
> Using those constructs, it is really doable to guess ownership of
> resources, whereas without them it becomes
> 
> much more hazardous.
> 
> So, should we make this new sm -adroitly called sm-ownership- C++-
> only ?
> 
> 
> Doing so would allow the sm to reuse code from under cp/*, thus it'd
> reduce
> duplicating code and would
> 
> likely lead to less false positives in C++ -more precise function
> checks-,
> though it would make any future C-support more tedious.
> 
> It's also going against the current flow of porting what's already
> done for
> C to C++.

A gnu::owner attribute is certainly something which also make sense
for C.

Martin