Page MenuHomePhabricator

[Sema] Perform call checking when building CXXNewExpr
Needs ReviewPublic

Authored by lebedev.ri on Mon, Jan 20, 1:57 AM.



There was even a TODO for this.
The main motivation is to make use of call-site based
__attribute__((alloc_align(param_idx))) validation (D72996).

Diff Detail

Event Timeline

lebedev.ri planned changes to this revision.Mon, Jan 20, 2:38 AM

Ah right, hmm, posted too soon.
I actually need to synthesize the size/align arguments.

Not only do we need to 'insert' size implicit argument,
but we may need to insert alignment implicit argument.
And while for size we can only insert nullptr,
for alignment we can actually insert the correct value.

Rebased, NFC.

rsmith added a comment.Mon, Feb 3, 6:39 PM

It would be really useful to provide these extra parameters to FindAllocationFunctions too. For example, we could support direct dispatch to a size-class-specific allocation function:

void *operator new(std::size_t n) __attribute__((enable_if(n == 16, "optimized allocator for 16-byte objects")));
14469 ↗(On Diff #240196)

This is wrong and may result in user-visible non-conformance; an expression of type std::align_val_t should never return true from isIntegerConstantExpr, because align_val_t is a scoped enumeration.

Can we instead change the caller (presumably the checking for the alloc_align attribute) to call a more general evaluation function rather than requiring an ICE?


If this is not an array new, or it's an array new with a constant array bound, we can pass in a correct size. For the remaining (runtime array bound) cases, please conjure up an OpaqueValueExpr as the corresponding argument here rather than working around the nullptr argument in EvaluateWithSubstitution.


Please put these on the stack rather than ASTContext-allocating them. Both of these classes already support stack allocation.

lebedev.ri marked 3 inline comments as done.

@rsmith Thank you for taking a look!
Addressed all review comments.

Looks good if you change the error to a warning.


Oh, I wasn't aware we gave an error for this case. We can't issue an error for passing a bad alignment to operator new -- it's valid for code to pass a bad alignment, but results in undefined behavior. The same is true for aligned_alloc and posix_memalign, so we should presumably just downgrade this from an error to a warning in general rather than (say) treating operator new as a special case.

For example, this is valid and may be reasonable in some cases:

template<int N> void *make() {
  if ((N & (N-1)) == 0)
    return operator new(N, std::align_val_t(N));
    return operator new(N);
void *p = make<7>();
lebedev.ri added inline comments.Tue, Feb 4, 12:59 PM

I'll post that as a followup patch here.

lebedev.ri marked 3 inline comments as done.
lebedev.ri added inline comments.

Posted D73996.
I'm not sure which cases should remain as error?

lebedev.ri marked an inline comment as done.Sun, Feb 9, 11:07 PM

Bump @rsmith