Re: BPF selftests and strict aliasing

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

> On 1/29/24 10:43 AM, Jose E. Marchesi wrote:
>>> On 1/29/24 9:05 AM, Jose E. Marchesi wrote:
>>>>> On Sun, 2024-01-28 at 21:33 -0800, Yonghong Song wrote:
>>>>> [...]
>>>>>> I tried below example with the above prog/dynptr_fail.c case with gcc 11.4
>>>>>> for native x86 target and didn't trigger the warning. Maybe this requires
>>>>>> latest gcc? Or test C file is not sufficient enough to trigger the warning?
>>>>>>
>>>>>> [~/tmp1]$ cat t.c
>>>>>> struct t {
>>>>>>      char a;
>>>>>>      short b;
>>>>>>      int c;
>>>>>> };
>>>>>> void init(struct t *);
>>>>>> long foo() {
>>>>>>      struct t dummy;
>>>>>>      init(&dummy);
>>>>>>      return *(int *)&dummy;
>>>>>> }
>>>>>> [~/tmp1]$ gcc -Wall -Werror -O2 -g -Wno-compare-distinct-pointer-types -c t.c
>>>>>> [~/tmp1]$ gcc --version
>>>>>> gcc (GCC) 11.4.1 20230605 (Red Hat 11.4.1-2)
>>>>> I managed to trigger this warning for gcc 13.2.1:
>>>>>
>>>>>       $ gcc -fstrict-aliasing -Wstrict-aliasing=1 -c test-punning.c -o /dev/null
>>>>>       test-punning.c: In function ‘foo’:
>>>>>       test-punning.c:10:19: warning: dereferencing type-punned pointer might break strict-aliasing rules [-Wstrict-aliasing]
>>>>>          10 |    return *(int *)&dummy;
>>>>>             |                   ^~~~~~
>>>>>       Note the -Wstrict-aliasing=1 option, w/o =1 suffix it does not
>>>>> trigger.
>>>>>
>>>>> Grepping words "strict-aliasing", "strictaliasing", "strict_aliasing"
>>>>> through clang code-base does not show any diagnostic related tests or
>>>>> detection logic. It appears to me clang does not warn about strict
>>>>> aliasing violations at all and -Wstrict-aliasing=* are just stubs at
>>>>> the moment.
>>>> Detecting strict aliasing violations can only be done by looking at
>>>> particular code constructions (casts immediately followed by
>>>> dereferencing for example) so GCC provides these three levels: 1, 2, and
>>>> 3 which is the default.  Level 1 can result in false positives (hence
>>>> the "might" in the warning message) while higher levels have less false
>>>> positives, but will likely miss lots of real positives.
>>> clang has not implemented this yet.
>>>
>>>> In this case, it seems to me clear that a pointer to int does not alias
>>>> a pointer to struct t.  So I would say, in this little program
>>>> strict-aliasing=1 catches a real positive, while strict-aliasing=3
>>>> misses a real positive.
>>> This make sense. But could you pose the exact bpf compilation command
>>> line which triggers strict-aliasing warning? Does the compiler command
>>> line have -fstrict-aliasing?
>> In GCC -fstrict-aliasing is activated at levels -O2, -O3 and -Os.  From
>> a quick look at Clang.cpp, I _think_ it generally assumes strict
>> aliasing when optimizing except when it tries to be compatible with
>> Visual Studio C++ compilers (that clang-cl driver thingie.)
>
> I double checked again. You are right, -fno-strict-aliasing does work
> to disable strict-aliasing. Looks like clang also has -fstrict-alaising
> as the default if optimization level is not O0. But somehow, clang
> did not issue warnings...
>
>>
>>  From the GCC manual:
>>
>> '-fstrict-aliasing'
>>       Allow the compiler to assume the strictest aliasing rules
>>       applicable to the language being compiled.  For C (and C++), this
>>       activates optimizations based on the type of expressions.  In
>>       particular, an object of one type is assumed never to reside at the
>>       same address as an object of a different type, unless the types are
>>       almost the same.  For example, an 'unsigned int' can alias an
>>       'int', but not a 'void*' or a 'double'.  A character type may alias
>>       any other type.
>>
>>       Pay special attention to code like this:
>>            union a_union {
>>              int i;
>>              double d;
>>            };
>>
>>            int f() {
>>              union a_union t;
>>              t.d = 3.0;
>>              return t.i;
>>            }
>>       The practice of reading from a different union member than the one
>>       most recently written to (called "type-punning") is common.  Even
>>       with '-fstrict-aliasing', type-punning is allowed, provided the
>>       memory is accessed through the union type.  So, the code above
>>       works as expected.  *Note Structures unions enumerations and
>>       bit-fields implementation::.  However, this code might not:
>>            int f() {
>>              union a_union t;
>>              int* ip;
>>              t.d = 3.0;
>>              ip = &t.i;
>>              return *ip;
>>            }
>>
>>       Similarly, access by taking the address, casting the resulting
>>       pointer and dereferencing the result has undefined behavior, even
>
> This is an alarm since enabling -fstrict-aliasing may produce
> undefined behavior if the code is written in a strange way which
> violates some casting rules. So -fno-strict-aliasing is the
> right solution to address this potential undefined behavior.
> We probably should not recommend -fno-strict-aliasing sicne it
> may hurt performance and production bpf programs should be
> more type friendly.
>
> So I think your option (b) sounds good. Thanks!

Will send a patch tomorrow.
Thanks for the feedback.

>
>>       if the cast uses a union type, e.g.:
>>            int f() {
>>              double d = 3.0;
>>              return ((union a_union *) &d)->i;
>>            }
>>
>>       The '-fstrict-aliasing' option is enabled at levels '-O2', '-O3',
>>       '-Os'.