Re: [Lldb-commits] [PATCH] D19991: [LLDB] Fix standalone build on RHEL6
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. looks fine. Out of curiosity, where were the undefined symbols coming from? I'm curious to know why they weren't showing up in none of our builds? Repository: rL LLVM http://reviews.llvm.org/D19991 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.
labath added a subscriber: labath. labath added a comment. I am glad to see more testing of the modules debugging. I have a couple of small comments though: - `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is supposed to be invoked? (I am not very familiar clang modules) - there is a `@skipUnlessClangModules` decorator in decorators.py. As far as I can see, this patch should now make it obsolete. It seems that it can be removed and all invocations replaced with `add_test_categories(["gmodules"])` And one meta-comment not directly related to this patch: We already run most of the tests two times. Now we will be doing it once more, which will increase the test times even more. I think it's important for each debug info format to have good coverage, but I also feel that there are tests which have nothing to do with debug info (or their connection to debug info is only very peripheral), and it does not make to sense to slow down the tests runs by running those tests so many times. We already have a (not very elegant, but working) mechanism to avoid this (`NO_DEBUG_INFO_TESTCASE` member). I propose that we be more aggressive in using it for new tests which do not specifically test debug info. Also when looking at existing tests, we should re-evaluate whether the test really needs to be run that many times (right now, the largest candidate that comes to mind is TestConcurrentEvents, but I am sure there are others I can't think of by name now). Comment at: packages/Python/lldbsuite/test/lldbtest.py:1488 @@ +1487,3 @@ +@wraps(attrvalue) +def dwarf_test_method(self, attrvalue=attrvalue): +self.debug_info = "gmodules" Shouldn't this be `gmodules_test_method`? http://reviews.llvm.org/D19998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.
aprantl added a comment. In http://reviews.llvm.org/D19998#423277, @labath wrote: > I am glad to see more testing of the modules debugging. I have a couple of > small comments though: > > - `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is > supposed to be invoked? (I am not very familiar clang modules) C++ modules are still a work in progress and not supported on all platforms (particularly on Darwin due to the way the system module maps interact with libc++, see https://llvm.org/bugs/show_bug.cgi?id=26928 for examples). On the platforms where C++ modules work well (such as Linux) on the other hand, module debugging hasn't been productized so far. Due to the way module debugging reuses DWO mechanisms I don't expect it to work without some fine-tuning. > - there is a `@skipUnlessClangModules` decorator in decorators.py. As far as > I can see, this patch should now make it obsolete. It seems that it can be > removed and all invocations replaced with `add_test_categories(["gmodules"])` Good catch. I didn't notice that! > And one meta-comment not directly related to this patch: > We already run most of the tests two times. Now we will be doing it once > more, which will increase the test times even more. I think it's important > for each debug info format to have good coverage, but I also feel that there > are tests which have nothing to do with debug info (or their connection to > debug info is only very peripheral), and it does not make to sense to slow > down the tests runs by running those tests so many times. We already have a > (not very elegant, but working) mechanism to avoid this > (`NO_DEBUG_INFO_TESTCASE` member). I propose that we be more aggressive in > using it for new tests which do not specifically test debug info. Also when > looking at existing tests, we should re-evaluate whether the test really > needs to be run that many times (right now, the largest candidate that comes > to mind is TestConcurrentEvents, but I am sure there are others I can't think > of by name now). That sounds like an all-around good idea, but probably out of scope for this patch. http://reviews.llvm.org/D19998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.
tfiala added a comment. > I propose that we be more aggressive in using it for new tests which do not > specifically test debug info. I totally agree, but I also caution that, as a debugger, the heart of much of what we do does use debug info. So we need to be careful to make sure that we don't disable the fan-out across all debug styles if we are in fact using debug info. http://reviews.llvm.org/D19998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19991: [LLDB] Fix standalone build on RHEL6
Eugene.Zelenko added a comment. Undefined symbols were used in clang::CodeGen::CoverageMappingGen. Repository: rL LLVM http://reviews.llvm.org/D19991 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268750 - Fix standalone build on RHEL6.
Author: eugenezelenko Date: Fri May 6 12:03:09 2016 New Revision: 268750 URL: http://llvm.org/viewvc/llvm-project?rev=268750&view=rev Log: Fix standalone build on RHEL6. Differential revision: http://reviews.llvm.org/D19991 Modified: lldb/trunk/cmake/LLDBDependencies.cmake Modified: lldb/trunk/cmake/LLDBDependencies.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/LLDBDependencies.cmake?rev=268750&r1=268749&r2=268750&view=diff == --- lldb/trunk/cmake/LLDBDependencies.cmake (original) +++ lldb/trunk/cmake/LLDBDependencies.cmake Fri May 6 12:03:09 2016 @@ -195,6 +195,7 @@ set(LLVM_LINK_COMPONENTS runtimedyld option support + coverage ) if ( NOT LLDB_DISABLE_PYTHON ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19991: [LLDB] Fix standalone build on RHEL6
This revision was automatically updated to reflect the committed changes. Closed by commit rL268750: Fix standalone build on RHEL6. (authored by eugenezelenko). Changed prior to commit: http://reviews.llvm.org/D19991?vs=56330&id=56430#toc Repository: rL LLVM http://reviews.llvm.org/D19991 Files: lldb/trunk/cmake/LLDBDependencies.cmake Index: lldb/trunk/cmake/LLDBDependencies.cmake === --- lldb/trunk/cmake/LLDBDependencies.cmake +++ lldb/trunk/cmake/LLDBDependencies.cmake @@ -195,6 +195,7 @@ runtimedyld option support + coverage ) if ( NOT LLDB_DISABLE_PYTHON ) Index: lldb/trunk/cmake/LLDBDependencies.cmake === --- lldb/trunk/cmake/LLDBDependencies.cmake +++ lldb/trunk/cmake/LLDBDependencies.cmake @@ -195,6 +195,7 @@ runtimedyld option support + coverage ) if ( NOT LLDB_DISABLE_PYTHON ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268755 - debugserver: fix some -Wformat-pedantic warnings
Author: compnerd Date: Fri May 6 12:32:58 2016 New Revision: 268755 URL: http://llvm.org/viewvc/llvm-project?rev=268755&view=rev Log: debugserver: fix some -Wformat-pedantic warnings Perform explicit casts for the log message to address some `-Wformat-pedantic` warnings from clang. NFC. Modified: lldb/trunk/tools/debugserver/source/DNB.cpp Modified: lldb/trunk/tools/debugserver/source/DNB.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNB.cpp?rev=268755&r1=268754&r2=268755&view=diff == --- lldb/trunk/tools/debugserver/source/DNB.cpp (original) +++ lldb/trunk/tools/debugserver/source/DNB.cpp Fri May 6 12:32:58 2016 @@ -360,21 +360,13 @@ DNBProcessLaunch (const char *path, char *err_str, size_t err_len) { -DNBLogThreadedIf(LOG_PROCESS, "%s ( path='%s', argv = %p, envp = %p, working_dir=%s, stdin=%s, stdout=%s, stderr=%s, no-stdio=%i, launch_flavor = %u, disable_aslr = %d, err = %p, err_len = %llu) called...", - __FUNCTION__, - path, - argv, - envp, - working_directory, - stdin_path, - stdout_path, - stderr_path, - no_stdio, - launch_flavor, - disable_aslr, - err_str, - (uint64_t)err_len); - +DNBLogThreadedIf(LOG_PROCESS, "%s ( path='%s', argv = %p, envp = %p, working_dir=%s, stdin=%s, stdout=%s, " + "stderr=%s, no-stdio=%i, launch_flavor = %u, disable_aslr = %d, err = %p, err_len = " + "%llu) called...", + __FUNCTION__, path, static_cast(argv), static_cast(envp), working_directory, + stdin_path, stdout_path, stderr_path, no_stdio, launch_flavor, disable_aslr, + static_cast(err_str), static_cast(err_len)); + if (err_str && err_len > 0) err_str[0] = '\0'; struct stat path_stat; @@ -548,7 +540,6 @@ DNBProcessAttach (nub_process_t attach_p switch (pid_state) { -default: case eStateInvalid: case eStateUnloaded: case eStateAttaching: ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268756 - debugserver: fix a few -Wcovered-swift-default warnings
Author: compnerd Date: Fri May 6 12:33:01 2016 New Revision: 268756 URL: http://llvm.org/viewvc/llvm-project?rev=268756&view=rev Log: debugserver: fix a few -Wcovered-swift-default warnings Remove a couple of `default` cases from switches which are covered. This is beneficial since it would allow the compiler to indicate when a new enum value is added and the switch is not updated. Fixes some warnings from clang. NFC. Modified: lldb/trunk/tools/debugserver/source/DNBDataRef.cpp lldb/trunk/tools/debugserver/source/RNBRemote.cpp Modified: lldb/trunk/tools/debugserver/source/DNBDataRef.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNBDataRef.cpp?rev=268756&r1=268755&r2=268756&view=diff == --- lldb/trunk/tools/debugserver/source/DNBDataRef.cpp (original) +++ lldb/trunk/tools/debugserver/source/DNBDataRef.cpp Fri May 6 12:33:01 2016 @@ -362,7 +362,6 @@ DNBDataRef::Dump // the snprintf call each time through this loop switch (type) { -default: case TypeUInt8: str_offset += snprintf(str + str_offset, sizeof(str) - str_offset, format ? format : " %2.2x", Get8(&offset)); break; case TypeChar: { Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268756&r1=268755&r2=268756&view=diff == --- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original) +++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May 6 12:33:01 2016 @@ -1026,7 +1026,6 @@ RNBRemote::ThreadFunctionReadRemoteData( case rnb_success: break; -default: case rnb_err: DNBLogThreadedIf (LOG_RNB_REMOTE, "RNBSocket::GetCommData returned error %u", err); done = true; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268759 - debugserver; fix -Wunused-local-typedef, -Wunused-variable warnings
Author: compnerd Date: Fri May 6 12:33:13 2016 New Revision: 268759 URL: http://llvm.org/viewvc/llvm-project?rev=268759&view=rev Log: debugserver; fix -Wunused-local-typedef, -Wunused-variable warnings Remove the typedef and local structure which was unused. Fixes last of the new clang warnings in the debugserver build. NFC. Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268759&r1=268758&r2=268759&view=diff == --- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original) +++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May 6 12:33:13 2016 @@ -3555,13 +3555,6 @@ RNBRemote::HandlePacket_v (const char *p } else if (strstr (p, "vCont") == p) { -typedef struct -{ -nub_thread_t tid; -char action; -int signal; -} vcont_action_t; - DNBThreadResumeActions thread_actions; char *c = (char *)(p += strlen("vCont")); char *c_end = c + strlen(c); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268757 - debugserver: fix some -Wpessimizing-move warnings
Author: compnerd Date: Fri May 6 12:33:04 2016 New Revision: 268757 URL: http://llvm.org/viewvc/llvm-project?rev=268757&view=rev Log: debugserver: fix some -Wpessimizing-move warnings Remove the unnecessary use of std::move to permit the compiler to perform NVRO instead. Fixes more warnings from clang. NFC. Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268757&r1=268756&r2=268757&view=diff == --- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original) +++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May 6 12:33:04 2016 @@ -2570,13 +2570,13 @@ RNBRemote::DispatchQueueOffsets::GetThre nub_addr_t pointer_to_label_address = dispatch_queue_t + dqo_label; nub_addr_t label_addr = DNBProcessMemoryReadPointer (pid, pointer_to_label_address); if (label_addr) -queue_name = std::move(DNBProcessMemoryReadCString (pid, label_addr)); +queue_name = DNBProcessMemoryReadCString(pid, label_addr); } else { // libdispatch versions 1-3, dispatch name is a fixed width char array // in the queue structure. -queue_name = std::move(DNBProcessMemoryReadCStringFixed(pid, dispatch_queue_t + dqo_label, dqo_label_size)); +queue_name = DNBProcessMemoryReadCStringFixed(pid, dispatch_queue_t + dqo_label, dqo_label_size); } } } @@ -5687,7 +5687,7 @@ RNBRemote::HandlePacket_qSymbol (const c if (*p) { // We have a symbol name -symbol_name = std::move(decode_hex_ascii_string(p)); +symbol_name = decode_hex_ascii_string(p); if (!symbol_value_str.empty()) { nub_addr_t symbol_value = decode_uint64(symbol_value_str.c_str(), 16); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268758 - debugserver: fix a couple of -Wmissing-field-initializers warnings
Author: compnerd Date: Fri May 6 12:33:09 2016 New Revision: 268758 URL: http://llvm.org/viewvc/llvm-project?rev=268758&view=rev Log: debugserver: fix a couple of -Wmissing-field-initializers warnings Explicitly provide an initializer for the std::vector in the constructed type. Addresses -Wmissing-field-initializers warnings from clang. NFC. Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=268758&r1=268757&r2=268758&view=diff == --- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original) +++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Fri May 6 12:33:09 2016 @@ -1227,7 +1227,9 @@ RNBRemote::InitializeRegisters (bool for register_map_entry_t reg_entry = { regnum++, // register number starts at zero and goes up with no gaps reg_data_offset,// Offset into register context data, no gaps between registers -reg_sets[set].registers[reg]// DNBRegisterInfo +reg_sets[set].registers[reg], // DNBRegisterInfo +{}, +{}, }; name_to_regnum[reg_entry.nub_info.name] = reg_entry.debugserver_regnum; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19998: Add a "-gmodules" category to the test suite.
labath added a comment. In http://reviews.llvm.org/D19998#423541, @aprantl wrote: > In http://reviews.llvm.org/D19998#423277, @labath wrote: > > > I am glad to see more testing of the modules debugging. I have a couple of > > small comments though: > > > > - `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is > > supposed to be invoked? (I am not very familiar clang modules) > > > C++ modules are still a work in progress and not supported on all platforms > (particularly on Darwin due to the way the system module maps interact with > libc++, see https://llvm.org/bugs/show_bug.cgi?id=26928 for examples). On the > platforms where C++ modules work well (such as Linux) on the other hand, > module debugging hasn't been productized so far. Due to the way module > debugging reuses DWO mechanisms I don't expect it to work without some > fine-tuning. Thank you for the comprehensive explanation. This information is interesting. Does that mean that in case of tests with c++ source code, there will be no difference in the test executables between modules and non-modules versions of the tests? I am wondering whether we should avoid running the modules tests in this case... I just did a quick count and we seem to have 129 C source files vs. 262 C++ files, so the difference is not actually as big as I expected, but it is still a substantial amount of test time going to waste. Maybe it's not that important though, we can possibly optimize that later, if it turns out to be necessary. > > And one meta-comment not directly related to this patch: > > > We already run most of the tests two times. Now we will be doing it once > > more, which will increase the test times even more. I think it's important > > for each debug info format to have good coverage, but I also feel that > > there are tests which have nothing to do with debug info (or their > > connection to debug info is only very peripheral), and it does not make to > > sense to slow down the tests runs by running those tests so many times. We > > already have a (not very elegant, but working) mechanism to avoid this > > (`NO_DEBUG_INFO_TESTCASE` member). I propose that we be more aggressive in > > using it for new tests which do not specifically test debug info. Also when > > looking at existing tests, we should re-evaluate whether the test really > > needs to be run that many times (right now, the largest candidate that > > comes to mind is TestConcurrentEvents, but I am sure there are others I > > can't think of by name now). > > > That sounds like an all-around good idea, but probably out of scope for this > patch. Yes, I am definitely not requesting you make any changes like that here. I using just testing the waters about what people think about this idea. It's a bit of a hijack though, so sorry about that. We can move the discussion outside, if it starts to get more involved. In http://reviews.llvm.org/D19998#423562, @tfiala wrote: > > I propose that we be more aggressive in using it for new tests which do > > not specifically test debug info. > > > I totally agree, but I also caution that, as a debugger, the heart of much of > what we do does use debug info. So we need to be careful to make sure that > we don't disable the fan-out across all debug styles if we are in fact using > debug info. I guess the question here is what do we consider as "using the debug info". To take TestConcurrentEvents as an example, it certainly *uses* debug info, in the sense that it sets a breakpoint at a certain line, and sets some variables, but I don't think this is what the test is about. It is about testing that the debugger handles the situation where there are multiple events happening in the target concurrently, which should not be related to the debug info at all. Any failure it this test which would be caused by a debug info problem should already be caught by some other test which actually targets these things. At least, that's my opinion... Comment at: packages/Python/lldbsuite/test/plugins/builder_base.py:144 @@ -143,1 +143,3 @@ +def buildDwarf(sender=None, architecture=None, compiler=None, dictionary=None, clean=True): +"""Build the binaries with dwarf debug info.""" shouldn't this be `buildModules` or something? http://reviews.llvm.org/D19998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268802 - Fix LLDB after removal of PDB_ErrorCode
Author: zturner Date: Fri May 6 16:35:47 2016 New Revision: 268802 URL: http://llvm.org/viewvc/llvm-project?rev=268802&view=rev Log: Fix LLDB after removal of PDB_ErrorCode 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=268802&r1=268801&r2=268802&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Fri May 6 16:35:47 2016 @@ -118,7 +118,7 @@ SymbolFilePDB::CalculateAbilities() // Lazily load and match the PDB file, but only do this once. std::string exePath = m_obj_file->GetFileSpec().GetPath(); auto error = loadDataForEXE(PDB_ReaderType::DIA, llvm::StringRef(exePath), m_session_up); -if (error != PDB_ErrorCode::Success) +if (error) return 0; } return CompileUnits | LineTables; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D20035: Add source file mapping to inline frames' SymbolContext line_entry.file
ted created this revision. ted added a reviewer: jingham. ted added a subscriber: lldb-commits. new StackFrame objects created in StackFrameList::GetFramesUpTo are constructed with SymbolContext set to nullptr, except for inline frames. Calling the ctor with nullptr causes StackFrame::GetSymbolContext to apply the target.source-map to line_entry.file. Calling it with a valid SC will cause line_entry.file to have the original file from the DWARF info, so the file comparison in ThreadPlanStepRange::InRange will fail when it should succeed. This patch adds the mapping from StackFrame::GetSymbolContext to the inline frame case in StackFrameList::GetFramesUpTo. http://reviews.llvm.org/D20035 Files: source/Target/StackFrameList.cpp Index: source/Target/StackFrameList.cpp === --- source/Target/StackFrameList.cpp +++ source/Target/StackFrameList.cpp @@ -350,6 +350,8 @@ if (unwind_block) { Address curr_frame_address (unwind_frame_sp->GetFrameCodeAddress()); +TargetSP target_sp = m_thread.CalculateTarget(); + // Be sure to adjust the frame address to match the address // that was used to lookup the symbol context above. If we are // in the first concrete frame, then we lookup using the current @@ -362,9 +364,8 @@ // If curr_frame_address points to the first address in a section then after // adjustment it will point to an other section. In that case resolve the // address again to the correct section plus offset form. -TargetSP target = m_thread.CalculateTarget(); -addr_t load_addr = curr_frame_address.GetOpcodeLoadAddress(target.get(), eAddressClassCode); -curr_frame_address.SetOpcodeLoadAddress(load_addr - 1, target.get(), eAddressClassCode); +addr_t load_addr = curr_frame_address.GetOpcodeLoadAddress(target_sp.get(), eAddressClassCode); +curr_frame_address.SetOpcodeLoadAddress(load_addr - 1, target_sp.get(), eAddressClassCode); } else { @@ -377,17 +378,25 @@ while (unwind_sc.GetParentOfInlinedScope(curr_frame_address, next_frame_sc, next_frame_address)) { -StackFrameSP frame_sp(new StackFrame (m_thread.shared_from_this(), - m_frames.size(), - idx, - unwind_frame_sp->GetRegisterContextSP (), - cfa, - next_frame_address, - &next_frame_sc)); - -m_frames.push_back (frame_sp); -unwind_sc = next_frame_sc; -curr_frame_address = next_frame_address; +if (target_sp) +{ +// Be sure to apply and file remappings to our file and line +// entries when handing out a line entry +FileSpec new_file_spec; +if (target_sp->GetSourcePathMap().FindFile(next_frame_sc.line_entry.file, new_file_spec)) +next_frame_sc.line_entry.file = new_file_spec; +} +StackFrameSP frame_sp(new StackFrame(m_thread.shared_from_this(), + m_frames.size(), + idx, + unwind_frame_sp->GetRegisterContextSP (), + cfa, + next_frame_address, + &next_frame_sc)); + +m_frames.push_back (frame_sp); +unwind_sc = next_frame_sc; +curr_frame_address = next_frame_address; } } } while (m_frames.size() - 1 < end_idx); Index: source/Target/StackFrameList.cpp === --- source/Target/StackFrameList.cpp +++ source/Target/StackFrameList.cpp @@ -350,6 +350,8 @@ if (unwind_block) { Address curr_frame_address (unwind_frame_sp->GetFrameCodeAddress()); +TargetSP target_sp = m_thread.CalculateTarget(); + // Be
[Lldb-commits] [lldb] r268823 - Fix the way the ShouldStopHere checker handles the general case of "stepping through line 0 code".
Author: jingham Date: Fri May 6 18:44:10 2016 New Revision: 268823 URL: http://llvm.org/viewvc/llvm-project?rev=268823&view=rev Log: Fix the way the ShouldStopHere checker handles the general case of "stepping through line 0 code". That's good 'cause it means all the different kinds of source line stepping won't leave user in the middle of compiler implementation code or code inlined from odd places, etc. But it turns out that the compiler also marks functions it MIGHT inline as all being of line 0. That would mean we single step through this code instead of just stepping out. That is both inefficient, and more error prone 'cause these little nuggets tend to be bits of hand-written assembly and the like and are hard to step through. This change just checks and if the entire function is marked with line 0, we step out rather than step through. Modified: lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp Modified: lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp?rev=268823&r1=268822&r2=268823&view=diff == --- lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp (original) +++ lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp Fri May 6 18:44:10 2016 @@ -11,10 +11,11 @@ // C++ Includes // Other libraries and framework includes // Project includes +#include "lldb/Core/Log.h" +#include "lldb/Symbol/Symbol.h" #include "lldb/Target/RegisterContext.h" #include "lldb/Target/Thread.h" #include "lldb/Target/ThreadPlanShouldStopHere.h" -#include "lldb/Core/Log.h" using namespace lldb; using namespace lldb_private; @@ -113,19 +114,45 @@ ThreadPlanShouldStopHere::DefaultStepFro ThreadPlanSP return_plan_sp; // If we are stepping through code at line number 0, then we need to step over this range. Otherwise // we will step out. +Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP)); + StackFrame *frame = current_plan->GetThread().GetStackFrameAtIndex(0).get(); if (!frame) return return_plan_sp; SymbolContext sc; sc = frame->GetSymbolContext (eSymbolContextLineEntry); +sc = frame->GetSymbolContext (eSymbolContextLineEntry|eSymbolContextSymbol); if (sc.line_entry.line == 0) { AddressRange range = sc.line_entry.range; -return_plan_sp = current_plan->GetThread().QueueThreadPlanForStepOverRange(false, - range, - sc, - eOnlyDuringStepping, - eLazyBoolNo); + + // If the whole function is marked line 0 just step out, that's easier & faster than continuing +// to step through it. + bool just_step_out = false; + if (sc.symbol && sc.symbol->ValueIsAddress()) + { + Address symbol_end = sc.symbol->GetAddress(); + symbol_end.Slide(sc.symbol->GetByteSize() - 1); +if (range.ContainsFileAddress(sc.symbol->GetAddress()) && range.ContainsFileAddress(symbol_end)) + { + if (log) + log->Printf("Stopped in a function with only line 0 lines, just stepping out."); +just_step_out = true; + } + } +if (!just_step_out) +{ +if (log) +log->Printf ("ThreadPlanShouldStopHere::DefaultStepFromHereCallback Queueing StepInRange plan to step through line 0 code."); + +return_plan_sp = current_plan->GetThread().QueueThreadPlanForStepInRange(false, + range, + sc, + NULL, + eOnlyDuringStepping, + eLazyBoolCalculate, + eLazyBoolNo); +} } if (!return_plan_sp) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268827 - Remove some lldbassert's from the packet checking code.
Author: jingham Date: Fri May 6 19:52:18 2016 New Revision: 268827 URL: http://llvm.org/viewvc/llvm-project?rev=268827&view=rev Log: Remove some lldbassert's from the packet checking code. Greg says he doesn't need these asserts anymore and since they cause occasional test suite crashes, out they go. Modified: lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp Modified: lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp?rev=268827&r1=268826&r2=268827&view=diff == --- lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp (original) +++ lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp Fri May 6 19:52:18 2016 @@ -14,7 +14,6 @@ // Other libraries and framework includes // Project includes #include "Utility/StringExtractorGDBRemote.h" -#include "lldb/Utility/LLDBAssert.h" StringExtractorGDBRemote::ResponseType StringExtractorGDBRemote::GetResponseType () const @@ -405,7 +404,6 @@ OKErrorNotSupportedResponseValidator(voi case StringExtractorGDBRemote::eResponse: break; } -lldbassert(!"Packet validatation failed, check why this is happening"); return false; } @@ -438,7 +436,6 @@ JSONResponseValidator(void *, const Stri } break; } -lldbassert(!"Packet validatation failed, check why this is happening"); return false; } @@ -463,7 +460,6 @@ ASCIIHexBytesResponseValidator(void *, c { if (!isxdigit(ch)) { -lldbassert(!"Packet validatation failed, check why this is happening"); return false; } if (++valid_count >= 16) @@ -473,7 +469,6 @@ ASCIIHexBytesResponseValidator(void *, c } break; } -lldbassert(!"Packet validatation failed, check why this is happening"); return false; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r268828 - Take the API lock in SBThread::IsValid & SBFrame::IsValid.
Author: jingham Date: Fri May 6 19:54:56 2016 New Revision: 268828 URL: http://llvm.org/viewvc/llvm-project?rev=268828&view=rev Log: Take the API lock in SBThread::IsValid & SBFrame::IsValid. The IsValid calls can try to reconstruct the thread & frame, which can take various internal locks. This can cause A/B locking issues with the Target lock, so these calls need to that the Target lock. Modified: lldb/trunk/source/API/SBFrame.cpp lldb/trunk/source/API/SBThread.cpp Modified: lldb/trunk/source/API/SBFrame.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFrame.cpp?rev=268828&r1=268827&r2=268828&view=diff == --- lldb/trunk/source/API/SBFrame.cpp (original) +++ lldb/trunk/source/API/SBFrame.cpp Fri May 6 19:54:56 2016 @@ -105,7 +105,20 @@ SBFrame::SetFrameSP (const StackFrameSP bool SBFrame::IsValid() const { -return GetFrameSP().get() != nullptr; +Mutex::Locker api_locker; +ExecutionContext exe_ctx (m_opaque_sp.get(), api_locker); + +Target *target = exe_ctx.GetTargetPtr(); +Process *process = exe_ctx.GetProcessPtr(); +if (target && process) +{ +Process::StopLocker stop_locker; +if (stop_locker.TryLock(&process->GetRunLock())) +return GetFrameSP().get() != nullptr; +} + +// Without a target & process we can't have a valid stack frame. +return false; } SBSymbolContext Modified: lldb/trunk/source/API/SBThread.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBThread.cpp?rev=268828&r1=268827&r2=268828&view=diff == --- lldb/trunk/source/API/SBThread.cpp (original) +++ lldb/trunk/source/API/SBThread.cpp Fri May 6 19:54:56 2016 @@ -130,7 +130,19 @@ SBThread::GetQueue () const bool SBThread::IsValid() const { -return m_opaque_sp->GetThreadSP().get() != NULL; +Mutex::Locker api_locker; +ExecutionContext exe_ctx (m_opaque_sp.get(), api_locker); + +Target *target = exe_ctx.GetTargetPtr(); +Process *process = exe_ctx.GetProcessPtr(); +if (target && process) +{ +Process::StopLocker stop_locker; +if (stop_locker.TryLock(&process->GetRunLock())) +return m_opaque_sp->GetThreadSP().get() != NULL; +} +// Without a valid target & process, this thread can't be valid. +return false; } void ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X
stigger created this revision. stigger added a reviewer: zturner. stigger added a subscriber: lldb-commits. - File path comparisons should be case-insensitive on OS X - Fixed TestBreakpointCaseSensitivity http://reviews.llvm.org/D20041 Files: include/lldb/Host/FileSpec.h packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/TestBreakpointCaseSensitivity.py source/Host/common/FileSpec.cpp source/Host/common/HostInfoBase.cpp Index: source/Host/common/HostInfoBase.cpp === --- source/Host/common/HostInfoBase.cpp +++ source/Host/common/HostInfoBase.cpp @@ -59,15 +59,15 @@ ArchSpec m_host_arch_32; ArchSpec m_host_arch_64; -FileSpec m_lldb_so_dir; -FileSpec m_lldb_support_exe_dir; -FileSpec m_lldb_headers_dir; -FileSpec m_lldb_python_dir; -FileSpec m_lldb_clang_resource_dir; -FileSpec m_lldb_system_plugin_dir; -FileSpec m_lldb_user_plugin_dir; -FileSpec m_lldb_process_tmp_dir; -FileSpec m_lldb_global_tmp_dir; +FileSpec m_lldb_so_dir = FileSpec(false); +FileSpec m_lldb_support_exe_dir = FileSpec(false); +FileSpec m_lldb_headers_dir = FileSpec(false); +FileSpec m_lldb_python_dir = FileSpec(false); +FileSpec m_lldb_clang_resource_dir = FileSpec(false); +FileSpec m_lldb_system_plugin_dir = FileSpec(false); +FileSpec m_lldb_user_plugin_dir = FileSpec(false); +FileSpec m_lldb_process_tmp_dir = FileSpec(false); +FileSpec m_lldb_global_tmp_dir = FileSpec(false); }; HostInfoBaseFields *g_fields = nullptr; Index: source/Host/common/FileSpec.cpp === --- source/Host/common/FileSpec.cpp +++ source/Host/common/FileSpec.cpp @@ -36,6 +36,7 @@ #include "lldb/Host/FileSpec.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/Host.h" +#include "lldb/Host/HostInfo.h" #include "lldb/Utility/CleanUp.h" #include "llvm/ADT/StringRef.h" @@ -304,9 +305,15 @@ } FileSpec::FileSpec() : +FileSpec{IsCaseSensitive(ePathSyntaxHostNative)} +{ +} + +FileSpec::FileSpec(bool case_sensitive) : m_directory(), m_filename(), -m_syntax(FileSystem::GetNativePathSyntax()) +m_syntax(FileSystem::GetNativePathSyntax()), +m_case_sensitive(case_sensitive) { } @@ -318,15 +325,17 @@ m_directory(), m_filename(), m_is_resolved(false), -m_syntax(syntax) +m_syntax(syntax), +m_case_sensitive(IsCaseSensitive(syntax)) { if (pathname && pathname[0]) -SetFile(pathname, resolve_path, syntax); +SetFile(pathname, resolve_path, m_case_sensitive, syntax); } FileSpec::FileSpec(const char *pathname, bool resolve_path, ArchSpec arch) : FileSpec{pathname, resolve_path, arch.GetTriple().isOSWindows() ? ePathSyntaxWindows : ePathSyntaxPosix} { +m_case_sensitive = IsCaseSensitive(m_syntax, &arch.GetTriple()); } FileSpec::FileSpec(const std::string &path, bool resolve_path, PathSyntax syntax) : @@ -346,7 +355,8 @@ m_directory (rhs.m_directory), m_filename (rhs.m_filename), m_is_resolved (rhs.m_is_resolved), -m_syntax (rhs.m_syntax) +m_syntax (rhs.m_syntax), +m_case_sensitive (rhs.m_case_sensitive) { } @@ -380,6 +390,7 @@ m_filename = rhs.m_filename; m_is_resolved = rhs.m_is_resolved; m_syntax = rhs.m_syntax; +m_case_sensitive = rhs.m_case_sensitive; } return *this; } @@ -390,11 +401,12 @@ // string values for quick comparison and efficient memory usage. //-- void -FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax) +FileSpec::SetFile(const char *pathname, bool resolve, bool case_sensitive, PathSyntax syntax) { m_filename.Clear(); m_directory.Clear(); m_is_resolved = false; +m_case_sensitive = case_sensitive; m_syntax = (syntax == ePathSyntaxHostNative) ? FileSystem::GetNativePathSyntax() : syntax; if (pathname == NULL || pathname[0] == '\0') @@ -431,12 +443,19 @@ } void +FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax, llvm::Triple *triple) +{ +return SetFile(pathname, resolve, IsCaseSensitive(syntax, triple), syntax); +} + +void FileSpec::SetFile(const char *pathname, bool resolve, ArchSpec arch) { return SetFile(pathname, resolve, arch.GetTriple().isOSWindows() ? ePathSyntaxWindows -: ePathSyntaxPosix); +: ePathSyntaxPosix, +&arch.GetTriple()); } void @@ -850,7 +869,7 @@ if (!GetPath (path_buf, PATH_MAX, false)) return false; // SetFile(...) will set m_is_resolved correctly if it can resolve the path -SetFile (path_buf, true); +SetFile(path_buf, true, m_case_sensitive, m_syntax); return m_is_r
Re: [Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X
jasonmolenda added a subscriber: jasonmolenda. jasonmolenda added a comment. I don't know if this is a safe change. Mac OS X filesystems like HFS Plus may be case-insensitive (but case preserving) or they may be case sensitive. Network filesystems mounted on a Mac may be case sensitive. Without doing some attribute check on the filesystem (I don't know what API can tell you this off-hand) where the FileSpec is, I don't think this is a good idea. http://reviews.llvm.org/D20041 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X
stigger added a comment. These are valid points, but they apply to Linux and Windows as well: nothing prevents you from mounting a case-insensitive FS on Linux or a case-sensitive network share on Windows. However, in most cases the file system is going to be case-sensitive on Linux and case-insensitive on Windows. It is true, that HFS supports optional case-sensitivity, but by default it is not being used, and 99% of Mac users out there are on case-insensitive FS. Of course, it would be nice to dynamically check case sensitivity for every path, but that should be done for all platforms and is kind of out of scope of the problem that I tried to solve here. Plus, it might affect the performance. http://reviews.llvm.org/D20041 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20041: File path comparisons should be case-insensitive on OS X
jasonmolenda added a comment. Agreed, filesystems are almost always case sensitive on mac/ios. I'd hate for anyone to make the assumption that it's always the case, though - I work with network filesystems that are case sensitive every day so I'm very aware of the possibility. :) http://reviews.llvm.org/D20041 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits