Re: [PATCH] cmd/mem.c: fix undefined behavior in mem cmp

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Rasmus,

On 9/30/24 11:54 AM, Rasmus Villemoes wrote:
> Quentin Schulz <quentin.schulz@cherry.de> writes:
> 
>> Hi Rasmus,
>>
>> On 9/27/24 8:56 PM, Rasmus Villemoes wrote:
>>> Quentin Schulz <foss+uboot@0leil.net> writes:
>>>
>>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>>
>>>> My linter complains that "When using void pointers in calculations, the
>>>> behaviour is undefined".
>>>>
>>>> GCC does say that "In GNU C, addition and subtraction operations are
>>>> supported on pointers to void"[1] but this hints at this only being
>>>> supported in the GNU flavor of C. And I assume U-Boot may want to be
>>>> compiled with clang/llvm?
>>>>
>>>> Let's fix that warning by casting the void pointer to a u8 pointer since
>>>> the size variable unit is byte.
>>>>
>>>> [1] https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html
>>>>
>>> No, let's please not. Try enabling -Wpointer-arith and see how much
>>> churn that would require all over the tree (doing it in this one place
>>> would be pointless), and all the casts would make the code much much
>>> harder to read.
>>>
>>
>> That alone is not a justification.
>>
>>> We do rely on lots of gcc extensions, and Clang has documented that it
>>> "aims to support a broad range of GCC extensions". Arithmetic on void is
>>> one of them, and that's not going to go away.
>>>
>>
>> But this very well could/is.
>>
>> 1) is there a way to tell Clang that we want to follow this GNU
>> extension for that case?
> 
> For that case? No. But we do -std=gnu11 which is understood by both
> compilers and tells clang that we do use GNU extensions. I don't know
> the clang internals, but I strongly suspect that that flag would make
> sure that defaults for various warnings are set appropriately (that is,
> not warning for something which is explicitly guaranteed to work in GNU
> C).
> 
>> Is that the default?
> 
> In our build, yes. See CSTD_FLAG. But both gcc and clang default to some
> -std=gnuXX, with the exact value of XX depending on compiler version.
> 
>> 2) do we document somewhere the GNU C extensions we use on the whole
>> tree?
> 
> Not that I know of explicitly, but CSTD_FLAG=-std=gnu11 kind of
> documents that gnu extensions are used.
> 
> Note that typeof() and inline asm() statements are also gnu extensions
> that we make heavy use of, as are various __attribute__(()), statement
> expressions (i.e. ({ stuff; }) ), omitting the middle expression in ?: ,
> special handling of token paste ## after a comma, case ranges 'case LOW
> ... HIGH', etc. etc.
> 

Ack, thanks for the info!

>>
>> 3) is anyone aware of a way to silence some specific warnings at the
>> project level so that clang takes care of it so that linters use those
>> settings so we avoid people sending more commits for that "issue"?
> 
> I don't quite parse that sentence. Are you saying that your linter is
> being invoked by clang, and you want clang to pass certain options to
> the linter to disable certain warnings? I don't know how clang could
> know that options those would be.
> 

My understanding is that my linter is calling clang. So I thought it 
would be a nice thing to have a way to specify options for clang at the 
root of the project so that any linter "wrapping" clang would use those 
options and not warn/error on things we don't want to check.

I spent 5min looking for it and it seems we already have that: 
https://docs.u-boot.org/en/latest/build/gen_compile_commands.html

After running that script, my linter is happy.

Thanks!
Cheers,
Quentin