[libcxxabi patch] Add Registers_arm class and unw_getcontext implementation

[Nico Weber]

On Tue, Dec 17, 2013 at 12:13 PM, Nick Kledzik <kledzik at apple.com> wrote:

> 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.
>

Thanks, I landed the assembly.h file in r197523.


>
> * 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.
>

Fixed the two I found.


>
>
>
> 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'm guessing this refers to "@ FIXME: VFP registers" that this patch adds
to UnwindRegistersSave.S – we don't save these registers correctly yet (and
don't restore them either).


>
> 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.
>

Would that work for unw_set_fpreg / unw_get_fpreg? (I don't know if users
call this function directly). Also, 9.3 in
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038a/IHI0038A_ehabi.pdf(remark
d, and instruction 11000111 0000iiii and similar) suggest that the
exception bytecode modifies float registers in the virtual register bank,
and if those modifications are done in real registers, wouldn't things be
in an inconsistent state early during phase 2 unwinding, when the bytecode
was processed once for phase 1 already?


>
>
> 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/b5bb2514/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: armregs.patch
Type: application/octet-stream
Size: 16384 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131217/b5bb2514/attachment.obj>