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: [email protected] [mailto:[email protected]] 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: [email protected] [mailto:[email protected]] 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: [email protected] [mailto:[email protected]] 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 <[email protected]> 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 [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev _______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
log-regs.patch
Description: log-regs.patch
_______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
