gcc warn when pointers not checked non-null before de-referencing.

gcc warn when pointers not checked non-null before de-referencing.

[Jonny Grant]

Hello

This isn't real code, just an example to show.

I've tried with:  -Wall -Wextra -O2  and some other warnings, but couldn't get this to generate a warning that *g was possibly de-referenced. May I ask, does GCC have a way to get warnings when pointers are not checked?
I had a look but -Wnull-dereference didn't help.

#include <stdlib.h>

#include <cstddef>
void f(int * g)
{
    *g = 1;

    if(NULL == g)
    {
        exit(1);
    }
}

Best regards Jonny

Re: gcc warn when pointers not checked non-null before de-referencing.

[Xi Ruoyao]

On Sun, 2021-06-13 at 23:30 +0100, Jonny Grant wrote:
> Hello
> 
> This isn't real code, just an example to show.
> 
> I've tried with:  -Wall -Wextra -O2  and some other warnings, but
> couldn't get this to generate a warning that *g was possibly de-
> referenced. May I ask, does GCC have a way to get warnings when pointers
> are not checked?
> I had a look but -Wnull-dereference didn't help.
> 
> #include <stdlib.h>
> 
> #include <cstddef>
> void f(int * g)
> {
>     *g = 1;
> 
>     if(NULL == g)
>     {
>         exit(1);
>     }
> }
> 
> Best regards Jonny

It was explained by Chris Lattner at
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_21.html

GCC (<= 4.4) had -Wunreachable-code which might work for this case.  But
it was too unreliable (as Chris said, generally there is no reliable way
to do this) and removed in later releases.
-- 
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University

Re: gcc warn when pointers not checked non-null before de-referencing.

[Martin Sebor]

On 6/13/21 4:30 PM, Jonny Grant wrote:
> Hello
> 
> This isn't real code, just an example to show.
> 
> I've tried with:  -Wall -Wextra -O2  and some other warnings, but couldn't get this to generate a warning that *g was possibly de-referenced. May I ask, does GCC have a way to get warnings when pointers are not checked?
> I had a look but -Wnull-dereference didn't help.

The pointer test is eliminated before -Wnull-dereference runs so it
can't do anything in this case.  -fno-delete-null-pointer-checks
disables the optimization and retains the test so -Wnull-dereference
could in theory detect it but it isn't designed to do it.  But if
it did, I suspect it would be quite noisy as a result of other
transformations like inlining and constant propagation.  So making
it work with an acceptable S/R ratio would take more work than just
enhancing -Wnull-dereference.

Martin

> 
> #include <stdlib.h>
> 
> #include <cstddef>
> void f(int * g)
> {
>      *g = 1;
> 
>      if(NULL == g)
>      {
>          exit(1);
>      }
> }
> 
> Best regards Jonny
> 

Re: gcc warn when pointers not checked non-null before de-referencing.

[Jonny Grant]

On 14/06/2021 06:15, Xi Ruoyao wrote:
> On Sun, 2021-06-13 at 23:30 +0100, Jonny Grant wrote:
>> Hello
>>
>> This isn't real code, just an example to show.
>>
>> I've tried with:  -Wall -Wextra -O2  and some other warnings, but
>> couldn't get this to generate a warning that *g was possibly de-
>> referenced. May I ask, does GCC have a way to get warnings when pointers
>> are not checked?
>> I had a look but -Wnull-dereference didn't help.
>>
>> #include <stdlib.h>
>>
>> #include <cstddef>
>> void f(int * g)
>> {
>>     *g = 1;
>>
>>     if(NULL == g)
>>     {
>>         exit(1);
>>     }
>> }
>>
>> Best regards Jonny
> 
> It was explained by Chris Lattner at
> http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_21.html
> 
> GCC (<= 4.4) had -Wunreachable-code which might work for this case.  But
> it was too unreliable (as Chris said, generally there is no reliable way
> to do this) and removed in later releases.
> 

Thank you for your reply and the link. And for Martin's reply.

I guess a separate static analyser would do it, GCC is more focused on compilation so I shouldn't ask for it to have so many features it can't support.


Chris Latner also mentioned integer overflow being undefined, that crops up too. There's no easy solution right, we need to hand write code the checks?  It's human-error prone if we need to manually code each check. throwing in C++, or handling in C.

if(N >= INT_MAX)
{
    throw std::overflow_error("N >= INT_MAX would overflow in for loop");
}

for (i = 0; i <= N; ++i)
{
// ...
 }

Cheers, Jonny

Re: gcc warn when pointers not checked non-null before de-referencing.

[Xi Ruoyao]

On Wed, 2021-06-16 at 14:01 +0100, Jonny Grant wrote:

> Chris Latner also mentioned integer overflow being undefined, that
> crops up too. There's no easy solution right, we need to hand write
> code the checks?  It's human-error prone if we need to manually code
> each check. throwing in C++, or handling in C.
> 
> if(N >= INT_MAX)
> {
>     throw std::overflow_error("N >= INT_MAX would overflow in for
> loop");
> }
> 
> for (i = 0; i <= N; ++i)
> {
> // ...
>  }

For debugging use -fsanitize=undefined.

And this is buggy anyway, no matter if there is an UB:

for (unsigned i = 0; i <= N; i++)
    make_some_side_effect_without_any_undefined_behavior(i);

If N may be UINT_MAX, this is not UB, but a dead loop. Programming is
just human-error prone, even if you use "some programming language
claimed to be able to eliminate many human errors" (I'll not say its
name, to prevent a flame war).
-- 
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University

Re: gcc warn when pointers not checked non-null before de-referencing.

[Segher Boessenkool]

On Wed, Jun 16, 2021 at 02:01:05PM +0100, Jonny Grant wrote:
> I guess a separate static analyser would do it, GCC is more focused on compilation so I shouldn't ask for it to have so many features it can't support.

-fsanitize=undefined already catches null pointer dereferences, is that
enough for your case?


Segher

Re: gcc warn when pointers not checked non-null before de-referencing.

[Jonny Grant]

On 16/06/2021 18:59, Segher Boessenkool wrote:
> On Wed, Jun 16, 2021 at 02:01:05PM +0100, Jonny Grant wrote:
>> I guess a separate static analyser would do it, GCC is more focused on compilation so I shouldn't ask for it to have so many features it can't support.
> 
> -fsanitize=undefined already catches null pointer dereferences, is that
> enough for your case?
> 
> 
> Segher

Hello
Thank you for the suggestion, yes, I had used that before. I did just check, it's runtime checks. I had hoped for something at compile time. warning for every function that didn't check pointer for NULL before de-referencing.

$ ./null
null.c:7:5: runtime error: load of null pointer of type 'int'
Segmentation fault (core dumped)



// gcc -Wall -fsanitize=undefined -o null null.c

#include <stdio.h>

void f(int * p)
{
    printf("%d\n", *p);
}

int main()
{
    f(NULL);
}

Re: gcc warn when pointers not checked non-null before de-referencing.

[Xi Ruoyao]

On Thu, 2021-06-17 at 21:44 +0100, Jonny Grant wrote:
> 
> 
> On 16/06/2021 18:59, Segher Boessenkool wrote:
> > On Wed, Jun 16, 2021 at 02:01:05PM +0100, Jonny Grant wrote:
> > > I guess a separate static analyser would do it, GCC is more
> > > focused on compilation so I shouldn't ask for it to have so many
> > > features it can't support.
> > 
> > -fsanitize=undefined already catches null pointer dereferences, is
> > that
> > enough for your case?
> > 
> > 
> > Segher
> 
> Hello
> Thank you for the suggestion, yes, I had used that before. I did just
> check, it's runtime checks. I had hoped for something at compile time.
> warning for every function that didn't check pointer for NULL before
> de-referencing.

There is no way to do this.

int foo(struct dev *dev)
{
    int r = sanitize_input(dev);
    if (r)
        return r;

    bar(&dev->id);
    return dev->id->result;
}

While there is no way to know the behavior of `sanitize_input` (it may
be defined in another TU), there is no way to tell if `foo` has done "a
proper non-null check".

And this "warning" does not make any sense.  It's perfectly legal for a
function to assume the input is not null and the caller should guarantee
it.  Adding a non-null check in every function is just paranoid.  This
really looks like a suggestion from some professors who don't program at
all, but unfortunately teach C and tell their own misconcept of
"defensive programming" everywhere.
-- 
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University

Re: gcc warn when pointers not checked non-null before de-referencing.

[Liu Hao]

[-- Attachment #1.1: Type: text/plain, Size: 576 bytes --]

在 2021-06-18 04:44, Jonny Grant 写道:
> 
> Hello
> Thank you for the suggestion, yes, I had used that before. I did just check, it's runtime checks. I had hoped for something at compile time. warning for every function that didn't check pointer for NULL before de-referencing.
> 

You might want to have a look at <http://cppcheck.sourceforge.net/demo/>. There are some 'NULL 
pointers' examples which answers your exact question.

If you use Debian or Ubuntu, you can install cppcheck with `sudo aptitude install cppcheck`.



-- 
Best regards,
Liu Hao


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: gcc warn when pointers not checked non-null before de-referencing.

[Segher Boessenkool]

On Thu, Jun 17, 2021 at 09:44:27PM +0100, Jonny Grant wrote:
> On 16/06/2021 18:59, Segher Boessenkool wrote:
> > On Wed, Jun 16, 2021 at 02:01:05PM +0100, Jonny Grant wrote:
> >> I guess a separate static analyser would do it, GCC is more focused on compilation so I shouldn't ask for it to have so many features it can't support.
> > 
> > -fsanitize=undefined already catches null pointer dereferences, is that
> > enough for your case?
> 
> Hello
> Thank you for the suggestion, yes, I had used that before. I did just check, it's runtime checks. I had hoped for something at compile time. warning for every function that didn't check pointer for NULL before de-referencing.

That doesn't make too much sense really.  Check pointer and then do
what?  Your code can check for null pointers itself, of course.

Anyway, without any sanitize options, from the following code:

int f(int *p) { return *p; }
int g(void) { return f(0); }

you get on powerpc64le:

f:
	lwa 3,0(3)
	blr
g:
	li 9,0
	lfiwax 0,0,9
	trap

or on aarch64:

f:
	ldr     w0, [x0]
	ret
g:
	mov     x0, 0
	ldr     w0, [x0]
	brk #1000

or on x86_64:

f:
	movl    (%rdi), %eax
	ret
g:
	movl    0, %eax
	ud2

GCC knows the code after the load is not reachable, that is why it
generates a trap instruction there.  I will still do the load though, so
that you get good errors and a reasonable debug experience.


Segher

Re: gcc warn when pointers not checked non-null before de-referencing.

[Jonny Grant]

On 18/06/2021 05:16, Xi Ruoyao wrote:
> On Thu, 2021-06-17 at 21:44 +0100, Jonny Grant wrote:
>>
>>
>> On 16/06/2021 18:59, Segher Boessenkool wrote:
>>> On Wed, Jun 16, 2021 at 02:01:05PM +0100, Jonny Grant wrote:
>>>> I guess a separate static analyser would do it, GCC is more
>>>> focused on compilation so I shouldn't ask for it to have so many
>>>> features it can't support.
>>>
>>> -fsanitize=undefined already catches null pointer dereferences, is
>>> that
>>> enough for your case?
>>>
>>>
>>> Segher
>>
>> Hello
>> Thank you for the suggestion, yes, I had used that before. I did just
>> check, it's runtime checks. I had hoped for something at compile time.
>> warning for every function that didn't check pointer for NULL before
>> de-referencing.
> 
> There is no way to do this.
> 
> int foo(struct dev *dev)
> {
>     int r = sanitize_input(dev);
>     if (r)
>         return r;
> 
>     bar(&dev->id);
>     return dev->id->result;
> }
> 
> While there is no way to know the behavior of `sanitize_input` (it may
> be defined in another TU), there is no way to tell if `foo` has done "a
> proper non-null check".

Yes, this is a good point, could only check the file that is being compiled.

> 
> And this "warning" does not make any sense.  It's perfectly legal for a
> function to assume the input is not null and the caller should guarantee
> it.  Adding a non-null check in every function is just paranoid.  This
> really looks like a suggestion from some professors who don't program at
> all, but unfortunately teach C and tell their own misconcept of
> "defensive programming" everywhere.

We were taught to be careful what we pass to functions, and careful what we accept in functions, feels appropriate.

While I can see that for developer builds, a NULL dereference and core dump would allow a developer to investigate. assert is just as good assert(NULL != ptr);

We'd rather have stability and logging of errors than a core dump. Especially on embedded systems that need to guarantee safety. It's easy enough to check for NULL and check the bounds of buffers etc before using. Asserts would trigger in a debug build.

I know GCC is a compiler, not a static analyser, so I'll probably run cppcheck

Kind regards
Jonny

Re: gcc warn when pointers not checked non-null before de-referencing.

[Jonny Grant]

On 16/06/2021 14:36, Xi Ruoyao wrote:
> On Wed, 2021-06-16 at 14:01 +0100, Jonny Grant wrote:
> 
>> Chris Latner also mentioned integer overflow being undefined, that
>> crops up too. There's no easy solution right, we need to hand write
>> code the checks?  It's human-error prone if we need to manually code
>> each check. throwing in C++, or handling in C.
>>
>> if(N >= INT_MAX)
>> {
>>     throw std::overflow_error("N >= INT_MAX would overflow in for
>> loop");
>> }
>>
>> for (i = 0; i <= N; ++i)
>> {
>> // ...
>>  }
> 
> For debugging use -fsanitize=undefined.
> 
> And this is buggy anyway, no matter if there is an UB:
> 
> for (unsigned i = 0; i <= N; i++)
>     make_some_side_effect_without_any_undefined_behavior(i);
> 
> If N may be UINT_MAX, this is not UB, but a dead loop. Programming is
> just human-error prone, even if you use "some programming language
> claimed to be able to eliminate many human errors" (I'll not say its
> name, to prevent a flame war).
> 
Hi Xi


Checking the UINT_MAX would at least prevent the continual running of any such buggy loop where it increments right? and the code within the loop does not modify 'i'

for (unsigned i = 0; (i <= N) && (i != UINT_MAX); i++)
    make_some_side_effect_without_any_undefined_behavior(i);

Is there any way to have a way to make loop variables like this 'i' const within the body of the loop, to avoid accidental changing of 'i' by the body of the loop

Jonny

Re: gcc warn when pointers not checked non-null before de-referencing.

[Segher Boessenkool]

On Sat, Jul 03, 2021 at 03:14:21PM +0100, Jonny Grant wrote:
> We'd rather have stability and logging of errors than a core dump. Especially on embedded systems that need to guarantee safety. It's easy enough to check for NULL and check the bounds of buffers etc before using. Asserts would trigger in a debug build.

There *is* no generic way you can sanely and even safely handle null
pointer dereferences at all.  You *have to* separately write code for
every place you want to do something special with null pointers (or any
other special case for that matter).

In case you are talking about a system with an event loop (or similar),
where you can just abort the task you are doing, and get back to waiting
for more work to do: you can just catch *all* fatal errors (which a null
dereference normally is), log it, and go back to the main loop.  But
even then the null dereference could be caused by something that is not
yet fixed.  If you are lucky other tasks can still be done.   OTOH,
perhaps just as often nothing will work anymore.

In any case, there is nothing the compiler can do for you here.  If
programming were a mechanical job, we wouldn't need all those pesky
programmers :-)


Segher

Re: gcc warn when pointers not checked non-null before de-referencing.

[Jonny Grant]

On 03/07/2021 17:22, Segher Boessenkool wrote:
> On Sat, Jul 03, 2021 at 03:14:21PM +0100, Jonny Grant wrote:
>> We'd rather have stability and logging of errors than a core dump. Especially on embedded systems that need to guarantee safety. It's easy enough to check for NULL and check the bounds of buffers etc before using. Asserts would trigger in a debug build.
> 
> There *is* no generic way you can sanely and even safely handle null
> pointer dereferences at all.  You *have to* separately write code for
> every place you want to do something special with null pointers (or any
> other special case for that matter).
> 
> In case you are talking about a system with an event loop (or similar),
> where you can just abort the task you are doing, and get back to waiting
> for more work to do: you can just catch *all* fatal errors (which a null
> dereference normally is), log it, and go back to the main loop.  But
> even then the null dereference could be caused by something that is not
> yet fixed.  If you are lucky other tasks can still be done.   OTOH,
> perhaps just as often nothing will work anymore.

Yes, you are quite right! The error could be handled by the calling code, or logged for later debugging use, and hopefully for a safety critical system it continues to run. In some instances we would restart that module after a recurring error, and use a watchdog thread to reboot if something severe that can't be recovered on an overall system.

An example would be as follows sample function:

int component_msg(unsigned int component, const char * msg)
{
    if(NULL == msg)
    {
        log_msg("component_msg component %d called with NULL msg\n", component);
        return EFAULT;
    }
    else
    {
        log_msg("%s", msg);
        return 0;
    }
}

The calling code could also check the return of component_msg() and handle some appropriate way. Restarting the module, notifying another system, ignoring a faulty device etc

> 
> In any case, there is nothing the compiler can do for you here.  If
> programming were a mechanical job, we wouldn't need all those pesky
> programmers :-)
> 
> 
> Segher
> 


Cheers, Jonny

Re: gcc warn when pointers not checked non-null before de-referencing.

[Xi Ruoyao]

On Sat, 2021-07-03 at 16:36 +0100, Jonny Grant wrote:
> 
> 
> On 16/06/2021 14:36, Xi Ruoyao wrote:
> > On Wed, 2021-06-16 at 14:01 +0100, Jonny Grant wrote:
> > 
> > > Chris Latner also mentioned integer overflow being undefined, that
> > > crops up too. There's no easy solution right, we need to hand write
> > > code the checks?  It's human-error prone if we need to manually code
> > > each check. throwing in C++, or handling in C.
> > > 
> > > if(N >= INT_MAX)
> > > {
> > >     throw std::overflow_error("N >= INT_MAX would overflow in for
> > > loop");
> > > }
> > > 
> > > for (i = 0; i <= N; ++i)
> > > {
> > > // ...
> > >  }
> > 
> > For debugging use -fsanitize=undefined.
> > 
> > And this is buggy anyway, no matter if there is an UB:
> > 
> > for (unsigned i = 0; i <= N; i++)
> >     make_some_side_effect_without_any_undefined_behavior(i);
> > 
> > If N may be UINT_MAX, this is not UB, but a dead loop. Programming is
> > just human-error prone, even if you use "some programming language
> > claimed to be able to eliminate many human errors" (I'll not say its
> > name, to prevent a flame war).
> > 
> Hi Xi
> 
> 
> Checking the UINT_MAX would at least prevent the continual running of
> any such buggy loop where it increments right? and the code within the
> loop does not modify 'i'
> 
> for (unsigned i = 0; (i <= N) && (i != UINT_MAX); i++)
>     make_some_side_effect_without_any_undefined_behavior(i);

Even if i is signed, it will still "work" if you modify the &&
expression a little:

for (int i = 0; i != UINT_MAX && i < N; i++)
     make_some_side_effect_without_any_undefined_behavior(i);

The problem is, now the behavior when N == UINT_MAX is same with when N
== (UINT_MAX - 1).  This can really puzzle someone who will call your
function.

If I'm designing this function I'd make it to interpret N as [0, N),
instead of [0, N]:

// Do something for each integer in [0, N).
void do_something(int N)
{
    for (int i = 0; i < N; i++)
        do_something_once(i);
}

> Is there any way to have a way to make loop variables like this 'i'
> const within the body of the loop, to avoid accidental changing of 'i'
> by the body of the loop

I don't think there is one in C.  Perhaps, maybe use some "nasty"
macros.
-- 
Xi Ruoyao <xry111@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University

Re: gcc warn when pointers not checked non-null before de-referencing.

[Jonny Grant]

On Tue, 6 Jul 2021 at 16:39, Xi Ruoyao <xry111@mengyan1223.wang> wrote:
>
> On Sat, 2021-07-03 at 16:36 +0100, Jonny Grant wrote:
> >
> >
> > On 16/06/2021 14:36, Xi Ruoyao wrote:
> > > On Wed, 2021-06-16 at 14:01 +0100, Jonny Grant wrote:
> > >
> > > > Chris Latner also mentioned integer overflow being undefined, that
> > > > crops up too. There's no easy solution right, we need to hand write
> > > > code the checks?  It's human-error prone if we need to manually code
> > > > each check. throwing in C++, or handling in C.
> > > >
> > > > if(N >= INT_MAX)
> > > > {
> > > >     throw std::overflow_error("N >= INT_MAX would overflow in for
> > > > loop");
> > > > }
> > > >
> > > > for (i = 0; i <= N; ++i)
> > > > {
> > > > // ...
> > > >  }
> > >
> > > For debugging use -fsanitize=undefined.
> > >
> > > And this is buggy anyway, no matter if there is an UB:
> > >
> > > for (unsigned i = 0; i <= N; i++)
> > >     make_some_side_effect_without_any_undefined_behavior(i);
> > >
> > > If N may be UINT_MAX, this is not UB, but a dead loop. Programming is
> > > just human-error prone, even if you use "some programming language
> > > claimed to be able to eliminate many human errors" (I'll not say its
> > > name, to prevent a flame war).
> > >
> > Hi Xi
> >
> >
> > Checking the UINT_MAX would at least prevent the continual running of
> > any such buggy loop where it increments right? and the code within the
> > loop does not modify 'i'
> >
> > for (unsigned i = 0; (i <= N) && (i != UINT_MAX); i++)
> >     make_some_side_effect_without_any_undefined_behavior(i);
>
> Even if i is signed, it will still "work" if you modify the &&
> expression a little:
>
> for (int i = 0; i != UINT_MAX && i < N; i++)
>      make_some_side_effect_without_any_undefined_behavior(i);
>
> The problem is, now the behavior when N == UINT_MAX is same with when N
> == (UINT_MAX - 1).  This can really puzzle someone who will call your
> function.
>
> If I'm designing this function I'd make it to interpret N as [0, N),
> instead of [0, N]:
>
> // Do something for each integer in [0, N).
> void do_something(int N)
> {
>     for (int i = 0; i < N; i++)
>         do_something_once(i);
> }

That's good, specify the limit.
>
> > Is there any way to have a way to make loop variables like this 'i'
> > const within the body of the loop, to avoid accidental changing of 'i'
> > by the body of the loop
>
> I don't think there is one in C.  Perhaps, maybe use some "nasty"
> macros.

Probably I could use this pattern to avoid the risk of code modifying
the loop counter by accident.

for (int loop_var = 0; loop_var < N; loop_var++)
{
    const int i = loop_var;
    printf("i: %d\n", i);
}

Jonny