public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug analyzer/109266] New: Wanalyzer-null-dereference does not warn when struct is at null
@ 2023-03-23 22:17 jg at jguk dot org
  2023-03-26 14:58 ` [Bug analyzer/109266] " dmalcolm at gcc dot gnu.org
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: jg at jguk dot org @ 2023-03-23 22:17 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109266

            Bug ID: 109266
           Summary: Wanalyzer-null-dereference does not warn when struct
                    is at null
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: jg at jguk dot org
  Target Milestone: ---

Couldn't find an existing report for this.
Hope the very useful Analyzer can be enhanced to handle nullptr for structs.
Which means that when reading members of the struct they might be at 0x4 etc,
not directly 0x0

Analyzer does detect if the first 'int' in this struct at address nullptr is
read. If the code reads the bytes after in the struct, it doesn't identify that
0x4 address is also inaccessible.

Only way to ensure to get a warning is to copy the struct to a local variable
(before reading those bytes at offset 0x4 from the copy).

Try it live:
https://godbolt.org/z/9a611jvfM

-fanalyzer -Wall -O2

typedef struct a
{
    int b;
    char c[3];
} a_t;

void f(a_t * s)
{
    //s->b = 0;
    s->c[0] = 'b';
}

int main()
{
    a_t * s = nullptr;
    f(s);
}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug analyzer/109266] Wanalyzer-null-dereference does not warn when struct is at null
  2023-03-23 22:17 [Bug analyzer/109266] New: Wanalyzer-null-dereference does not warn when struct is at null jg at jguk dot org
@ 2023-03-26 14:58 ` dmalcolm at gcc dot gnu.org
  2023-03-26 21:08 ` jg at jguk dot org
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-03-26 14:58 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109266

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this bug.

We probably want to allow accesses to hard-coded addresses, for the case of
embedded development, so we presumably need some way to distinguish between
accesses of:
  ((struct foo *)NULL->field)
due to buggy code versus poking specific memory locations.

One approach to this might be for the analyzer to complain if it sees a memory
access to a "low enough" address: rather than merely complaining about accesses
to memory location 0, to have some kind of configurable threshold of pointer
values below which to complain, e.g. "complain about accesses to locations 0
through 4095".

I dimly recall something similar to this inside the Linux kernel at run time -
what memory accesses close to NULL should trigger a crash - but I can't find a
reference right now.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug analyzer/109266] Wanalyzer-null-dereference does not warn when struct is at null
  2023-03-23 22:17 [Bug analyzer/109266] New: Wanalyzer-null-dereference does not warn when struct is at null jg at jguk dot org
  2023-03-26 14:58 ` [Bug analyzer/109266] " dmalcolm at gcc dot gnu.org
@ 2023-03-26 21:08 ` jg at jguk dot org
  2023-03-27 23:14 ` dmalcolm at gcc dot gnu.org
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: jg at jguk dot org @ 2023-03-26 21:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109266

--- Comment #2 from Jonny Grant <jg at jguk dot org> ---
Thank you for your reply David. Your analyzer is very good already.

I played around a bit, a base of nullptr doesn't give a warning. But changing
to 0x10 does give array-bounds warning.
cc1plus: note: source object is likely at address zero
<source>:13:13: warning: array subscript 0 is outside array bounds of 'a_t [0]'
[-Warray-bounds=]

https://godbolt.org/z/PhhT48xxP

Found Andrew Pinski comment says 4096 is not accessible:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106699#c1

I wondered if you know how to turn on that "cc1plus: note: source object is
likely at address zero? It seems different from normal warnings.


It would be fantastic if there was a way for me to specify on the gcc command
line an address range I didn't want read and/or writable. That would be great
to get build warnings from those addresses if the compiler could see them being
accessed.

At the moment, I always need to use the JTAG debugger to set some hw
breakpoints on read from various addresses to catch those accesses (as they are
mapped to the interrupt vector from 0x0). On Windows I've had various crashes
where the access was address 0x10 so felt like that was probably a struct
offset too

I don't know very much about gcc internals. I did wonder if the analyzer can
see the base address of the struct being passed as 0x0 in the RTL file?
I tried -fdump-rtl-all but couldn't see the 0x0 address, or when I changed to
0x10 either

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug analyzer/109266] Wanalyzer-null-dereference does not warn when struct is at null
  2023-03-23 22:17 [Bug analyzer/109266] New: Wanalyzer-null-dereference does not warn when struct is at null jg at jguk dot org
  2023-03-26 14:58 ` [Bug analyzer/109266] " dmalcolm at gcc dot gnu.org
  2023-03-26 21:08 ` jg at jguk dot org
@ 2023-03-27 23:14 ` dmalcolm at gcc dot gnu.org
  2023-03-27 23:22 ` dmalcolm at gcc dot gnu.org
  2023-03-31 21:30 ` jg at jguk dot org
  4 siblings, 0 replies; 6+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-03-27 23:14 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109266

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Jonny Grant from comment #2)
> Thank you for your reply David. Your analyzer is very good already.
> 
> I played around a bit, a base of nullptr doesn't give a warning. But
> changing to 0x10 does give array-bounds warning.
> cc1plus: note: source object is likely at address zero
> <source>:13:13: warning: array subscript 0 is outside array bounds of 'a_t
> [0]' [-Warray-bounds=]
> 
> https://godbolt.org/z/PhhT48xxP

FWIW, note that [-Warray-bounds=] is separate from -fanalyzer.

> 
> Found Andrew Pinski comment says 4096 is not accessible:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106699#c1

Aha - thanks for the link!  I think that's the thing that I was
half-remembering (well, its dup, PR 99578), and that it was, in fact, in GCC.

Looks like I should extend -Wanalyzer-null-dereference to warn about accesses
to constant addresses, but have it respect --param=min-pagesize=
(see r11-9731-g91f7d7e1bb6827bf8e0b7ba7eb949953a5b1bd18).  This would have to
wait for GCC 14 at this point in the release cycle.

> 
> I wondered if you know how to turn on that "cc1plus: note: source object is
> likely at address zero? It seems different from normal warnings.

Grepping the sources shows it's from gcc/pointer-query.cc:
access_ref::inform_access; I think it's one of the middle-end warnings that
triggers that, but I'm not sure exactly which (the analyzer doesn't use that at
the moment).


> It would be fantastic if there was a way for me to specify on the gcc
> command line an address range I didn't want read and/or writable. That would
> be great to get build warnings from those addresses if the compiler could
> see them being accessed.

Is this always for stuff near the 0 address, or are there other use cases? 
(embedded?)  Are you able to post an example here of what the input might look
like?


> 
> At the moment, I always need to use the JTAG debugger to set some hw
> breakpoints on read from various addresses to catch those accesses (as they
> are mapped to the interrupt vector from 0x0). On Windows I've had various
> crashes where the access was address 0x10 so felt like that was probably a
> struct offset too
> 
> I don't know very much about gcc internals. I did wonder if the analyzer can
> see the base address of the struct being passed as 0x0 in the RTL file?
> I tried -fdump-rtl-all but couldn't see the 0x0 address, or when I changed
> to 0x10 either

The analyzer works on the gimple-ssa representation, which is before it become
RTL.  If you want to see the gory details, have a look in:
  https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html
in the gcc internal docs, and:
  https://gcc-newbies-guide.readthedocs.io/en/latest/inside-cc1.html
in the guide for new gcc contributors I wrote.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug analyzer/109266] Wanalyzer-null-dereference does not warn when struct is at null
  2023-03-23 22:17 [Bug analyzer/109266] New: Wanalyzer-null-dereference does not warn when struct is at null jg at jguk dot org
                   ` (2 preceding siblings ...)
  2023-03-27 23:14 ` dmalcolm at gcc dot gnu.org
@ 2023-03-27 23:22 ` dmalcolm at gcc dot gnu.org
  2023-03-31 21:30 ` jg at jguk dot org
  4 siblings, 0 replies; 6+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-03-27 23:22 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109266

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #3)
> (In reply to Jonny Grant from comment #2)
[...]
> > 
> > I wondered if you know how to turn on that "cc1plus: note: source object is
> > likely at address zero? It seems different from normal warnings.
> 
> Grepping the sources shows it's from gcc/pointer-query.cc:
> access_ref::inform_access; I think it's one of the middle-end warnings that
> triggers that, but I'm not sure exactly which (the analyzer doesn't use that
> at the moment).

Grepping for calls to "inform_access" shows that it can come from:
 -Warray_bounds=
 -Wstringop_overflow=
 -Wstringop_overread

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug analyzer/109266] Wanalyzer-null-dereference does not warn when struct is at null
  2023-03-23 22:17 [Bug analyzer/109266] New: Wanalyzer-null-dereference does not warn when struct is at null jg at jguk dot org
                   ` (3 preceding siblings ...)
  2023-03-27 23:22 ` dmalcolm at gcc dot gnu.org
@ 2023-03-31 21:30 ` jg at jguk dot org
  4 siblings, 0 replies; 6+ messages in thread
From: jg at jguk dot org @ 2023-03-31 21:30 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109266

--- Comment #5 from Jonny Grant <jg at jguk dot org> ---
(In reply to David Malcolm from comment #3)
> (In reply to Jonny Grant from comment #2)
> > Thank you for your reply David. Your analyzer is very good already.
> > 
> > I played around a bit, a base of nullptr doesn't give a warning. But
> > changing to 0x10 does give array-bounds warning.
> > cc1plus: note: source object is likely at address zero
> > <source>:13:13: warning: array subscript 0 is outside array bounds of 'a_t
> > [0]' [-Warray-bounds=]
> > 
> > https://godbolt.org/z/PhhT48xxP
> 
> FWIW, note that [-Warray-bounds=] is separate from -fanalyzer.
> 
> > 
> > Found Andrew Pinski comment says 4096 is not accessible:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106699#c1
> 
> Aha - thanks for the link!  I think that's the thing that I was
> half-remembering (well, its dup, PR 99578), and that it was, in fact, in GCC.
> 
> Looks like I should extend -Wanalyzer-null-dereference to warn about
> accesses to constant addresses, but have it respect --param=min-pagesize=
> (see r11-9731-g91f7d7e1bb6827bf8e0b7ba7eb949953a5b1bd18).  This would have
> to wait for GCC 14 at this point in the release cycle.
> 
> > 
> > I wondered if you know how to turn on that "cc1plus: note: source object is
> > likely at address zero? It seems different from normal warnings.
> 
> Grepping the sources shows it's from gcc/pointer-query.cc:
> access_ref::inform_access; I think it's one of the middle-end warnings that
> triggers that, but I'm not sure exactly which (the analyzer doesn't use that
> at the moment).

Many thanks for your investigation and reply.


> > It would be fantastic if there was a way for me to specify on the gcc
> > command line an address range I didn't want read and/or writable. That would
> > be great to get build warnings from those addresses if the compiler could
> > see them being accessed.
> 
> Is this always for stuff near the 0 address, or are there other use cases? 
> (embedded?)  Are you able to post an example here of what the input might
> look like?

For me, I was mainly thinking issues due to reading near the 0x0 address (which
is valid containing some stuff like the interrupt vector of function pointers
etc). The best example would be the struct at NULL I put on godbolt.

However, it is true that reading from other addresses that aren't HW mapped IO,
RAM or Flash etc would also lead to faults, so if there was a way to specify
the valid address ranges that would be great. I recall another compiler I used
did let me specify valid ranges.

Jonathan Wakely mentioned  __attribute__((access(read_only, 1)));  elsewhere it
could imply the pointer was non-null (I couldn't get any build warnings through
use of that attribute though in GCC 12). I don't know if the Analyzer could use
that attribute too.


> > 
> > At the moment, I always need to use the JTAG debugger to set some hw
> > breakpoints on read from various addresses to catch those accesses (as they
> > are mapped to the interrupt vector from 0x0). On Windows I've had various
> > crashes where the access was address 0x10 so felt like that was probably a
> > struct offset too
> > 
> > I don't know very much about gcc internals. I did wonder if the analyzer can
> > see the base address of the struct being passed as 0x0 in the RTL file?
> > I tried -fdump-rtl-all but couldn't see the 0x0 address, or when I changed
> > to 0x10 either
> 
> The analyzer works on the gimple-ssa representation, which is before it
> become RTL.  If you want to see the gory details, have a look in:
>   https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html
> in the gcc internal docs, and:
>   https://gcc-newbies-guide.readthedocs.io/en/latest/inside-cc1.html
> in the guide for new gcc contributors I wrote.

Thank thanks for the links.

I modified the above example to have a base of 0x100 and noticed it in the SSA
file

MEM[(struct a_t *)256B].c[0] = 98;

In the resulting ASM output, it was 8 bytes offset from the base. 0x108

Would be great if Analyzer could report on these in a future release.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-31 21:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-23 22:17 [Bug analyzer/109266] New: Wanalyzer-null-dereference does not warn when struct is at null jg at jguk dot org
2023-03-26 14:58 ` [Bug analyzer/109266] " dmalcolm at gcc dot gnu.org
2023-03-26 21:08 ` jg at jguk dot org
2023-03-27 23:14 ` dmalcolm at gcc dot gnu.org
2023-03-27 23:22 ` dmalcolm at gcc dot gnu.org
2023-03-31 21:30 ` jg at jguk dot org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).