This is an archive of the discontinued LLVM Phabricator instance.

[libcxxabi] Fix layout of __cxa_exception for win64, fixing static assert failures
ClosedPublic

Authored by mstorsjo on Feb 1 2020, 1:04 PM.

Details

Summary

Win64 isn't LP64, it's LLP64, but there's no __LLP64__ predefined - just check _WIN64 in addition to __LP64__.

This fixes compilation after static asserts about the struct layout were added in f2a436058fcbc11291e73badb44e243f61046183.

Diff Detail

Event Timeline

mstorsjo created this revision.Feb 1 2020, 1:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2020, 1:04 PM
mstorsjo edited the summary of this revision. (Show Details)Feb 1 2020, 1:04 PM
steven_wu accepted this revision.Feb 1 2020, 3:50 PM

Looks like this is an ABI change for WIN64. If that is intended then LGTM

This revision is now accepted and ready to land.Feb 1 2020, 3:50 PM

There's a Windows-based target that uses libUnwind-based exceptions?

Looks like this is an ABI change for WIN64. If that is intended then LGTM

I think that's ok - I don't think it's worth complicating this further to preserve compatibility with a previous binary layout which was accidental (using !LP64 layout on a platform with 64 bit pointers).

I presume this is a break for cases where an exception is thrown across libraries that have statically linked different versions of libcxxabi?

For cases where libcxxabi is linked in dynamically, all modules would be using the same copy of libcxxabi (either old or new) and it'd be consistent? I presume the __cxa_exception offsets don't end up in the calling application's binary anywhere, unless statically linked?

Is the layout of __cxa_exception supposed to match that of libgcc/libstdc++/libsupc++? Because they don't have e.g. the reference count stuck in a padding hole for 32 bit architectures.

There's a Windows-based target that uses libUnwind-based exceptions?

MinGW toolchains normally use libgcc/libstdc++, but one can build a toolchain that uses libunwind/libcxxabi/libcxx instead; I distribute such a toolchain at https://github.com/mstorsjo/llvm-mingw. But I don't really give any ABI guarantees; I've e.g. switched unwinding mechanisms (between dwarf, sjlj and SEH) before, to avoid bugs until they're fixed properly.

Looks like this is an ABI change for WIN64. If that is intended then LGTM

I think that's ok - I don't think it's worth complicating this further to preserve compatibility with a previous binary layout which was accidental (using !LP64 layout on a platform with 64 bit pointers).

Yeah, I think it's okay to consider this a bug.

I presume this is a break for cases where an exception is thrown across libraries that have statically linked different versions of libcxxabi?

You really aren't supposed to redundantly statically link libcxxabi like that; there are things in there that need to be unique.

For cases where libcxxabi is linked in dynamically, all modules would be using the same copy of libcxxabi (either old or new) and it'd be consistent? I presume the __cxa_exception offsets don't end up in the calling application's binary anywhere, unless statically linked?

It's supposed to have a stable layout (modulo the ability to extend it with prefixes). However, I'm not aware of any direct use of its fields by the compiler.

Is the layout of __cxa_exception supposed to match that of libgcc/libstdc++/libsupc++? Because they don't have e.g. the reference count stuck in a padding hole for 32 bit architectures.

That's interesting. Most details of the layout are indeed supposed to be portable across compilers and runtimes.

There's a Windows-based target that uses libUnwind-based exceptions?

MinGW toolchains normally use libgcc/libstdc++, but one can build a toolchain that uses libunwind/libcxxabi/libcxx instead; I distribute such a toolchain at https://github.com/mstorsjo/llvm-mingw. But I don't really give any ABI guarantees; I've e.g. switched unwinding mechanisms (between dwarf, sjlj and SEH) before, to avoid bugs until they're fixed properly.

Okay.

I presume this is a break for cases where an exception is thrown across libraries that have statically linked different versions of libcxxabi?

You really aren't supposed to redundantly statically link libcxxabi like that; there are things in there that need to be unique.

Out of curiosity - which are these? Things like typeids not being unique, or are there things with global state as well?

I presume this is a break for cases where an exception is thrown across libraries that have statically linked different versions of libcxxabi?

You really aren't supposed to redundantly statically link libcxxabi like that; there are things in there that need to be unique.

Out of curiosity - which are these? Things like typeids not being unique, or are there things with global state as well?

typeids are the big thing, but there's global state associated with __cxa_atexit, thread-local variables, initialization guards, the terminate and unexpected handlers, and so on.

I presume this is a break for cases where an exception is thrown across libraries that have statically linked different versions of libcxxabi?

You really aren't supposed to redundantly statically link libcxxabi like that; there are things in there that need to be unique.

Out of curiosity - which are these? Things like typeids not being unique, or are there things with global state as well?

typeids are the big thing, but there's global state associated with __cxa_atexit, thread-local variables, initialization guards, the terminate and unexpected handlers, and so on.

Right - ok, thanks!

The only case I know which can break after ABI change is the application has its own implementations to inspecting and handling exceptions. Almost everything go through libunwind and libcxxabi so as long as they are agree on the ABI, you have most of the cases covered.

This revision was automatically updated to reflect the committed changes.