Warnings from GCC -fanalyzer

[samth]

GCC has a relatively-new static analysis pass that can warn about various problems. I ran it (using GCC v13.2) on Chez Scheme as built by Racket. Below are the first few warnings it generated, at verbosity level 0. If it's helpful I can provide more of the control flow that the analysis thinks can lead to the problem. If these problems seem worth fixing then I'll include the rest.

../ChezScheme/c/segment.c: In function ‘contract_segment_table’:
../ChezScheme/c/segment.c:565:15: warning: dereference of NULL ‘t2i’ [CWE-476] [-Wanalyzer-null-dereference]
  565 |       if ((t2i->refcount -= 1) == 0) {
      |            ~~~^~~~~~~~~~
  ‘S_free_chunk’: event 1
    |
    |  452 |   contract_segment_table(chunk->base, chunk->base + chunk->segs);
    |      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |   |
    |      |   (1) calling ‘contract_segment_table’ from ‘S_free_chunk’
    |
    +--> ‘contract_segment_table’: events 2-4
           |
           |  553 |     t2i = S_segment_info[SEGMENT_T3_IDX(base)];
           |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |         |
           |      |         (3) ‘t2i’ is NULL
           |......
           |  565 |       if ((t2i->refcount -= 1) == 0) {
           |      |            ~~~~~~~~~~~~~                      
           |      |               |
           |      |               (4) dereference of NULL ‘t2i’
           |  566 |         S_freemem((void *)t2i, sizeof(t2table), 0);
           |  567 |         S_segment_info[SEGMENT_T3_IDX(base)] = NULL;
           |      |                                              ^
           |      |                                              |
           |      |                                              (2) ‘0’ is NULL

../ChezScheme/c/vfasl.c: In function ‘S_vfasl’:
../ChezScheme/c/vfasl.c:163:17: warning: use of uninitialized value ‘vspaces[s]’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
  163 |     if (!vspaces[s])
      |          ~~~~~~~^~~
  ‘S_vfasl_to’: event 1
    |
    |  465 |   return S_vfasl(bv, NULL, 0, Sbytevector_length(bv));
    |      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |          |
    |      |          (1) calling ‘S_vfasl’ from ‘S_vfasl_to’
    |
    +--> ‘S_vfasl’: events 2-3
           |
           |   94 |   ptr vspaces[vspaces_count];
           |      |       ^~~~~~~
           |      |       |
           |      |       (2) region created on stack here
           |......
           |  163 |     if (!vspaces[s])
           |      |          ~~~~~~~~~~
           |      |                 |
           |      |                 (3) use of uninitialized value ‘vspaces[s]’ here
           |

../ChezScheme/c/prim5.c: In function ‘s_process’:
../ChezScheme/c/prim5.c:899:22: warning: leak of file descriptor ‘dup(tofds[0])’ [CWE-775] [-Wanalyzer-fd-leak]
  899 |         CLOSE(0); if (dup(tofds[0]) != 0) _exit(1);
      |                      ^
  ‘s_process’: events 1-4
    |
    |  879 |     if (pipe(tofds)) S_error("process","cannot open pipes");
    |      |         ^~~~~~~~~~~
    |      |         |
    |      |         (1) when ‘pipe’ succeeds
    |  880 |     if (pipe(fromfds)) {
    |      |         ~~~~~~~~~~~~~
    |      |         |
    |      |         (2) when ‘pipe’ succeeds
    |......
    |  899 |         CLOSE(0); if (dup(tofds[0]) != 0) _exit(1);
    |      |                      ~~~~~~~~~~~~~~
    |      |                      ||
    |      |                      |(3) opened here
    |      |                      (4) ‘dup(tofds[0])’ leaks here; was opened at (3)
    |

[mnieper]

I think this is useful in any case. Either it is a bogus warning, in which case it should be reported to the GCC maintainers, or it is an actual error in Chez's source, in which case it should be fixed.

[mflatt]

For the first one above, I can't tell what the report is trying to say.

For the second one, I don't see how spaces[s] would be uninitialized. It should be initialized in the preceding loop at either line 136 (the macro expands to an assignment to that argument), line 138 (ditto), or line 160. Am I missing something there? I can see worrying about the spaces[s+1] access on the next line, because the analysis cannot know that vspaces[vspaces_count-1] will never be null (there's always a relocation table), but it doesn't seem to be complaining about that.

For the third one, the analysis quite reasonably doesn't know or can't figure out that because fd 0 was just closed, the dup call will definitely allocate to fd 0.

[samth]

I think the first one is saying that if n is 0 on some iteration of the loop, then we will set S_segment_info[SEGMENT_T3_IDX(base)] to NULL and then on the next iteration, t2i will get that NULL and then be dereferenced, but I'll check the additional context.

I haven't figured out the issue with the second one.