[clang] Enable sized deallocation by default in C++14 onwards

[wangpc]

Summary

Since C++14 has been released for about seven years and most standard
libraries have implemented sized deallocation functions, it's time to
make this feature default again.

[• pcwang-thead]

Add -fno-sized-allocation to:

• clang\test\AST\ast-dump-expr-json.cpp
• clang\test\AST\ast-dump-stmt-json.cpp
• clang\test\CodeGenCXX\dllimport.cpp
• clang\test\SemaCXX\MicrosoftExtensions.cpp
• clang-tools-extra/clangd/unittests/FindTargetTests.cpp

[rnk]

Please add a note to clang/docs/ReleaseNotes.rst about the behavior change.

The clangd test failure seems related to this change, and the other ones could be as well.

clang/lib/Driver/ToolChains/Clang.cpp

6405

Can this be Args.AddLastArg(CmdArgs, ...);?

[• pcwang-thead]

• Changes to Args.AddLastArg(CmdArgs, ...)
• Adds a note to clang/docs/ReleaseNotes.rst
• Fixs clangd test failure.

[• pcwang-thead]

Removes unnecessary -fno-sized-deallocation and some comments.

[• pcwang-thead]

• Regenerate clang\test\AST\ast-dump-stmt-json.cpp and clang\test\AST\ast-dump-expr-json.cpp.
• Format code.

[ldionne]

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp

12 ↗

(On Diff #383981)

[• pcwang-thead]

Makes required changes

[rnk]

Looks good from my end.

[• pcwang-thead]

Adds ADDITIONAL_COMPILE_FLAGS and guard macros to:

• libcxx\test\std\language.support\support.dynamic\new.delete\new.delete.single\sized_delete14.pass.cpp
• libcxx\test\std\language.support\support.dynamic\new.delete\new.delete.array\sized_delete_array14.pass.cpp

[zixuan-wu]

Is this going to be reviewed again or committed?

[kwk]

You've disabled sized deallocation in some tests by providing -fno-sized-deallocation . I admit I haven't looked at those tests in detail but I would like to understand if those tests would otherwise fail. If they fail, is there work that needs to be done later to make them work again?

clang/lib/Driver/ToolChains/Clang.cpp

6403–6406

@rjmccall mentioned here that

Apple would like this to only be enabled conditionally based on deployment target.

Is this considered at all?

[Mordante]

In D112921#3126492, @zixuan-wu wrote:

Is this going to be reviewed again or committed?

This patch still requires approval by the libc++ group.
The last build failed for multiple libc++ CI jobs, these should be fixed.
(I haven't looked at the code.)

[rjmccall]

Yeah, I think this either needs deployment restriction on Apple platforms or it needs the thunk / weak-linking approach from the original patch when the target OS isn't recent enough. @ldionne should know the right OS versions.

[rnk]

Let's not bring back the weak function thunk approach. That approach caused problems described in my commit, D8467, which is why the code was removed.

The next steps are to sort out right defaults for Apple and understand the libc++ test failures. Would it be reasonable to take a shortcut here, leave the feature disabled for Apple targets, and defer those details to those that own the target?

[rjmccall]

In D112921#3127767, @rnk wrote:

Let's not bring back the weak function thunk approach. That approach caused problems described in my commit, D8467, which is why the code was removed.

The weak function that intercedes if the strong function isn't found statically was, yeah, a poorly thought out idea. Weak *imports* work reliably, at least on Darwin, but they do require SDK support.

The next steps are to sort out right defaults for Apple and understand the libc++ test failures. Would it be reasonable to take a shortcut here, leave the feature disabled for Apple targets, and defer those details to those that own the target?

Yes, I think that would be acceptable.

[ldionne]

Regarding the back-deployment target for Apple, I believe the first OSes where libc++.dylib had sized deallocation are macOS 10.12, iOS 10.0, watchOS 3.0, and tvOS 10.0. We should be able to enable reliance on sized deallocation functions automatically whenever the deployment target is greater than or equal to these versions.

The comments I left in the test file apply to both test files.

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp

12–13 ↗

(On Diff #384344)

The comment you removed is still relevant and should be kept for these compilers, since it applies to them.

15–16 ↗

(On Diff #384344)

This shouldn't be necessary anymore if it is enabled by default in Clang trunk, no? And in fact we definitely don't want to add these flags here, since we're trying to test the default Clang behavior. I think that's why CI is failing with a bunch of XPASSes.

17 ↗

(On Diff #384344)

We will also need to add the following annotation to express that the test is expected to fail when run on targets where sized deallocation wasn't available yet:

// Sized deallocation was added in macOS 10.12 and aligned OSes.
// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}

[MaskRay]

Beside the concern raised by platform maintainers: the cc1 default switch part should be made separately from the patch.
This makes revert easy and leaves fewer churn to the test suite.

If -fno-sized-deallocation is a better cc1 default (but not Driver's), I can make such a change.

[• pcwang-thead]

• Disable sized deallocation for Apple targets.
• Update tests and don't use -fno-sized-deallocation any more.
  • With one exception: clang/test/SemaCXX/builtin-operator-new-delete.cpp, which will generate two notes without line infos if using default settings.

[• pcwang-thead]

In D112921#3453114, @MaskRay wrote:

Beside the concern raised by platform maintainers: the cc1 default switch part should be made separately from the patch.
This makes revert easy and leaves fewer churn to the test suite.

If -fno-sized-deallocation is a better cc1 default (but not Driver's), I can make such a change.

That will be helpful, thanks!

[Mordante]

I didn't look at the code, but I have some hints how we can test libc++.

[• pcwang-thead]

In D112921#3462592, @Mordante wrote:

I didn't look at the code, but I have some hints how we can test libc++.

Thanks! I ran tests with no error occurred on my local machine and I really want to know how to test it!

[Mordante]

In D112921#3463887, @pcwang-thead wrote:

In D112921#3462592, @Mordante wrote:

I didn't look at the code, but I have some hints how we can test libc++.

Thanks! I ran tests with no error occurred on my local machine and I really want to know how to test it!

I added similar information yesterday, but I somehow didn't properly submit it.

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp

17 ↗

(On Diff #423555)

This // XFAIL: clang-12, clang-13 is still needed but should be // XFAIL: clang-13, clang-14. (Since the LLVM 14 release we only support these two versions.)

For testing it's the easiest to remove line 147 of libcxx/utils/ci/buildkite-pipeline.yml
https://github.com/llvm/llvm-project/blob/afcc6baac52fcc91d1636f6803f5c230e7018016/libcxx/utils/ci/buildkite-pipeline.yml#L147

That way all builds run. The Bootstrapping build builds clang and uses that clang to test libc++. That way we can validate which builds fail and which succeed. Maybe some more builds need to be (temporary) disabled.

Once we verify that works we need to:

• undo the buildkite changes
• disable this test temporary (UNSUPPORTED: clang-15)
• create a followup patch to reenable the test

After the change has landed it will take some time before the change is propagated to the CI. Once it's propagated the followup patch can be landed. I'm willing to create the followup patch and land it at the proper time.

[• pcwang-thead]

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp

17 ↗

(On Diff #423555)

Thanks! I will have a try later. :)

[ldionne]

clang/lib/Frontend/CompilerInvocation.cpp

4078–4082 ↗

(On Diff #423555)

Why not implement it correctly from the start? @arphaman @ahatanak What would be the right incantation to enable sized deallocation starting in macOS 10.12, iOS 10.0, watchOS 3.0, and tvOS 10.0?

[ldionne]

(BTW I strongly support this patch, I just think we should do it properly on all platforms from the start :-)

[rjmccall]

Ideally, I think, we would set this up to work something like ObjCRuntime, where we're making queries to a common place that contains all the information necessary to decide what runtime features are available. In particular, we shouldn't treat Apple platforms as forever unique in providing a stable runtime interface with availability gating.

Now, we don't necessarily need the same complexity that ObjCRuntime supports, where the user can tell us to use a different runtime and runtime version from the default for the platform. On the other hand, maybe we want that, because it's a good way to deal with the compatibility problem that we have on non-Apple platforms. Users could tell us that they're targeting e.g. libsupc++ v4.8, and we could tell them in response that sized allocation isn't supported. And if we get them to tell us that, rather than "I have sized allocation support" specifically, it sets us up well to solve similar problems in the future.

[• pcwang-thead]

In D112921#3473022, @ldionne wrote:

(BTW I strongly support this patch, I just think we should do it properly on all platforms from the start :-)

I couldn't agree with you more, but I have no idea how to implement it. :-(

In D112921#3473080, @rjmccall wrote:

Ideally, I think, we would set this up to work something like ObjCRuntime, where we're making queries to a common place that contains all the information necessary to decide what runtime features are available. In particular, we shouldn't treat Apple platforms as forever unique in providing a stable runtime interface with availability gating.

Now, we don't necessarily need the same complexity that ObjCRuntime supports, where the user can tell us to use a different runtime and runtime version from the default for the platform. On the other hand, maybe we want that, because it's a good way to deal with the compatibility problem that we have on non-Apple platforms. Users could tell us that they're targeting e.g. libsupc++ v4.8, and we could tell them in response that sized allocation isn't supported. And if we get them to tell us that, rather than "I have sized allocation support" specifically, it sets us up well to solve similar problems in the future.

You mean that we may provide a option -fc++-runtime likes -fobjc-runtime, or extend -stdlib to specify version number in the form of -stdlib=libsupc++-v4.8?

[rjmccall]

Hmm. Allowing a version on -stdlib is intuitively appealing, but I'm not sure it actually gives us the information we need. As I recall, -stdlib selects the high-level stdlib and not the low-level one, and those are related in code but not necessarily at runtime; for example, you can (or at least could, historically) use libstdc++ on macOS, but the underlying low-level stdlib is going to be libc++abi, not libsupc++. And the low-level runtime is the one that actually provides global operator new functions. Is there a way to bridge that gap?

[ldionne]

In D112921#3473632, @pcwang-thead wrote:

In D112921#3473022, @ldionne wrote:

(BTW I strongly support this patch, I just think we should do it properly on all platforms from the start :-)

I couldn't agree with you more, but I have no idea how to implement it. :-(

I was thinking about doing what we do for aligned allocation here: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2652-L2673

I think this should work:

bool Darwin::isSizedDeallocationUnavailable() const {
  llvm::Triple::OSType OS;

  if (isTargetMacCatalyst())
    return TargetVersion < sizedDeallocMinVersion(llvm::Triple::MacOSX);
  switch (TargetPlatform) {
  case MacOS:
    OS = llvm::Triple::MacOSX;
    break;
  case IPhoneOS:
    OS = llvm::Triple::IOS;
    break;
  case TvOS:
    OS = llvm::Triple::TvOS;
    break;
  case WatchOS:
    OS = llvm::Triple::WatchOS;
    break;
  }

  return TargetVersion < sizedDeallocMinVersion(OS);
}

// This could go in clang/include/clang/Basic/AlignedAllocation.h with a suitable rename, or another header
inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) {
  switch (OS) {
  default:
    break;
  case llvm::Triple::Darwin:
  case llvm::Triple::MacOSX: // Earliest supporting version is 10.12.
    return llvm::VersionTuple(10U, 12U);
  case llvm::Triple::IOS:
  case llvm::Triple::TvOS: // Earliest supporting version is 10.0.0.
    return llvm::VersionTuple(10U);
  case llvm::Triple::WatchOS: // Earliest supporting version is 3.0.0.
    return llvm::VersionTuple(3U);
  case llvm::Triple::ZOS:
    return llvm::VersionTuple(); // All z/OS versions have no support.
  }

  llvm_unreachable("Unexpected OS");
}

Alternatively, @rjmccall 's approach might be better, but I'm not familiar with what's done for ObjCRuntime so I can't comment. I'm personally neutral on what approach is taken.

[• pcwang-thead]

In D112921#3474964, @rjmccall wrote:

Hmm. Allowing a version on -stdlib is intuitively appealing, but I'm not sure it actually gives us the information we need. As I recall, -stdlib selects the high-level stdlib and not the low-level one, and those are related in code but not necessarily at runtime; for example, you can (or at least could, historically) use libstdc++ on macOS, but the underlying low-level stdlib is going to be libc++abi, not libsupc++. And the low-level runtime is the one that actually provides global operator new functions. Is there a way to bridge that gap?

Hmm, that's really a tough nut. We have high-level standard libraries like libc++ and libstdc++, and low-level runtimes like libc++abi, libsupc++ and libcxxrt. There could be a lot of situations and we haven't think about MSVC yet.
I don't know if adding an option -cxxabilib=libsupc++-v4.8 or something like this would make sense, but I think we should make another revision to do it. I will apply @ldionne 's solution first.

[• pcwang-thead]

• Add handling of Apple targets.
• Update libc++ tests.

[Romain-Geissler-1A]

Hi,

Is there any update about this ?

[• pcwang-thead]

In D112921#4003939, @Romain-Geissler-1A wrote:

Hi,

Is there any update about this ?

Currently, no.
If someone is interesting in this, please feel free to commandeer. :-)

[MaskRay]

Changing the default improves GCC compatibility (Enabled by default under -std=c++14 and above.) and I think is the right direction.

@wangpc You can use your new username to commandeer this revision?

To other reviewers, what's still blocked now that the Apple target issue appears to be addressed?
Note, we could also keep Apple behavior unchanged in this patch and let folks more familiar with it to handle the change.

We probably need to make a decision whether this should be the release/17.x branch or not.

[Mordante]

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp

16 ↗

(On Diff #425443)

Maybe as a pragmatic solution

// TODO(mordante) fix this test after updating clang in Docker
// UNSUPPORTED: clang-17

The same for the other failing test. Then I'll fix this once the libc++ CI can be updated with this fix included.

libcxx/utils/ci/buildkite-pipeline.yml

148 ↗

(On Diff #425443)

Please undo the changes to this file.

[wangpc]

• Rebase
• Fix tests.

@ldionne I don't know the details about DriverKit so I assumed that sized deallocation is available just like aligned allocation.

@Mordante I will leave libcxx tests to folks who are more familiar with them.

[wangpc]

• Add comments.
• clang-format.

[wangpc]

Rebase and fix libcxx tests.

[wangpc]

Ping.

[Mordante]

I noticed some of the CI jobs are still failing with the patch, I didn't look into them.

[wangpc]

Rebase.

[wangpc]

In D112921#4529182, @Mordante wrote:

I noticed some of the CI jobs are still failing with the patch, I didn't look into them.

I don't think they are related to this patch, so I rebased again. If still failed, I will try to fix them later.

[Mordante]

In D112921#4530916, @wangpc wrote:

In D112921#4529182, @Mordante wrote:

I noticed some of the CI jobs are still failing with the patch, I didn't look into them.

I don't think they are related to this patch, so I rebased again. If still failed, I will try to fix them later.

I see changes in the libc++ ABI list output

Symbol added: _ZdlPvmSt11align_val_t
    {'name': '_ZdlPvmSt11align_val_t', 'type': 'FUNC', 'is_defined': False}

Symbol added: _ZdlPvm
    {'name': '_ZdlPvm', 'type': 'FUNC', 'is_defined': False}

SYMBOL REMOVED: _ZdlPvSt11align_val_t
    {'is_defined': False, 'name': '_ZdlPvSt11align_val_t', 'type': 'FUNC'}

Summary
    Added:   2
    Removed: 1
    Changed: 0

There seems to be small change in the symbol name. What does the m in the added symbol mean?

_ZdlPvmSt11align_val_t - added
_ZdlPvSt11align_val_t  - remove

There is also a new symbol _ZdlPvm added.

clang/test/CodeGenCoroutines/coro-aligned-alloc.cpp

29–31 ↗

(On Diff #543828)

Here I see changes in the symbol name. I see issues in the libc++ ABI list too.

[wangpc]

In D112921#4532378, @Mordante wrote:

In D112921#4530916, @wangpc wrote:

In D112921#4529182, @Mordante wrote:

I noticed some of the CI jobs are still failing with the patch, I didn't look into them.

I don't think they are related to this patch, so I rebased again. If still failed, I will try to fix them later.

I see changes in the libc++ ABI list output

Symbol added: _ZdlPvmSt11align_val_t
    {'name': '_ZdlPvmSt11align_val_t', 'type': 'FUNC', 'is_defined': False}

Symbol added: _ZdlPvm
    {'name': '_ZdlPvm', 'type': 'FUNC', 'is_defined': False}

SYMBOL REMOVED: _ZdlPvSt11align_val_t
    {'is_defined': False, 'name': '_ZdlPvSt11align_val_t', 'type': 'FUNC'}

Summary
    Added:   2
    Removed: 1
    Changed: 0

There seems to be small change in the symbol name. What does the m in the added symbol mean?

_ZdlPvmSt11align_val_t - added
_ZdlPvSt11align_val_t  - remove

There is also a new symbol _ZdlPvm added.

m means unsigned long in mangled name (5.1.5.2 Builtin types), which is the size of sized deallocation.

$ c++filt _ZdlPvmSt11align_val_t _ZdlPvm
operator delete(void*, unsigned long, std::align_val_t)
operator delete(void*, unsigned long)

I am not familiar with libcxx, can you please help me to fix these tests? I hope we can catch up with the release of LLVM 17.

[Mordante]

In D112921#4535716, @wangpc wrote:

In D112921#4532378, @Mordante wrote:

In D112921#4530916, @wangpc wrote:

In D112921#4529182, @Mordante wrote:

I noticed some of the CI jobs are still failing with the patch, I didn't look into them.

I don't think they are related to this patch, so I rebased again. If still failed, I will try to fix them later.

I see changes in the libc++ ABI list output

Symbol added: _ZdlPvmSt11align_val_t
    {'name': '_ZdlPvmSt11align_val_t', 'type': 'FUNC', 'is_defined': False}

Symbol added: _ZdlPvm
    {'name': '_ZdlPvm', 'type': 'FUNC', 'is_defined': False}

SYMBOL REMOVED: _ZdlPvSt11align_val_t
    {'is_defined': False, 'name': '_ZdlPvSt11align_val_t', 'type': 'FUNC'}

Summary
    Added:   2
    Removed: 1
    Changed: 0

There seems to be small change in the symbol name. What does the m in the added symbol mean?

_ZdlPvmSt11align_val_t - added
_ZdlPvSt11align_val_t  - remove

There is also a new symbol _ZdlPvm added.

m means unsigned long in mangled name (5.1.5.2 Builtin types), which is the size of sized deallocation.

$ c++filt _ZdlPvmSt11align_val_t _ZdlPvm
operator delete(void*, unsigned long, std::align_val_t)
operator delete(void*, unsigned long)

I am not familiar with libcxx, can you please help me to fix these tests? I hope we can catch up with the release of LLVM 17.

Looking at the ABI list we currently have

_ZdlPv -> operator delete(void*)                                   // this one is kept
_ZdlPvSt11align_val_t -> operator delete(void*, std::align_val_t)  // this one is removed

You add the overloads

operator delete(void*, unsigned long, std::align_val_t)
operator delete(void*, unsigned long)

Looking in the current WP http://eel.is/c++draft/replacement.functions#lib:new,operator the removed overload is still listed there.

Do you know why operator delete(void*) and operator delete(void*, unsigned long) are both available?

clang/include/clang/Driver/Options.td

2345

Can you update the test test/Lexer/cxx-features.cpp and remove -fsized-deallocation in C++14 and newer? This should no longer be required.

[wangpc]

• Remove usages of -fsized-deallocation in some tests.
• Rebase.

[wangpc]

In D112921#4536088, @Mordante wrote:

In D112921#4535716, @wangpc wrote:

In D112921#4532378, @Mordante wrote:

In D112921#4530916, @wangpc wrote:

In D112921#4529182, @Mordante wrote:

I noticed some of the CI jobs are still failing with the patch, I didn't look into them.

I don't think they are related to this patch, so I rebased again. If still failed, I will try to fix them later.

I see changes in the libc++ ABI list output

Symbol added: _ZdlPvmSt11align_val_t
    {'name': '_ZdlPvmSt11align_val_t', 'type': 'FUNC', 'is_defined': False}

Symbol added: _ZdlPvm
    {'name': '_ZdlPvm', 'type': 'FUNC', 'is_defined': False}

SYMBOL REMOVED: _ZdlPvSt11align_val_t
    {'is_defined': False, 'name': '_ZdlPvSt11align_val_t', 'type': 'FUNC'}

Summary
    Added:   2
    Removed: 1
    Changed: 0

There seems to be small change in the symbol name. What does the m in the added symbol mean?

_ZdlPvmSt11align_val_t - added
_ZdlPvSt11align_val_t  - remove

There is also a new symbol _ZdlPvm added.

m means unsigned long in mangled name (5.1.5.2 Builtin types), which is the size of sized deallocation.

$ c++filt _ZdlPvmSt11align_val_t _ZdlPvm
operator delete(void*, unsigned long, std::align_val_t)
operator delete(void*, unsigned long)

I am not familiar with libcxx, can you please help me to fix these tests? I hope we can catch up with the release of LLVM 17.

Looking at the ABI list we currently have

_ZdlPv -> operator delete(void*)                                   // this one is kept
_ZdlPvSt11align_val_t -> operator delete(void*, std::align_val_t)  // this one is removed

You add the overloads

operator delete(void*, unsigned long, std::align_val_t)
operator delete(void*, unsigned long)

Looking in the current WP http://eel.is/c++draft/replacement.functions#lib:new,operator the removed overload is still listed there.

Do you know why operator delete(void*) and operator delete(void*, unsigned long) are both available?

I don't know the details about C++ spec, but I think they should be available for runtimes with/without sized deallocations.

[Mordante]

I see one libc++ failure, I have not looked into the other failures.

libcxx/test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp

29 ↗

(On Diff #544640)

The bootstrap build fails. Currently main is clang-18. Since you intend to backport the patch it needs to be disabled on clang-17 too.

[wangpc]

• Rebase.
• Unsupport clang-18.

[aaron.ballman]

In D112921#4535716, @wangpc wrote:

I am not familiar with libcxx, can you please help me to fix these tests? I hope we can catch up with the release of LLVM 17.

I think it's too late to put this into Clang 17. This is a significant amount of change to add to the release branch; I'd prefer if it stayed in Clang 18 to give it some extra time to bake.

[Mordante]

In D112921#4541063, @wangpc wrote:

• Rebase.
• Unsupport clang-18.

I see the libc++ build failed. It seems like timeouts uploading artifacts, so not related to your patch. Can you rebase and give it another run on the CI? (I didn't look at the other build failures.)

[wangpc]

Rebase.

[rnk]

What are the next steps here? Have concerns been addressed? The CI failures seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an interpreter test that seems unrelated, but please confirm).

[aaron.ballman]

In D112921#4581641, @rnk wrote:

What are the next steps here? Have concerns been addressed? The CI failures seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an interpreter test that seems unrelated, but please confirm).

We definitely need the libc++ reviewers to approve before we move forward. IMO. the clang bits look to be in reasonable shape in terms of code quality.

[Mordante]

I'm happy with the libc++ changes, please wait a few days for @ldionne to react.

In D112921#4584704, @aaron.ballman wrote:

In D112921#4581641, @rnk wrote:

What are the next steps here? Have concerns been addressed? The CI failures seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an interpreter test that seems unrelated, but please confirm).

We definitely need the libc++ reviewers to approve before we move forward. IMO. the clang bits look to be in reasonable shape in terms of code quality.

I leave the approval of the clang part to @aaron.ballman

libcxx/test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp

29 ↗

(On Diff #544640)

Based on the timing I'm no longer in favor of backporting this patch.

For the future please mark comments as done when addressed. That makes reviewing easier.

[wangpc]

• Rebase.
• Update tests, remove clang-17.

[aaron.ballman]

LGTM!

[Mordante]

In D112921#4594092, @wangpc wrote:

• Rebase.
• Update tests, remove clang-17.

The removal of the markers in the libc++ tests breaks these test. FYI the tests are using a Clang build of apt.llvm.org and are not using this patch for testing.

[wangpc]

In D112921#4595918, @Mordante wrote:

In D112921#4594092, @wangpc wrote:

• Rebase.
• Update tests, remove clang-17.

The removal of the markers in the libc++ tests breaks these test. FYI the tests are using a Clang build of apt.llvm.org and are not using this patch for testing.

Thanks! I didn't know this. I will add it back before I commit this patch.

[wangpc]

Rebase and fix failed tests.

[wangpc]

• Rebase.
• Remove abilist changes.
• Add clang-18.

[wangpc]

Rebase

[wangpc]

@ldionne Sorry for bothering, what do you think about landing this patch?

[wangpc]

Gentle ping. Can I move forward and land this?

[aaron.ballman]

In D112921#4620399, @wangpc wrote:

Gentle ping. Can I move forward and land this?

What name and email address would you like us to use for patch attribution? @ldionne do you have any remaining concerns (I think all the ones you laid out have already been addressed, so if I don't hear back, I'll assume anything remaining can be handled post-commit)?

[wangpc]

In D112921#4621019, @aaron.ballman wrote:

In D112921#4620399, @wangpc wrote:

Gentle ping. Can I move forward and land this?

What name and email address would you like us to use for patch attribution?

Thanks! I have commit access.

@ldionne do you have any remaining concerns (I think all the ones you laid out have already been addressed, so if I don't hear back, I'll assume anything remaining can be handled post-commit)?

I will leave it for some days and commit it before Friday if it's OK. :-)

[aaron.ballman]

In D112921#4621039, @wangpc wrote:

In D112921#4621019, @aaron.ballman wrote:

In D112921#4620399, @wangpc wrote:

Gentle ping. Can I move forward and land this?

What name and email address would you like us to use for patch attribution?

Thanks! I have commit access.

@ldionne do you have any remaining concerns (I think all the ones you laid out have already been addressed, so if I don't hear back, I'll assume anything remaining can be handled post-commit)?

I will leave it for some days and commit it before Friday if it's OK. :-)

Oh great! Yeah, I think committing later in the week if we don't hear back is perfectly reasonable.

[ldionne]

Sorry for missing a couple of direct pings. I have no concerns with the patch, perhaps I should not have marked my review as blocking in the first place. Thanks for doing this work, it's about time we updated this default!

[MaskRay]

clang/include/clang/Basic/SizedDeallocation.h

23 ↗

(On Diff #551890)

Does this need to be in Basic/? It's only used by clang/lib/Driver/ToolChains/Darwin.cpp

36 ↗

(On Diff #551890)

This is major=minor=0, which is probably not desired.

We can just omit ZOS.

[wangpc]

clang/include/clang/Basic/SizedDeallocation.h

23 ↗

(On Diff #551890)

This file is just copied and changed from clang/include/clang/Basic/AlignedAllocation.h actually, I don't know which directory is more suitable.

36 ↗

(On Diff #551890)

We have ZOS in alignedAllocMinVersion in clang/include/clang/Basic/AlignedAllocation.h too.

[wangpc]

Rebase.

[wangpc]

Thanks all! I will land this patch later.
If there are some failures (especially libcxx part @Mordante :-) ), please help me to fix them. Thanks in advance!

[nikic]

FYI this resulted in some pretty wild code size swings, in particular between -10% and -15% for tramp3d-v4 (http://llvm-compile-time-tracker.com/compare.php?from=6cde64a94986165547ae5237ac7dd4bddfc9f2a7&to=2916b125f686115deab2ba573dcaff3847566ab9&stat=size-text). Not sure whether that's an expected result of this change or not.

[thakis]

Looks like this breaks tests on windows: http://45.33.8.238/win/83485/step_7.txt

Please take a look and revert for now if it takes a while to fix.

(Also, if the patch doesn't already do it, it probably shouldn't change defaults in clang-cl mode?)

[jhuber6]

This caused some linking errors with the GPU libc test suite, see https://lab.llvm.org/staging/#/builders/247/builds/5659.

clang++: error: ld.lld command failed with exit code 1 (use -v to see invocation)
[331/473] Linking CXX executable libc/test/src/__support/libc.test.src.__support.uint_test.__hermetic__.__build__
FAILED: libc/test/src/__support/libc.test.src.__support.uint_test.__hermetic__.__build__ 
: && /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang++ --target=x86_64-unknown-linux-gnu -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -Wl,--color-diagnostics    -nostdlib -static libc/startup/gpu/amdgpu/CMakeFiles/libc.startup.gpu.amdgpu.crt1.dir/start.cpp.o libc/test/src/__support/CMakeFiles/libc.test.src.__support.uint_test.__hermetic__.__build__.dir/uint_test.cpp.o -o libc/test/src/__support/libc.test.src.__support.uint_test.__hermetic__.__build__  libc/test/UnitTest/libLibcTest.hermetic.a  libc/test/UnitTest/libLibcHermeticTestSupport.hermetic.a  libc/test/src/__support/liblibc.test.src.__support.uint_test.__hermetic__.libc.a  -mcpu=gfx906  --target=amdgcn-amd-amdhsa  -flto  -Wl,-mllvm,-amdgpu-lower-global-ctor-dtor=0 && :
ld.lld: error: undefined symbol: operator delete(void*, unsigned long)
>>> referenced by lto.tmp:(LlvmLibcUIntClassTest_ConstructorFromUInt128Tests::~LlvmLibcUIntClassTest_ConstructorFromUInt128Tests())
>>> referenced by lto.tmp:(LlvmLibcUIntClassTest_ConstructorFromUInt128Tests::~LlvmLibcUIntClassTest_ConstructorFromUInt128Tests())
>>> referenced by lto.tmp:(LlvmLibcUIntClassTest_BasicArithmeticInt128Tests::~LlvmLibcUIntClassTest_BasicArithmeticInt128Tests())
>>> referenced 41 more times
>>> did you mean: operator delete(void*)
>>> defined in: lto.tmp
clang++: error: ld.lld command failed with exit code 1 (use -v to see invocation)

[wangpc]

In D112921#4624488, @thakis wrote:

Looks like this breaks tests on windows: http://45.33.8.238/win/83485/step_7.txt

Please take a look and revert for now if it takes a while to fix.

(Also, if the patch doesn't already do it, it probably shouldn't change defaults in clang-cl mode?)

The two failed cases are about Interpreter, which is not related to clang-cl mode I think? Is it because the windows environment doesn't support sized deallocation?
I don't know how to fix it, can someone help me?

[wangpc]

In D112921#4624547, @jhuber6 wrote:

This caused some linking errors with the GPU libc test suite, see https://lab.llvm.org/staging/#/builders/247/builds/5659.

clang++: error: ld.lld command failed with exit code 1 (use -v to see invocation)
[331/473] Linking CXX executable libc/test/src/__support/libc.test.src.__support.uint_test.__hermetic__.__build__
FAILED: libc/test/src/__support/libc.test.src.__support.uint_test.__hermetic__.__build__ 
: && /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang++ --target=x86_64-unknown-linux-gnu -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -Wl,--color-diagnostics    -nostdlib -static libc/startup/gpu/amdgpu/CMakeFiles/libc.startup.gpu.amdgpu.crt1.dir/start.cpp.o libc/test/src/__support/CMakeFiles/libc.test.src.__support.uint_test.__hermetic__.__build__.dir/uint_test.cpp.o -o libc/test/src/__support/libc.test.src.__support.uint_test.__hermetic__.__build__  libc/test/UnitTest/libLibcTest.hermetic.a  libc/test/UnitTest/libLibcHermeticTestSupport.hermetic.a  libc/test/src/__support/liblibc.test.src.__support.uint_test.__hermetic__.libc.a  -mcpu=gfx906  --target=amdgcn-amd-amdhsa  -flto  -Wl,-mllvm,-amdgpu-lower-global-ctor-dtor=0 && :
ld.lld: error: undefined symbol: operator delete(void*, unsigned long)
>>> referenced by lto.tmp:(LlvmLibcUIntClassTest_ConstructorFromUInt128Tests::~LlvmLibcUIntClassTest_ConstructorFromUInt128Tests())
>>> referenced by lto.tmp:(LlvmLibcUIntClassTest_ConstructorFromUInt128Tests::~LlvmLibcUIntClassTest_ConstructorFromUInt128Tests())
>>> referenced by lto.tmp:(LlvmLibcUIntClassTest_BasicArithmeticInt128Tests::~LlvmLibcUIntClassTest_BasicArithmeticInt128Tests())
>>> referenced 41 more times
>>> did you mean: operator delete(void*)
>>> defined in: lto.tmp
clang++: error: ld.lld command failed with exit code 1 (use -v to see invocation)

It seems that the linker can't find sized deallocator (no support in the environment or AMDGPU libraries?).

[wangpc]

In D112921#4624141, @nikic wrote:

FYI this resulted in some pretty wild code size swings, in particular between -10% and -15% for tramp3d-v4 (http://llvm-compile-time-tracker.com/compare.php?from=6cde64a94986165547ae5237ac7dd4bddfc9f2a7&to=2916b125f686115deab2ba573dcaff3847566ab9&stat=size-text). Not sure whether that's an expected result of this change or not.

I'm surprised. From the tests diff in this patch, it should increase code size I think.

[jhuber6]

In D112921#4624580, @wangpc wrote:

It seems that the linker can't find sized deallocation (no support in the environment or AMDGPU libraries?).

We should have some implementations here I thought https://github.com/llvm/llvm-project/blob/main/libc/src/__support/CPP/new.cpp, maybe @sivachandra can elucidate on that. I actually don't know what the expected behavior is here, since we don't really have any of this support on GPU targets. I'm wondering why we're currently fine with compiling delete but not with the sized version, since their definitions should both be present in that file.

[uabelho]

I've no idea what's happening but it seems like linking compiler-rt/lib/memprof/tests/MemProfUnitTests fails with this commit:

[17/17] Linking CXX executable compiler-rt/lib/memprof/tests/MemProfUnitTests
FAILED: compiler-rt/lib/memprof/tests/MemProfUnitTests 
: && /repo/uabelho/dev-main/llvm/build-all-builtins/./bin/clang++ --target=x86_64-unknown-linux-gnu [...] compiler-rt/lib/memprof/tests/MemProfUnitTests  -lstdc++  -ldl && :
clang++: error: unable to execute command: Segmentation fault (core dumped)
clang++: error: linker command failed due to signal (use -v to see invocation)
ninja: build stopped: subcommand failed.

[TWeaver]

Good afternoon from the UK!

It looks as though this change has caused the following buildbot to start failing:

https://lab.llvm.org/buildbot/#/builders/216/builds/26407

are you able to take a look?

Thanks in advance,
Tom W

[aaron.ballman]

Because the issues have been ongoing for a few hours now, I think it'd make sense to revert these changes while trying to determine what the appropriate fix is. @wangpc would you mind doing the revert?

[wangpc]

In D112921#4624674, @aaron.ballman wrote:

Because the issues have been ongoing for a few hours now, I think it'd make sense to revert these changes while trying to determine what the appropriate fix is. @wangpc would you mind doing the revert?

Yeah I think we should. Can you revert it for me? I am away from my computer now.

[aaron.ballman]

In D112921#4624674, @aaron.ballman wrote:
Because the issues have been ongoing for a few hours now, I think it'd make sense to revert these changes while trying to determine what the appropriate fix is. @wangpc would you mind doing the revert?

Yeah I think we should. Can you revert it for me? I am away from my computer now.

Sure can -- I've reverted in a02f9a7756e5e4c76b422f9948d5dc3f56a1d139 and am reopening the review. Thanks!

[MaskRay]

clang/include/clang/Basic/SizedDeallocation.h

23 ↗

(On Diff #551890)

If this is only used by clangDriver, I am not sure we want to expose it as a public header under include/.

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

TEST_F(TargetDeclTest, OverloadExpr) {
   )cpp";
   EXPECT_DECLS("CXXDeleteExpr", "static void operator delete(void *) noexcept");
   Code = R"cpp(
     void operator delete(void *) noexcept;
     void k(int* x) {
       [[delete]] x;
     }
   )cpp";
-  EXPECT_DECLS("CXXDeleteExpr", "void operator delete(void *) noexcept");
+  // Sized deallocation is enabled by default in C++14 onwards.
+  EXPECT_DECLS("CXXDeleteExpr", "void operator delete(void *, unsigned long) noexcept");
 }
 
 TEST_F(TargetDeclTest, DependentExprs) {
   // Heuristic resolution of method of dependent field
   Code = R"cpp(
         struct A { void foo() {} };
         template <typename T>
         struct B {

clang-tools-extra/test/clang-tidy/checkers/misc-new-delete-overloads.cpp

-// RUN: %check_clang_tidy %s misc-new-delete-overloads %t
+// RUN: %check_clang_tidy %s misc-new-delete-overloads %t -- -- -fno-sized-deallocation
 
 typedef decltype(sizeof(int)) size_t;
 
 struct S {
   // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope [misc-new-delete-overloads]
   void *operator new(size_t size) noexcept;
   // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new[]' has no matching declaration of 'operator delete[]' at the same scope
   void *operator new[](size_t size) noexcept;
 };
 
 // CHECK-MESSAGES: :[[@LINE+1]]:7: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
 void *operator new(size_t size) noexcept(false);
 
 struct T {
-  // Sized deallocations are not enabled by default, and so this new/delete pair
+  // Sized deallocations are not enabled, and so this new/delete pair
   // does not match. However, we expect only one warning, for the new, because
   // the operator delete is a placement delete and we do not warn on mismatching
   // placement operations.
   // CHECK-MESSAGES: :[[@LINE+1]]:9: warning: declaration of 'operator new' has no matching declaration of 'operator delete' at the same scope
   void *operator new(size_t size) noexcept;
   void operator delete(void *ptr, size_t) noexcept; // ok only if sized deallocation is enabled
 };
 

clang/docs/ReleaseNotes.rst

- Support for ``__attribute__((error("")))`` and
   ``__attribute__((warning("")))`` function attributes have been added.
 - The maximum allowed alignment has been increased from 2^29 to 2^32.
 
 C++ Language Changes in Clang
 -----------------------------
 
 - ...
 
+C++14 Feature Support
+^^^^^^^^^^^^^^^^^^^^^
+- Sized deallocation is enabled by default in C++14 onwards.
+
 C++20 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 ...
 
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 - Implemented `P1938R3: if consteval <https://wg21.link/P1938R3>`_.
 - Implemented `P2360R0: Extend init-statement to allow alias-declaration <https://wg21.link/P2360R0>`_.

clang/include/clang/Driver/Options.td

class MarshallingInfoVisibility<KeyPathAndMacro kpm, code default>
   : MarshallingInfoEnum<kpm, default>,
     Values<"default,hidden,internal,protected">,
     NormalizedValues<["DefaultVisibility", "HiddenVisibility",
                       "HiddenVisibility", "ProtectedVisibility"]> {}
 
 // Key paths that are constant during parsing of options with the same key path prefix.
 defvar cplusplus = LangOpts<"CPlusPlus">;
 defvar cpp11 = LangOpts<"CPlusPlus11">;
+defvar cpp14 = LangOpts<"CPlusPlus14">;
 defvar cpp17 = LangOpts<"CPlusPlus17">;
 defvar cpp20 = LangOpts<"CPlusPlus20">;
 defvar c99 = LangOpts<"C99">;
 defvar c2x = LangOpts<"C2x">;
 defvar lang_std = LangOpts<"LangStd">;
 defvar open_cl = LangOpts<"OpenCL">;
 defvar cuda = LangOpts<"CUDA">;
 defvar render_script = LangOpts<"RenderScript">;
defm application_extension : BoolFOption<"application-extension",
   LangOpts<"AppExt">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Restrict code to those available for App Extensions">,
   NegFlag<SetFalse>>;
 defm relaxed_template_template_args : BoolFOption<"relaxed-template-template-args",
   LangOpts<"RelaxedTemplateTemplateArgs">, DefaultTrue,
   PosFlag<SetTrue>,
   NegFlag<SetFalse, [CC1Option], "Disable C++17 relaxed template template argument matching">>;
 defm sized_deallocation : BoolFOption<"sized-deallocation",
-  LangOpts<"SizedDeallocation">, DefaultFalse,
-  PosFlag<SetTrue, [CC1Option], "Enable C++14 sized global deallocation functions">,
-  NegFlag<SetFalse>>;
+  LangOpts<"SizedDeallocation">, Default<cpp14.KeyPath>,
+  PosFlag<SetTrue, [], "Enable C++14 sized global deallocation functions">,
+  NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
 defm aligned_allocation : BoolFOption<"aligned-allocation",
   LangOpts<"AlignedAllocation">, Default<cpp17.KeyPath>,
   PosFlag<SetTrue, [], "Enable C++17 aligned allocation functions">,
   NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
 def fnew_alignment_EQ : Joined<["-"], "fnew-alignment=">,
   HelpText<"Specifies the largest alignment guaranteed by '::operator new(size_t)'">,
   MetaVarName<"<align>">, Group<f_Group>, Flags<[CC1Option]>,
   MarshallingInfoInt<LangOpts<"NewAlignOverride">>;

clang/lib/Driver/ToolChains/Clang.cpp

   if (Arg *A =
           Args.getLastArg(options::OPT_frelaxed_template_template_args,
                           options::OPT_fno_relaxed_template_template_args)) {
     D.Diag(diag::warn_drv_deprecated_arg) << A->getAsString(Args) << false;
     if (A->getOption().matches(options::OPT_fno_relaxed_template_template_args))
       CmdArgs.push_back("-fno-relaxed-template-template-args");
   }
 
-  // -fsized-deallocation is off by default, as it is an ABI-breaking change for
-  // most platforms.
-  if (Args.hasFlag(options::OPT_fsized_deallocation,
-                   options::OPT_fno_sized_deallocation, false))
-    CmdArgs.push_back("-fsized-deallocation");
+  // -fsized-deallocation is on by default in C++14 onwards and otherwise off
+  // by default.
+  Args.AddLastArg(CmdArgs, options::OPT_fsized_deallocation,
+                  options::OPT_fno_sized_deallocation);
 
   // -faligned-allocation is on by default in C++17 onwards and otherwise off
   // by default.
   if (Arg *A = Args.getLastArg(options::OPT_faligned_allocation,
                                options::OPT_fno_aligned_allocation,
                                options::OPT_faligned_new_EQ)) {
     if (A->getOption().matches(options::OPT_fno_aligned_allocation))
       CmdArgs.push_back("-fno-aligned-allocation");

clang/test/AST/ast-dump-expr.cpp

 // Test without serialization:
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -ast-dump %s \
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fno-sized-deallocation -Wno-unused-value -fcxx-exceptions -std=gnu++17 -ast-dump %s \
 // RUN: | FileCheck --strict-whitespace %s
 //
 // Test with serialization:
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -emit-pch -o %t %s
-// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 \
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fno-sized-deallocation -Wno-unused-value -fcxx-exceptions -std=gnu++17 -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -fno-sized-deallocation -Wno-unused-value -fcxx-exceptions -std=gnu++17 \
 // RUN: -include-pch %t -ast-dump-all /dev/null \
 // RUN: | sed -e "s/ <undeserialized declarations>//" -e "s/ imported//" \
 // RUN: | FileCheck --strict-whitespace %s
 
 namespace std {
 using size_t = decltype(sizeof(0));
 
 class type_info {

clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp

 // Check that delete exprs call the sized deallocation function if
 // -fsized-deallocation is passed in both C++11 and C++14.
 // RUN: %clang_cc1 -std=c++11 -fsized-deallocation %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s
 // RUN: %clang_cc1 -std=c++14 -fsized-deallocation %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s
 
 // Check that we don't used sized deallocation without -fsized-deallocation and
 // C++14.
-// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK-UNSIZED
-// RUN: %clang_cc1 -std=c++14 %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK-UNSIZED
+// RUN: %clang_cc1 -std=c++11 -fno-sized-deallocation %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK-UNSIZED
+// RUN: %clang_cc1 -std=c++14 -fno-sized-deallocation %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK-UNSIZED
 
 // CHECK-UNSIZED-NOT: _ZdlPvm
 // CHECK-UNSIZED-NOT: _ZdaPvm
 
 typedef decltype(sizeof(0)) size_t;
 
 typedef int A;
 struct B { int n; };

clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp

-// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ITANIUM,CHECK-64BIT
-// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple x86_64-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI64,CHECK-64BIT
-// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm %s -triple i386-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI32,CHECK-32BIT
+// RUN: %clang_cc1 -std=c++2a -fno-sized-deallocation -fexceptions -emit-llvm %s -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefixes=CHECK,CHECK-ITANIUM,CHECK-64BIT
+// RUN: %clang_cc1 -std=c++2a -fno-sized-deallocation -fexceptions -emit-llvm %s -triple x86_64-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI64,CHECK-64BIT
+// RUN: %clang_cc1 -std=c++2a -fno-sized-deallocation -fexceptions -emit-llvm %s -triple i386-windows -o - | FileCheck %s --check-prefixes=CHECK,CHECK-MSABI,CHECK-MSABI32,CHECK-32BIT
 
 // PR46908: ensure the IR passes the verifier with optimizations enabled.
-// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple x86_64-linux-gnu -O2
-// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple x86_64-windows -O2
-// RUN: %clang_cc1 -std=c++2a -fexceptions -emit-llvm-only %s -triple i386-windows -O2
+// RUN: %clang_cc1 -std=c++2a -fno-sized-deallocation -fexceptions -emit-llvm-only %s -triple x86_64-linux-gnu -O2
+// RUN: %clang_cc1 -std=c++2a -fno-sized-deallocation -fexceptions -emit-llvm-only %s -triple x86_64-windows -O2
+// RUN: %clang_cc1 -std=c++2a -fno-sized-deallocation -fexceptions -emit-llvm-only %s -triple i386-windows -O2
 
 namespace std {
   using size_t = decltype(sizeof(0));
   enum class align_val_t : size_t;
   struct destroying_delete_t {};
 }
 
 struct A {

clang/test/CodeGenCXX/delete-two-arg.cpp

-// RUN: %clang_cc1 -triple i686-pc-linux-gnu %s -o - -emit-llvm -verify | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fno-sized-deallocation %s -o - -emit-llvm -verify | FileCheck %s
 // expected-no-diagnostics
 
 typedef __typeof(sizeof(int)) size_t;
 
 namespace test1 {
   struct A { void operator delete(void*,size_t); int x; };
 
   // CHECK-LABEL: define{{.*}} void @_ZN5test11aEPNS_1AE(

clang/test/CodeGenCXX/delete.cpp

-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOSIZE
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 %s -emit-llvm -o - -Oz -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,CHECK-SIZE
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fno-sized-deallocation %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOSIZE
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fno-sized-deallocation %s -emit-llvm -o - -Oz -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,CHECK-SIZE
 
 void t1(int *a) {
   delete a;
 }
 
 struct S {
   int a;
 };

clang/test/CodeGenCXX/dllimport.cpp

-// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M32 %s
-// RUN: %clang_cc1 -triple x86_64-windows-msvc -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M64 %s
-// RUN: %clang_cc1 -triple i686-windows-gnu    -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s         -w | FileCheck --check-prefix=GNU --check-prefix=G32 %s
-// RUN: %clang_cc1 -triple x86_64-windows-gnu  -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s         -w | FileCheck --check-prefix=GNU %s
-// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-rtti -fno-threadsafe-statics -fms-extensions -fms-compatibility-version=18.00 -emit-llvm -std=c++1y -O1 -disable-llvm-passes -o - %s -DMSABI -w | FileCheck --check-prefix=MO1 --check-prefix=M18 %s
-// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-rtti -fno-threadsafe-statics -fms-extensions -fms-compatibility-version=19.00 -emit-llvm -std=c++1y -O1 -disable-llvm-passes -o - %s -DMSABI -w | FileCheck --check-prefix=MO1 --check-prefix=M19 %s
-// RUN: %clang_cc1 -triple i686-windows-gnu    -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O1 -fno-experimental-new-pass-manager -o - %s         -w | FileCheck --check-prefix=GO1 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M32 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC --check-prefix=M64 %s
+// RUN: %clang_cc1 -triple i686-windows-gnu    -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s         -w | FileCheck --check-prefix=GNU --check-prefix=G32 %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu  -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s         -w | FileCheck --check-prefix=GNU %s
+// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -fms-compatibility-version=18.00 -emit-llvm -std=c++1y -O1 -disable-llvm-passes -o - %s -DMSABI -w | FileCheck --check-prefix=MO1 --check-prefix=M18 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -fms-compatibility-version=19.00 -emit-llvm -std=c++1y -O1 -disable-llvm-passes -o - %s -DMSABI -w | FileCheck --check-prefix=MO1 --check-prefix=M19 %s
+// RUN: %clang_cc1 -triple i686-windows-gnu    -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O1 -fno-experimental-new-pass-manager -o - %s         -w | FileCheck --check-prefix=GO1 %s
 
 // CHECK-NOT doesn't play nice with CHECK-DAG, so use separate run lines.
-// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC2 %s
-// RUN: %clang_cc1 -triple i686-windows-gnu    -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s         -w | FileCheck --check-prefix=GNU2 %s
+// RUN: %clang_cc1 -triple i686-windows-msvc   -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=MSC2 %s
+// RUN: %clang_cc1 -triple i686-windows-gnu    -fno-sized-deallocation -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s         -w | FileCheck --check-prefix=GNU2 %s
 
 // Helper structs to make templates more expressive.
 struct ImplicitInst_Imported {};
 struct ImplicitInst_NotImported {};
 struct ExplicitDecl_Imported {};
 struct ExplicitInst_Imported {};
 struct ExplicitSpec_Imported {};
 struct ExplicitSpec_Def_Imported {};

clang/test/CodeGenCXX/new.cpp

-// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -fno-sized-deallocation -triple x86_64-unknown-unknown %s -emit-llvm -o - | FileCheck %s
 
 typedef __typeof__(sizeof(0)) size_t;
 
 // Declare an 'operator new' template to tickle a bug in __builtin_operator_new.
 template<typename T> void *operator new(size_t, int (*)(T));
 
 // Ensure that this declaration doesn't cause operator new to lose its
 // 'noalias' attribute.

clang/test/CodeGenCoroutines/coro-alloc.cpp

-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 \
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -fno-sized-deallocation \
 // RUN:    -Wno-coroutine-missing-unhandled-exception -emit-llvm %s -o - -disable-llvm-passes \
 // RUN:   | FileCheck %s
 
 namespace std {
 namespace experimental {
 template <typename... T>
 struct coroutine_traits; // expected-note {{declared here}}
 

clang/test/CodeGenCoroutines/coro-cleanup.cpp

 // Verify that coroutine promise and allocated memory are freed up on exception.
-// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++1z -fno-sized-deallocation -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
 
 namespace std::experimental {
 template <typename... T> struct coroutine_traits;
 
 template <class Promise = void> struct coroutine_handle {
   coroutine_handle() = default;
   static coroutine_handle from_address(void *) noexcept;
 };

clang/test/CodeGenCoroutines/coro-gro.cpp

 // Verifies lifetime of __gro local variable
 // Verify that coroutine promise and allocated memory are freed up on exception.
-// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++1z -fno-sized-deallocation -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
 
 namespace std::experimental {
 template <typename... T> struct coroutine_traits;
 
 template <class Promise = void> struct coroutine_handle {
   coroutine_handle() = default;
   static coroutine_handle from_address(void *) noexcept;
 };

clang/test/SemaCXX/MicrosoftExtensions.cpp

-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
-// RUN: %clang_cc1 -std=c++98 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
-// RUN: %clang_cc1 -std=c++11 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fexceptions -fcxx-exceptions -DTEST2
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 -fms-compatibility -verify -DTEST3
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -fno-sized-deallocation -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
+// RUN: %clang_cc1 -std=c++98 %s -triple i686-pc-win32 -fsyntax-only -fno-sized-deallocation -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
+// RUN: %clang_cc1 -std=c++11 %s -triple i686-pc-win32 -fsyntax-only -fno-sized-deallocation -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -fno-sized-deallocation -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fexceptions -fcxx-exceptions -DTEST2
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -fno-sized-deallocation -std=c++11 -fms-compatibility -verify -DTEST3
 
 #if TEST1
 
 // MSVC allows type definition in anonymous union and struct
 struct A
 {
   union
   {

clang/test/SemaCXX/builtin-operator-new-delete.cpp

-// RUN: %clang_cc1 -std=c++1z -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++03 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++03 -faligned-allocation -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -fsized-deallocation %s
+// RUN: %clang_cc1 -std=c++1z -fno-sized-deallocation -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++03 -fno-sized-deallocation -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++03 -fno-sized-deallocation -faligned-allocation -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fno-sized-deallocation -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fno-sized-deallocation -fsyntax-only -verify -fsized-deallocation %s
 
 #if !__has_builtin(__builtin_operator_new) || !__has_builtin(__builtin_operator_delete)
 #error builtins should always be available
 #endif
 
 #if __has_builtin(__builtin_operator_new) != 201802L || \
     __has_builtin(__builtin_operator_delete) != 201802L
 #error builtin should report updated value

clang/test/SemaCXX/unavailable_aligned_allocation.cpp

-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DMACOS %s
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fexceptions -std=c++1z -verify -DNO_ERRORS %s
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fexceptions -faligned-allocation -faligned-alloc-unavailable -std=c++14 -verify -DMACOS %s
-// RUN: %clang_cc1 -triple arm64-apple-ios10.0.0 -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DIOS %s
-// RUN: %clang_cc1 -triple arm64-apple-ios10.0.0 -fexceptions -std=c++1z -verify -DNO_ERRORS %s
-// RUN: %clang_cc1 -triple arm64-apple-tvos10.0.0 -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DTVOS %s
-// RUN: %clang_cc1 -triple arm64-apple-tvos10.0.0 -fexceptions -std=c++1z -verify -DNO_ERRORS %s
-// RUN: %clang_cc1 -triple armv7k-apple-watchos3.0.0 -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DWATCHOS %s
-// RUN: %clang_cc1 -triple armv7k-apple-watchos3.0.0 -fexceptions -std=c++1z -verify -DNO_ERRORS %s
-// RUN: %clang_cc1 -triple s390x-none-zos -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DZOS %s
-// RUN: %clang_cc1 -triple s390x-none-zos -fexceptions -std=c++1z -verify -DNO_ERRORS %s
-// RUN: %clang_cc1 -triple s390x-none-zos -fexceptions -faligned-allocation -faligned-alloc-unavailable -std=c++14 -verify -DZOS %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fno-sized-deallocation -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DMACOS %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fno-sized-deallocation -fexceptions -std=c++1z -verify -DNO_ERRORS %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -fno-sized-deallocation -fexceptions -faligned-allocation -faligned-alloc-unavailable -std=c++14 -verify -DMACOS %s
+// RUN: %clang_cc1 -triple arm64-apple-ios10.0.0 -fno-sized-deallocation -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DIOS %s
+// RUN: %clang_cc1 -triple arm64-apple-ios10.0.0 -fno-sized-deallocation -fexceptions -std=c++1z -verify -DNO_ERRORS %s
+// RUN: %clang_cc1 -triple arm64-apple-tvos10.0.0 -fno-sized-deallocation -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DTVOS %s
+// RUN: %clang_cc1 -triple arm64-apple-tvos10.0.0 -fno-sized-deallocation -fexceptions -std=c++1z -verify -DNO_ERRORS %s
+// RUN: %clang_cc1 -triple armv7k-apple-watchos3.0.0 -fno-sized-deallocation -fexceptions -faligned-alloc-unavailable -std=c++1z -verify -DWATCHOS %s
+// RUN: %clang_cc1 -triple armv7k-apple-watchos3.0.0 -fno-sized-deallocation -fexceptions -std=c++1z -verify -DNO_ERRORS %s
+// RUN: %clang_cc1 -triple s390x-none-zos -fexceptions -fno-sized-deallocation -faligned-alloc-unavailable -std=c++1z -verify -DZOS %s
+// RUN: %clang_cc1 -triple s390x-none-zos -fexceptions -fno-sized-deallocation -std=c++1z -verify -DNO_ERRORS %s
+// RUN: %clang_cc1 -triple s390x-none-zos -fexceptions -fno-sized-deallocation -faligned-allocation -faligned-alloc-unavailable -std=c++14 -verify -DZOS %s
 
 namespace std {
   typedef decltype(sizeof(0)) size_t;
   enum class align_val_t : std::size_t {};
   struct nothrow_t {};
   nothrow_t nothrow;
 }
 

clang/unittests/StaticAnalyzer/CallEventTest.cpp

EXPECT_TRUE(runCheckerOnCode<addCXXDeallocatorChecker>(R"(
     struct A {};
 
     void f() {
       A *a = new A;
       delete a;
     }
   )",
                                                          Diags));
-  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 1\n");
+  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 2\n");
 }
 
 } // namespace
 } // namespace ento
 } // namespace clang

clang/www/cxx_status.html

</tr>
     <tr>
       <td>C++ Sized Deallocation</td>
       <td><a href="https://wg21.link/n3778">N3778</a></td>
       <td class="full" align="center">Clang 3.4 <a href="#n3778">(7)</a></td>
     </tr>
 </table>
 
 <p>
-<span id="n3778">(7): In Clang 3.7 and later, sized deallocation is only enabled
-if the user passes the <code>-fsized-deallocation</code> flag. The user must
-supply definitions of the sized deallocation functions, either by providing them
-explicitly or by using a C++ standard library that does. <code>libstdc++</code>
-added these functions in version 5.0, and <code>libc++</code> added them in
-version 3.7.
+<span id="n3778">(7): The user must supply definitions of the sized deallocation
+functions, either by providing them explicitly or by using a C++ standard library
+that does. <code>libstdc++</code> added these functions in version 5.0, and
+<code>libc++</code> added them in version 3.7. The user can also use the
+<code>-fno-sized-deallocation</code> option to disable sized deallocation.
 </span>
 </p>
 </details>
 
 <h2 id="cxx17">C++17 implementation status</h2>
 
 <p>Clang 5 and later implement all the features of the
 <a href="https://www.iso.org/standard/68564.html">ISO C++ 2017 standard</a>.</p>