[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D33826#868185, @aaron.ballman wrote:

> In https://reviews.llvm.org/D33826#868167, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D33826#868085, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D33826#866170, @lebedev.ri wrote:
> > >
> > > > In https://reviews.llvm.org/D33826#866161, @JonasToth wrote:
> > > >
> > > > > There is an exception to the general rule (EXP36-C-EX2), stating that the result of `malloc` and friends is allowed to be casted to stricter alignments, since the pointer is known to be of correct alignment.
> > > >
> > > >
> > > > Quote for the reference:
> > > >
> > > > > EXP36-C-EX2: If a pointer is known to be correctly aligned to the target type, then a cast to that type is permitted. There are several cases where a pointer is known to be correctly aligned to the target type. The pointer could point to an object declared with a suitable alignment specifier. It could point to an object returned by aligned_alloc(), calloc(), malloc(), or realloc(), as per the C standard, section 7.22.3, paragraph 1  [ISO/IEC 9899:2011].
> > > >
> > > > For plain `calloc(), malloc(), or realloc()`, i would guess it's related to `max_align_t` / `std::max_align_t` / `__STDCPP_DEFAULT_NEW_ALIGNMENT__`, which is generally just `16` bytes.
> > >
> > >
> > > It's a requirement from the C standard that malloc, calloc, and realloc return a suitably-aligned pointer for *any* type.
> >
> >
> > We are probably arguing about the details, or i'm just misguided, but you mean any *standard* type, right?
> >
> >   MALLOC(3)
> >    ...
> >    The  malloc()  and calloc() functions return a pointer to the allocated memory, which is suitably aligned for any built-in type.
> >   
> >
> > E.g. `__m256` needs to be 32-byte aligned, and  user type, with the help of `alignas()`, can have different alignment requirements
>
>
> C11 7.22.3p1 (in part):
>
>   The pointer returned if the allocation succeeds is suitably aligned so that it may be assigned to
>    a pointer to any type of object with a fundamental alignment requirement and then used
>    to access such an object or an array of such objects in the space allocated (until the space
>    is explicitly deallocated). 
>
> C11 6.2.8p2 defines fundamental alignment:
>
>   A fundamental alignment is represented by an alignment less than or equal to the greatest
>    alignment supported by the implementation in all contexts, which is equal to
>    _Alignof (max_align_t).
>
> So I don't think this applies to only builtin types. Which makes sense, otherwise you couldn't rely on malloc to do the correct thing with `struct S { char c; int i; }; struct S s = (struct S *)malloc(sizeof(struct S));` However, you are correct that it doesn't need to return a suitably aligned pointer for *any* type.


I agree!
Thus, my point being, *if* the cast in question is not from `void*` (if it is, i don't think it is possible to warn, or prevent the warning for that matter),
shouldn't the alignment requirements of the target type be considered, even if the pointer was just returned by `malloc` and friends?

>> 
>> 
>>>>> Could you add a testcase for this case, i think there would currenlty be a false positive.
>>>>> 
>>>>> And is there a general way of knowing when the pointer is of correct alignment, or is it necessary to keep a list of functions like `malloc` that are just known? 
>>>>>  If yes, i think it would be nice if this list is configurable (maybe like in cppcoreguidelines-no-malloc, where that functionality could be refactored out).
>>> 
>>> Agreed, this should be a configurable list, but is should be pre-populated with the listed functions from the CERT standard.


Repository:
  rL LLVM

https://reviews.llvm.org/D33826