Re: [Lldb-commits] [PATCH] D20565: Add MemoryRegionInfo to SB API
hhellyer marked 2 inline comments as done. hhellyer added a comment. In http://reviews.llvm.org/D20565#442373, @clayborg wrote: > This looks good as long as SBMemoryRegionInfo and SBMemoryRegionInfoList are > ready only and will never have any setter functions. If we plan to ever have > the SBMemoryRegionInfo or SBMemoryRegionInfoList be modifiable, then we > should make each contain a std::unique_ptr instead of a std::shared_ptr. I’m not sure if there’s ever going to be a need for any setter methods on the SBMemoryRegionInfo classes. Can you suggest any use cases? In my mind they just reflect the state of the memory in the process, I'm not sure if they allow that state to be changed. For example a user wouldn’t ever directly create a new memory region by creating an SBMemoryRegionInfo. The process would allocate memory, memory map a file or load a module and a new one (might) be created. Other functions in lldb might cause the process to allocate or free memory and indirectly create or extend a memory region. Similarly changing a regions properties (e.g. making a region read only or changing it’s size) doesn’t necessarily make sense as those changes won't be reflected back in the process. If SBMemoryRegion had setters, what the desired behaviour would be? The original purpose of SBMemoryInfoList to take a snap shot of memory regions (instead of using GetNumRegions and GetRegionAtIndex which would give unreliable results if the process was allowed to continue) so I'm not sure how it would all fit together if memory regions could be created or updated especially if the process continued. I guess the other case is setting something on the memory region that changes the behaviour of lldb with respect to it or using an SBMemoryRegionInfo as an argument to another method. Then you might want to create a custom region and there may be some of those that make more sense. > So I propose the following changes: > > - make both SBMemoryRegionInfo and SBMemoryRegionInfoList contain > std::unique_ptr objects > - have their constructors always create a default object > - change copy constructors to copy the objects instead of sharing a reference > - change assignment operators to copy the objects instead of sharing a > reference I'll make the changes and upload a new patch, it certainly doesn't hurt to enable it as a possibility, I'm just not sure what the behaviour would be with modifiable memory regions. I can always go back to the shared pointer version. (Sorry for the delay in replying, it was a bank holiday here yesterday.) http://reviews.llvm.org/D20565 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20565: Add MemoryRegionInfo to SB API
clayborg added a comment. In http://reviews.llvm.org/D20565#444223, @hhellyer wrote: > In http://reviews.llvm.org/D20565#442373, @clayborg wrote: > > > This looks good as long as SBMemoryRegionInfo and SBMemoryRegionInfoList > > are ready only and will never have any setter functions. If we plan to ever > > have the SBMemoryRegionInfo or SBMemoryRegionInfoList be modifiable, then > > we should make each contain a std::unique_ptr instead of a std::shared_ptr. > > > I’m not sure if there’s ever going to be a need for any setter methods on the > SBMemoryRegionInfo classes. Can you suggest any use cases? In my mind they > just reflect the state of the memory in the process, I'm not sure if they > allow that state to be changed. > > For example a user wouldn’t ever directly create a new memory region by > creating an SBMemoryRegionInfo. The process would allocate memory, memory map > a file or load a module and a new one (might) be created. Other functions in > lldb might cause the process to allocate or free memory and indirectly create > or extend a memory region. Similarly changing a regions properties (e.g. > making a region read only or changing it’s size) doesn’t necessarily make > sense as those changes won't be reflected back in the process. > > If SBMemoryRegion had setters, what the desired behaviour would be? The > original purpose of SBMemoryInfoList to take a snap shot of memory regions > (instead of using GetNumRegions and GetRegionAtIndex which would give > unreliable results if the process was allowed to continue) so I'm not sure > how it would all fit together if memory regions could be created or updated > especially if the process continued. It would be if we expose allocating/deallocating memory regions. User would create a memory region, set read/write/execute and a size, then call: SBError SBProcess::AllocateMemory(SBMemoryRegionInfo ®ion); > I guess the other case is setting something on the memory region that changes > the behaviour of lldb with respect to it or using an SBMemoryRegionInfo as an > argument to another method. Then you might want to create a custom region and > there may be some of those that make more sense. Yes, it is in case we enable calls that would require users to fill out memory regions. If we somehow exposed mmap through the public API, you can also request a certain address for the allocation in some calls. > > > > So I propose the following changes: > > > > > > - make both SBMemoryRegionInfo and SBMemoryRegionInfoList contain > > std::unique_ptr objects > > > - have their constructors always create a default object > > > - change copy constructors to copy the objects instead of sharing a > > reference > > > - change assignment operators to copy the objects instead of sharing a > > reference > > > I'll make the changes and upload a new patch, it certainly doesn't hurt to > enable it as a possibility, I'm just not sure what the behaviour would be > with modifiable memory regions. I can always go back to the shared pointer > version. (Sorry for the delay in replying, it was a bank holiday here > yesterday.) My main reason for making sure we use std::unique_ptr is for API compatibility in the future. The size of std::unique_ptr and std::shared_ptr differs and if anyone wrote an IDE or tool that links against the LLDB public API, then we can't switch the layout of a class without breaking our ABI. So it is safest to go with a std::unique_ptr since the backing class is so simple and isn't expensive to copy. http://reviews.llvm.org/D20565 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D20826: [CMake] Update to retiring CMake 3.4.3
beanz created this revision. beanz added a subscriber: lldb-commits. This is as per the discussions on developer lists: http://lists.llvm.org/pipermail/llvm-dev/2016-April/098780.html http://lists.llvm.org/pipermail/llvm-dev/2016-May/100058.html http://reviews.llvm.org/D20826 Files: CMakeLists.txt Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12.2) +cmake_minimum_required(VERSION 3.4.3) include(cmake/modules/LLDBStandalone.cmake) include(cmake/modules/LLDBConfig.cmake) Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12.2) +cmake_minimum_required(VERSION 3.4.3) include(cmake/modules/LLDBStandalone.cmake) include(cmake/modules/LLDBConfig.cmake) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20722: Implement ProcessInfo::Dump(), log gdb-remote stub launch
tfiala updated this revision to Diff 59105. tfiala added a comment. Updates per Greg. - Args::Dump(stream, label) now takes a default label. - Dropped the Args::Dump(stream) call. - Converted the Args stream parameter to a ref. It is not valid to call this without a stream. - Updated in-source docs to reflect changes. http://reviews.llvm.org/D20722 Files: include/lldb/Interpreter/Args.h source/Interpreter/Args.cpp source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp source/Target/ProcessInfo.cpp Index: source/Target/ProcessInfo.cpp === --- source/Target/ProcessInfo.cpp +++ source/Target/ProcessInfo.cpp @@ -15,6 +15,8 @@ // Project includes #include "lldb/Target/ProcessInfo.h" +#include "lldb/Core/Stream.h" + using namespace lldb; using namespace lldb_private; @@ -65,6 +67,21 @@ } void +ProcessInfo::Dump (Stream &s, Platform *platform) const +{ +s << "Executable: " << GetName() << "\n"; +s << "Triple: "; +m_arch.DumpTriple(s); +s << "\n"; + +s << "Arguments:\n"; +m_arguments.Dump(s); + +s << "Environment:\n"; +m_environment.Dump(s, "env"); +} + +void ProcessInfo::SetExecutableFile (const FileSpec &exe_file, bool add_exe_file_as_first_arg) { if (exe_file) Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp === --- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1120,7 +1120,7 @@ { Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS)); if (log) -log->Printf ("GDBRemoteCommunication::%s(url=%s, port=%" PRIu16, __FUNCTION__, url ? url : "", port ? *port : uint16_t(0)); +log->Printf ("GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")", __FUNCTION__, url ? url : "", port ? *port : uint16_t(0)); Error error; // If we locate debugserver, keep that located version around @@ -1352,7 +1352,14 @@ launch_info.AppendSuppressFileAction (STDIN_FILENO, true, false); launch_info.AppendSuppressFileAction (STDOUT_FILENO, false, true); launch_info.AppendSuppressFileAction (STDERR_FILENO, false, true); - + +if (log) +{ +StreamString string_stream; +Platform *const platform = nullptr; +launch_info.Dump(string_stream, platform); +log->Printf("launch info for gdb-remote stub:\n%s", string_stream.GetString().c_str()); +} error = Host::LaunchProcess(launch_info); if (error.Success() && Index: source/Interpreter/Args.cpp === --- source/Interpreter/Args.cpp +++ source/Interpreter/Args.cpp @@ -83,19 +83,22 @@ } void -Args::Dump (Stream *s) +Args::Dump (Stream &s, const char *label_name) const { +if (!label_name) +return; + const size_t argc = m_argv.size(); for (size_t i=0; iIndent(); +s.Indent(); const char *arg_cstr = m_argv[i]; if (arg_cstr) -s->Printf("argv[%zi]=\"%s\"\n", i, arg_cstr); +s.Printf("%s[%zi]=\"%s\"\n", label_name, i, arg_cstr); else -s->Printf("argv[%zi]=NULL\n", i); +s.Printf("%s[%zi]=NULL\n", label_name, i); } -s->EOL(); +s.EOL(); } bool Index: include/lldb/Interpreter/Args.h === --- include/lldb/Interpreter/Args.h +++ include/lldb/Interpreter/Args.h @@ -91,14 +91,20 @@ ~Args(); //-- -/// Dump all arguments to the stream \a s. +/// Dump all entries to the stream \a s using label \a label_name. +/// +/// If label_name is nullptr, the dump operation is skipped. /// /// @param[in] s /// The stream to which to dump all arguments in the argument /// vector. +/// @param[in] label_name +/// The label_name to use as the label printed for each +/// entry of the args like so: +/// {label_name}[{index}]={value} //-- void -Dump (Stream *s); +Dump (Stream &s, const char *label_name = "argv") const; //-- /// Sets the command string contained by this object. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20722: Implement ProcessInfo::Dump(), log gdb-remote stub launch
tfiala marked 2 inline comments as done. tfiala added a comment. Marked comments as done. http://reviews.llvm.org/D20722 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r271312 - Implement ProcessInfo::Dump(), log gdb-remote stub launch
Author: tfiala Date: Tue May 31 13:32:20 2016 New Revision: 271312 URL: http://llvm.org/viewvc/llvm-project?rev=271312&view=rev Log: Implement ProcessInfo::Dump(), log gdb-remote stub launch This change implements dumping the executable, triple, args and environment when using ProcessInfo::Dump(). It also tweaks the way Args::Dump() works so that it prints a configurable label rather than argv[{index}]={value}. By default it behaves the same, but if the Dump() method with the additional arg is provided, it can be overridden. The environment variables dumped as part of ProcessInfo::Dump() make use of that. lldb-server has been modified to dump the gdb-remote stub's ProcessInfo before launching if the "gdb-remote process" channel is logged. Modified: lldb/trunk/include/lldb/Interpreter/Args.h lldb/trunk/source/Interpreter/Args.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/trunk/source/Target/ProcessInfo.cpp Modified: lldb/trunk/include/lldb/Interpreter/Args.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=271312&r1=271311&r2=271312&view=diff == --- lldb/trunk/include/lldb/Interpreter/Args.h (original) +++ lldb/trunk/include/lldb/Interpreter/Args.h Tue May 31 13:32:20 2016 @@ -91,14 +91,20 @@ public: ~Args(); //-- -/// Dump all arguments to the stream \a s. +/// Dump all entries to the stream \a s using label \a label_name. +/// +/// If label_name is nullptr, the dump operation is skipped. /// /// @param[in] s /// The stream to which to dump all arguments in the argument /// vector. +/// @param[in] label_name +/// The label_name to use as the label printed for each +/// entry of the args like so: +/// {label_name}[{index}]={value} //-- void -Dump (Stream *s); +Dump (Stream &s, const char *label_name = "argv") const; //-- /// Sets the command string contained by this object. Modified: lldb/trunk/source/Interpreter/Args.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=271312&r1=271311&r2=271312&view=diff == --- lldb/trunk/source/Interpreter/Args.cpp (original) +++ lldb/trunk/source/Interpreter/Args.cpp Tue May 31 13:32:20 2016 @@ -83,19 +83,22 @@ Args::~Args () } void -Args::Dump (Stream *s) +Args::Dump (Stream &s, const char *label_name) const { +if (!label_name) +return; + const size_t argc = m_argv.size(); for (size_t i=0; iIndent(); +s.Indent(); const char *arg_cstr = m_argv[i]; if (arg_cstr) -s->Printf("argv[%zi]=\"%s\"\n", i, arg_cstr); +s.Printf("%s[%zi]=\"%s\"\n", label_name, i, arg_cstr); else -s->Printf("argv[%zi]=NULL\n", i); +s.Printf("%s[%zi]=NULL\n", label_name, i); } -s->EOL(); +s.EOL(); } bool Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=271312&r1=271311&r2=271312&view=diff == --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Tue May 31 13:32:20 2016 @@ -1120,7 +1120,7 @@ GDBRemoteCommunication::StartDebugserver { Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS)); if (log) -log->Printf ("GDBRemoteCommunication::%s(url=%s, port=%" PRIu16, __FUNCTION__, url ? url : "", port ? *port : uint16_t(0)); +log->Printf ("GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")", __FUNCTION__, url ? url : "", port ? *port : uint16_t(0)); Error error; // If we locate debugserver, keep that located version around @@ -1352,7 +1352,14 @@ GDBRemoteCommunication::StartDebugserver launch_info.AppendSuppressFileAction (STDIN_FILENO, true, false); launch_info.AppendSuppressFileAction (STDOUT_FILENO, false, true); launch_info.AppendSuppressFileAction (STDERR_FILENO, false, true); - + +if (log) +{ +StreamString string_stream; +Platform *const platform = nullptr; +launch_info.Dump(string_stream, platform); +log->Printf("launch info for gdb-remote stub:\n%s", string_stream.GetString().c_str()); +} error = Host::LaunchProcess(launch_info); if (error.Success() && Modifie
Re: [Lldb-commits] [PATCH] D20722: Implement ProcessInfo::Dump(), log gdb-remote stub launch
tfiala closed this revision. tfiala added a comment. Closed by commit r271312. http://reviews.llvm.org/D20722 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r270891 - Make sure that we succeed in starting a definition before we complete it and emit an error if we fail to start the definition.
On 26 May 2016 at 15:24, Greg Clayton via lldb-commits wrote: > Author: gclayton > Date: Thu May 26 14:24:02 2016 > New Revision: 270891 > > URL: http://llvm.org/viewvc/llvm-project?rev=270891&view=rev > Log: > Make sure that we succeed in starting a definition before we complete it and > emit an error if we fail to start the definition. > ... > +{ > +module_sp->ReportError ("DWARF DIE at > 0x%8.8x was not able to start its definition.\nPlease file a bug and attach > the file at the start of this error message", > + > type_die_ref.die_offset, > +type_name_cstr); > +} ../tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1762:61: warning: data argument not used by format string [-Wformat-extra-args] type_name_cstr); ^ ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r270891 - Make sure that we succeed in starting a definition before we complete it and emit an error if we fail to start the definition.
On 31 May 2016 at 16:24, Ed Maste wrote: > > ../tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1762:61: > warning: data argument not used by format string [-Wformat-extra-args] > type_name_cstr); > ^ Oops, hit send too early. I didn't make a change because it wasn't obvious from the diff context if the format string should have a %s added, or if the extraneous type_name_cstr arg should be deleted. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r271328 - [CMake] Update to requiring CMake 3.4.3
Author: cbieneman Date: Tue May 31 15:21:44 2016 New Revision: 271328 URL: http://llvm.org/viewvc/llvm-project?rev=271328&view=rev Log: [CMake] Update to requiring CMake 3.4.3 Summary: This is as per the discussions on developer lists: http://lists.llvm.org/pipermail/llvm-dev/2016-April/098780.html http://lists.llvm.org/pipermail/llvm-dev/2016-May/100058.html Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D20826 Modified: lldb/trunk/CMakeLists.txt Modified: lldb/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=271328&r1=271327&r2=271328&view=diff == --- lldb/trunk/CMakeLists.txt (original) +++ lldb/trunk/CMakeLists.txt Tue May 31 15:21:44 2016 @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12.2) +cmake_minimum_required(VERSION 3.4.3) include(cmake/modules/LLDBStandalone.cmake) include(cmake/modules/LLDBConfig.cmake) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20826: [CMake] Update to retiring CMake 3.4.3
This revision was automatically updated to reflect the committed changes. Closed by commit rL271328: [CMake] Update to requiring CMake 3.4.3 (authored by cbieneman). Changed prior to commit: http://reviews.llvm.org/D20826?vs=59096&id=59124#toc Repository: rL LLVM http://reviews.llvm.org/D20826 Files: lldb/trunk/CMakeLists.txt Index: lldb/trunk/CMakeLists.txt === --- lldb/trunk/CMakeLists.txt +++ lldb/trunk/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12.2) +cmake_minimum_required(VERSION 3.4.3) include(cmake/modules/LLDBStandalone.cmake) include(cmake/modules/LLDBConfig.cmake) Index: lldb/trunk/CMakeLists.txt === --- lldb/trunk/CMakeLists.txt +++ lldb/trunk/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12.2) +cmake_minimum_required(VERSION 3.4.3) include(cmake/modules/LLDBStandalone.cmake) include(cmake/modules/LLDBConfig.cmake) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
aprantl created this revision. aprantl added reviewers: jingham, clayborg. aprantl added a subscriber: lldb-commits. aprantl set the repository for this revision to rL LLVM. Here's a fun little idea plus a preliminary patch implementing it. When I'm debugging programs I often wonder what exactly will happen when I step-in. This is particularly a problem with code that has lots of control flow happening on a single line, such as a C++ for loop with iterators, a function call with a lambda or block definition, or plain old nested function calls. For example, let's say I'm stopped inside lldb in SourceManager.cpp:93 — I would really like to know which function I'll be stepping into next. Will the next "step" take me into get(), new, File(), or reset()? ``` 90 // If file_sp is no good or it points to a non-existent file, reset it. 91 if (!file_sp || !file_sp->GetFileSpec().Exists()) 92 { -> 93 file_sp.reset (new File (file_spec, target_sp.get())); 94 95 if (debugger_sp) 96 debugger_sp->GetSourceFileCache().AddSourceFile(file_sp); (lldb) step ``` Of course a debugger cannot predict the future, but what it can do is tell me exactly where I am stopped now! Compilers like clang already include column information in the debug info by default. The attached patch makes use of this by adding an underline attribute to the character on the current column to indicate the exact breakpoint on the current line (here simulated with a caret): ``` 90 // If file_sp is no good or it points to a non-existent file, reset it. 91 if (!file_sp || !file_sp->GetFileSpec().Exists()) 92 { -> 93 file_sp.reset (new File (file_spec, target_sp.get())); ^ 94 95 if (debugger_sp) 96 debugger_sp->GetSourceFileCache().AddSourceFile(file_sp); (lldb) step ``` With this markup I may now assume that "step" will take me into the File() constructor. Just what I wanted to know. This is of course just scratching the surface of what we could do with column information, but probably a good starting point. Having a more fine-grained visualization, for example, it might be interesting to have "next" take me to the next "is_stmt" in the line table instead of always to the next line, and so on. Let me know what you think! Repository: rL LLVM http://reviews.llvm.org/D20835 Files: include/lldb/API/SBSourceManager.h include/lldb/Core/SourceManager.h scripts/interface/SBSourceManager.i scripts/interface/SBStream.i source/API/SBSourceManager.cpp source/Commands/CommandObjectSource.cpp source/Core/Disassembler.cpp source/Core/SourceManager.cpp source/Target/StackFrame.cpp Index: source/Target/StackFrame.cpp === --- source/Target/StackFrame.cpp +++ source/Target/StackFrame.cpp @@ -1579,6 +1579,7 @@ { size_t num_lines = target->GetSourceManager().DisplaySourceLinesWithLineNumbers (m_sc.line_entry.file, m_sc.line_entry.line, + m_sc.line_entry.column, source_lines_before, source_lines_after, "->", Index: source/Core/SourceManager.cpp === --- source/Core/SourceManager.cpp +++ source/Core/SourceManager.cpp @@ -22,6 +22,7 @@ #include "lldb/Symbol/Function.h" #include "lldb/Symbol/SymbolContext.h" #include "lldb/Target/Target.h" +#include "lldb/Utility/AnsiTerminal.h" using namespace lldb; using namespace lldb_private; @@ -101,6 +102,7 @@ SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile (uint32_t start_line, uint32_t count, uint32_t curr_line, + uint32_t curr_column, const char* current_line_cstr, Stream *s, const SymbolContextList *bp_locs) @@ -152,7 +154,8 @@ prefix, line == curr_line ? current_line_cstr : "", line); -size_t this_line_size = m_last_file_sp->DisplaySourceLines (line, 0, 0, s); +size_t this_line_size = +
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This looks fine. Can you add a setting in the "stop-line" vein that will turn on and off the column listing. Seems odd to control all the other stuff, but not this. Repository: rL LLVM http://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
aprantl added a comment. I was also wondering if there was a preferred way to detect whether LLDB is connected to a dumb terminal. I noticed that Stream has an eANSI flag, but it seemed to be always zero. Repository: rL LLVM http://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r271343 - Add more verification on consectutive bitfields otherwise clang will assert.
Author: gclayton Date: Tue May 31 17:29:56 2016 New Revision: 271343 URL: http://llvm.org/viewvc/llvm-project?rev=271343&view=rev Log: Add more verification on consectutive bitfields otherwise clang will assert. We need to verify that consecutive bitfields have higher offsets and don't overlap. The issues was found by running a broken version of recent clangs where the bitfield offsets were being emitted incorrectly. To guard against this we now verify and toss out any invalid bitfields and print a message that indicates to file a bug against the compiler. Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=271343&r1=271342&r2=271343&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Tue May 31 17:29:56 2016 @@ -99,9 +99,9 @@ struct BitfieldInfo uint64_t bit_size; uint64_t bit_offset; -BitfieldInfo () : -bit_size (LLDB_INVALID_ADDRESS), -bit_offset (LLDB_INVALID_ADDRESS) +BitfieldInfo() : +bit_size(LLDB_INVALID_ADDRESS), +bit_offset(LLDB_INVALID_ADDRESS) { } @@ -112,10 +112,28 @@ struct BitfieldInfo bit_offset = LLDB_INVALID_ADDRESS; } -bool IsValid () +bool +IsValid() const { return (bit_size != LLDB_INVALID_ADDRESS) && -(bit_offset != LLDB_INVALID_ADDRESS); + (bit_offset != LLDB_INVALID_ADDRESS); +} + +bool +NextBitfieldOffsetIsValid(const uint64_t next_bit_offset) const +{ +if (IsValid()) +{ +// This bitfield info is valid, so any subsequent bitfields +// must not overlap and must be at a higher bit offset than +// any previous bitfield + size. +return (bit_size + bit_offset) <= next_bit_offset; +} +else +{ +// If the this BitfieldInfo is not valid, then any offset isOK +return true; +} } }; @@ -2965,24 +2983,24 @@ DWARFASTParserClang::ParseChildMembers(c { this_field_info.bit_offset += byte_size * 8; this_field_info.bit_offset -= (bit_offset + bit_size); - -if (this_field_info.bit_offset >= parent_bit_size) -{ - objfile->GetModule()->ReportWarning("0x%8.8" PRIx64 ": %s bitfield named \"%s\" has invalid bit offset (0x%8.8" PRIx64 ") member will be ignored. Please file a bug against the compiler and include the preprocessed output for %s\n", - die.GetID(), - DW_TAG_value_to_name(tag), - name, - this_field_info.bit_offset, - sc.comp_unit ? sc.comp_unit->GetPath().c_str() : "the source file"); -this_field_info.Clear(); -continue; -} } else { this_field_info.bit_offset += bit_offset; } +if ((this_field_info.bit_offset >= parent_bit_size) || !last_field_info.NextBitfieldOffsetIsValid(this_field_info.bit_offset)) +{ + objfile->GetModule()->ReportWarning("0x%8.8" PRIx64 ": %s bitfield named \"%s\" has invalid bit offset (0x%8.8" PRIx64 ") member will be ignored. Please file a bug against the compiler and include the preprocessed output for %s\n", + die.GetID(), + DW_TAG_value_to_name(tag), + name, + this_field_info.bit_offset, + sc.comp_unit ? sc.comp_unit->GetPath().c_str() : "the source file"); +
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
aprantl added a subscriber: friss. aprantl removed rL LLVM as the repository for this revision. aprantl updated this revision to Diff 59141. aprantl added a comment. Added a use-column-info setting. http://reviews.llvm.org/D20835 Files: include/lldb/API/SBSourceManager.h include/lldb/Core/Debugger.h include/lldb/Core/SourceManager.h packages/Python/lldbsuite/test/settings/TestSettings.py scripts/interface/SBSourceManager.i scripts/interface/SBStream.i source/API/SBSourceManager.cpp source/Commands/CommandObjectSource.cpp source/Core/Debugger.cpp source/Core/Disassembler.cpp source/Core/SourceManager.cpp source/Target/StackFrame.cpp Index: source/Target/StackFrame.cpp === --- source/Target/StackFrame.cpp +++ source/Target/StackFrame.cpp @@ -1579,6 +1579,7 @@ { size_t num_lines = target->GetSourceManager().DisplaySourceLinesWithLineNumbers (m_sc.line_entry.file, m_sc.line_entry.line, + m_sc.line_entry.column, source_lines_before, source_lines_after, "->", Index: source/Core/SourceManager.cpp === --- source/Core/SourceManager.cpp +++ source/Core/SourceManager.cpp @@ -22,6 +22,7 @@ #include "lldb/Symbol/Function.h" #include "lldb/Symbol/SymbolContext.h" #include "lldb/Target/Target.h" +#include "lldb/Utility/AnsiTerminal.h" using namespace lldb; using namespace lldb_private; @@ -101,6 +102,7 @@ SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile (uint32_t start_line, uint32_t count, uint32_t curr_line, + uint32_t curr_column, const char* current_line_cstr, Stream *s, const SymbolContextList *bp_locs) @@ -152,7 +154,8 @@ prefix, line == curr_line ? current_line_cstr : "", line); -size_t this_line_size = m_last_file_sp->DisplaySourceLines (line, 0, 0, s); +size_t this_line_size = +m_last_file_sp->DisplaySourceLines(line, line == curr_line ? curr_column : 0, s); if (this_line_size == 0) { m_last_line = UINT32_MAX; @@ -170,6 +173,7 @@ ( const FileSpec &file_spec, uint32_t line, +uint32_t column, uint32_t context_before, uint32_t context_after, const char* current_line_cstr, @@ -192,7 +196,7 @@ m_last_line = 0; m_last_file_sp = file_sp; } -return DisplaySourceLinesWithLineNumbersUsingLastFile (start_line, count, line, current_line_cstr, s, bp_locs); +return DisplaySourceLinesWithLineNumbersUsingLastFile (start_line, count, line, column, current_line_cstr, s, bp_locs); } size_t @@ -240,7 +244,7 @@ else m_last_line = 1; -return DisplaySourceLinesWithLineNumbersUsingLastFile (m_last_line, m_last_count, UINT32_MAX, "", s, bp_locs); +return DisplaySourceLinesWithLineNumbersUsingLastFile (m_last_line, m_last_count, UINT32_MAX, UINT32_MAX, "", s, bp_locs); } return 0; } @@ -548,17 +552,17 @@ } size_t -SourceManager::File::DisplaySourceLines (uint32_t line, uint32_t context_before, uint32_t context_after, Stream *s) +SourceManager::File::DisplaySourceLines (uint32_t line, uint32_t column, Stream *s) { // Sanity check m_data_sp before proceeding. if (!m_data_sp) return 0; -const uint32_t start_line = line <= context_before ? 1 : line - context_before; +const uint32_t start_line = line <= 0 ? 1 : line; const uint32_t start_line_offset = GetLineOffset (start_line); if (start_line_offset != UINT32_MAX) { -const uint32_t end_line = line + context_after; +const uint32_t end_line = line; uint32_t end_line_offset = GetLineOffset (end_line + 1); if (end_line_offset == UINT32_MAX) end_line_offset = m_data_sp->GetByteSize(); @@ -569,7 +573,20 @@ { size_t count = end_line_offset - start_line_offset; const uint8_t *cstr = m_data_sp->GetBytes() + start_line_offset; -
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. I wouldn't call the setting "use-column-info". It would be better to call it "stop-print-column-info" or something that makes it clear this is just for printing. If we get better column info & start using it for stepping, etc, we might want to provide a way to turn that off too, but independently from this stop-info printing. http://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
clayborg added a comment. "display-column-info"? http://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] LLVM buildmaster will be restarted tonight
Hello everyone, LLVM buildmaster will be restarted after 6 PM Pacific time today. Thank you for understanding. Thanks Galina ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] Buildbot numbers for the last week of 5/22/2016 - 5/28/2016
Hello everyone, Below are some buildbot numbers for the last week of 5/22/2016 - 5/28/2016. Thanks Galina "Status change ratio" by active builder (percent of builds that changed the builder status from greed to red or from red to green): buildername| builds | changes | status change ratio ++-+- perf-x86_64-penryn-O3-polly| 54 | 24 |44.4 clang-cmake-armv7-a15-selfhost-neon| 23 | 8 |34.8 clang-ppc64le-linux-lnt|214 | 70 |32.7 lldb-x86_64-darwin-13.4|141 | 44 |31.2 clang-ppc64le-linux-multistage |106 | 28 |26.4 lldb-windows7-android |137 | 36 |26.3 clang-ppc64be-linux-multistage | 97 | 24 |24.7 clang-cmake-mipsel | 21 | 5 |23.8 clang-cmake-armv7-a15-selfhost | 30 | 7 |23.3 clang-cmake-thumbv7-a15-full-sh| 26 | 6 |23.1 sanitizer-x86_64-linux-bootstrap | 46 | 9 |19.6 clang-cmake-armv7-a15-full | 99 | 17 |17.2 clang-x86-win2008-selfhost | 94 | 16 |17.0 libcxx-sphinx-docs | 12 | 2 |16.7 clang-cmake-aarch64-full | 54 | 8 |14.8 llvm-mips-linux| 62 | 9 |14.5 sanitizer-ppc64le-linux| 60 | 8 |13.3 clang-native-aarch64-full | 15 | 2 |13.3 lldb-x86_64-ubuntu-14.04-android |137 | 18 |13.1 clang-atom-d525-fedora-rel | 77 | 10 |13.0 sanitizer-ppc64be-linux| 98 | 12 |12.2 clang-cmake-thumbv7-a15|180 | 21 |11.7 clang-cmake-armv7-a15 |148 | 17 |11.5 clang-ppc64be-linux-lnt|262 | 30 |11.5 sanitizer-x86_64-linux-fast|202 | 23 |11.4 clang-cmake-aarch64-quick |178 | 20 |11.2 clang-ppc64le-linux|243 | 26 |10.7 clang-cmake-aarch64-42vma |113 | 12 |10.6 llvm-clang-lld-x86_64-debian-fast |172 | 18 |10.5 clang-x86_64-debian-fast |138 | 14 |10.1 lldb-x86-windows-msvc2015 |271 | 27 |10.0 clang-ppc64be-linux|346 | 34 | 9.8 clang-cmake-mips |108 | 10 | 9.3 lldb-x86_64-ubuntu-14.04-cmake |248 | 22 | 8.9 clang-s390x-linux |378 | 32 | 8.5 clang-x86_64-linux-selfhost-modules|247 | 20 | 8.1 clang-hexagon-elf |354 | 28 | 7.9 clang-bpf-build|394 | 30 | 7.6 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast |469 | 34 | 7.2 perf-x86_64-penryn-O3-polly-fast | 30 | 2 | 6.7 llvm-hexagon-elf |274 | 18 | 6.6 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast |467 | 28 | 6.0 clang-x64-ninja-win7 |154 | 9 | 5.8 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable |275 | 16 | 5.8 sanitizer-x86_64-linux | 74 | 4 | 5.4 lldb-amd64-ninja-netbsd7 |167 | 8 | 4.8 lld-x86_64-freebsd |2
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
All the other controls for printing the stop info have "stop-*". It would be clearer to keep doing this, I think. Jim > On May 31, 2016, at 4:30 PM, Greg Clayton wrote: > > clayborg added a comment. > > "display-column-info"? > > > http://reviews.llvm.org/D20835 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
jingham added a subscriber: jingham. jingham added a comment. All the other controls for printing the stop info have "stop-*". It would be clearer to keep doing this, I think. Jim http://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
aprantl updated this revision to Diff 59154. aprantl added a comment. Rename the property to "stop-show-column" and query the stream whether ansi escape sequences are allowed. http://reviews.llvm.org/D20835 Files: include/lldb/API/SBSourceManager.h include/lldb/Core/Debugger.h include/lldb/Core/SourceManager.h packages/Python/lldbsuite/test/settings/TestSettings.py scripts/interface/SBSourceManager.i scripts/interface/SBStream.i source/API/SBSourceManager.cpp source/Commands/CommandObjectSource.cpp source/Core/Debugger.cpp source/Core/Disassembler.cpp source/Core/SourceManager.cpp source/Core/StreamAsynchronousIO.cpp source/Target/StackFrame.cpp Index: source/Target/StackFrame.cpp === --- source/Target/StackFrame.cpp +++ source/Target/StackFrame.cpp @@ -1579,6 +1579,7 @@ { size_t num_lines = target->GetSourceManager().DisplaySourceLinesWithLineNumbers (m_sc.line_entry.file, m_sc.line_entry.line, + m_sc.line_entry.column, source_lines_before, source_lines_after, "->", Index: source/Core/StreamAsynchronousIO.cpp === --- source/Core/StreamAsynchronousIO.cpp +++ source/Core/StreamAsynchronousIO.cpp @@ -15,12 +15,11 @@ using namespace lldb; using namespace lldb_private; - -StreamAsynchronousIO::StreamAsynchronousIO (Debugger &debugger, bool for_stdout) : -Stream (0, 4, eByteOrderBig), -m_debugger (debugger), -m_data (), -m_for_stdout (for_stdout) +StreamAsynchronousIO::StreamAsynchronousIO(Debugger &debugger, bool for_stdout) +: Stream(debugger.GetUseColor() ? eANSIColor : 0, 4, eByteOrderBig), + m_debugger(debugger), + m_data(), + m_for_stdout(for_stdout) { } Index: source/Core/SourceManager.cpp === --- source/Core/SourceManager.cpp +++ source/Core/SourceManager.cpp @@ -22,6 +22,7 @@ #include "lldb/Symbol/Function.h" #include "lldb/Symbol/SymbolContext.h" #include "lldb/Target/Target.h" +#include "lldb/Utility/AnsiTerminal.h" using namespace lldb; using namespace lldb_private; @@ -101,6 +102,7 @@ SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile (uint32_t start_line, uint32_t count, uint32_t curr_line, + uint32_t curr_column, const char* current_line_cstr, Stream *s, const SymbolContextList *bp_locs) @@ -152,7 +154,8 @@ prefix, line == curr_line ? current_line_cstr : "", line); -size_t this_line_size = m_last_file_sp->DisplaySourceLines (line, 0, 0, s); +size_t this_line_size = +m_last_file_sp->DisplaySourceLines(line, line == curr_line ? curr_column : 0, s); if (this_line_size == 0) { m_last_line = UINT32_MAX; @@ -170,6 +173,7 @@ ( const FileSpec &file_spec, uint32_t line, +uint32_t column, uint32_t context_before, uint32_t context_after, const char* current_line_cstr, @@ -192,7 +196,7 @@ m_last_line = 0; m_last_file_sp = file_sp; } -return DisplaySourceLinesWithLineNumbersUsingLastFile (start_line, count, line, current_line_cstr, s, bp_locs); +return DisplaySourceLinesWithLineNumbersUsingLastFile (start_line, count, line, column, current_line_cstr, s, bp_locs); } size_t @@ -240,7 +244,7 @@ else m_last_line = 1; -return DisplaySourceLinesWithLineNumbersUsingLastFile (m_last_line, m_last_count, UINT32_MAX, "", s, bp_locs); +return DisplaySourceLinesWithLineNumbersUsingLastFile (m_last_line, m_last_count, UINT32_MAX, UINT32_MAX, "", s, bp_locs); } return 0; } @@ -548,17 +552,17 @@ } size_t -SourceManager::File::DisplaySourceLines (uint32_t line, uint32_t context_before, uint32_t context_after, Stream *s) +SourceManager::File::DisplaySourceLines (uint32_t line, uint32_t column, Stream *s) { // Sanity check m_data_sp before proceeding. if (!m_data_sp)
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Looks fine to me. http://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits