[libcxxabi patch] Add Registers_arm class and unw_getcontext implementation

[Nick Kledzik]

On Dec 14, 2013, at 12:25 PM, Nico Weber <thakis at chromium.org> wrote:
> Hi Nick,
> 
> I just realized we didn't cc you on our replies here:
> Albert: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131202/095019.html
> Me: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131209/095089.html
> 
> Sorry!
> 
> Adding Albert to this branch of the thread, to hopefully bring us all on the same page again. I'm also attaching a new version of the patch, with the following changes:
> 
> * Add an assembly.h file, use that to hide the leading '_' difference in the two .S files
Looks good.  Thanks for fixing the other arches too.

> * Change the arm 32bit register enum in libunwind.h
> * Change the float register stuff in Registers_arm

There are a couple of typos in comments.  If you open the patch in a word processor, you’ll see them in red underlines.


> 
> On Fri, Dec 6, 2013 at 11:37 AM, Nick Kledzik <kledzik at apple.com> wrote:
> 
> On Dec 5, 2013, at 10:32 PM, Nico Weber <thakis at chromium.org> wrote:
> 
> > Hi,
> >
> > the attached patch adds Registers_arm and assembly for unw_getcontext. VFP support is still missing. The class isn't used by anything yet.
> 
> 
> > +// 32-bit ARM64 registers
> > +enum {
> > +  UNW_ARM_R0  = 0,
> > +  UNW_ARM_R1  = 1,
> > ...
> > +  UNW_ARM_D0  = 64,
> > +  UNW_ARM_D1  = 65,
> This register numbering needs some comments.  I’m not even sure what the right numbering should be.  The numbers do need to match what the compiler thinks the numbering is (e.g. the dwarf register numbering).  I’ve heard that for arm dwarf, 64+ is legacy for single precision registers, and that when they were widened to double, new register numbers (256+) were used.
> 
> Albert: """Hmm...section 3.1 of the DWARF for ARM spec seems to agree with you.
> 
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0040b/IHI0040B_aadwarf.pdf
> 
> I think that implies that the ARM64 enum constants are also incorrect.
>  AFAICT though, these enums aren't actually evaluated for their integer
> representation in either libc++abi or libc++. Is the need to match the
> DWARF register numbering just part of the libunwind.h API or am I missing
> some usage somewhere?
The 64-bit arm (aka AArch64 or arm64) dwarf register numbers are define at:
  http://infocenter.arm.com/help/topic/com.arm.doc.ihi0057b/IHI0057B_aadwarf64.pdf
and look correct to me. 

> +inline Registers_arm::Registers_arm(const void *registers) {
> +  static_assert(sizeof(Registers_arm) < sizeof(unw_context_t),
> +                    "arm registers do not fit into unw_context_t");
> +  // See unw_getcontext() note about data.
> +  memcpy(&_registers, registers, sizeof(_registers));
> +  bzero(_vfpRegisterData, sizeof(_vfpRegisterData));
> +  bzero(_wmmxData, sizeof(_wmmxData));
> +  bzero(_wmmxControl, sizeof(_wmmxControl));
> +}
I don’t see how this is supposed to work. There is no comment at the arm version of unw_getcontext().  
How are float registers saved? 

I have not investigated deeply how _Unwind_VRS_Get() and friends work.  I thought part of the model was that arm could ship one static library that worked with all processors, and it did so via runtime checks for what registers the processor supported, and the core unwinder was always compiled to only use integer registers.  Meaning the float/vector registers where never saved/restored to the unwind buffer.  That is, calling a setFloatRegister() would change the real processor register and not something in the register set struct.  

How do you want to reconcile that model with how libunwind does it for other architectures?  

-Nick

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131217/77a19ef7/attachment.html>