On Tue, Sep 12, 2017 at 1:02 PM, Roman Lebedev via Phabricator <revi...@reviews.llvm.org> wrote: > 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?
Ah, I think I'm catching on to the point you're raising (thank you for the patience). If the return type is void *, we don't have enough information to sensibly diagnose (not without data flow analysis to determine where the original value came from, anyway), and if the return type is not void *, we should be checking the alignment anyway. That suggests we don't need the exception in this check at all, unless we find extant implementations that return something other than void *. ~Aaron > >>> >>> >>>>>> 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 > > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits