Commit Graph

5 Commits

Author SHA1 Message Date
Roman Lebedev 9ea5d17cc9
[Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning
Summary:
As @rsmith notes in https://reviews.llvm.org/D73020#inline-672219
while that is certainly UB land, it may not be actually reachable at runtime, e.g.:
```
template<int N> void *make() {
  if ((N & (N-1)) == 0)
    return operator new(N, std::align_val_t(N));
  else
    return operator new(N);
}
void *p = make<7>();
```
and we shouldn't really error-out there.

That being said, i'm not really following the logic here.
Which ones of these cases should remain being an error?

Reviewers: rsmith, erichkeane

Reviewed By: erichkeane

Subscribers: cfe-commits, rsmith

Tags: #clang

Differential Revision: https://reviews.llvm.org/D73996
2020-02-20 16:39:26 +03:00
Roman Lebedev ba545c814b
[Sema] Try 2: Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation
Summary:
`alloc_align` attribute takes parameter number, not the alignment itself,
so given **just** the attribute/function declaration we can't do any
sanity checking for said alignment.

However, at call site, given the actual `Expr` that is passed
into that parameter, we //might// be able to evaluate said `Expr`
as Integer Constant Expression, and perform the sanity checks.
But since there is no requirement for that argument to be an immediate,
we may fail, and that's okay.

However if we did evaluate, we should enforce the same constraints
as with `__builtin_assume_aligned()`/`__attribute__((assume_aligned(imm)))`:
said alignment is a power of two, and is not greater than our magic threshold


This was initially committed in c2a9061ac5
but reverted in 00756b1823 because of
suspicious bot failures.

Reviewers: erichkeane, aaron.ballman, hfinkel, rsmith, jdoerfert

Reviewed By: erichkeane

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D72996
2020-01-24 14:42:45 +03:00
Roman Lebedev 00756b1823
Revert "[Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation"
Likely makes bots angry.

This reverts commit c2a9061ac5.
2020-01-23 23:10:34 +03:00
Roman Lebedev c2a9061ac5
[Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation
Summary:
`alloc_align` attribute takes parameter number, not the alignment itself,
so given **just** the attribute/function declaration we can't do any
sanity checking for said alignment.

However, at call site, given the actual `Expr` that is passed
into that parameter, we //might// be able to evaluate said `Expr`
as Integer Constant Expression, and perform the sanity checks.
But since there is no requirement for that argument to be an immediate,
we may fail, and that's okay.

However if we did evaluate, we should enforce the same constraints
as with `__builtin_assume_aligned()`/`__attribute__((assume_aligned(imm)))`:
said alignment is a power of two, and is not greater than our magic threshold

Reviewers: erichkeane, aaron.ballman, hfinkel, rsmith, jdoerfert

Reviewed By: erichkeane

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D72996
2020-01-23 22:50:49 +03:00
Erich Keane 623efd8a75 Clang changes for alloc_align attribute
GCC has the alloc_align attribute, which is similar to assume_aligned, except the attribute's parameter is the index of the integer parameter that needs aligning to.

Differential Revision: https://reviews.llvm.org/D29599

llvm-svn: 299117
2017-03-30 21:48:55 +00:00