unwind's permanent residence

[Renato_Golin1]

Talked it over with Saleem on IRC, and I've come around to thinking libunwind is a better default for --rtlib=compiler-rt. Reason being that --rtlib=compiler-rt means libgcc probably isn't even available.

It's not just that, it's about making it self-contained. In a system
with both gcc and llvm libraries, with a package structure, you have
to depend on other packages. If I was to build a compiler-rt package,
I'd have to either include libunwind in it or create another package
for that and mark it as dependency for RT.

Since we do provide an unwinder, there is no point in making
compiler-rt depend on libgcc just because it provides libgcc_s. It's
the same as making libc++ depend on stdlibc++ because of some missing
components that we already ship on a different package.

On FreeBSD, we install compiler-rt as libgcc (or, at least, symlink it to libgcc). This means that all compilers can find it and we don't have issues when a gcc-compiled program loads a clang-compiled library (or vice versa).

This is the other end of the spectrum, where there is no gcc at all.
This is somewhat easier, since there is no cross-linking.

My worry of using libunwind by default on a libgcc-system is that
people will start finding a lot of interoperating trouble when they
dynamically link a gcc-compiled library which throws an exception and
you catch on your code. However, that kind of interoperation is
assumed to be correct, so we will have to cope with that anyway.

All in all, making it the default, at least on the master branch, will
expose those kind of problems earlier, so we can fix them before
shipping any stable compiler.

If, during release 3.7, there are still too many bugs, we can revert
the patch on branch release_37, so to have another six months to fix
them, and so on.

cheers,
--renato

[jroelofs]

Talked it over with Saleem on IRC, and I've come around to thinking libunwind is a better default for --rtlib=compiler-rt. Reason being that --rtlib=compiler-rt means libgcc probably isn't even available.

It's not just that, it's about making it self-contained. In a system
with both gcc and llvm libraries, with a package structure, you have
to depend on other packages. If I was to build a compiler-rt package,
I'd have to either include libunwind in it or create another package
for that and mark it as dependency for RT.

Since we do provide an unwinder, there is no point in making
compiler-rt depend on libgcc just because it provides libgcc_s. It's
the same as making libc++ depend on stdlibc++ because of some missing
components that we already ship on a different package.

On FreeBSD, we install compiler-rt as libgcc (or, at least, symlink it to libgcc). This means that all compilers can find it and we don't have issues when a gcc-compiled program loads a clang-compiled library (or vice versa).

This is the other end of the spectrum, where there is no gcc at all.
This is somewhat easier, since there is no cross-linking.

My worry of using libunwind by default on a libgcc-system is that
people will start finding a lot of interoperating trouble when they
dynamically link a gcc-compiled library which throws an exception and
you catch on your code. However, that kind of interoperation is
assumed to be correct, so we will have to cope with that anyway.

My worry is the scenario of throwing in one dso, then running a cleanup as you're unwinding through another dso. If that cleanup throws, and invokes a different unwinder, there's no way for that second unwinder to know about the state of the first. If this were c++, we'd break the requirement that throwing from a destructor during exception propagation causes the app to terminate.

I think there can really only be one unwinder per application.

Jon

[Dan_Albert]

This was my concern as well, but we are only talking about the default behavior of --rtlib=compiler-rt, which is a case where we have no extra information. For triples that we know for a fact use libgcc as the system unwinder it would make sense to have that target’s default be libgcc.

[Renato_Golin1]

In theory, the Itanium ABI doesn’t require that any implementation cooperates with any other, but in practice, libunwinder lives in a world where GNU is the law.

If we want to migrate one application at a time, we’ll have to interoperate. Not sure how, though…

Cheers,
Renato

[Renato_Golin1]

I agree, but it should be easy to change that behaviour by replacing the unwinder, not just adding a new one via -l.

Cheers,
Renato

[Renato_Golin1]

So, do we have a consensus?

AFAICS, the most agree solution (with optionals to be defined):

1. Move Unwinder to its own repository in the LLVM server
2. Make the CMake connections from libc++abi and compiler-rt
  2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT
  2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT
  2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder
3. Change clang to assume -lunwind when --rtlib=compiler-rt
  3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s
  3.2 OPTIONAL 5: Add option to change unwinder library by not adding
-lunwind/-lgcc_s, but whatever comes as argument

1, 2, and 3 must be changed.

I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later.

My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or
something like that.

I personally don't think the front-end scanning existing libraries is
a good thing to do, but I'm not against the idea, if anyone feels
strongly about it.

If all of us could agree to a common solution, and make sure all
interested parties are in, we should do the move before 3.7.

Please, cast your votes.

cheers,
--renato

[_C_Bergstrom]

I know I'm in some minority, but whatever ends up happening please be aware
that some platforms and targets aren't using the bundled unwind.

Specifically this non-gnu one (aka HP's libunwind permissively licensed)
www.nongnu.org/libunwind/download.html

In my experience when it's used in the compiler the name (libgcc, libeh,
libunwind) isn't always the same. (So searching for a particular lib name
as the unwinder is fragile)

[jroelofs]

Last time we brought this up, there was only partial consensus, and then
someone arbitrarily declared total consensus (without compelling arguments
in any particular direction) that we were going to move it to compiler_rt.
Then the discussion fell on the floor because no-one had time to actually do
the move. Please, let's not let that happen again this time.

So, do we have a consensus?

Wheeee Two Generals problem...

AFAICS, the most agree solution (with optionals to be defined):

1. Move Unwinder to its own repository in the LLVM server

      1.1 Freeze the code
      1.2 Copy unwinder to its own repo
      1.3 Migrate all the buildbots that care about libc++abi's unwinder
      1.4 Delete the old copy
      1.5 Unfreeze the code

2. Make the CMake connections from libc++abi and compiler-rt
   2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT
   2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT
   2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder
3. Change clang to assume -lunwind when --rtlib=compiler-rt
   3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s
   3.2 OPTIONAL 5: Add option to change unwinder library by not adding
-lunwind/-lgcc_s, but whatever comes as argument

1, 2, and 3 must be changed.

I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later.

+1

My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or
something like that.

I personally don't think the front-end scanning existing libraries is
a good thing to do,

Agreed.

but I'm not against the idea, if anyone feels
strongly about it.

If all of us could agree to a common solution, and make sure all
interested parties are in, we should do the move before 3.7.

+1

[jroelofs]

    I thought the ARM EHABI added a twist to this because it created
    some upward dependency from the unwinder to libc++abi.

ARM EHABI does have this upward dependency on the private type-infos for matching catch descriptors. Our implementation of EHABI doesn't really implement this part of the spec (See ARM EHABI, section #9.2 [1]). IIRC, we get away with that because clang/llvm doesn't code that needs them. (yet?)

Jon

    Other than that, I don’t have any strong feeling where it lives.

+1 to this. If you don't break the EHABI unwinder while moving things
around, I'm happy.

+Dan, he probably has an opinion on this topic.

    -Nick

[1]: Documentation – Arm Developer

[Saleem_Abdulrasool1]

> Last time we brought this up, there was only partial consensus, and then
> someone arbitrarily declared total consensus (without compelling
arguments
> in any particular direction) that we were going to move it to
compiler_rt.
> Then the discussion fell on the floor because no-one had time to
actually do
> the move. Please, let's not let that happen again this time.

So, do we have a consensus?

I think so.

AFAICS, the most agree solution (with optionals to be defined):

1. Move Unwinder to its own repository in the LLVM server
2. Make the CMake connections from libc++abi and compiler-rt
  2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT
  2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT
  2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder
3. Change clang to assume -lunwind when --rtlib=compiler-rt
  3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s
  3.2 OPTIONAL 5: Add option to change unwinder library by not adding
-lunwind/-lgcc_s, but whatever comes as argument

1, 2, and 3 must be changed.

I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later.

It seems that there is some interest in making the unwinder a build time
option, so that should be as good as 2.2 right? I am definitely fine with
3.1 (its no better or worse from the status quo).

I would like say that although Im not a fan of options 2.1 and 2.2, I do
think that 2.3 is a bad idea. There is far too much complexity involved,
and worse yet, it needs a number of knobs to get right -- what if I have
gcc 4.7, 4.8, 4.9 (each has its own libgcc_s), how do you select one? What
if the layout differs between Linux distributions? There is just too much
variability. Trying to reuse part of another compiler's resources is just
too brittle IMO, and Id rather not introduce more of that behavior into
clang.

My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or
something like that.

Thats fine; it is effectively what I was suggesting that we do if people
feel strongly that we need to support clang_rt + gcc_s.

I personally don't think the front-end scanning existing libraries is
a good thing to do, but I'm not against the idea, if anyone feels
strongly about it.

Strongly agreed. This can easily change subtly and diverge in behavior
from the linker.

If all of us could agree to a common solution, and make sure all
interested parties are in, we should do the move before 3.7.

I was hoping to do this much sooner (in the next few days) for the 3.7
release :-).

[Renato_Golin1]

This dependency is just for exception handling, not forced unwind, so
this could be a call into a handler that is in libc++abi.

Isn't that the difference between libgcc_s and libgcc_eh? Maybe we
could do the same, but let _eh live in libc++abi and _s live in
libunwind.

Having said that, it does sound a bit complicated...

cheers,
--renato

[Nick_Kledzik]

Last time we brought this up, there was only partial consensus, and then
someone arbitrarily declared total consensus (without compelling arguments
in any particular direction) that we were going to move it to compiler_rt.
Then the discussion fell on the floor because no-one had time to actually do
the move. Please, let's not let that happen again this time.

So, do we have a consensus?

AFAICS, the most agree solution (with optionals to be defined):

1. Move Unwinder to its own repository in the LLVM server

This is fine with me. But I thought when I first submitted the source there was push back because we “already have too many repositories”.

2. Make the CMake connections from libc++abi and compiler-rt
2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT
2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT
2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder
3. Change clang to assume -lunwind when --rtlib=compiler-rt
3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s
3.2 OPTIONAL 5: Add option to change unwinder library by not adding
-lunwind/-lgcc_s, but whatever comes as argument

I assume these are all just about ELF platforms. Darwin does not need any of this logic.

-Nick

[Renato_Golin1]

This is fine with me. But I thought when I first submitted the source there was push back because we “already have too many repositories”.

I know, but after the third attempt to put it in a better place, we
kind of agreed there is no better place than on its own.

I assume these are all just about ELF platforms. Darwin does not need any of this logic.

Yes, nor does FreeBSD, and probably other BSDs. This is a platform
specific setting in the Clang driver.

cheers,
--renato

[jroelofs]

+Iain

Last time we brought this up, there was only partial consensus, and then
someone arbitrarily declared total consensus (without compelling arguments
in any particular direction) that we were going to move it to compiler_rt.
Then the discussion fell on the floor because no-one had time to actually do
the move. Please, let's not let that happen again this time.

So, do we have a consensus?

AFAICS, the most agree solution (with optionals to be defined):

1. Move Unwinder to its own repository in the LLVM server
2. Make the CMake connections from libc++abi and compiler-rt
   2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT
   2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT
   2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder
3. Change clang to assume -lunwind when --rtlib=compiler-rt
   3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s
   3.2 OPTIONAL 5: Add option to change unwinder library by not adding
-lunwind/-lgcc_s, but whatever comes as argument

1, 2, and 3 must be changed.

I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later.

My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or
something like that.

I personally don't think the front-end scanning existing libraries is
a good thing to do, but I'm not against the idea, if anyone feels
strongly about it.

If all of us could agree to a common solution, and make sure all
interested parties are in, we should do the move before 3.7.

Please, cast your votes.

Out-of-band +1 on this from Iain Sandoe.

[Saleem_Abdulrasool1]

> Last time we brought this up, there was only partial consensus, and then
> someone arbitrarily declared total consensus (without compelling
arguments
> in any particular direction) that we were going to move it to
compiler_rt.
> Then the discussion fell on the floor because no-one had time to
actually do
> the move. Please, let's not let that happen again this time.

So, do we have a consensus?

AFAICS, the most agree solution (with optionals to be defined):

1. Move Unwinder to its own repository in the LLVM server

I think that we have consensus here, and we've given people time to chime
in. What exactly is the process for making this happen? I assume that
aKor or someone else would need to create the repository on the server side
so that it can be populated or is there someone else who should be
contacted so that we can get this going?

2. Make the CMake connections from libc++abi and compiler-rt

[Renato_Golin1]

Maybe ask Marshall for a final blessing, since he's the code owner of libc++.

We should also send an email to the list warning about the code freeze.

cheers,
--renato

[Saleem_Abdulrasool1]

> I think that we have consensus here, and we've given people time to chime
> in. What exactly is the process for making this happen? I assume that
aKor
> or someone else would need to create the repository on the server side so
> that it can be populated or is there someone else who should be
contacted so
> that we can get this going?

Maybe ask Marshall for a final blessing, since he's the code owner of
libc++.

A gentle ping for mclow :-).

[Marshall_Clow1]

I’m OK with this.

— Marshall

[Renato_Golin1]

Thanks Marshall.

Saleem, we need to create a new repository, copy the Unwind source
tree in there, and get CMake to work. I'm not experienced enough with
CMake to know heads or tails of it, but I can help you test it.

cheers,
-renato

[Saleem_Abdulrasool1]

> I’m OK with this.

Thanks Marshall.

Saleem, we need to create a new repository, copy the Unwind source
tree in there, and get CMake to work. I'm not experienced enough with
CMake to know heads or tails of it, but I can help you test it.

So after a bit of a hiatus (sorry, other stuff has been eating up my free
time), Id like to pick this up again. I think that its a matter of just
copying the unwind sources into the right place. Im hoping to do this
sometime this Friday (or perhaps Saturday). Any objections? I can
probably try to take a stab with the CMake side of things once the repo is
copied over.