[Lldb-commits] [lldb] r293269 - Unroll r292930 due to TestCallThatThrows test fail is not fixed in reasonable time.
Author: bulasevich Date: Fri Jan 27 01:51:43 2017 New Revision: 293269 URL: http://llvm.org/viewvc/llvm-project?rev=293269&view=rev Log: Unroll r292930 due to TestCallThatThrows test fail is not fixed in reasonable time. Removed: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/ Modified: lldb/trunk/include/lldb/Target/Thread.h lldb/trunk/include/lldb/Target/ThreadPlan.h lldb/trunk/source/Target/StopInfo.cpp lldb/trunk/source/Target/Thread.cpp lldb/trunk/source/Target/ThreadPlanStepInstruction.cpp lldb/trunk/source/Target/ThreadPlanStepRange.cpp Modified: lldb/trunk/include/lldb/Target/Thread.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Thread.h?rev=293269&r1=293268&r2=293269&view=diff == --- lldb/trunk/include/lldb/Target/Thread.h (original) +++ lldb/trunk/include/lldb/Target/Thread.h Fri Jan 27 01:51:43 2017 @@ -126,7 +126,6 @@ public: // bit of data. lldb::StopInfoSP stop_info_sp; // You have to restore the stop info or you // might continue with the wrong signals. -std::vector m_completed_plan_stack; lldb::RegisterCheckpointSP register_backup_sp; // You need to restore the registers, of course... uint32_t current_inlined_depth; @@ -1030,15 +1029,6 @@ public: bool WasThreadPlanDiscarded(ThreadPlan *plan); //-- - /// Check if we have completed plan to override breakpoint stop reason - /// - /// @return - /// Returns true if completed plan stack is not empty - /// false otherwise. - //-- - bool CompletedPlanOverridesBreakpoint(); - - //-- /// Queues a generic thread plan. /// /// @param[in] plan_sp @@ -1223,8 +1213,6 @@ public: void SetStopInfo(const lldb::StopInfoSP &stop_info_sp); - void ResetStopInfo(); - void SetShouldReportStop(Vote vote); //-- Modified: lldb/trunk/include/lldb/Target/ThreadPlan.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ThreadPlan.h?rev=293269&r1=293268&r2=293269&view=diff == --- lldb/trunk/include/lldb/Target/ThreadPlan.h (original) +++ lldb/trunk/include/lldb/Target/ThreadPlan.h Fri Jan 27 01:51:43 2017 @@ -40,10 +40,9 @@ namespace lldb_private { // The thread maintaining a thread plan stack, and you program the actions of a // particular thread // by pushing plans onto the plan stack. -// There is always a "Current" plan, which is the top of the plan stack, +// There is always a "Current" plan, which is the head of the plan stack, // though in some cases -// a plan may defer to plans higher in the stack for some piece of information -// (let us define that the plan stack grows downwards). +// a plan may defer to plans higher in the stack for some piece of information. // // The plan stack is never empty, there is always a Base Plan which persists // through the life @@ -110,15 +109,6 @@ namespace lldb_private { // plans in the time between when // your plan gets unshipped and the next resume. // -// Thread State Checkpoint: -// -// Note that calling functions on target process (ThreadPlanCallFunction) changes -// current thread state. The function can be called either by direct user demand or -// internally, for example lldb allocates memory on device to calculate breakpoint -// condition expression - on Linux it is performed by calling mmap on device. -// ThreadStateCheckpoint saves Thread state (stop info and completed -// plan stack) to restore it after completing function call. -// // Over the lifetime of the plan, various methods of the ThreadPlan are then // called in response to changes of state in // the process we are debugging as follows: @@ -159,7 +149,7 @@ namespace lldb_private { // If the Current plan answers "true" then it is asked if the stop should // percolate all the way to the // user by calling the ShouldStop method. If the current plan doesn't explain -// the stop, then we query up +// the stop, then we query down // the plan stack for a plan that does explain the stop. The plan that does // explain the stop then needs to // figure out what to do about the plans below it in the stack. If the stop is @@ -180,7 +170,7 @@ namespace lldb_private { // event it didn't directly handle // it can designate itself a "Master" plan by responding true to IsMasterPlan, // and then if it wants not to be -// discarded, it can return false to OkayToDiscard, and it and all its dependent +// discarded, it can retur
[Lldb-commits] [PATCH] D29215: [LLDB][MIPS] Fix TestMiniDumpNew
nitesh.jain created this revision. In case of a core file, if the core file architecture(like x86_64) is incompatible with that of Host architecture(like Mips64) then platform is set to remote-platform. If the remote-platform is not connected then SBPlatform::GetTriple() will return none. Hence we assume target platform is same as the host platform. (lldb) script Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D. >>> import lldb >>> dbg = lldb.SBDebugger.Create() >>> print dbg.GetSelectedPlatform().GetName() host >>> print dbg.GetSelectedPlatform().GetTriple() mips64el--linux-gnu >>> dbg.CreateTarget("linux-x86_64") > >>> print dbg.GetSelectedPlatform().GetName() remote-linux >>> print dbg.GetSelectedPlatform().GetTriple() None >>> print dbg.GetSelectedPlatform().IsConnected() False >>> Repository: rL LLVM https://reviews.llvm.org/D29215 Files: packages/Python/lldbsuite/test/lldbplatformutil.py Index: packages/Python/lldbsuite/test/lldbplatformutil.py === --- packages/Python/lldbsuite/test/lldbplatformutil.py +++ packages/Python/lldbsuite/test/lldbplatformutil.py @@ -127,7 +127,15 @@ def getPlatform(): """Returns the target platform which the tests are running on.""" -platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2] +if lldb.DBG.GetSelectedPlatform().GetTriple(): +platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2] +else: +# In case of a core file, if the core file architecture(like x86_64) +# is incompatible with that of Host architecture(like Mips64) +# then platform is set to remote-platform. If the remote-platform is +# not connected then SBPlatform::GetTriple() will return none. +# Hence we assume target platform is same as the host platform. +platform = sys.platform if platform.startswith('freebsd'): platform = 'freebsd' elif platform.startswith('netbsd'): Index: packages/Python/lldbsuite/test/lldbplatformutil.py === --- packages/Python/lldbsuite/test/lldbplatformutil.py +++ packages/Python/lldbsuite/test/lldbplatformutil.py @@ -127,7 +127,15 @@ def getPlatform(): """Returns the target platform which the tests are running on.""" -platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2] +if lldb.DBG.GetSelectedPlatform().GetTriple(): +platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2] +else: +# In case of a core file, if the core file architecture(like x86_64) +# is incompatible with that of Host architecture(like Mips64) +# then platform is set to remote-platform. If the remote-platform is +# not connected then SBPlatform::GetTriple() will return none. +# Hence we assume target platform is same as the host platform. +platform = sys.platform if platform.startswith('freebsd'): platform = 'freebsd' elif platform.startswith('netbsd'): ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29215: [LLDB][MIPS] Fix TestMiniDumpNew
labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. This seems like it's fixing the problem in the wrong place. Also, the assumption that the platform == host_platform is not correct (what about when the test is run on windows?) I think we should either fix SBPlatform::GetTriple to return a correct triple (lldb already knows the triple since it has the core file. Whether that triple should be returned by the platform is a somewhat different question though) or make the test suite not depend on that. I am confused as to why would this behavior occur only on mips hosts. I've seen the platform being set to remote-linux when the test is run from windows, and it works fine there. What is the actual error message you are getting? Repository: rL LLVM https://reviews.llvm.org/D29215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29215: [LLDB][MIPS] Fix TestMiniDumpNew
nitesh.jain added a comment. Hi Labath, I think on window ur host architecture is x86_64 hence when TargetList::CreateTargetInternal(..) is call, at line 269 the code is if (!prefer_platform_arch && arch.IsValid()) { if (!platform_sp->IsCompatibleArchitecture(arch, false, &platform_arch)) { platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch); if (!is_dummy_target && platform_sp) debugger.GetPlatformList().SetSelectedPlatform(platform_sp); The line platform_sp->IsCompatibleArchitecture(arch, false, &platform_arch) will be true , if ur host architecture is x86(which is ur case). In our case since host architecture is Mips it will return false. hence Platform::GetPlatformForArchitecture(arch, &platform_arch) will be set platform to remote-linux Repository: rL LLVM https://reviews.llvm.org/D29215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29215: [LLDB][MIPS] Fix TestMiniDumpNew
nitesh.jain added a comment. ERROR: test_two_cores_same_pid (TestMiniDumpNew.MiniDumpNewTestCase) Test that we handle the situation if we have two core files with the same PID -- Traceback (most recent call last): File "/export/tmp/nin/LLVM/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1884, in setUp Base.setUp(self) File "/export/tmp/nin/LLVM/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 835, in setUp self.darwinWithFramework = self.platformIsDarwin() File "/export/tmp/nin/LLVM/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1300, in platformIsDarwin return lldbplatformutil.platformIsDarwin() File "/export/tmp/nin/LLVM/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py", line 140, in platformIsDarwin return getPlatform() in getDarwinOSTriples() File "/export/tmp/nin/LLVM/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py", line 130, in getPlatform platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2] AttributeError: 'NoneType' object has no attribute 'split' Repository: rL LLVM https://reviews.llvm.org/D29215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29215: [LLDB][MIPS] Fix TestMiniDumpNew
labath added a comment. Ok, I've just checked and this does not happen on windows. It does however happen (GetTriple() returning None) when we try to open the s390x core file (functionalities/postmortem/elf-core/linux-s390x.out). The test suite seems to be handling it fine. The only difference I see there is that the other test has a bit of setUp/tearDown code which saves and restores the original platform after each run. Can you check whether that will make things work for you ? (It's still not ideal, but at least it will be consistent with the other test). As an alternative fix -- the checking of the *target* platform in the code which is throwing this exception seems to be a bug. "Whether we have an LLDB.framework" is a property of the host, not the target. Changing that would probably allow you to make progress here. (although it won't fix the underlying problem). Repository: rL LLVM https://reviews.llvm.org/D29215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r293281 - Refactor the android accept hack
Author: labath Date: Fri Jan 27 06:23:51 2017 New Revision: 293281 URL: http://llvm.org/viewvc/llvm-project?rev=293281&view=rev Log: Refactor the android accept hack This moves the accept hack from the android toolchain file into LLDBConfig.cmake. This allows successful lldb android compilation without relying on our custom toolchain file. Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake lldb/trunk/cmake/platforms/Android.cmake lldb/trunk/source/Host/common/Socket.cpp Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=293281&r1=293280&r2=293281&view=diff == --- lldb/trunk/cmake/modules/LLDBConfig.cmake (original) +++ lldb/trunk/cmake/modules/LLDBConfig.cmake Fri Jan 27 06:23:51 2017 @@ -412,4 +412,8 @@ if(LLDB_USE_BUILTIN_DEMANGLER) add_definitions(-DLLDB_USE_BUILTIN_DEMANGLER) endif() +if ((CMAKE_SYSTEM_NAME MATCHES "Android") AND LLVM_BUILD_STATIC) + add_definitions(-DANDROID_BUILD_STATIC) +endif() + find_package(Backtrace) Modified: lldb/trunk/cmake/platforms/Android.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/platforms/Android.cmake?rev=293281&r1=293280&r2=293281&view=diff == --- lldb/trunk/cmake/platforms/Android.cmake (original) +++ lldb/trunk/cmake/platforms/Android.cmake Fri Jan 27 06:23:51 2017 @@ -102,10 +102,6 @@ elseif( ANDROID_ABI STREQUAL "armeabi" ) # 64 bit atomic operations used in c++ libraries require armv7-a instructions # armv5te and armv6 were tried but do not work. set( ANDROID_CXX_FLAGS "${ANDROID_CXX_FLAGS} -march=armv7-a -mthumb" ) - if( LLVM_BUILD_STATIC ) - # Temporary workaround for static linking with the latest API. - set( ANDROID_CXX_FLAGS "${ANDROID_CXX_FLAGS} -DANDROID_ARM_BUILD_STATIC" ) - endif() elseif( ANDROID_ABI STREQUAL "mips" ) # http://b.android.com/182094 list( FIND LLDB_SYSTEM_LIBS atomic index ) @@ -113,10 +109,6 @@ elseif( ANDROID_ABI STREQUAL "mips" ) list( APPEND LLDB_SYSTEM_LIBS atomic ) set( LLDB_SYSTEM_LIBS ${LLDB_SYSTEM_LIBS} CACHE INTERNAL "" FORCE ) endif() - if( LLVM_BUILD_STATIC ) - # Temporary workaround for static linking with the latest API. - set( ANDROID_CXX_FLAGS "${ANDROID_CXX_FLAGS} -DANDROID_MIPS_BUILD_STATIC" ) - endif() endif() # Use gold linker and enable safe ICF in case of x86, x86_64 and arm Modified: lldb/trunk/source/Host/common/Socket.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Socket.cpp?rev=293281&r1=293280&r2=293281&view=diff == --- lldb/trunk/source/Host/common/Socket.cpp (original) +++ lldb/trunk/source/Host/common/Socket.cpp Fri Jan 27 06:23:51 2017 @@ -38,11 +38,9 @@ #include #include #include -#if defined(ANDROID_ARM_BUILD_STATIC) || defined(ANDROID_MIPS_BUILD_STATIC) #include #include #include -#endif // ANDROID_ARM_BUILD_STATIC || ANDROID_MIPS_BUILD_STATIC #endif // __ANDROID__ using namespace lldb; @@ -424,9 +422,13 @@ NativeSocket Socket::AcceptSocket(Native socklen_t *addrlen, bool child_processes_inherit, Error &error) { error.Clear(); -#if defined(ANDROID_ARM_BUILD_STATIC) || defined(ANDROID_MIPS_BUILD_STATIC) - // Temporary workaround for statically linking Android lldb-server with the - // latest API. +#if defined(ANDROID_BUILD_STATIC) + // Hack: + // This enables static linking lldb-server to an API 21 libc, but still having + // it run on older devices. It is necessary because API 21 libc's + // implementation of accept() uses the accept4 syscall(), which is not + // available in older kernels. Using an older libc would fix this issue, but + // introduce other ones, as the old libraries were quite buggy. int fd = syscall(__NR_accept, sockfd, addr, addrlen); if (fd >= 0 && !child_processes_inherit) { int flags = ::fcntl(fd, F_GETFD); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r293282 - Fix android-i386 build broken by previous commit
Author: labath Date: Fri Jan 27 06:58:23 2017 New Revision: 293282 URL: http://llvm.org/viewvc/llvm-project?rev=293282&view=rev Log: Fix android-i386 build broken by previous commit I foolishly thought I could simplify the condition to cover all android targets. I was wrong - i386 headers don't define __NR_accept. Revert back to enabling the workaround on arm an mips only. Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=293282&r1=293281&r2=293282&view=diff == --- lldb/trunk/cmake/modules/LLDBConfig.cmake (original) +++ lldb/trunk/cmake/modules/LLDBConfig.cmake Fri Jan 27 06:58:23 2017 @@ -412,7 +412,8 @@ if(LLDB_USE_BUILTIN_DEMANGLER) add_definitions(-DLLDB_USE_BUILTIN_DEMANGLER) endif() -if ((CMAKE_SYSTEM_NAME MATCHES "Android") AND LLVM_BUILD_STATIC) +if ((CMAKE_SYSTEM_NAME MATCHES "Android") AND LLVM_BUILD_STATIC AND +((ANDROID_ABI MATCHES "armeabi") OR (ANDROID_ABI MATCHES "mips"))) add_definitions(-DANDROID_BUILD_STATIC) endif() ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
labath added a subscriber: lldb-commits. labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I've added a quick test to demonstrate what I had in mind. > Unfortunately, unlike release_39 branch, I cannot open my core dump, but the > problem seems unrelated. So I think I want to push it through and continue > investigation what is not working this time. That's kinda the reason I wanted to add tests. :) This test would not have actually caught your new problem, but it could add at least some protection for the future, as I doubt many people will run into real files with this many sections. I think this should be ok to put in if there are no objections from anyone else. Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.h:71 elf_half e_phentsize;///< Size of a program header table entry. -elf_half e_phnum;///< Number of program header entries. +elf_half e_phnum_hdr;///< Number of program header entries. elf_half e_shentsize;///< Size of a section header table entry. EugeneBi wrote: > labath wrote: > > I am wondering whether these is any use in keeping these old values. It > > sounds like it's a recipe for someone getting confused and using the wrong > > ones. What do you think about just deleting these? > I will leave it to you - just tell me what you prefer. I see both pros and > cons of my current code. Ok, let's leave it this way then. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r293287 - Address post-commit review remarks
Author: labath Date: Fri Jan 27 09:19:03 2017 New Revision: 293287 URL: http://llvm.org/viewvc/llvm-project?rev=293287&view=rev Log: Address post-commit review remarks Tamas pointed out that the macro name I used in r293282 was too vague. Rename it to better reflect what it is used for. Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake lldb/trunk/source/Host/common/Socket.cpp Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=293287&r1=293286&r2=293287&view=diff == --- lldb/trunk/cmake/modules/LLDBConfig.cmake (original) +++ lldb/trunk/cmake/modules/LLDBConfig.cmake Fri Jan 27 09:19:03 2017 @@ -414,7 +414,7 @@ endif() if ((CMAKE_SYSTEM_NAME MATCHES "Android") AND LLVM_BUILD_STATIC AND ((ANDROID_ABI MATCHES "armeabi") OR (ANDROID_ABI MATCHES "mips"))) - add_definitions(-DANDROID_BUILD_STATIC) + add_definitions(-DANDROID_USE_ACCEPT_WORKAROUND) endif() find_package(Backtrace) Modified: lldb/trunk/source/Host/common/Socket.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Socket.cpp?rev=293287&r1=293286&r2=293287&view=diff == --- lldb/trunk/source/Host/common/Socket.cpp (original) +++ lldb/trunk/source/Host/common/Socket.cpp Fri Jan 27 09:19:03 2017 @@ -422,7 +422,7 @@ NativeSocket Socket::AcceptSocket(Native socklen_t *addrlen, bool child_processes_inherit, Error &error) { error.Clear(); -#if defined(ANDROID_BUILD_STATIC) +#if defined(ANDROID_USE_ACCEPT_WORKAROUND) // Hack: // This enables static linking lldb-server to an API 21 libc, but still having // it run on older devices. It is necessary because API 21 libc's ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29126: [cmake] Remove VERSION property from executable targets
labath added a comment. Thanks. I guess I'll give this one more week, and then commit it if noone objects. https://reviews.llvm.org/D29126 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
davidb added inline comments. Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108 + if (ok) { +if (e_phnum_hdr == 0x) + e_phnum = section_zero.sh_info; Would this make more sense to compare against a named constant ELF::PN_XNUM? Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:110 + e_phnum = section_zero.sh_info; +if (e_shnum_hdr == 0) + e_shnum = section_zero.sh_size; And this with SHN_UNDEF? Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:112 + e_shnum = section_zero.sh_size; +if (e_shstrndx_hdr == 0x) + e_shstrndx = section_zero.sh_link; and this with ELF::SHN_XINDEX. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r293269 - Unroll r292930 due to TestCallThatThrows test fail is not fixed in reasonable time.
Thanks for following up! It's really appreciated! -Tim On Thu, Jan 26, 2017 at 11:51 PM, Boris Ulasevich via lldb-commits < lldb-commits@lists.llvm.org> wrote: > Author: bulasevich > Date: Fri Jan 27 01:51:43 2017 > New Revision: 293269 > > URL: http://llvm.org/viewvc/llvm-project?rev=293269&view=rev > Log: > Unroll r292930 due to TestCallThatThrows test fail is not fixed in > reasonable time. > > Removed: > lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/ > step_over_breakpoint/ > Modified: > lldb/trunk/include/lldb/Target/Thread.h > lldb/trunk/include/lldb/Target/ThreadPlan.h > lldb/trunk/source/Target/StopInfo.cpp > lldb/trunk/source/Target/Thread.cpp > lldb/trunk/source/Target/ThreadPlanStepInstruction.cpp > lldb/trunk/source/Target/ThreadPlanStepRange.cpp > > Modified: lldb/trunk/include/lldb/Target/Thread.h > URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/ > lldb/Target/Thread.h?rev=293269&r1=293268&r2=293269&view=diff > > == > --- lldb/trunk/include/lldb/Target/Thread.h (original) > +++ lldb/trunk/include/lldb/Target/Thread.h Fri Jan 27 01:51:43 2017 > @@ -126,7 +126,6 @@ public: > // bit of data. > lldb::StopInfoSP stop_info_sp; // You have to restore the stop info > or you > // might continue with the wrong > signals. > -std::vector m_completed_plan_stack; > lldb::RegisterCheckpointSP > register_backup_sp; // You need to restore the registers, of > course... > uint32_t current_inlined_depth; > @@ -1030,15 +1029,6 @@ public: >bool WasThreadPlanDiscarded(ThreadPlan *plan); > >//-- > - /// Check if we have completed plan to override breakpoint stop reason > - /// > - /// @return > - /// Returns true if completed plan stack is not empty > - /// false otherwise. > - //-- > - bool CompletedPlanOverridesBreakpoint(); > - > - //-- >/// Queues a generic thread plan. >/// >/// @param[in] plan_sp > @@ -1223,8 +1213,6 @@ public: > >void SetStopInfo(const lldb::StopInfoSP &stop_info_sp); > > - void ResetStopInfo(); > - >void SetShouldReportStop(Vote vote); > >//-- > > > Modified: lldb/trunk/include/lldb/Target/ThreadPlan.h > URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/ > lldb/Target/ThreadPlan.h?rev=293269&r1=293268&r2=293269&view=diff > > == > --- lldb/trunk/include/lldb/Target/ThreadPlan.h (original) > +++ lldb/trunk/include/lldb/Target/ThreadPlan.h Fri Jan 27 01:51:43 2017 > @@ -40,10 +40,9 @@ namespace lldb_private { > // The thread maintaining a thread plan stack, and you program the > actions of a > // particular thread > // by pushing plans onto the plan stack. > -// There is always a "Current" plan, which is the top of the plan stack, > +// There is always a "Current" plan, which is the head of the plan stack, > // though in some cases > -// a plan may defer to plans higher in the stack for some piece of > information > -// (let us define that the plan stack grows downwards). > +// a plan may defer to plans higher in the stack for some piece of > information. > // > // The plan stack is never empty, there is always a Base Plan which > persists > // through the life > @@ -110,15 +109,6 @@ namespace lldb_private { > // plans in the time between when > // your plan gets unshipped and the next resume. > // > -// Thread State Checkpoint: > -// > -// Note that calling functions on target process > (ThreadPlanCallFunction) changes > -// current thread state. The function can be called either by direct > user demand or > -// internally, for example lldb allocates memory on device to calculate > breakpoint > -// condition expression - on Linux it is performed by calling mmap on > device. > -// ThreadStateCheckpoint saves Thread state (stop info and completed > -// plan stack) to restore it after completing function call. > -// > // Over the lifetime of the plan, various methods of the ThreadPlan are > then > // called in response to changes of state in > // the process we are debugging as follows: > @@ -159,7 +149,7 @@ namespace lldb_private { > // If the Current plan answers "true" then it is asked if the stop should > // percolate all the way to the > // user by calling the ShouldStop method. If the current plan doesn't > explain > -// the stop, then we query up > +// the stop, then we query down > // the plan stack for a plan that does explain the stop. The plan that > does > // explain the stop then needs to > // figure out what to
[Lldb-commits] [PATCH] D29144: LLDB: fix for TestCallThatThrows.py test fail
boris.ulasevich added a comment. In https://reviews.llvm.org/D29144#657798, @jingham wrote: > When you go to pick the plan to report for the stop it should be the top of > the completed plan stack, the "thread plan to call function". Why in this > particular case is the bottom of the completed stack getting handed out? "thread plan to call function" do stay on the top of the completed plan stack, but GetStopInfo returns another value because ProcessGDBRemote::SetThreadStopInfo have put StopInfoBreakpoint to Thread::m_stop_info_sp, and preset value have priority over completed plan. Repository: rL LLVM https://reviews.llvm.org/D29144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29144: LLDB: fix for TestCallThatThrows.py test fail
boris.ulasevich updated this revision to Diff 86091. boris.ulasevich added a comment. I made another diff with using GetCompletedPlan call. Hope it makes the code clear. https://reviews.llvm.org/D29144 Files: lldb/source/Target/Process.cpp Index: lldb/source/Target/Process.cpp === --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -5248,15 +5248,9 @@ do_resume = false; handle_running_event = true; } else { - StopInfoSP stop_info_sp(thread_sp->GetStopInfo()); - StopReason stop_reason = eStopReasonInvalid; - if (stop_info_sp) -stop_reason = stop_info_sp->GetStopReason(); - - // FIXME: We only check if the stop reason is plan complete, - // should we make sure that - // it is OUR plan that is complete? - if (stop_reason == eStopReasonPlanComplete) { + ThreadPlanSP plan_sp = thread->GetCompletedPlan(); + if (plan_sp == thread_plan_sp && plan_sp->PlanSucceeded()) { + if (log) log->PutCString("Process::RunThreadPlan(): execution " "completed successfully."); @@ -5267,9 +5261,11 @@ return_value = eExpressionCompleted; } else { +StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); // Something restarted the target, so just wait for it to // stop for real. -if (stop_reason == eStopReasonBreakpoint) { +if (stop_info_sp && +stop_info_sp->GetStopReason() == eStopReasonBreakpoint) { if (log) log->Printf("Process::RunThreadPlan() stopped for " "breakpoint: %s.", Index: lldb/source/Target/Process.cpp === --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -5248,15 +5248,9 @@ do_resume = false; handle_running_event = true; } else { - StopInfoSP stop_info_sp(thread_sp->GetStopInfo()); - StopReason stop_reason = eStopReasonInvalid; - if (stop_info_sp) -stop_reason = stop_info_sp->GetStopReason(); - - // FIXME: We only check if the stop reason is plan complete, - // should we make sure that - // it is OUR plan that is complete? - if (stop_reason == eStopReasonPlanComplete) { + ThreadPlanSP plan_sp = thread->GetCompletedPlan(); + if (plan_sp == thread_plan_sp && plan_sp->PlanSucceeded()) { + if (log) log->PutCString("Process::RunThreadPlan(): execution " "completed successfully."); @@ -5267,9 +5261,11 @@ return_value = eExpressionCompleted; } else { +StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); // Something restarted the target, so just wait for it to // stop for real. -if (stop_reason == eStopReasonBreakpoint) { +if (stop_info_sp && +stop_info_sp->GetStopReason() == eStopReasonBreakpoint) { if (log) log->Printf("Process::RunThreadPlan() stopped for " "breakpoint: %s.", ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29144: LLDB: fix for TestCallThatThrows.py test fail
jingham added a comment. If things were working generally the way your explanation states, then I don't understand why this doesn't cause all expression evaluations to fail. We have lots of tests that do expression evaluation. Why is only this test failing. Your change looks reasonable to me, but I'm a bit worried that we don't understand why the code as it was isn't failing all the time. What's special about the TestCallThatThrows test? https://reviews.llvm.org/D29144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29144: LLDB: fix for TestCallThatThrows.py test fail
boris.ulasevich added a comment. In https://reviews.llvm.org/D29144#659052, @jingham wrote: > What's special about the TestCallThatThrows test? Good question! It must be related with SetIgnoreBreakpoints(True) option used in the test: with this option StopInfoBreakpoint::PerformAction is not performed and preset StopInfoBreakpoint value is not cleared :) https://reviews.llvm.org/D29144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r293336 - NFC: Improve comments in SymbolFilePDB.cpp
Author: amccarth Date: Fri Jan 27 15:42:28 2017 New Revision: 293336 URL: http://llvm.org/viewvc/llvm-project?rev=293336&view=rev Log: NFC: Improve comments in SymbolFilePDB.cpp Mostly this just fixes bad wrapping caused by the reformat, with tiny changes sprinkled here and there. Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=293336&r1=293335&r2=293336&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Fri Jan 27 15:42:28 2017 @@ -125,9 +125,8 @@ uint32_t SymbolFilePDB::GetNumCompileUni m_cached_compile_unit_count = compilands->getChildCount(); // The linker can inject an additional "dummy" compilation unit into the -// PDB. -// Ignore this special compile unit for our purposes, if it is there. It is -// always the last one. +// PDB. Ignore this special compile unit for our purposes, if it is there. +// It is always the last one. auto last_cu = compilands->getChildAtIndex(m_cached_compile_unit_count - 1); std::string name = last_cu->getName(); if (name == "* Linker *") @@ -187,12 +186,10 @@ bool SymbolFilePDB::ParseCompileUnitSupp return false; // In theory this is unnecessary work for us, because all of this information - // is easily - // (and quickly) accessible from DebugInfoPDB, so caching it a second time - // seems like a waste. - // Unfortunately, there's no good way around this short of a moderate - // refactor, since SymbolVendor - // depends on being able to cache this list. + // is easily (and quickly) accessible from DebugInfoPDB, so caching it a + // second time seems like a waste. Unfortunately, there's no good way around + // this short of a moderate refactor since SymbolVendor depends on being able + // to cache this list. auto cu = m_session_up->getConcreteSymbolById( sc.comp_unit->GetID()); if (!cu) @@ -269,9 +266,8 @@ lldb_private::CompilerDecl SymbolFilePDB lldb_private::CompilerDeclContext SymbolFilePDB::GetDeclContextForUID(lldb::user_id_t uid) { // PDB always uses the translation unit decl context for everything. We can - // improve this later - // but it's not easy because PDB doesn't provide a high enough level of type - // fidelity in this area. + // improve this later but it's not easy because PDB doesn't provide a high + // enough level of type fidelity in this area. return *m_tu_decl_ctx_up; } @@ -295,30 +291,25 @@ uint32_t SymbolFilePDB::ResolveSymbolCon uint32_t resolve_scope, lldb_private::SymbolContextList &sc_list) { if (resolve_scope & lldb::eSymbolContextCompUnit) { // Locate all compilation units with line numbers referencing the specified -// file. For example, if -// `file_spec` is , then this should return all source files and -// header files that reference -// , either directly or indirectly. +// file. For example, if `file_spec` is , then this should return +// all source files and header files that reference , either +// directly or indirectly. auto compilands = m_session_up->findCompilandsForSourceFile( file_spec.GetPath(), PDB_NameSearchFlags::NS_CaseInsensitive); -// For each one, either find get its previously parsed data, or parse it -// afresh and add it to -// the symbol context list. +// For each one, either find its previously parsed data or parse it afresh +// and add it to the symbol context list. while (auto compiland = compilands->getNext()) { // If we're not checking inlines, then don't add line information for this - // file unless the FileSpec - // matches. + // file unless the FileSpec matches. if (!check_inlines) { // `getSourceFileName` returns the basename of the original source file -// used to generate this compiland. -// It does not return the full path. Currently the only way to get that -// is to do a basename lookup to -// get the IPDBSourceFile, but this is ambiguous in the case of two -// source files with the same name -// contributing to the same compiland. This is a moderately extreme -// edge case, so we consider this ok -// for now, although we need to find a long term solution. +// used to generate this compiland. It does not return the full path. +// Currently the only way to get that is to do a basename lookup to get +// the IPDBSourceFile, but this is ambiguous in the case of two source +// files with the same name contributing to the same compiland. This is +// a moderately extreme edge case, so we consider this OK for now
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added inline comments. Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108 + if (ok) { +if (e_phnum_hdr == 0x) + e_phnum = section_zero.sh_info; davidb wrote: > Would this make more sense to compare against a named constant ELF::PN_XNUM? Would be nice, but - where is it defined? I tried ELF::PN_XNUM, elf::PN_XNUM, PN_XNUM - compiler barks anyway. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added inline comments. Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108 + if (ok) { +if (e_phnum_hdr == 0x) + e_phnum = section_zero.sh_info; EugeneBi wrote: > davidb wrote: > > Would this make more sense to compare against a named constant ELF::PN_XNUM? > Would be nice, but - where is it defined? I tried ELF::PN_XNUM, elf::PN_XNUM, > PN_XNUM - compiler barks anyway. > OK, I see the other two are defined in Support/ELF.h, I'll put PN_XNUM there then. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added inline comments. Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108 + if (ok) { +if (e_phnum_hdr == 0x) + e_phnum = section_zero.sh_info; EugeneBi wrote: > EugeneBi wrote: > > davidb wrote: > > > Would this make more sense to compare against a named constant > > > ELF::PN_XNUM? > > Would be nice, but - where is it defined? I tried ELF::PN_XNUM, > > elf::PN_XNUM, PN_XNUM - compiler barks anyway. > > > OK, I see the other two are defined in Support/ELF.h, I'll put PN_XNUM there > then. Oh, this ELF.h is a part of a different git repo - it lives in llvm. I am not sure what to do about it. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi updated this revision to Diff 86148. EugeneBi added a comment. Used named constants for SHN_UNDEF and SHN_XINDEX sentinels. Unfortunately ELF.h lacks definition of PN_XNUM, so left it as a comment. https://reviews.llvm.org/D29095 Files: source/Plugins/ObjectFile/ELF/ELFHeader.cpp source/Plugins/ObjectFile/ELF/ELFHeader.h source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp source/Plugins/Process/elf-core/ProcessElfCore.cpp unittests/CMakeLists.txt unittests/ObjectFile/CMakeLists.txt unittests/ObjectFile/ELF/CMakeLists.txt unittests/ObjectFile/ELF/TestELFHeader.cpp Index: unittests/ObjectFile/ELF/TestELFHeader.cpp === --- /dev/null +++ unittests/ObjectFile/ELF/TestELFHeader.cpp @@ -0,0 +1,62 @@ +//===-- TestELFHeader.cpp ---*- C++ -*-===// +// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Plugins/ObjectFile/ELF/ELFHeader.h" +#include "lldb/Core/DataExtractor.h" +#include "gtest/gtest.h" + +using namespace lldb; +using namespace lldb_private; + + +TEST(ELFHeader, ParseHeaderExtension) { + uint8_t data[] = { + // e_ident + 0x7f, 0x45, 0x4c, 0x46, 0x02, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // e_type, e_machine, e_version, e_entry + 0x03, 0x00, 0x3e, 0x00, 0x01, 0x00, 0x00, 0x00, 0x90, 0x48, 0x40, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // e_phoff, e_shoff + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // e_flags, e_ehsize, e_phentsize, e_phnum, e_shentsize, e_shnum, + // e_shstrndx + 0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x38, 0x00, 0xff, 0xff, 0x40, 0x00, + 0x00, 0x00, 0xff, 0xff, + + // sh_name, sh_type, sh_flags + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // sh_addr, sh_offset + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + + // sh_size, sh_link, sh_info + 0x23, 0x45, 0x67, 0x00, 0x00, 0x00, 0x00, 0x00, 0x34, 0x56, 0x78, 0x00, + 0x12, 0x34, 0x56, 0x00, + + // sh_addralign, sh_entsize + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, + }; + + DataExtractor extractor(data, sizeof data, eByteOrderLittle, 8); + elf::ELFHeader header; + offset_t offset = 0; + ASSERT_TRUE(header.Parse(extractor, &offset)); + EXPECT_EQ(0x563412u, header.e_phnum); + EXPECT_EQ(0x785634u, header.e_shstrndx); + EXPECT_EQ(0x674523u, header.e_shnum); +} Index: unittests/ObjectFile/ELF/CMakeLists.txt === --- /dev/null +++ unittests/ObjectFile/ELF/CMakeLists.txt @@ -0,0 +1,3 @@ +add_lldb_unittest(ObjectFileELFTests + TestELFHeader.cpp + ) Index: unittests/ObjectFile/CMakeLists.txt === --- /dev/null +++ unittests/ObjectFile/CMakeLists.txt @@ -0,0 +1 @@ +add_subdirectory(ELF) Index: unittests/CMakeLists.txt === --- unittests/CMakeLists.txt +++ unittests/CMakeLists.txt @@ -53,6 +53,7 @@ add_subdirectory(Host) add_subdirectory(Interpreter) add_subdirectory(Language) +add_subdirectory(ObjectFile) add_subdirectory(Platform) add_subdirectory(Process) add_subdirectory(ScriptInterpreter) Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp === --- source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -56,6 +56,8 @@ lldb::ProcessSP process_sp; if (crash_file) { // Read enough data for a ELF32 header or ELF64 header +// Note: Here we care about e_type field only, so it is safe +// to ignore possible presence of the header extension. const size_t header_size = sizeof(llvm::ELF::Elf64_Ehdr); lldb::DataBufferSP data_sp(crash_file->ReadFileContents(0, header_size)); Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp === --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -610,7 +610,8 @@ DataExtractor data; data.SetData(data_sp); elf::ELFHeader header; -if (header.Parse(data, &data_offset)) { +lldb::offset_t header_offset = data_offset; +if (header.Parse(data, &header_offset)) { if (data_sp) { ModuleSpec spec(file); @@ -645,10 +646,24 @@ __FUNCTION__, file.GetPath().c_str());
[Lldb-commits] [lldb] r293366 - One of the changes Jim made in r286288 (cleaning up the stop print
Author: jmolenda Date: Fri Jan 27 20:54:10 2017 New Revision: 293366 URL: http://llvm.org/viewvc/llvm-project?rev=293366&view=rev Log: One of the changes Jim made in r286288 (cleaning up the stop print header line, backtrace output) was to remove the current pc value from frames where we have source level information. We've been discussing this for the past week and based on input from a group of low level users, I believe this is the wrong default behavior for the command line lldb tool. lldb's backtrace will include the pc value for all stack frames regardless of whether they have source level debug information or not. A related part of r286288 removes the byte offset printing for functions with source level information (e.g. "main + 22 sourcefile.c:10" is printed as "main sourcefile.c:10"). I don't see a compelling case for changing this part of 286288 so I'm leaving that as-is (in addition to the rest of 286288 which is clearly better than the previous output style). Modified: lldb/trunk/source/Core/Debugger.cpp Modified: lldb/trunk/source/Core/Debugger.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=293366&r1=293365&r2=293366&view=diff == --- lldb/trunk/source/Core/Debugger.cpp (original) +++ lldb/trunk/source/Core/Debugger.cpp Fri Jan 27 20:54:10 2017 @@ -125,7 +125,7 @@ OptionEnumValueElement g_language_enumer "\\n" #define DEFAULT_FRAME_FORMAT \ - "frame #${frame.index}:{ ${frame.no-debug}${frame.pc}}" MODULE_WITH_FUNC FILE_AND_LINE \ + "frame #${frame.index}: ${frame.pc}" MODULE_WITH_FUNC FILE_AND_LINE \ IS_OPTIMIZED "\\n" // Three parts to this disassembly format specification: ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits