Missing [[no_unique_address]] support on Windows

[ClawmanCat]

Bugzilla Link	50014
Version	trunk
OS	Windows NT
Attachments	test.cpp: the source file that triggers the warning. log.txt: console output from compiling test.cpp.
CC	@AaronBallman,@CaseyCarter,@Ivan171,@zygoloid

Extended Description

https://clang.llvm.org/cxx_status.html#cxx20 lists the [[no_unique_address]] attribute as supported since Clang 9. However, the attribute does not seem to currently be supported on Windows platforms.

Compiling the following source code on Windows with Clang 12.0.0 with -std=c++20:

struct X {};

struct Y1 {
	[[no_unique_address]] X x;
	long long a;
};

struct Y2 {
	X x;
	long long a;
};

int main() {}

gives the following warning:

test.cpp:4:4: warning: unknown attribute 'no_unique_address' ignored [-Wunknown-attributes]
        [[no_unique_address]] X x;
          ^~~~~~~~~~~~~~~~~
1 warning generated.

The size of objects from Y1 are also the same size as those from Y2, indicating that an unique address is indeed still generated:

std::cout << sizeof(Y1) << ", " << sizeof(Y2);

prints "16, 16" compared to "8, 16" on Linux: https://godbolt.org/z/nfaf3KeP3

[Ivan171]

*** Bug llvm/llvm-bugzilla-archive#51051 has been marked as a duplicate of this bug. ***

[AaronBallman]

This is not a bug. Microsoft has not defined the ABI for this attribute on their platform, so Clang needs to wait for Microsoft to define that before we can support [[no_unique_address]] there (otherwise we run into potential ABI incompatibilities).

However, I'm leaving this request open because presumably Microsoft will define the ABI on their platform for this feature, and then Clang can follow suit.

[ClawmanCat]

This is not a bug. Microsoft has not defined the ABI for this attribute on
their platform, so Clang needs to wait for Microsoft to define that before
we can support [[no_unique_address]] there (otherwise we run into potential
ABI incompatibilities).

However, I'm leaving this request open because presumably Microsoft will
define the ABI on their platform for this feature, and then Clang can follow
suit.

Microsoft already added support for this as of VS 2019 16.9.
(https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-160)

[AaronBallman]

This is not a bug. Microsoft has not defined the ABI for this attribute on
their platform, so Clang needs to wait for Microsoft to define that before
we can support [[no_unique_address]] there (otherwise we run into potential
ABI incompatibilities).

However, I'm leaving this request open because presumably Microsoft will
define the ABI on their platform for this feature, and then Clang can follow
suit.

Microsoft already added support for this as of VS 2019 16.9.
(https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-
conformance?view=msvc-160)

Alas, but I don't believe the documentation based on how their compiler behaves in practice (which is not something Clang will be emulating because it looks to be a buggy implementation). See https://godbolt.org/z/j1PzbK68P, but basically, __has_cpp_attribute returns false for both no_unique_address and msvc::no_unique_address, yet the latter actually does the correct thing that the former is standardized to do while the former is silently ignored.

[CaseyCarter]

It is not intended that MSVC predefines __has_cpp_attribute(msvc::no_unique_address) to 0. (Copy-pasta strikes again!) I'll get it changed to 201803L. It is intentional that MSVC predefines __has_cpp_attribute(no_unique_address) to 0, however - we don't consider the no-op implementation to be worthy of claiming support for the attribute.

As an STL developer, I would greatly appreciate if clang-cl were to implement [[msvc::no_unique_address]]. I'm willing to bribe people with adult beverages =)

[Ivan171]

mentioned in issue #50395

[AaronBallman]

It is not intended that MSVC predefines
__has_cpp_attribute(msvc::no_unique_address) to 0. (Copy-pasta strikes again!) I'll get it changed to 201803L.

Thanks!

It is intentional that MSVC predefines __has_cpp_attribute(no_unique_address) to 0, however - we don't consider the no-op implementation to be worthy of claiming support for the attribute.

Personally, I think it's unfortunate that Microsoft is not implementing the standard attribute but are instead creating problems by introducing a vendor-specific attribute that behaves the same as the standard attribute.

As an STL developer, I would greatly appreciate if clang-cl were to implement [[msvc::no_unique_address]]. I'm willing to bribe people with adult beverages =)

As someone who writes portable code and hates jumping through ridiculous hoops to do so, I would greatly appreciate if cl were to implement [[no_unique_address]] (I, too, can bribe people with adult beverages!).

That said, I have no problem with Clang implementing [[msvc::no_unique_address]] so long as we match Microsoft's ABI for it. Is there any documentation available on the ABI for this?

[zmodem]

I'm not sure what the best way forward is for us here. Since MSVC treats [[no_unique_address]] as a no-op, we can't implement it in a way that's both compatible and standards-conforming. But just waiting isn't great either.

Is [[no_unique_address]] used in any MS system headers yet? How bad would it be if we ignore MSVC here and just use clang's current implementation?

Also, like it or not, if there is a bunch of code using [[msvc::no_unique_address]], we'll probably have to implement it. I see it's mentioned in libc++ already (https://github.com/llvm/llvm-project/blob/05337a7/libcxx/include/__config#L1404). I wonder if just aliasing it to the current attribute would work..

cc'ing @mstorsjo @nico and @rnk

[AaronBallman]

But just waiting isn't great either.

Agreed that it's not great, but rushing is also not great given the ABI concerns.

How bad would it be if we ignore MSVC here and just use clang's current implementation?

I'm strongly opposed because of ABI. Using fields with [[no_unique_address]] will have potentially different offsets depending on whether compiling with clang-cl or cl.

Also, like it or not, if there is a bunch of code using [[msvc::no_unique_address]], we'll probably have to implement it.

I'm absolutely happy to see an implementation for it once we know we can match the Microsoft ABI for it. Without that, I have the same ABI concerns as above.

[nico]

It sounds like supporting [[msvc::no_unique_address]] is uncontroversial, so we should do that.

For [[no_unique_address]], how about we make up a mangling (that's unlikely to colilde with cl), use clang's existing codepath for it, and emit some -Wmicrosoft- warning that says "ABI-incompatible with cl.exe" when it's used in ms mode?

[AaronBallman]

It sounds like supporting [[msvc::no_unique_address]] is uncontroversial, so we should do that.

+1, hopefully someone from Microsoft can help with the ABI questions so we don't have to reverse engineer it.

For [[no_unique_address]], how about we make up a mangling (that's unlikely to colilde with cl), use clang's existing codepath for it, and emit some -Wmicrosoft- warning that says "ABI-incompatible with cl.exe" when it's used in ms mode?

The warning helps but it doesn't actually address the ABI issues (users are still going to have a hard ABI break and this decreases our compatibility with MSVC). I think when the user passes -fms-compatibility, we should be compatible with MSVC.

[nico]

+1, hopefully someone from Microsoft can help with the ABI questions so we don't have to reverse engineer it.

Note that "reverse engineer" is often understood as reverse engineering cl.exe and looking at its assembly. This is not something we've ever done.

We've very often compiled test programs with cl.exe and looked at its output. That's how most of clang-cl's behavior was implemented. This is not reverse engineering. (This distinction is important for EULA reasons etc.)

With that out of the way: Sure, it'd be nice to have docs, but we wrote most of clang-cl without such docs.

Re non-prefixed [[no_unique_address]]:

We have a history of inventing ABI stuff until actual ABI arrives (eg https://reviews.llvm.org/rG2ac58801873ab99dd5de48ad7557b76f1803100b – but see git log clang/lib/AST/MicrosoftMangle.cpp for many more examples). If there's a warning, that's better than we usually do. I think most people don't use C++ classes across ABI boundaries, which is why we can usually get away with this. (I suppose MFC is an exception, but that luckily doesn't adopt new features aggressively.)

We also have a history for being standards-compliant by default where it makes sense (different defaults for various of the /Zc: flags).

I guess we can also add a /Zc: flag to pick what [[no_unique_address]] is supposed to do in ms mode. (If this attribute ends up being used in system headers, that'd be a problem though.)

Given that __has_cpp_attribute(no_unique_address) is 0 for cl.exe (or intended to be 0 at least), you'd probably know that you're getting into trouble if you still end up using it in both cl and clang-cl and expect ABI compat.

[AaronBallman]

Note that "reverse engineer" is often understood as reverse engineering cl.exe and looking at its assembly. This is not something we've ever done.
We've very often compiled test programs with cl.exe and looked at its output. That's how most of clang-cl's behavior was implemented. This is not reverse engineering. (This distinction is important for EULA reasons etc.)

Good points, I definitely did not mean the former interpretation. :-)

With that out of the way: Sure, it'd be nice to have docs, but we wrote most of clang-cl without such docs.

It doesn't have to be docs; it can be extensive tests, for example. However, there are folks from Microsoft on this thread asking us to support their attribute, so I'm hoping they can volunteer to help verify the feature (either with documentation, test cases, etc).

We have a history of inventing ABI stuff until actual ABI arrives...

My point is that the ABI has already arrived and we need to match it otherwise we're making extra work for ourselves and our users.

I think most people don't use C++ classes across ABI boundaries...

I don't think we can say what most people do or don't do.

I guess we can also add a /Zc: flag to pick what [[no_unique_address]] is supposed to do in ms mode. (If this attribute ends up being used in system headers, that'd be a problem though.)

I'm not super keen on adding a /Zc: flag for this (I have no reason to believe this won't show up in system headers on some configurations like Cygwin etc), but hiding it behind a feature flag of some sort seems more plausible. However, what's the use case for deviating from MSVC's behavior when the user passes -fms-compatibility?

[mstorsjo]

Given that __has_cpp_attribute(no_unique_address) is 0 for cl.exe (or intended to be 0 at least), you'd probably know that you're getting into trouble if you still end up using it in both cl and clang-cl and expect ABI compat.

Actually, no - earlier, when using [[no_unique_address]] in cl.exe, you got a warning about it being unknown/unsupported - but since 16.9, it no longer warns. The attribute is considered known and implemented - it's just implemented as doing nothing (until the next time they break their C++ ABI, if they ever do that again - after that, it's supposed to be changed to doing the right thing).

[nico]

However, what's the use case for deviating from MSVC's behavior when the user passes -fms-compatibility?

We gave a long talk about this a while ago: https://docs.google.com/presentation/d/1oxNHaVjA9Gn_rTzX6HIpJHP7nXRua_0URXxxJ3oYRq0/edit#slide=id.p

The use case is "I want to be able to build software against the Windows SDK". Under this interpretation, as long as you can link against system libs, all's well.

But @mstorsjo's point makes sense – by default we have to match what cl.exe does. But having some opt-in thing (/Zc: flag to toggle it globally, and maybe a [[clang::no_unique_address_and_i_really_mean_it]] to opt in on a case-by-case basis still seems good?

[rnk]

I fired off an email to some folks at Microsoft asking for some guidance, FWIW. I think it would be reasonable to expose an alternative attribute spelling in the meantime, as annoying as that is. Is [[clang::no_unique_address]] possible?

I think historically we have aimed for more ABI compatibility than just the ability to link against the SDK. Back when we supported MSVC fallback, you could get arbitrary mixes of translation units between MSVC and clang-cl.

[mstorsjo]

I fired off an email to some folks at Microsoft asking for some guidance, FWIW. I think it would be reasonable to expose an alternative attribute spelling in the meantime, as annoying as that is. Is [[clang::no_unique_address]] possible?

Isn't [[msvc::no_unique_address]] such an alternative spelling, which cl.exe already supports?

[AaronBallman]

The use case is "I want to be able to build software against the Windows SDK".

Is there evidence that [[no_unique_address]] is being used in the Windows SDK? I had the impression the situation was that the standard attribute is not supported by MSVC or used by any Windows platform SDK, but that the msvc variant was supported (and maybe used in the system headers).

Isn't [[msvc::no_unique_address]] such an alternative spelling, which cl.exe already supports?

That's my belief. I think we should just support that form rather than introduce a third form of the attribute.

[nico]

Isn't [[msvc::no_unique_address]] such an alternative spelling, which cl.exe already supports?

Ah, I had misread the bug and had misunderstood what that does.

I think then it's probably best to just make [[no_unique_address]] a no-op in clang-cl and give [[msvc::no_unique_address]] the behavior that [[no_unique_address]] has elsewhere, and punt picking the right one to all projects via _MSC_VER check :/

[rnk]

Yeah, I agree. I would still like input from MSVC folks on why they made their own spelling here and make the standard feature a no-op.

[mstorsjo]

Yeah, I agree. I would still like input from MSVC folks on why they made their own spelling here and make the standard feature a no-op.

I got it explained to me in microsoft/STL#1364 (comment), and https://devblogs.microsoft.com/cppblog/msvc-cpp20-and-the-std-cpp20-switch/#msvc-extensions-and-abi also hints at the same: Due to the fact that they silently accepted the attribute without actually doing anything about it, for a number of releases, they considered that "accepting and not doing anything about it" had been settled in ABI as users could have ended up using it blindly, so that changing it could break existing deployed code.

[JVApen]

I think then it's probably best to just make [[no_unique_address]] a no-op in clang-cl and give [[msvc::no_unique_address]] the behavior that [[no_unique_address]] has elsewhere, and punt picking the right one to all projects via _MSC_VER check :/

As concerned user, this sounds like a really good solution! Can we also get a warning about the no-op behavior. So that users that want to use this, know it ain't working and how they should adapt their code to get the intended behavior?

[akukh]

I want to update the question about this, as I understand at the moment there is still no support for [[no_unique_address]] for Windows. But, is there a chance to get support for the attribute in the near future? Or perhaps there is some workaround to get [[no_unique_address]] under Windows working?

[zmodem]

But, is there a chance to get support for the attribute in the near future?

It depends on what you mean by "near". It's certainly on the todo list, but I don't think anyone is prepared to commit to a timeline.

[AaronBallman]

Support for [[no_unique_address]] on Windows is waiting for Microsoft to define what that actually means for the ABI; there is no timeline for this.

Support for [[msvc::no_unique_address]]` is waiting for someone to do the implementation work to ensure we're matching Microsoft's ABI for their vendor attribute; I don't know anyone is prepared to commit to a timeline for that.

[akukh]

But, is there a chance to get support for the attribute in the near future?

It depends on what you mean by "near". It's certainly on the todo list, but I don't think anyone is prepared to commit to a timeline.

For myself, I defined the "near future" in the probable next major (?) release (15.x). But yes, I understand that it still depends on the guys from Microsoft. Just as I understand, Microsoft has already implemented its [[msvc::no_unique_address]] and I was just wondering if it would be possible to use it somehow.

However, Aaron gave the answer I was expecting, thank you.

[AaronBallman]

I think the next steps here are to:

• Add an alias for the attribute
• Modify MicrosoftRecordLayoutBuilder in RecordLayoutBuilder.cpp to implement the attribute
• Ignore the attribute if it does not have the proper spelling (the msvc-namespace spelling)
• Incorporate the above test cases and ensure our empty base subobject avoidance is compatible with MSVC

I think those next steps sound correct to me, and thank you to @amykhuang for looking into the effort!

[amykhuang]

forgot to update here that I have a patch at https://reviews.llvm.org/D157762 (the implementation is still very much a work in progress)

There are a couple of cases (that I've found so far) where I can't figure out MSVC's intended behavior, such as https://godbolt.org/z/6cP554ddb. @CaseyCarter is this something you could help with?

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[CaseyCarter]

There are a couple of cases (that I've found so far) where I can't figure out MSVC's intended behavior, such as https://godbolt.org/z/6cP554ddb. @CaseyCarter is this something you could help with?

This is DevCom-10416202, with an added "Oops, we sometimes layout distinct objects at the same address". I sincerely hope this is not intended behavior since it's very much not conformant. I've pinged the internal bug tracker and added a slightly reduced version of your repro (https://godbolt.org/z/3GGj9xsfe).

[rnk]

@CaseyCarter if this layout is a bug, can you provide some guidance on how and where Clang should aim for MSVC compatibility? In other words, should we expect a bug fix that changes the layout of these structs in future compiler versions, and if so, should Clang just not worry about being bug for bug compatible here?

There are some other ways we could approach this:

• Implement bug-for-bug compatibility with [[msvc::no_unique_address]], and in the future add version checks to align clang-cl ABI with MSVC ABI. This is a lot of work. MSVC's record layout is quite convoluted, and matching compatibly it was a many month project, IIRC. See the extensive comments about vtordisp/vfptr/vbptr injection. 😨
• Implement [[clang::no_unique_address]], which uses the logic of [[no_unique_address]] from other platforms, and is known to be ABI incompatible with both [[no_unique_address]] and [[msvc::no_unique_address]].
• Do the above, but use either the [[no_unique_address]] or [[msvc::no_unique_address]] spelling.

The main top priority use case for some form of no_unique_address support in libc++ is to eliminate compressed_pair. I believe folks using libc++ generally do not aim for ABI compatibility with the MSVC compiler, so for libc++ purposes, we don't need an attribute that is perfectly ABI compatible with the MSVC compiler. If users want Clang/MSVC ABI compatibility, those users typically use the MSVC STL, which is what any MSVC-precompiled code would use.

---

After re-reading what I've written, I'm unfortunately leading myself to the conclusion that we should implement [[clang::no_unique_address]]. This is not a good outcome for users, who probably just wanted one standard attribute with a stable ABI, but here we are.

[CaseyCarter]

@CaseyCarter if this layout is a bug, can you provide some guidance on how and where Clang should aim for MSVC compatibility? In other words, should we expect a bug fix that changes the layout of these structs in future compiler versions, and if so, should Clang just not worry about being bug for bug compatible here?

I poked the pertinent compiler dev about DevCom-10416202 - which they fixed just in time for VS2022 17.8 preview 3 - but I didn't make it clear that I had an expanded repro from you folks with more non-conforming class layouts involving [[msvc::no_unique_address]]. Most of the failures you observe are still present even after the fix, so I've filed another issue at DevCom-10456269 and asked said compiler dev (who's off today) if they can take a look early next week and give me an idea of whether it will be trivial to fix soon, or have to be deprioritized for weeks/months.

There are some other ways we could approach this:

• Implement bug-for-bug compatibility with [[msvc::no_unique_address]], and in the future add version checks to align clang-cl ABI with MSVC ABI. This is a lot of work. MSVC's record layout is quite convoluted, and matching compatibly it was a many month project, IIRC. See the extensive comments about vtordisp/vfptr/vbptr injection. 😨

I do not recommend bug compatibility here. This isn't a part of the ABI that's been baked-in for decades, I don't even know of any current users of [[msvc::no_unique_address]]. MSVC should fix our mess.

[amykhuang]

sounds like because the attribute is relatively unused and we're only planning to use it in libc++, it shouldn't be too much of an issue if the ABI doesn't match up at the moment?

also, have a few more questions on layout here https://godbolt.org/z/nMexrWxG4, sorry it's a bit long

[rnk]

Thanks for the input! I think we can proceed with landing an implementation of [[msvc::no_unique_address]] that isn't 100% ABI compatible with MSVC. Our understanding is that neither compiler guarantees ABI stability for layouts using this attribute yet, and we can use it in libc++ now, and improve MSVC layout compatibility later.

[CaseyCarter]

also, have a few more questions on layout here https://godbolt.org/z/nMexrWxG4, sorry it's a bit long

(Unrelated nit: offsetof is defined in <cstddef>, not <cstdlib>.)

The struct F case here is identical to the A2 case in the previous repro; I'm still calling it a bug.
The struct Z case no longer reproes with the tip-of-trunk compiler; alignof(Z) is now 8. I believe the not-yet-released fix for DevCom-10416202 fixed this.

I'll send this on the compiler team and see if it elucidates any comments.

[AaronBallman]

Thanks for the input! I think we can proceed with landing an implementation of [[msvc::no_unique_address]] that isn't 100% ABI compatible with MSVC. Our understanding is that neither compiler guarantees ABI stability for layouts using this attribute yet, and we can use it in libc++ now, and improve MSVC layout compatibility later.

Based on the discussion in this thread, I concur. When landing the changes, I think we should explicitly document the expectations so users are aware we're not making them any promises regarding ABI compatibility or stability.

Thank you to everyone for pulling this together!

[zmodem]

Does 4a55d42 mean this is done?

[philnik777]

I think so.