public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Differences between clang and gcc handling of int[static n] function arguments
@ 2023-05-21 23:40 peter0x44
  2023-05-23 17:47 ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: peter0x44 @ 2023-05-21 23:40 UTC (permalink / raw)
  To: gcc

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



Hi,

So, recently I learned about the c99 feature to get NULL pointer checks 
for array function arguments.
I have really never seen this feature used in an actual codebase. It's 
definitely something I wanted on a few occasions.

To be clear, I'm talking about specifically:
void foo(int array[static 1]);

I checked what warnings this produces - gcc by default produces, none, 
but with -Wall it produces for this code:

int foo(int array[static 1]){ return array[0]; }

int main(void)
{
#define NULL (void*)0
        foo(NULL);
}

bruh.c: In function 'main':
bruh.c:8:9: warning: argument 1 to 'int[static 1]' is null where 
non-null expected [-Wnonnull]
    8 |         foo(NULL);
      |         ^~~~~~~~~
bruh.c:3:5: note: in a call to function 'foo'
    3 | int foo(int array[static 1]){ return array[0]; }
      |     ^~~
I think this warning is acceptable, but has some scope for improvement.

I checked what clang did instead, and it seemed nicer, for sure.

bruh.c:8:2: warning: null passed to a callee that requires a non-null 
argument [-Wnonnull]
        foo(NULL);
         ^   ~~~~
bruh.c:3:13: note: callee declares array parameter as static here
int foo(int array[static 1]){ return array[0]; }
             ^    ~~~~~~~~~~
  It's pointing me exactly to the parameter with the static directly, so 
there is no ambiguity

Also, this is a warning enabled by default, no need to pass -Wall.

Is there a reason gcc doesn't enable this by default? To me, it seems 
like a warning that's desirable always.

You are explicitly agreeing to never call these functions with NULL, any 
code doing that is surely broken.

There's no way this gives a false positive, ever.

I'm definitely adding this warning to -Werror on all of my future 
projects, now that I know about it.

One last thing worth mentioning, is that GCC makes a nicer warning than 
clang when this is done through __attribute__((nonnull))

bruh.c: In function 'main':
bruh.c:8:9: warning: argument 1 null where non-null expected [-Wnonnull]
    8 |         foo(NULL);
      |         ^~~
bruh.c:3:5: note: in a call to function 'foo' declared 'nonnull'
    3 | int foo(int array[1]){ return array[0]; }
      |     ^~~
  It points out specifically that that it is done through the attribute.

I think it would be nice if the attribute could be underlined also, 
though.

Clang produces:

bruh.c:8:10: warning: null passed to a callee that requires a non-null 
argument [-Wnonnull]
        foo(NULL);
             ~~~~^
  with no mention that the warning is specifically because of the 
attribute.

I tried looking on the bug tracker and I could find nothing elaborating 
on this. Maybe I'm not looking hard enough.

I would be happy to open a PR to improve this warning, if there isn't 
one already.

It seems it might even be trivial enough for me to investigate and 
tackle myself, in some spare time.

I see very little code using either of these features, so it's 
definitely not a high priority task regardless.

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

* Re: Differences between clang and gcc handling of int[static n] function arguments
  2023-05-21 23:40 Differences between clang and gcc handling of int[static n] function arguments peter0x44
@ 2023-05-23 17:47 ` Jonathan Wakely
  2023-05-24  9:06   ` Florian Weimer
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2023-05-23 17:47 UTC (permalink / raw)
  To: peter0x44; +Cc: gcc

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

On Mon, 22 May 2023, 00:40 peter0x44 via Gcc, <gcc@gcc.gnu.org> wrote:

>
> I would be happy to open a PR to improve this warning, if there isn't
> one already.
>

Please do.


>
> It seems it might even be trivial enough for me to investigate and
> tackle myself, in some spare time.
>
> I see very little code using either of these features, so it's
> definitely not a high priority task regardless.
>

Glibc uses the nonnull attribute in many places. Libstdc++ uses it in a few
places.

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

* Re: Differences between clang and gcc handling of int[static n] function arguments
  2023-05-23 17:47 ` Jonathan Wakely
@ 2023-05-24  9:06   ` Florian Weimer
  2023-05-24  9:26     ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2023-05-24  9:06 UTC (permalink / raw)
  To: Jonathan Wakely via Gcc; +Cc: peter0x44, Jonathan Wakely

* Jonathan Wakely via Gcc:

>> It seems it might even be trivial enough for me to investigate and
>> tackle myself, in some spare time.
>>
>> I see very little code using either of these features, so it's
>> definitely not a high priority task regardless.
>>
>
> Glibc uses the nonnull attribute in many places. Libstdc++ uses it in a few
> places.

Note that the glibc uses are frequently incompatible with libstdc++
(std::vector in particular).  Unfortunately, there is no consensus to
fix this.  For example, one issue is that

  memset(vec.data(), 0, sizeof(decltype(vec)::value_type) * vec.size())

isn't necessarily defined even for vectors of POD type.

Thanks,
Florian


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

* Re: Differences between clang and gcc handling of int[static n] function arguments
  2023-05-24  9:06   ` Florian Weimer
@ 2023-05-24  9:26     ` Jonathan Wakely
  2023-05-24  9:42       ` Florian Weimer
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2023-05-24  9:26 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Jonathan Wakely via Gcc, peter0x44

On Wed, 24 May 2023 at 10:06, Florian Weimer wrote:
>
> * Jonathan Wakely via Gcc:
>
> >> It seems it might even be trivial enough for me to investigate and
> >> tackle myself, in some spare time.
> >>
> >> I see very little code using either of these features, so it's
> >> definitely not a high priority task regardless.
> >>
> >
> > Glibc uses the nonnull attribute in many places. Libstdc++ uses it in a few
> > places.
>
> Note that the glibc uses are frequently incompatible with libstdc++

Do you mean incompatible with libstdc++ specifically, or incompatible
with "common C++ idioms"?

> (std::vector in particular).  Unfortunately, there is no consensus to
> fix this.  For example, one issue is that
>
>   memset(vec.data(), 0, sizeof(decltype(vec)::value_type) * vec.size())
>
> isn't necessarily defined even for vectors of POD type.

It's not really idiomatic C++ code anyway. Why would you write that, instead of:

vec.assign(vec.size(), {});
or:
std::fill_n(vec.begin(), vec.size(), range_value_t<decltype(vec)>:{});
?

(The latter optimizes to the ideal code, but I think I know how to
make the former optimize to the same thing.)

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

* Re: Differences between clang and gcc handling of int[static n] function arguments
  2023-05-24  9:26     ` Jonathan Wakely
@ 2023-05-24  9:42       ` Florian Weimer
  2023-05-24  9:51         ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2023-05-24  9:42 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Jonathan Wakely via Gcc, peter0x44

* Jonathan Wakely:

> On Wed, 24 May 2023 at 10:06, Florian Weimer wrote:
>>
>> * Jonathan Wakely via Gcc:
>>
>> >> It seems it might even be trivial enough for me to investigate and
>> >> tackle myself, in some spare time.
>> >>
>> >> I see very little code using either of these features, so it's
>> >> definitely not a high priority task regardless.
>> >>
>> >
>> > Glibc uses the nonnull attribute in many places. Libstdc++ uses it in a few
>> > places.
>>
>> Note that the glibc uses are frequently incompatible with libstdc++
>
> Do you mean incompatible with libstdc++ specifically, or incompatible
> with "common C++ idioms"?

The latter (common C++ idioms as encouraged by the standard library).

>> (std::vector in particular).  Unfortunately, there is no consensus to
>> fix this.  For example, one issue is that
>>
>>   memset(vec.data(), 0, sizeof(decltype(vec)::value_type) * vec.size())
>>
>> isn't necessarily defined even for vectors of POD type.
>
> It's not really idiomatic C++ code anyway. Why would you write that,
> instead of:
>
> vec.assign(vec.size(), {});
> or:
> std::fill_n(vec.begin(), vec.size(), range_value_t<decltype(vec)>:{});
> ?
>
> (The latter optimizes to the ideal code, but I think I know how to
> make the former optimize to the same thing.)

Well, one might be tempted to write that explicit memset to eliminate
the conditional branch, except that it's technically required because
data() might return nullptr. 8-/

Thanks,
Florian


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

* Re: Differences between clang and gcc handling of int[static n] function arguments
  2023-05-24  9:42       ` Florian Weimer
@ 2023-05-24  9:51         ` Jonathan Wakely
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2023-05-24  9:51 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Jonathan Wakely via Gcc, peter0x44

On Wed, 24 May 2023 at 10:42, Florian Weimer wrote:
> Well, one might be tempted to write that explicit memset to eliminate
> the conditional branch, except that it's technically required because
> data() might return nullptr. 8-/

Which is why you shouldn't write that code :-)

But this is a digression from the original point, which is that
nonnull attributes are used in glibc and libstdc++, which are widely
used. So improvements to diagnostics for the attribute would not be a
waste of time.

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

end of thread, other threads:[~2023-05-24  9:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-21 23:40 Differences between clang and gcc handling of int[static n] function arguments peter0x44
2023-05-23 17:47 ` Jonathan Wakely
2023-05-24  9:06   ` Florian Weimer
2023-05-24  9:26     ` Jonathan Wakely
2023-05-24  9:42       ` Florian Weimer
2023-05-24  9:51         ` Jonathan Wakely

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).