[lldb-dev] Patch to add two new classes - RegisterContextLinux_x86_64 and RegisterContextFreeBSD_x86_64

[Thirumurthi, Ashok]

Hi Samuel,

I missed the fact that the derived classes create copies of m_register_infos that should be used by non-static methods of the base class.  The attached patch addresses this point while aiming to simplify the derived class.  Note that we will have a matrix of derived classes for os x arch that can get large, so a simpler derivation has advantages.  This approach does update the register info every time that a specialization is constructed, but I think that is defensible since any bugs will be easier to reproduce.

FYI, I also added a test so that asserts for invariants will have some coverage in our test suite,

- Ashok


-----Original Message-----
From: lldb-dev-bounces at cs.uiuc.edu [mailto:lldb-dev-bounces at cs.uiuc.edu] On Behalf Of Thirumurthi, Ashok
Sent: Wednesday, May 01, 2013 10:51 AM
To: Samuel Jacob; lldb-dev
Subject: Re: [lldb-dev] Patch to add two new classes - RegisterContextLinux_x86_64 and RegisterContextFreeBSD_x86_64

And this time with the patch!

-----Original Message-----
From: lldb-dev-bounces at cs.uiuc.edu [mailto:lldb-dev-bounces at cs.uiuc.edu] On Behalf Of Thirumurthi, Ashok
Sent: Wednesday, May 01, 2013 10:33 AM
To: Samuel Jacob; lldb-dev
Subject: Re: [lldb-dev] Patch to add two new classes - RegisterContextLinux_x86_64 and RegisterContextFreeBSD_x86_64

Hi Samuel,

Thanks for preparing the patch.  I ran the 64-bit test suite on Ubuntu 12.04 with no regressions.

I noticed there is no matching delete for m_register_infos (Linux/FreeBSD).  The attached patch proposes a fix by deleting on destruction and allocating on demand.  Note that EntityRegister::Materialize makes calls to ReadRegister which relies on m_register_infos in the call to GetRegisterOffset.  Previously, the following tests relied on m_register_infos living past the destruction (i.e. a side-effect of the unmatched new).  The attached patch handles this case while avoiding the memory leak:
    tools/lldb/test$ python dotest.py --executable /path/to/lldb expression_command/issue_11588
    tools/lldb/test$ python dotest.py --executable /path/to/lldb functionalities/register

I also added a comment for DEFINE_GPR to explain that the 0 size and offset will be filled in by platform-specific classes.  I'll commit this patch based on your review,

- Ashok


-----Original Message-----
From: lldb-dev-bounces at cs.uiuc.edu [mailto:lldb-dev-bounces at cs.uiuc.edu] On Behalf Of Samuel Jacob
Sent: Tuesday, April 30, 2013 6:31 PM
To: lldb-dev
Subject: Re: [lldb-dev] Patch to add two new classes - RegisterContextLinux_x86_64 and RegisterContextFreeBSD_x86_64

Somebody please review and commit.

Thanks
Samuel

On Fri, Apr 26, 2013 at 3:17 PM, Samuel Jacob <samueldotj at gmail.com> wrote:
> RegisterContext_x86_64->GPR is defined based on the host. This is done 
> at compile time by including OS specific files.
>
> This caused a problem in elf-coredump plugin - RegisterContext_x86_64 
> with FreeBSD GPR format cant be instantiated in Linux and vice versa.
> The need for this is based on Greg's suggestion - coredump files 
> should be platform independent.(Refer
> http://lists.cs.uiuc.edu/pipermail/lldb-dev/2013-February/001477.html)
>
> This patch adds two new classes RegisterContextLinux_x86_64 and
> RegisterContextFreeBSD_x86_64 which can instantiated on any platform.
>
> I verified it compiles on a Ubuntu and on a MacMini. I also verified 
> it shows correct backtrace and correct register content on a simple 
> program. I can run more test cases for that somebody please point me 
> how do I run test suites.
>
> Once this is done I will send a new elf-core patch based on this.
>
> Thanks
> Samuel
_______________________________________________
lldb-dev mailing list
lldb-dev at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev

_______________________________________________
lldb-dev mailing list
lldb-dev at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: log-regs.patch
Type: application/octet-stream
Size: 9902 bytes
Desc: log-regs.patch
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20130506/f82f4a22/attachment.obj>