[Bug analyzer/115436] New: False positive with -Wanalyzer-malloc-leak

2024-06-11 Thread rickobranimir at gmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115436

Bug ID: 115436
   Summary: False positive with -Wanalyzer-malloc-leak
   Product: gcc
   Version: 14.1.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: analyzer
  Assignee: dmalcolm at gcc dot gnu.org
  Reporter: rickobranimir at gmail dot com
  Target Milestone: ---

Hello,
I've been playing with the gcc -fanalyzer. It works great, really impressive
stuff.
But I think I have found a bug with it.

This is a simplest cast where I get the wrong analysis:

This is a command that I run:
```
gcc -c -fanalyzer main.c
```

INPUT:
```c
// File: main.c
#include 
#include 
#include 

typedef struct {
  char* str;
  size_t len, cap;
} my_str;

static bool my_str_realloc(my_str* s, size_t new_cap) {
  if (s->cap == 0) {
s->str = malloc(new_cap > 8 ? new_cap : 8);
if (s->str == NULL) return false;
s->cap = new_cap > 8 ? new_cap : 8;
return true;
  }
  if (s->cap < new_cap) {
char * newS = realloc(s->str, new_cap);
if (newS == NULL) return false;
s->str = newS;
s->cap = (unsigned int)new_cap;
return true;
  }
  return false;
}

static bool my_str_push_char(my_str* s, char c) {
  if (s->len >= s->cap) if (!my_str_realloc(s, s->cap * 2)) return false;
  s->str[s->len++] = c;
  return true;
}

int foo(my_str* s) {
  my_str_push_char(s, '/');
  my_str_push_char(s, '/');
  return 0;
}
```

OUTPUT:
```
main.c: In function ‘my_str_realloc’:
main.c:12:12: warning: leak of ‘*s.str’ [CWE-401] [-Wanalyzer-malloc-leak]
   12 | s->str = malloc(new_cap > 8 ? new_cap : 8);
  | ~~~^~~
  ‘foo’: events 1-2
|
|   35 | int foo(my_str* s) {
|  | ^~~
|  | |
|  | (1) entry to ‘foo’
|   36 |   my_str_push_char(s, '/');
|  |   
|  |   |
|  |   (2) calling ‘my_str_push_char’ from ‘foo’
|
+--> ‘my_str_push_char’: events 3-6
   |
   |   29 | static bool my_str_push_char(my_str* s, char c) {
   |  | ^~~~
   |  | |
   |  | (3) entry to ‘my_str_push_char’
   |   30 |   if (s->len >= s->cap) if (!my_str_realloc(s, s->cap * 2))
return false;
   |  |  ~   ~
   |  |  |   |  |
   |  |  |   |  (5) ...to
here
   |  |  |   (6) calling ‘my_str_realloc’
from ‘my_str_push_char’
   |  |  (4) following ‘true’ branch...
   |
   +--> ‘my_str_realloc’: events 7-13
  |
  |   10 | static bool my_str_realloc(my_str* s, size_t
new_cap) {
  |  | ^~
  |  | |
  |  | (7) entry to ‘my_str_realloc’
  |   11 |   if (s->cap == 0) {
  |  |  ~  
  |  |  |
  |  |  (8) following ‘true’ branch...
  |   12 | s->str = malloc(new_cap > 8 ? new_cap : 8);
  |  |  ~
  |  |  |
  |  |  (9) ...to here
  |  |  (10) allocated here
  |   13 | if (s->str == NULL) return false;
  |  |~
  |  ||
  |  |(11) assuming ‘*s.str’ is non-NULL
  |  |(12) following ‘false’ branch...
  |   14 | s->cap = new_cap > 8 ? new_cap : 8;
  |  |  ~
  |  ||
  |  |(13) ...to here
  |
   <--+
   |
 ‘my_str_push_char’: events 14-16
   |
   |   30 |   if (s->len >= s->cap) if (!my_str_realloc(s, s->cap * 2))
return false;
   |  |~ ^
   |  || |
   |  || (14) returning to
‘my_str_push_char’ from ‘my_str_realloc’
   |  |(15) following ‘false’ branch...
   |   31 |   s->str[s->len++] = c;
   |  |   ~~  
   |  ||
   |  |(16) ...to here
   |
<--+
|
  ‘foo’: events 17-18
|
|   36 |   my_str_push_char(s, '/');
|  |   ^~~~
|  |   |
|  |   (17

[Bug analyzer/115436] False positive with -Wanalyzer-malloc-leak

2024-06-12 Thread rickobranimir at gmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115436

--- Comment #2 from Branimir Ričko  ---
```
I think there *might* be a true positive here for the case where s->cap ==
0x8000, so that s->cap * 2 becomes 0 due to overflow; should my_str_realloc
be checking for s->str being null for the "needs malloc" case?
```

Yea, you are right. When I was making the example shorter, I must have made it
too short...

This should not be affected by overflows: https://godbolt.org/z/dhPcz8Kj4

Analyzer says that on first call to push, *len* is >= *cap*.
That means that cap is 0.
And then it mallocs on first call.
But then it also deduces that on second call *len* can be >= *cap*.
This can not be because *cap* was set to 8 on the first call and *len* to
1.

```
(b) there's a definite bug in binding_map, where __analyzer_dump () shows an
overlapping concrete binding:

clusters within root region
  cluster for: (*INIT_VAL(s_2(D)))
ESCAPED
key:   {bytes 0-7}
value: 'char *' {UNKNOWN(char *)}
key:   {bytes 0-23}
value: 'struct my_str' {UNKNOWN(struct my_str)}
key:   {bytes 16-23}
value: 'unsigned int' {UNKNOWN(unsigned int)}

where the binding for bytes 0-23 overlaps that for bytes 0-7.
```

Idk what this is or should be, but to me it looks like `struct my_str` should
overlap with `char *`.
`struct my_str` is *s
and
`char *` is `s->str`

Or am I missing something?