Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

[Philippe Mathieu-Daudé]

On 01/17/2018 11:56 AM, Eric Blake wrote:
> On 01/17/2018 08:39 AM, Daniel P. Berrange wrote:
> 
>>>>
>>>> GCC may or may not warn you about passing NULL for the 'bar'
>>>> parameter, but it will none the less assume nothing passes
>>>> NULL, and thus remove the 'if (!bar)' conditional during
>>>> optimization. IOW, adding nonnull annotations can actually
>>>> make your code less robust :-(
> 
> The gcc bug mentioned in the libvirt code says that newer gcc may be
> smarter than when we added the libvirt workaround:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308
> 
> But I haven't played with it in a long time (the sour experience with
> misoptimized code with no warning has made me wary of re-trying).

I have been there...

>>>
>>> Why do you use __attribute__(()) ? Isn't this enough:
>>
>> No idea offhand - Eric wrote this so perhaps he had a reason for that
>> else branch style.
>>
>>>
>>> #if defined __clang_analyzer__ || defined __COVERITY__
>>> #define QEMU_STATIC_ANALYSIS 1
>>> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
>>> +#else
>>> +#define QEMU_WARN_NONNULL_ARGS(args...)
>>> #endif
> 
> My reasoning for using the empty attribute was to at least ensure that
> the gcc compilation agrees that the annotation is syntactically valid
> (nothing worse than sticking an __attribute__ code in a place that gcc
> doesn't like, but only for static checker builds, so you don't learn
> about it right away).  Defining the macro to nothing, rather than an
> empty attribute, makes it harder for the common-case compilation to
> detect placement problems.

I wouldn't have thought of that, thanks!

Phil.

signature.asc
Description: OpenPGP digital signature