Re: macppc libunwind without altivec
> Date: Thu, 2 Apr 2020 16:05:09 -0400 > From: George Koehler > > Hello tech, > > powerpc libunwind is broken on machines without altivec. It crashes > SIGILL when code (built with base-clang++) throws a C++ exception, > because libunwind always saves the altivec registers. You don't have > altivec if sysctl machdep.altivec is 0. I believe that G3 cpus don't > have altivec, and G4 and G5 do have it. > > This diff defers saving the altivec registers until we need to access > them. I took the idea from arm, which defers saving VFP registers. > The diff fixes a small C++ demo on my G3. I didn't try other C++ > code; I was building ports with macppc base-clang but stopped at an > error from lang/python/3.7. > > Registers_arm has members like "bool _saved_vfp_d0_d15;" to know if > VFP got saved. I can't add a "bool _saved_vrs" to Registers_ppc, > because some assertions would fail, because unw_context_t would be > too small. I don't enlarge unw_context_t (an opaque struct of 117 * 8 > bytes) because it is in /usr/include/c++/v1/libunwind.h and I don't > want to change the ABI. I instead check if vrsave != 0; vrsave is a > bitset of altivec registers in use (Altivec Programming Environments > Manual, ALTIVECPEM.pdf, section 2.3.3). > > libunwind operates on a saved context, not real registers. If a > compiler exists that would tell libunwind to set v31 when vrsave == 0, > this diff would break it. I have no bool to check if I have already > saved the vrs but vrsave == 0. > > I also stop using the red zone, because it doesn't exist: a signal > handler may clobber anything below the stack pointer. > > Is the diff OK to commit? It applies to src/lib/libunwind but you > build it in src/lib/libcxxabi > > $ cat thrown.cpp > #include > #include > > using std::cout; > using std::runtime_error; > > int > main() > { > try { > throw runtime_error("ouch"); > } catch(runtime_error e) { > cout << "caught " << e.what() << "\n"; > } > } > $ clang++ -o thrown thrown.cpp > $ ./thrown # without the fix > Illegal instruction (core dumped) > $ ./thrown # with the fixed libc++abi.so.2.0 > caught ouch ok kettenis@ > Index: src/Registers.hpp > === > RCS file: /cvs/src/lib/libunwind/src/Registers.hpp,v > retrieving revision 1.8 > diff -u -p -r1.8 Registers.hpp > --- src/Registers.hpp 17 Jun 2019 22:28:51 - 1.8 > +++ src/Registers.hpp 2 Apr 2020 16:40:32 - > @@ -630,7 +630,7 @@ private: > unsigned int __lr; /* Link register */ > unsigned int __ctr;/* Count register */ > unsigned int __mq; /* MQ register (601 only) */ > -unsigned int __vrsave; /* Vector Save Register */ > +mutable unsigned int __vrsave; /* Vector Save Register */ >}; > >struct ppc_float_state_t { > @@ -640,9 +640,11 @@ private: > unsigned int __fpscr; /* floating point status register */ >}; > > + void saveVectorRegisters() const; > + >ppc_thread_state_t _registers; >ppc_float_state_t _floatRegisters; > - v128 _vectorRegisters[32]; // offset 424 > + mutable v128 _vectorRegisters[32]; // offset 424 > }; > > inline Registers_ppc::Registers_ppc(const void *registers) { > @@ -657,10 +659,8 @@ inline Registers_ppc::Registers_ppc(cons > sizeof(_floatRegisters)); >static_assert(sizeof(ppc_thread_state_t) + sizeof(ppc_float_state_t) == > 424, > "expected vector register offset to be 424 bytes"); > - memcpy(_vectorRegisters, > - static_cast(registers) + > sizeof(ppc_thread_state_t) + > - sizeof(ppc_float_state_t), > - sizeof(_vectorRegisters)); > + // no values until saveVectorRegisters() > + memset(&_vectorRegisters, 0, sizeof(_vectorRegisters)); > } > > inline Registers_ppc::Registers_ppc() { > @@ -780,6 +780,7 @@ inline uint32_t Registers_ppc::getRegist >case UNW_PPC_CR7: > return (_registers.__cr & 0x000F); >case UNW_PPC_VRSAVE: > +saveVectorRegisters(); > return _registers.__vrsave; >} >_LIBUNWIND_ABORT("unsupported ppc register"); > @@ -932,6 +933,7 @@ inline void Registers_ppc::setRegister(i > _registers.__cr |= (value & 0x000F); > return; >case UNW_PPC_VRSAVE: > +saveVectorRegisters(); > _registers.__vrsave = value; > return; > // not saved > @@ -976,12 +978,14 @@ inline bool Registers_ppc::validVectorRe > > inline v128 Registers_ppc::getVectorRegister(int regNum) const { >assert(validVectorRegister(regNum)); > + saveVectorRegisters(); >v128 result = _vectorRegisters[regNum - UNW_PPC_V0]; >return result; > } > > inline void Registers_ppc::setVectorRegister(int regNum, v128 value) { >assert(validVectorRegister(regNum)); > + saveVectorRegisters(); >_vectorRegisters[regNum - UNW_PPC_V0] = value; > } > > Index: src/UnwindRegistersRestore.S >
macppc libunwind without altivec
Hello tech, powerpc libunwind is broken on machines without altivec. It crashes SIGILL when code (built with base-clang++) throws a C++ exception, because libunwind always saves the altivec registers. You don't have altivec if sysctl machdep.altivec is 0. I believe that G3 cpus don't have altivec, and G4 and G5 do have it. This diff defers saving the altivec registers until we need to access them. I took the idea from arm, which defers saving VFP registers. The diff fixes a small C++ demo on my G3. I didn't try other C++ code; I was building ports with macppc base-clang but stopped at an error from lang/python/3.7. Registers_arm has members like "bool _saved_vfp_d0_d15;" to know if VFP got saved. I can't add a "bool _saved_vrs" to Registers_ppc, because some assertions would fail, because unw_context_t would be too small. I don't enlarge unw_context_t (an opaque struct of 117 * 8 bytes) because it is in /usr/include/c++/v1/libunwind.h and I don't want to change the ABI. I instead check if vrsave != 0; vrsave is a bitset of altivec registers in use (Altivec Programming Environments Manual, ALTIVECPEM.pdf, section 2.3.3). libunwind operates on a saved context, not real registers. If a compiler exists that would tell libunwind to set v31 when vrsave == 0, this diff would break it. I have no bool to check if I have already saved the vrs but vrsave == 0. I also stop using the red zone, because it doesn't exist: a signal handler may clobber anything below the stack pointer. Is the diff OK to commit? It applies to src/lib/libunwind but you build it in src/lib/libcxxabi $ cat thrown.cpp #include #include using std::cout; using std::runtime_error; int main() { try { throw runtime_error("ouch"); } catch(runtime_error e) { cout << "caught " << e.what() << "\n"; } } $ clang++ -o thrown thrown.cpp $ ./thrown # without the fix Illegal instruction (core dumped) $ ./thrown # with the fixed libc++abi.so.2.0 caught ouch Index: src/Registers.hpp === RCS file: /cvs/src/lib/libunwind/src/Registers.hpp,v retrieving revision 1.8 diff -u -p -r1.8 Registers.hpp --- src/Registers.hpp 17 Jun 2019 22:28:51 - 1.8 +++ src/Registers.hpp 2 Apr 2020 16:40:32 - @@ -630,7 +630,7 @@ private: unsigned int __lr; /* Link register */ unsigned int __ctr;/* Count register */ unsigned int __mq; /* MQ register (601 only) */ -unsigned int __vrsave; /* Vector Save Register */ +mutable unsigned int __vrsave; /* Vector Save Register */ }; struct ppc_float_state_t { @@ -640,9 +640,11 @@ private: unsigned int __fpscr; /* floating point status register */ }; + void saveVectorRegisters() const; + ppc_thread_state_t _registers; ppc_float_state_t _floatRegisters; - v128 _vectorRegisters[32]; // offset 424 + mutable v128 _vectorRegisters[32]; // offset 424 }; inline Registers_ppc::Registers_ppc(const void *registers) { @@ -657,10 +659,8 @@ inline Registers_ppc::Registers_ppc(cons sizeof(_floatRegisters)); static_assert(sizeof(ppc_thread_state_t) + sizeof(ppc_float_state_t) == 424, "expected vector register offset to be 424 bytes"); - memcpy(_vectorRegisters, - static_cast(registers) + sizeof(ppc_thread_state_t) + - sizeof(ppc_float_state_t), - sizeof(_vectorRegisters)); + // no values until saveVectorRegisters() + memset(&_vectorRegisters, 0, sizeof(_vectorRegisters)); } inline Registers_ppc::Registers_ppc() { @@ -780,6 +780,7 @@ inline uint32_t Registers_ppc::getRegist case UNW_PPC_CR7: return (_registers.__cr & 0x000F); case UNW_PPC_VRSAVE: +saveVectorRegisters(); return _registers.__vrsave; } _LIBUNWIND_ABORT("unsupported ppc register"); @@ -932,6 +933,7 @@ inline void Registers_ppc::setRegister(i _registers.__cr |= (value & 0x000F); return; case UNW_PPC_VRSAVE: +saveVectorRegisters(); _registers.__vrsave = value; return; // not saved @@ -976,12 +978,14 @@ inline bool Registers_ppc::validVectorRe inline v128 Registers_ppc::getVectorRegister(int regNum) const { assert(validVectorRegister(regNum)); + saveVectorRegisters(); v128 result = _vectorRegisters[regNum - UNW_PPC_V0]; return result; } inline void Registers_ppc::setVectorRegister(int regNum, v128 value) { assert(validVectorRegister(regNum)); + saveVectorRegisters(); _vectorRegisters[regNum - UNW_PPC_V0] = value; } Index: src/UnwindRegistersRestore.S === RCS file: /cvs/src/lib/libunwind/src/UnwindRegistersRestore.S,v retrieving revision 1.8 diff -u -p -r1.8 UnwindRegistersRestore.S --- src/UnwindRegistersRestore.S17 Jun 2019 22:28:51 - 1.8 +++ src/UnwindRegistersRestore.S2 Apr 2020 16:40:33 - @@ -476,11