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

[Samuel Jacob]

Hi Ashok,

Any update to RegisterContext_x86_64::m_register_infos should be
isolated to a particular platform(Linux, FreeBSD...).
That is why a copy was made for each platform.

With this patch, creating RegisterContext for multiple platforms would
become problem.
For example in the following code, both r1 and r2 would point to same
register info structure(which is wrong).
RegisterContextFreeBSD_x86_64 r1;
RegisterContextLinux_x86_64 r2;

Thanks
Samuel

On Mon, May 6, 2013 at 3:04 PM, Thirumurthi, Ashok
<ashok.thirumurthi at intel.com> wrote:
> 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