[Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[EricWF]

Summary

Libc++ needs to know when aligned allocation is supported by clang, but is otherwise unavailable at link time. Otherwise, libc++ will incorrectly end up generating calls to __builtin_operator_new/__builtin_operator_delete which alignment arguments.

This patch implements the following changes:

• The __cpp_aligned_new feature test macro to no longer be defined when aligned allocation is otherwise enabled but unavailable.

• The Darwin driver no longer passes -faligned-alloc-unavailable when the user manually specifies any of -faligned-allocation, -fno-aligned-allocation, or -nostdinc++. (Though I'm not sure we should this for -nostdinc++ since it makes this behavior to test within libc++, since the libc++ tests must always pass -nostdinc++).

• Instead of a warning Clang now generates a hard error when an aligned allocation or deallocation function is referenced but unavailable.

[EricWF]

Ping. I would like to get a way for libc++ to detect this case before the next release.

[vsapsai]

Another approach is __has_feature but I don't think it is applicable in this case.

Is there a way right now to detect that aligned allocation is supported by clang, regardless of link time? Asking to make sure we are consistent.

[EricWF]

In D45015#1064922, @vsapsai wrote:

Another approach is __has_feature but I don't think it is applicable in this case.

Is there a way right now to detect that aligned allocation is supported by clang, regardless of link time? Asking to make sure we are consistent.

There is a C++ feature test macro, as specified by the standard, __cpp_aligned_new. But I'm not sure how to be consistent with that.

[vsapsai]

In D45015#1064930, @EricWF wrote:

In D45015#1064922, @vsapsai wrote:

Another approach is __has_feature but I don't think it is applicable in this case.

Is there a way right now to detect that aligned allocation is supported by clang, regardless of link time? Asking to make sure we are consistent.

There is a C++ feature test macro, as specified by the standard, __cpp_aligned_new. But I'm not sure how to be consistent with that.

After more thinking I believe a predefined macro is the best option. And in spirit it is consistent with SD-6: SG10 Feature Test Recommendations.

As for the naming, I see it is consistent with -faligned-alloc-unavailable but for me it is hard to tell immediately if "unavailable" means "unsupported at all" or "supported but unavailable on the target". Maybe it is just me and others don't have such problem but probably including "runtime" in the macro name would help. Something like __ALIGNED_ALLOCATION_UNAVAILABLE_RUNTIME__.

For the overall aligned allocation plan overall. We also need to keep in mind users who provide their own aligned allocation / deallocation functions, so they can still do that. So far everything seems to be OK but we need to be careful.

lib/Frontend/InitPreprocessor.cpp

1059–1060 ↗

(On Diff #140181)

Don't know what the macro will be in the end, please consider adding a comment clarifying runtime availability.

test/Preprocessor/predefined-macros.c

288 ↗

(On Diff #140181)

Would it be useful to test -fno-aligned-allocation too? Extensive testing in different combinations doesn't add much value but -std=c++17 -fno-aligned-allocation can be useful.

[vsapsai]

Eric, do you have more thoughts on this issue?

[rsmith]

Hmm, perhaps our strategy for handling aligned allocation on Darwin should be revisited. We shouldn't be defining __cpp_aligned_allocation if we believe it doesn't work -- that will break code that uses aligned allocation where available and falls back to something else where it's unavailable.

@ahatanak: how about this:

• Change the driver to not pass -faligned-alloc-unavailable if an explicit -faligned-allocation or -fno-aligned-allocation flag is given; update Clang's note to suggest explicitly passing -faligned-allocation rather than -Wno-aligned-alloc-unavailable if the user provides their own aligned allocation function.
• Change -faligned-alloc-unavailable so that it does not define __cpp_aligned_allocation.
• Change Sema's handling of the "aligned allocation unavailable" case so that, after warning on selecting an aligned allocation function, it then removes those functions from the candidate set and tries again.

That is: on old Darwin, we should not define __cpp_aligned_allocation (even in C++17), produce the "no aligned allocation support" warning in C++17 mode, and then not try to call the aligned allocation function. But if -faligned-allocation or -fno-aligned-allocation is specified explicitly, then the user knows what they're doing and they get no warning.

[vsapsai]

In D45015#1105314, @rsmith wrote:

That is: on old Darwin, we should not define __cpp_aligned_allocation (even in C++17), produce the "no aligned allocation support" warning in C++17 mode, and then not try to call the aligned allocation function. But if -faligned-allocation or -fno-aligned-allocation is specified explicitly, then the user knows what they're doing and they get no warning.

What when compiler has __builtin_operator_new, __builtin_operator_delete? If I build libc++ tests with recent Clang which has these builtins and run tests with libc++.dylib from old Darwin, there are no linkage errors. Should we define __cpp_aligned_allocation in this case?

[rsmith]

In D45015#1105372, @vsapsai wrote:

What when compiler has __builtin_operator_new, __builtin_operator_delete? If I build libc++ tests with recent Clang which has these builtins and run tests with libc++.dylib from old Darwin, there are no linkage errors. Should we define __cpp_aligned_allocation in this case?

I don't see why that should make any difference -- those builtins call the same functions that the new and delete operator call. Perhaps libc++ isn't calling the forms of those builtins that take an alignment argument yet?

[ahatanak]

In D45015#1105314, @rsmith wrote:

Hmm, perhaps our strategy for handling aligned allocation on Darwin should be revisited. We shouldn't be defining __cpp_aligned_allocation if we believe it doesn't work -- that will break code that uses aligned allocation where available and falls back to something else where it's unavailable.

@ahatanak: how about this:

• Change the driver to not pass -faligned-alloc-unavailable if an explicit -faligned-allocation or -fno-aligned-allocation flag is given; update Clang's note to suggest explicitly passing -faligned-allocation rather than -Wno-aligned-alloc-unavailable if the user provides their own aligned allocation function.
• Change -faligned-alloc-unavailable so that it does not define __cpp_aligned_allocation.
• Change Sema's handling of the "aligned allocation unavailable" case so that, after warning on selecting an aligned allocation function, it then removes those functions from the candidate set and tries again.

That is: on old Darwin, we should not define __cpp_aligned_allocation (even in C++17), produce the "no aligned allocation support" warning in C++17 mode, and then not try to call the aligned allocation function. But if -faligned-allocation or -fno-aligned-allocation is specified explicitly, then the user knows what they're doing and they get no warning.

I talked with @vsapsai today and we think this looks like a better idea.

A couple of questions:

• Currently clang errors out when aligned operator new is selected but the OS's version is too old to support it. What's the reason we want to change this now to be a warning rather than an error?

• So clang no longer needs to define macro __ALIGNED_ALLOCATION_UNAVAILABLE__ and libc++ will use __cpp_aligned_new (I think you meant __cpp_aligned_new, not __cpp_aligned_allocation?) to determine whether aligned allocation functions should be defined or made available in the header?

[vsapsai]

Somewhat tangential, in discussion with Duncan he mentioned that -nostdinc++ should turn off assumptions about old Darwin. So if you build libc++ yourself, you don't care what does the system stdlib support. I agree with that and think it doesn't interfere with the latest proposal but adds more to it.

[rsmith]

In D45015#1109049, @ahatanak wrote:

• Currently clang errors out when aligned operator new is selected but the OS's version is too old to support it. What's the reason we want to change this now to be a warning rather than an error?

I think it's fine to leave it as a DefaultError warning. I think it'd also be fine to change it to simply be an error (rather than an error-with-warning-flag) and remove the error recovery, and that actually seems a bit better: then our behavior under -faligned-alloc-unavailable would be equivalent to treating the implicit aligned forms of operator new/delete as if they had an availability attribute on them by default, which I think seems very reasonable.

• So clang no longer needs to define macro __ALIGNED_ALLOCATION_UNAVAILABLE__ and libc++ will use __cpp_aligned_new (I think you meant __cpp_aligned_new, not __cpp_aligned_allocation?) to determine whether aligned allocation functions should be defined or made available in the header?

Yes, that's the idea. __cpp_aligned_new exists to allow code to conditionally use aligned new if it's available, and we should do our best to honor that.

[ahatanak]

In D45015#1105388, @rsmith wrote:

In D45015#1105372, @vsapsai wrote:

What when compiler has __builtin_operator_new, __builtin_operator_delete? If I build libc++ tests with recent Clang which has these builtins and run tests with libc++.dylib from old Darwin, there are no linkage errors. Should we define __cpp_aligned_allocation in this case?

I don't see why that should make any difference -- those builtins call the same functions that the new and delete operator call. Perhaps libc++ isn't calling the forms of those builtins that take an alignment argument yet?

It looks like clang currently doesn't issue a warning when a call to builtin_operator_new or builtin_operator_delete calls an aligned allocation function that is not support by the OS version. I suppose we should fix this?

// no warning issued when triple is "thumbv7-apple-ios5.0.0" even though aligned allocation is unavailable.
void *p = __builtin_operator_new(100, std::align_val_t(32));

[ahatanak]

I think clang should error out or warn when aligned operator or builtin operator new/delete functions are used when they are not available (r306722 should have handled them).

I'm also not sure not defining __cpp_aligned_new is sufficient. My understanding is that if __cpp_aligned_new is not defined, libc++ will define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION, which will cause the aligned new/delete functions declared in <new> to be removed. That is fine if the new/delete function that is used in a program is one of the function implicitly declared by the compiler (e.g., 'void operator delete ( void*, std::align_val_t)'), but if it isn't one of those functions (e.g., 'void operator delete (void *, std::align_val_t, const std::nothrow_t&)'), clang will produce diagnostics that are not very informative to the user (error: no matching function for call to 'operator delete') instead of the one added in r306722 (aligned allocation function of type is only available on ...).

For example:

#include <new>
  
void foo1(void *p, std::align_val_t a, std::nothrow_t t) {
  operator delete(p, a, t);
}

[EricWF]

The new path forward sounds good to me. I'll work on implementing it.

[EricWF]

In D45015#1119286, @ahatanak wrote:

In D45015#1105388, @rsmith wrote:

In D45015#1105372, @vsapsai wrote:

What when compiler has __builtin_operator_new, __builtin_operator_delete? If I build libc++ tests with recent Clang which has these builtins and run tests with libc++.dylib from old Darwin, there are no linkage errors. Should we define __cpp_aligned_allocation in this case?

I don't see why that should make any difference -- those builtins call the same functions that the new and delete operator call. Perhaps libc++ isn't calling the forms of those builtins that take an alignment argument yet?

It looks like clang currently doesn't issue a warning when a call to builtin_operator_new or builtin_operator_delete calls an aligned allocation function that is not support by the OS version. I suppose we should fix this?

// no warning issued when triple is "thumbv7-apple-ios5.0.0" even though aligned allocation is unavailable.
void *p = __builtin_operator_new(100, std::align_val_t(32));

If we switch to no longer defining __cpp_aligned_new when it's unavailable that means libc++ shouldn't generate calls to an aligned allocation or deallocation function. Do we need the compiler to protect libc++ from itself here?

[EricWF]

Update the patch with the form suggested in the previous conversation. See the updated summary for a description of the behavior.

The Darwin driver no longer passes -faligned-alloc-unavailable when -nostdinc++ is specified, but I'm not sure this is the behavior
we want. Having -nostdinc++ have side-effects which affect the well-formedness of new/delete expressions seems non-obvious and potentially problematic.

[ahatanak]

Could you elaborate on what kind of changes you are planning to make in libc++ after committing this patch?

[EricWF]

In D45015#1121581, @ahatanak wrote:

Could you elaborate on what kind of changes you are planning to make in libc++ after committing this patch?

Libc++ shouldn't actually need any changes if this current patch lands. Currently libc++ is in a "incorrect" state where
it generates calls to __builtin_operator_new(size_t, align_val_t) when __cpp_aligned_new is defined but when aligned new/delete
are actually unavailable.

If we change __cpp_aligned_new to no longer be defined when aligned new is unavailable, then libc++ will start doing the right thing.
See r328180 for the relevent commits which made these libc++ changes.

[ahatanak]

I see, thank you.

clang front-end currently fails to issue a warning or error when an aligned allocation/deallocation functions are required but not available in a few cases (e.g., delete called from a deleting destructor, calls to operator or builtin operator new/delete). I suppose those bugs should be fixed in separate patches.

[EricWF]

In D45015#1121874, @ahatanak wrote:

I see, thank you.

clang front-end currently fails to issue a warning or error when an aligned allocation/deallocation functions are required but not available in a few cases (e.g., delete called from a deleting destructor, calls to operator or builtin operator new/delete). I suppose those bugs should be fixed in separate patches.

I don't think we need to emit warnings from __builtin_operator_new/__builtin_operator_delete. Libc++ is the only consumer, and I think we can trust it to know what it's doing.

[ahatanak]

In D45015#1123097, @EricWF wrote:

In D45015#1121874, @ahatanak wrote:

I see, thank you.

clang front-end currently fails to issue a warning or error when an aligned allocation/deallocation functions are required but not available in a few cases (e.g., delete called from a deleting destructor, calls to operator or builtin operator new/delete). I suppose those bugs should be fixed in separate patches.

I don't think we need to emit warnings from __builtin_operator_new/__builtin_operator_delete. Libc++ is the only consumer, and I think we can trust it to know what it's doing.

Shouldn't clang warn when users explicitly call an aligned builtin operator new or delete in their code and the OS is too old to support the operator?

For example:

typedef __SIZE_TYPE__ size_t;
namespace std {
enum class align_val_t : size_t {};
}

int main() {
  void *p = __builtin_operator_new(100, std::align_val_t(32));
  return 0;
}

[EricWF]

In D45015#1123164, @ahatanak wrote:

In D45015#1123097, @EricWF wrote:

In D45015#1121874, @ahatanak wrote:

I see, thank you.

clang front-end currently fails to issue a warning or error when an aligned allocation/deallocation functions are required but not available in a few cases (e.g., delete called from a deleting destructor, calls to operator or builtin operator new/delete). I suppose those bugs should be fixed in separate patches.

I don't think we need to emit warnings from __builtin_operator_new/__builtin_operator_delete. Libc++ is the only consumer, and I think we can trust it to know what it's doing.

Shouldn't clang warn when users explicitly call an aligned builtin operator new or delete in their code and the OS is too old to support the operator?

For example:

typedef __SIZE_TYPE__ size_t;
namespace std {
enum class align_val_t : size_t {};
}

int main() {
  void *p = __builtin_operator_new(100, std::align_val_t(32));
  return 0;
}

Yeah, I think you're right. Initially I thought __builtin_operator_new was documented as being intended for usage only in the STL, but it doesn't quite say that.
I have a patch which moves the checks into DiagnoseUseOfDecl, which should catch all of the cases we haven't already handled.

[vsapsai]

Sorry for the churn but can you please take out -nostdinc++ part out of this change? After more thinking and discussion we think there is a chance developers can use -nostdinc++ not only for building the standard library. -nostdinc++ is a signal of building the standard library but it's not strong enough. Most likely developers will be unaware how -nostdinc++ affects aligned allocations and that can lead to linker failures at runtime. So if you deploy on older macOS versions it is safer to assume aligned allocation isn't available. But if you provide your own libc++ with aligned allocations, you can still use -faligned-allocation to make that work. For now we prefer to use that approach and if it proves to be too onerous for developers, we can reintroduce -nostdinc++ logic.

For building libc++ we rely on _LIBCPP_BUILDING_NEW to have support for aligned allocations even though the build host doesn't support them. So we don't need to change libc++ build.

[vsapsai]

In D45015#1121762, @EricWF wrote:

In D45015#1121581, @ahatanak wrote:

Could you elaborate on what kind of changes you are planning to make in libc++ after committing this patch?

Libc++ shouldn't actually need any changes if this current patch lands. Currently libc++ is in a "incorrect" state where
it generates calls to __builtin_operator_new(size_t, align_val_t) when __cpp_aligned_new is defined but when aligned new/delete
are actually unavailable.

If we change __cpp_aligned_new to no longer be defined when aligned new is unavailable, then libc++ will start doing the right thing.
See r328180 for the relevent commits which made these libc++ changes.

Looks like in libc++ we need to remove _LIBCPP_STD_VER check for aligned allocations, something like

 #if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
-    (!(defined(_LIBCPP_BUILDING_NEW) || _LIBCPP_STD_VER > 14 || \
+    (!(defined(_LIBCPP_BUILDING_NEW) || \
     (defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606)))
 # define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
 #endif

Is that correct? I didn't check the rest of the code, probably TEST_HAS_NO_ALIGNED_ALLOCATION needs some clean up too.

[EricWF]

In D45015#1127046, @vsapsai wrote:

In D45015#1121762, @EricWF wrote:

In D45015#1121581, @ahatanak wrote:

Could you elaborate on what kind of changes you are planning to make in libc++ after committing this patch?

Libc++ shouldn't actually need any changes if this current patch lands. Currently libc++ is in a "incorrect" state where
it generates calls to __builtin_operator_new(size_t, align_val_t) when __cpp_aligned_new is defined but when aligned new/delete
are actually unavailable.

If we change __cpp_aligned_new to no longer be defined when aligned new is unavailable, then libc++ will start doing the right thing.
See r328180 for the relevent commits which made these libc++ changes.

Looks like in libc++ we need to remove _LIBCPP_STD_VER check for aligned allocations, something like

 #if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
-    (!(defined(_LIBCPP_BUILDING_NEW) || _LIBCPP_STD_VER > 14 || \
+    (!(defined(_LIBCPP_BUILDING_NEW) || \
     (defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606)))
 # define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
 #endif

Is that correct? I didn't check the rest of the code, probably TEST_HAS_NO_ALIGNED_ALLOCATION needs some clean up too.

Huh, that original formulation is correct-ish, and so is your proposed changes. There are two problems here:

1. We want to expose the aligned allocation declarations to users in C++17, even if the compiler will never call them because

the feature has been disabled.

2. We don't want to generate calls to them via __builtin_operator_new when we've only enabled the declarations because of C++17.

I'll make some changes to libc++ to address this. Thanks for pointing it out.

[EricWF]

Remove -nostdinc++ check as requested.

[vsapsai]

With this change and the mentioned libc++ change the tests with old libc++ dylib are passing (didn't test all possible configurations though). Would like to get more feedback from other reviewers on this matter.

[EricWF]

@rsmith Ping. This needs to land before the next branch for release.

[EricWF]

Ping.

Are there any more reviewers I should add to this?

[• Quuxplusone]

include/clang/Basic/DiagnosticSemaKinds.td

6425 ↗

(On Diff #150635)

I observe that this mnemonic is misspelled; but that's not this patch's fault.
The traditional spelling would probably be just note_aligned_allocation_unavailable.

lib/Driver/ToolChains/Darwin.cpp

2027 ↗

(On Diff #150635)

Peanut gallery asks: Why is the cc1 option spelled differently from the clang driver option? Don't they do the same thing?

[rsmith]

lib/Driver/ToolChains/Darwin.cpp

2027 ↗

(On Diff #150635)

At the -cc1 level, there are three different levels:

• no aligned allocation in the language
• aligned allocation in the language but it doesn't work (eg, C++17 on old Darwin)
• aligned allocation in the language and it works

The driver aligned allocation flags force the compilation into case 1 or 3. Cases 1 and 2 do not advertise __cpp_aligned_new. Case 2 provides an error if an aligned allocation is attempted, whereas case 1 provides an underaligned allocation with a warning. This flag puts us into case 2, so it isn't the same as any driver flag.

[vsapsai]

Eric, will you have time to commit this patch? If you don't have time and don't have objections, I plan to land this change on your behalf.

[EricWF]

Hi, I'm in the process of moving cities. please take over.

Thanks, and sorry

[vsapsai]

In D45015#1188141, @EricWF wrote:

Hi, I'm in the process of moving cities. please take over.

Thanks, and sorry

No worries. Thanks for spending time on this issue and making the fix, committing is the easy part. Good luck with the move.

cfe/trunk/include/clang/Basic/DiagnosticGroups.td

 def NonNull : DiagGroup<"nonnull">;
 def NonPODVarargs : DiagGroup<"non-pod-varargs">;
 def ClassVarargs : DiagGroup<"class-varargs", [NonPODVarargs]>;
 def : DiagGroup<"nonportable-cfstrings">;
 def NonVirtualDtor : DiagGroup<"non-virtual-dtor">;
 def NullPointerArithmetic : DiagGroup<"null-pointer-arithmetic">;
 def : DiagGroup<"effc++", [NonVirtualDtor]>;
 def OveralignedType : DiagGroup<"over-aligned">;
-def AlignedAllocationUnavailable : DiagGroup<"aligned-allocation-unavailable">;
 def OldStyleCast : DiagGroup<"old-style-cast">;
 def : DiagGroup<"old-style-definition">;
 def OutOfLineDeclaration : DiagGroup<"out-of-line-declaration">;
 def : DiagGroup<"overflow">;
 def ForwardClassReceiver : DiagGroup<"receiver-forward-class">;
 def MethodAccess : DiagGroup<"objc-method-access">;
 def ObjCReceiver : DiagGroup<"receiver-expr">;
 def OperatorNewReturnsNull : DiagGroup<"new-returns-null">;

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def note_hidden_overloaded_virtual_declared_here : Note<
   "|: different exception specifications}1">;
 def warn_using_directive_in_header : Warning<
   "using namespace directive in global context in header">,
   InGroup<HeaderHygiene>, DefaultIgnore;
 def warn_overaligned_type : Warning<
   "type %0 requires %1 bytes of alignment and the default allocator only "
   "guarantees %2 bytes">,
   InGroup<OveralignedType>, DefaultIgnore;
-def warn_aligned_allocation_unavailable :Warning<
+def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
-  "available on %2 %3 or newer">, InGroup<AlignedAllocationUnavailable>, DefaultError;
+  "available on %2 %3 or newer">;
 def note_silence_unligned_allocation_unavailable : Note<
   "if you supply your own aligned allocation functions, use "
-  "-Wno-aligned-allocation-unavailable to silence this diagnostic">;
+  "-faligned-allocation to silence this diagnostic">;
 
 def err_conditional_void_nonvoid : Error<
   "%select{left|right}1 operand to ? is void, but %select{right|left}1 operand "
   "is of type %0">;
 def err_conditional_ambiguous : Error<
   "conditional expression is ambiguous; "
   "%diff{$ can be converted to $ and vice versa|"
   "types can be convert to each other}0,1">;

cfe/trunk/lib/Driver/ToolChains/Darwin.cpp

bool Darwin::isAlignedAllocationUnavailable() const {
   }
 
   return TargetVersion < alignedAllocMinVersion(OS);
 }
 
 void Darwin::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                                    llvm::opt::ArgStringList &CC1Args,
                                    Action::OffloadKind DeviceOffloadKind) const {
-  if (isAlignedAllocationUnavailable())
+  // Pass "-faligned-alloc-unavailable" only when the user hasn't manually
+  // enabled or disabled aligned allocations.
+  if (!DriverArgs.hasArgNoClaim(options::OPT_faligned_allocation,
+                                options::OPT_fno_aligned_allocation) &&
+      isAlignedAllocationUnavailable())
     CC1Args.push_back("-faligned-alloc-unavailable");
 }
 
 DerivedArgList *
 Darwin::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
                       Action::OffloadKind DeviceOffloadKind) const {
   // First get the generic Apple args, before moving onto Darwin-specific ones.
   DerivedArgList *DAL =

cfe/trunk/lib/Frontend/InitPreprocessor.cpp

if (LangOpts.CPlusPlus17) {
     Builder.defineMacro("__cpp_variadic_using", "201611L");
     Builder.defineMacro("__cpp_aggregate_bases", "201603L");
     Builder.defineMacro("__cpp_structured_bindings", "201606L");
     Builder.defineMacro("__cpp_nontype_template_args", "201411L");
     Builder.defineMacro("__cpp_fold_expressions", "201603L");
     Builder.defineMacro("__cpp_guaranteed_copy_elision", "201606L");
     Builder.defineMacro("__cpp_nontype_template_parameter_auto", "201606L");
   }
-  if (LangOpts.AlignedAllocation)
+  if (LangOpts.AlignedAllocation && !LangOpts.AlignedAllocationUnavailable)
     Builder.defineMacro("__cpp_aligned_new", "201606L");
   if (LangOpts.RelaxedTemplateTemplateArgs)
     Builder.defineMacro("__cpp_template_template_args", "201611L");
 
   // TS features.
   if (LangOpts.ConceptsTS)
     Builder.defineMacro("__cpp_experimental_concepts", "1L");
   if (LangOpts.CoroutinesTS)

cfe/trunk/lib/Sema/SemaExprCXX.cpp

if (FD.isDefined())
     return;
 
   bool IsAligned = false;
   if (FD.isReplaceableGlobalAllocationFunction(&IsAligned) && IsAligned) {
     const llvm::Triple &T = S.getASTContext().getTargetInfo().getTriple();
     StringRef OSName = AvailabilityAttr::getPlatformNameSourceSpelling(
         S.getASTContext().getTargetInfo().getPlatformName());
 
-    S.Diag(Loc, diag::warn_aligned_allocation_unavailable)
-         << IsDelete << FD.getType().getAsString() << OSName
-         << alignedAllocMinVersion(T.getOS()).getAsString();
+    S.Diag(Loc, diag::err_aligned_allocation_unavailable)
     S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
   }
 }
 
 ExprResult
 Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
                   SourceLocation PlacementLParen,
                   MultiExprArg PlacementArgs,

cfe/trunk/test/Driver/unavailable_aligned_allocation.cpp

 // RUN: %clang -target x86_64-apple-darwin -mtvos-simulator-version-min=11 \
 // RUN: -c -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=AVAILABLE
 //
 // RUN: %clang -target x86_64-apple-darwin -mwatchos-simulator-version-min=4 \
 // RUN: -c -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=AVAILABLE
 //
+// Check that passing -faligned-allocation or -fno-aligned-allocation stops the
+// driver from passing -faligned-alloc-unavailable to cc1.
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -faligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -fno-aligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+
 // AVAILABLE-NOT: "-faligned-alloc-unavailable"

cfe/trunk/test/Lexer/aligned-allocation.cpp

+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -DEXPECT_DEFINED
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-alloc-unavailable
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-allocation -faligned-alloc-unavailable
+
+// Test that __cpp_aligned_new is not defined when CC1 is passed
+// -faligned-alloc-unavailable by the Darwin driver, even when aligned
+// allocation is actually enabled.
+
+// expected-no-diagnostics
+#ifdef EXPECT_DEFINED
+# ifndef __cpp_aligned_new
+#   error "__cpp_aligned_new" should be defined
+# endif
+#else
+# ifdef __cpp_aligned_new
+#   error "__cpp_aligned_new" should not be defined
+# endif
+#endif