[Lldb-commits] [PATCH] D50087: Add doxygen comments to the StackFrameList API (NFC)
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I am not too familiar with this code, but the descriptions seem to make sense. However, since you have kind of opened up the `Optional` discussion, I'll use this opportunity to give my take on it: I've wanted to use `Optional` in this way in the past, but the thing that has always stopped me is that this essentially doubles the amount of storage required for the value (you only need one bit for the `IsValid` field, but due to alignment constraints, this will usually consume the same amount of space as the underlying int type). This may not matter for the StackFrameList class, but it does matter for classes which have a lot of instances floating around. I suspect this is also why LLVM does not generally use this idiom (the last example where I ran into this is the VersionTuple class, which hand-rolls a packed optional by stealing a bit from the int type, and then converts this to Optional for the public accessors). For this reason, I think it would be useful to have a specialized class (`OptionalInt`?), which has the same interface as `Optional`, but does not increase the storage size. It could be implemented the same way as we hand-roll the invalid values, only now the invalid values would be a part of the type. So, you could do something like: using tid_t = OptionalInt; tid_t my_tid; // initialized to "invalid"; my_tid = get_thread_id(); if (my_tid) // no need to remember what is the correct "invalid" value here do_stuff(*my_tid); else printf("no thread"); I am sure this is more than what you wanted to hear in this patch, but I wanted to make sure consider the tradeoffs before we start putting Optionals everywhere. https://reviews.llvm.org/D50087 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50038: Introduce install-lldb-framework target
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D50038 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49963: Preliminary patch to support prompt interpolation
labath added a comment. Sounds like a nice feature to have. In addition to the other feedback you've received, I'd suggest splitting out the addition of new format entities (frame count and friends) and the core interpolation feature into separate patches. Repository: rL LLVM https://reviews.llvm.org/D49963 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks great. I only noticed some typos when looking this over again. We can continue the register shuffling discussion offline. Comment at: include/lldb/Target/Target.h:929-939 + /// @param[in] adjust_platform + /// If \b true, then the platform will be adjusted if the currently + /// selected platform is not compatible with the archicture being set. + /// If \b false, then just the architecture will be set even if the + /// currently selected platform isn't compatible (in case it might be + /// manually set following this function call). + /// mismatch in the parameter name in the comment and signature Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:497 + if (!system_info) +return error; + should `error` be initialized here? Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp:56 + k_num_regs +}; + clayborg wrote: > I would rather define a new context and avoid mutating one register context > into another. I didn't really like the other register contexts for minidumps. > I like to show the actual data that is available, not a translation of one > set of data to another. Since, you're the one doing the work, I am fine leaving this up to you, but I'd like to understand what is it that you didn't like about the x86 register contexts. In what way do they misrepresent the data available? As far as I can tell, the difference here is just in the internal representation of the data. It should not change the actual values of the registers displayed to the user (if it does, I'd like to fix it). In other words, I should be able to rewrite the implementation here to reshuffle the register order in the same way as x86, and the tests should still pass. If this is true, then this is only a question of optimizing the internal implementation for some criteria. In this case, I would choose to optimize for code size/readability, and try to avoid defining 200 lines of constants, which largely duplicate what is already given in the other register contexts. Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp:221 + m_regs.context_flags = data.GetU64(&offset); + for (unsigned i=0; i<32; ++i) +m_regs.x[i] = data.GetU64(&offset); llvm style would put spaces between the operators here. Could you run clang-format over the diff? https://reviews.llvm.org/D49750 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
labath added a comment. Thanks for the patience. I've tried the patch out on our end. You shouldn't have any problems now. Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21 +@add_test_categories(["libc++"]) +@skipIf(oslist=no_match(["macosx"]), compiler="clang", compiler_version=['<', '5.0']) + shafik wrote: > labath wrote: > > Could you add another line for `gcc` here? The -std=c++17 flag seems to be > > supported starting with gcc-5.1. > > > > Also a comment that this is due to the -std flag would be helpful to people > > looking at this in the future. > Adding a comment makes sense. > > So `@add_test_categories(["libc++"])` won't skip for gcc bots then? It won't because it doesn't make sense to do that. Gcc is perfectly capable of using libc++. The only tricky thing is that it doesn't have the magic `-stdlib` argument, so you have to specify the include paths and libraries explicitly (which our makefiles do). Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp:17-20 +bool has_optional = false ; +#if HAVE_OPTIONAL == 1 +has_optional = true ; +#endif This could be simplified to `bool has_optional = HAVE_OPTIONAL;` https://reviews.llvm.org/D49271 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50027: Added initial unit test for LLDB's Stream class.
labath added inline comments. Comment at: unittests/Utility/StreamTest.cpp:38-41 +TEST_F(StreamTest, ChangingByteOrder) { + s.SetByteOrder(lldb::eByteOrderPDP); + EXPECT_EQ(lldb::eByteOrderPDP, s.GetByteOrder()); +} probinson wrote: > teemperor wrote: > > labath wrote: > > > I've been wondering for a while whether we shouldn't just remove > > > PDP byte order support. Most of our code doesn't really support it, and > > > neither does llvm's, so this is kind of a prerequisite for switching to > > > llvm streams. > > I support this notion. think most of LLDB's algorithms do not respect PDP > > ordering. > PDP ordering? As in, PDP-11? I could imagine GDB had to support it many > long ages ago, but AFAIK no such machine has been produced this century. This millenium, even. :P Ok, I'll see if I can dismantle it. Repository: rL LLVM https://reviews.llvm.org/D50027 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50087: Add doxygen comments to the StackFrameList API (NFC)
vsk added a comment. In https://reviews.llvm.org/D50087#1183982, @labath wrote: > I am not too familiar with this code, but the descriptions seem to make sense. > > However, since you have kind of opened up the `Optional` discussion, I'll use > this opportunity to give my take on it: > > I've wanted to use `Optional` in this way in the past, but the thing > that has always stopped me is that this essentially doubles the amount of > storage required for the value (you only need one bit for the `IsValid` > field, but due to alignment constraints, this will usually consume the same > amount of space as the underlying int type). This may not matter for the > StackFrameList class, but it does matter for classes which have a lot of > instances floating around. I suspect this is also why LLVM does not generally > use this idiom (the last example where I ran into this is the VersionTuple > class, which hand-rolls a packed optional by stealing a bit from the int > type, and then converts this to Optional for the public accessors). > > For this reason, I think it would be useful to have a specialized class > (`OptionalInt`?), which has the same interface as `Optional`, but does not > increase the storage size. It could be implemented the same way as we > hand-roll the invalid values, only now the invalid values would be a part of > the type. So, you could do something like: > > using tid_t = OptionalInt; > > tid_t my_tid; // initialized to "invalid"; > my_tid = get_thread_id(); > if (my_tid) // no need to remember what is the correct "invalid" value here > do_stuff(*my_tid); > else > printf("no thread"); > > > I am sure this is more than what you wanted to hear in this patch, but I > wanted to make sure consider the tradeoffs before we start putting Optionals > everywhere. No, I think that's a good point. I held off on introducing llvm::Optional here because of similar reservations. I think it'd make sense to introduce an OptionalInt facility, or possibly something more general that supports non-POD types & more exotic invalid values. Definitely something to revisit. https://reviews.llvm.org/D50087 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50038: Introduce install-lldb-framework target
clayborg added a comment. Might be nice to put a blurb in the build page about this in the MacOS section? https://reviews.llvm.org/D50038 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338588 - [StackFrame] Factor GetOnlyConcreteFramesUpTo out of GetFramesUpTo (NFC)
Author: vedantk Date: Wed Aug 1 10:07:40 2018 New Revision: 338588 URL: http://llvm.org/viewvc/llvm-project?rev=338588&view=rev Log: [StackFrame] Factor GetOnlyConcreteFramesUpTo out of GetFramesUpTo (NFC) Splitting GetOnlyConcreteFramesUpTo will make it easier to implement support for synthetic tail call frames in backtraces. This is just a prep change, no functionality is affected. Modified: lldb/trunk/include/lldb/Target/StackFrameList.h lldb/trunk/source/Target/StackFrameList.cpp Modified: lldb/trunk/include/lldb/Target/StackFrameList.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/StackFrameList.h?rev=338588&r1=338587&r2=338588&view=diff == --- lldb/trunk/include/lldb/Target/StackFrameList.h (original) +++ lldb/trunk/include/lldb/Target/StackFrameList.h Wed Aug 1 10:07:40 2018 @@ -83,6 +83,8 @@ protected: void GetFramesUpTo(uint32_t end_idx); + void GetOnlyConcreteFramesUpTo(uint32_t end_idx, Unwind *unwinder); + bool GetAllFramesFetched() { return m_concrete_frames_fetched == UINT32_MAX; } void SetAllFramesFetched() { m_concrete_frames_fetched = UINT32_MAX; } Modified: lldb/trunk/source/Target/StackFrameList.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrameList.cpp?rev=338588&r1=338587&r2=338588&view=diff == --- lldb/trunk/source/Target/StackFrameList.cpp (original) +++ lldb/trunk/source/Target/StackFrameList.cpp Wed Aug 1 10:07:40 2018 @@ -226,8 +226,27 @@ void StackFrameList::SetCurrentInlinedDe m_current_inlined_pc = m_thread.GetRegisterContext()->GetPC(); } +void StackFrameList::GetOnlyConcreteFramesUpTo(uint32_t end_idx, + Unwind *unwinder) { + assert(m_thread.IsValid() && "Expected valid thread"); + assert(m_frames.size() <= end_idx && "Expected there to be frames to fill"); + + if (end_idx < m_concrete_frames_fetched) +return; + + if (!unwinder) +return; + + uint32_t num_frames = unwinder->GetFramesUpTo(end_idx); + if (num_frames <= end_idx + 1) { +// Done unwinding. +m_concrete_frames_fetched = UINT32_MAX; + } + m_frames.resize(num_frames); +} + void StackFrameList::GetFramesUpTo(uint32_t end_idx) { - // this makes sure we do not fetch frames for an invalid thread + // Do not fetch frames for an invalid thread. if (!m_thread.IsValid()) return; @@ -238,201 +257,185 @@ void StackFrameList::GetFramesUpTo(uint3 Unwind *unwinder = m_thread.GetUnwinder(); - if (m_show_inlined_frames) { + if (!m_show_inlined_frames) { +GetOnlyConcreteFramesUpTo(end_idx, unwinder); +return; + } + #if defined(DEBUG_STACK_FRAMES) -StreamFile s(stdout, false); + StreamFile s(stdout, false); #endif -// If we are hiding some frames from the outside world, we need to add -// those onto the total count of frames to fetch. However, we don't need -// to do that if end_idx is 0 since in that case we always get the first -// concrete frame and all the inlined frames below it... And of course, if -// end_idx is UINT32_MAX that means get all, so just do that... - -uint32_t inlined_depth = 0; -if (end_idx > 0 && end_idx != UINT32_MAX) { - inlined_depth = GetCurrentInlinedDepth(); - if (inlined_depth != UINT32_MAX) { -if (end_idx > 0) - end_idx += inlined_depth; - } + // If we are hiding some frames from the outside world, we need to add + // those onto the total count of frames to fetch. However, we don't need + // to do that if end_idx is 0 since in that case we always get the first + // concrete frame and all the inlined frames below it... And of course, if + // end_idx is UINT32_MAX that means get all, so just do that... + + uint32_t inlined_depth = 0; + if (end_idx > 0 && end_idx != UINT32_MAX) { +inlined_depth = GetCurrentInlinedDepth(); +if (inlined_depth != UINT32_MAX) { + if (end_idx > 0) +end_idx += inlined_depth; } + } -StackFrameSP unwind_frame_sp; -do { - uint32_t idx = m_concrete_frames_fetched++; - lldb::addr_t pc = LLDB_INVALID_ADDRESS; - lldb::addr_t cfa = LLDB_INVALID_ADDRESS; - if (idx == 0) { -// We might have already created frame zero, only create it if we need -// to -if (m_frames.empty()) { - RegisterContextSP reg_ctx_sp(m_thread.GetRegisterContext()); - - if (reg_ctx_sp) { -const bool success = -unwinder && unwinder->GetFrameInfoAtIndex(idx, cfa, pc); -// There shouldn't be any way not to get the frame info for frame -// 0. But if the unwinder can't make one, lets make one by hand -// with the -// SP as the CFA and see if that gets any further. -if (!success) { - cfa = reg_ctx_sp->GetSP()
[Lldb-commits] [PATCH] D50087: Add doxygen comments to the StackFrameList API (NFC)
This revision was automatically updated to reflect the committed changes. Closed by commit rL338590: [StackFrame] Add doxygen comments to the StackFrameList API (NFC) (authored by vedantk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50087?vs=158327&id=158562#toc Repository: rL LLVM https://reviews.llvm.org/D50087 Files: lldb/trunk/include/lldb/Target/StackFrameList.h lldb/trunk/source/Target/StackFrameList.cpp Index: lldb/trunk/source/Target/StackFrameList.cpp === --- lldb/trunk/source/Target/StackFrameList.cpp +++ lldb/trunk/source/Target/StackFrameList.cpp @@ -436,11 +436,7 @@ if (can_create) GetFramesUpTo(UINT32_MAX); - uint32_t inlined_depth = GetCurrentInlinedDepth(); - if (inlined_depth == UINT32_MAX) -return m_frames.size(); - else -return m_frames.size() - inlined_depth; + return GetVisibleStackFrameIndex(m_frames.size()); } void StackFrameList::Dump(Stream *s) { @@ -620,7 +616,6 @@ return m_selected_frame_idx; } -// Mark a stack frame as the current frame using the frame index bool StackFrameList::SetSelectedFrameByIndex(uint32_t idx) { std::lock_guard guard(m_mutex); StackFrameSP frame_sp(GetFrameAtIndex(idx)); @@ -652,19 +647,6 @@ m_concrete_frames_fetched = 0; } -void StackFrameList::InvalidateFrames(uint32_t start_idx) { - std::lock_guard guard(m_mutex); - if (m_show_inlined_frames) { -Clear(); - } else { -const size_t num_frames = m_frames.size(); -while (start_idx < num_frames) { - m_frames[start_idx].reset(); - ++start_idx; -} - } -} - void StackFrameList::Merge(std::unique_ptr &curr_ap, lldb::StackFrameListSP &prev_sp) { std::unique_lock current_lock, previous_lock; Index: lldb/trunk/include/lldb/Target/StackFrameList.h === --- lldb/trunk/include/lldb/Target/StackFrameList.h +++ lldb/trunk/include/lldb/Target/StackFrameList.h @@ -10,14 +10,10 @@ #ifndef liblldb_StackFrameList_h_ #define liblldb_StackFrameList_h_ -// C Includes -// C++ Includes #include #include #include -// Other libraries and framework includes -// Project includes #include "lldb/Target/StackFrame.h" namespace lldb_private { @@ -32,39 +28,57 @@ ~StackFrameList(); + /// Get the number of visible frames. Frames may be created if \p can_create + /// is true. Synthetic (inline) frames expanded from the concrete frame #0 + /// (aka invisible frames) are not included in this count. uint32_t GetNumFrames(bool can_create = true); + /// Get the frame at index \p idx. Invisible frames cannot be indexed. lldb::StackFrameSP GetFrameAtIndex(uint32_t idx); + /// Get the first concrete frame with index greater than or equal to \p idx. + /// Unlike \ref GetFrameAtIndex, this cannot return a synthetic frame. lldb::StackFrameSP GetFrameWithConcreteFrameIndex(uint32_t unwind_idx); + /// Retrieve the stack frame with the given ID \p stack_id. lldb::StackFrameSP GetFrameWithStackID(const StackID &stack_id); - // Mark a stack frame as the current frame + /// Mark a stack frame as the currently selected frame and return its index. uint32_t SetSelectedFrame(lldb_private::StackFrame *frame); + /// Get the currently selected frame index. uint32_t GetSelectedFrameIndex() const; - // Mark a stack frame as the current frame using the frame index + /// Mark a stack frame as the currently selected frame using the frame index + /// \p idx. Like \ref GetFrameAtIndex, invisible frames cannot be selected. bool SetSelectedFrameByIndex(uint32_t idx); + /// If the current inline depth (i.e the number of invisible frames) is valid, + /// subtract it from \p idx. Otherwise simply return \p idx. uint32_t GetVisibleStackFrameIndex(uint32_t idx) { if (m_current_inlined_depth < UINT32_MAX) return idx - m_current_inlined_depth; else return idx; } + /// Calculate and set the current inline depth. This may be used to update + /// the StackFrameList's set of inline frames when execution stops, e.g when + /// a breakpoint is hit. void CalculateCurrentInlinedDepth(); + /// If the currently selected frame comes from the currently selected thread, + /// point the default file and line of the thread's target to the location + /// specified by the frame. void SetDefaultFileAndLineToSelectedFrame(); + /// Clear the cache of frames. void Clear(); - void InvalidateFrames(uint32_t start_idx); - void Dump(Stream *s); + /// If \p stack_frame_ptr is contained in this StackFrameList, return its + /// wrapping shared pointer. lldb::StackFrameSP GetStackFrameSPForStackFramePtr(StackFrame *stack_frame_ptr); @@ -101,14 +115,44 @@ typedef collection::iterator iterator; typedef collection::const_iterator const_iterator; + /// The th
[Lldb-commits] [lldb] r338590 - [StackFrame] Add doxygen comments to the StackFrameList API (NFC)
Author: vedantk Date: Wed Aug 1 10:08:11 2018 New Revision: 338590 URL: http://llvm.org/viewvc/llvm-project?rev=338590&view=rev Log: [StackFrame] Add doxygen comments to the StackFrameList API (NFC) Clarify how StackFrameList works by documenting its methods. Also, delete some dead code and insert some TODOs. Differential Revision: https://reviews.llvm.org/D50087 Modified: lldb/trunk/include/lldb/Target/StackFrameList.h lldb/trunk/source/Target/StackFrameList.cpp Modified: lldb/trunk/include/lldb/Target/StackFrameList.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/StackFrameList.h?rev=338590&r1=338589&r2=338590&view=diff == --- lldb/trunk/include/lldb/Target/StackFrameList.h (original) +++ lldb/trunk/include/lldb/Target/StackFrameList.h Wed Aug 1 10:08:11 2018 @@ -10,14 +10,10 @@ #ifndef liblldb_StackFrameList_h_ #define liblldb_StackFrameList_h_ -// C Includes -// C++ Includes #include #include #include -// Other libraries and framework includes -// Project includes #include "lldb/Target/StackFrame.h" namespace lldb_private { @@ -32,22 +28,33 @@ public: ~StackFrameList(); + /// Get the number of visible frames. Frames may be created if \p can_create + /// is true. Synthetic (inline) frames expanded from the concrete frame #0 + /// (aka invisible frames) are not included in this count. uint32_t GetNumFrames(bool can_create = true); + /// Get the frame at index \p idx. Invisible frames cannot be indexed. lldb::StackFrameSP GetFrameAtIndex(uint32_t idx); + /// Get the first concrete frame with index greater than or equal to \p idx. + /// Unlike \ref GetFrameAtIndex, this cannot return a synthetic frame. lldb::StackFrameSP GetFrameWithConcreteFrameIndex(uint32_t unwind_idx); + /// Retrieve the stack frame with the given ID \p stack_id. lldb::StackFrameSP GetFrameWithStackID(const StackID &stack_id); - // Mark a stack frame as the current frame + /// Mark a stack frame as the currently selected frame and return its index. uint32_t SetSelectedFrame(lldb_private::StackFrame *frame); + /// Get the currently selected frame index. uint32_t GetSelectedFrameIndex() const; - // Mark a stack frame as the current frame using the frame index + /// Mark a stack frame as the currently selected frame using the frame index + /// \p idx. Like \ref GetFrameAtIndex, invisible frames cannot be selected. bool SetSelectedFrameByIndex(uint32_t idx); + /// If the current inline depth (i.e the number of invisible frames) is valid, + /// subtract it from \p idx. Otherwise simply return \p idx. uint32_t GetVisibleStackFrameIndex(uint32_t idx) { if (m_current_inlined_depth < UINT32_MAX) return idx - m_current_inlined_depth; @@ -55,16 +62,23 @@ public: return idx; } + /// Calculate and set the current inline depth. This may be used to update + /// the StackFrameList's set of inline frames when execution stops, e.g when + /// a breakpoint is hit. void CalculateCurrentInlinedDepth(); + /// If the currently selected frame comes from the currently selected thread, + /// point the default file and line of the thread's target to the location + /// specified by the frame. void SetDefaultFileAndLineToSelectedFrame(); + /// Clear the cache of frames. void Clear(); - void InvalidateFrames(uint32_t start_idx); - void Dump(Stream *s); + /// If \p stack_frame_ptr is contained in this StackFrameList, return its + /// wrapping shared pointer. lldb::StackFrameSP GetStackFrameSPForStackFramePtr(StackFrame *stack_frame_ptr); @@ -101,14 +115,44 @@ protected: typedef collection::iterator iterator; typedef collection::const_iterator const_iterator; + /// The thread this frame list describes. Thread &m_thread; + + /// The old stack frame list. + // TODO: The old stack frame list is used to fill in missing frame info + // heuristically when it's otherwise unavailable (say, because the unwinder + // fails). We should have stronger checks to make sure that this is a valid + // source of information. lldb::StackFrameListSP m_prev_frames_sp; + + /// A mutex for this frame list. + // TODO: This mutex may not always be held when required. In particular, uses + // of the StackFrameList APIs in lldb_private::Thread look suspect. Consider + // passing around a lock_guard reference to enforce proper locking. mutable std::recursive_mutex m_mutex; + + /// A cache of frames. This may need to be updated when the program counter + /// changes. collection m_frames; + + /// The currently selected frame. uint32_t m_selected_frame_idx; + + /// The number of concrete frames fetched while filling the frame list. This + /// is only used when synthetic frames are enabled. uint32_t m_concrete_frames_fetched; + + /// The number of synthetic function activations (invisible frames) expanded +
[Lldb-commits] [lldb] r338589 - [StackFrame] Use early returns in ResetCurrentInlinedDepth (NFC)
Author: vedantk Date: Wed Aug 1 10:07:56 2018 New Revision: 338589 URL: http://llvm.org/viewvc/llvm-project?rev=338589&view=rev Log: [StackFrame] Use early returns in ResetCurrentInlinedDepth (NFC) Using early returns in this function substantially reduces the nesting level, making the logic easier to understand. Modified: lldb/trunk/include/lldb/Target/StackFrameList.h lldb/trunk/source/Target/StackFrameList.cpp Modified: lldb/trunk/include/lldb/Target/StackFrameList.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/StackFrameList.h?rev=338589&r1=338588&r2=338589&view=diff == --- lldb/trunk/include/lldb/Target/StackFrameList.h (original) +++ lldb/trunk/include/lldb/Target/StackFrameList.h Wed Aug 1 10:07:56 2018 @@ -109,7 +109,7 @@ protected: uint32_t m_concrete_frames_fetched; uint32_t m_current_inlined_depth; lldb::addr_t m_current_inlined_pc; - bool m_show_inlined_frames; + const bool m_show_inlined_frames; private: DISALLOW_COPY_AND_ASSIGN(StackFrameList); Modified: lldb/trunk/source/Target/StackFrameList.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrameList.cpp?rev=338589&r1=338588&r2=338589&view=diff == --- lldb/trunk/source/Target/StackFrameList.cpp (original) +++ lldb/trunk/source/Target/StackFrameList.cpp Wed Aug 1 10:07:56 2018 @@ -81,127 +81,119 @@ uint32_t StackFrameList::GetCurrentInlin } void StackFrameList::ResetCurrentInlinedDepth() { + if (!m_show_inlined_frames) +return; + std::lock_guard guard(m_mutex); - if (m_show_inlined_frames) { -GetFramesUpTo(0); -if (m_frames.empty()) - return; -if (!m_frames[0]->IsInlined()) { - m_current_inlined_depth = UINT32_MAX; - m_current_inlined_pc = LLDB_INVALID_ADDRESS; - Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); - if (log && log->GetVerbose()) -log->Printf( -"ResetCurrentInlinedDepth: Invalidating current inlined depth.\n"); -} else { - // We only need to do something special about inlined blocks when we are - // at the beginning of an inlined function: - // FIXME: We probably also have to do something special if the PC is at - // the END - // of an inlined function, which coincides with the end of either its - // containing function or another inlined function. - - lldb::addr_t curr_pc = m_thread.GetRegisterContext()->GetPC(); - Block *block_ptr = m_frames[0]->GetFrameBlock(); - if (block_ptr) { -Address pc_as_address; -pc_as_address.SetLoadAddress(curr_pc, - &(m_thread.GetProcess()->GetTarget())); -AddressRange containing_range; -if (block_ptr->GetRangeContainingAddress(pc_as_address, - containing_range)) { - if (pc_as_address == containing_range.GetBaseAddress()) { -// If we got here because of a breakpoint hit, then set the inlined -// depth depending on where the breakpoint was set. If we got here -// because of a crash, then set the inlined depth to the deepest -// most block. Otherwise, we stopped here naturally as the result -// of a step, so set ourselves in the containing frame of the whole -// set of nested inlines, so the user can then "virtually" step -// into the frames one by one, or next over the whole mess. Note: -// We don't have to handle being somewhere in the middle of the -// stack here, since ResetCurrentInlinedDepth doesn't get called if -// there is a valid inlined depth set. -StopInfoSP stop_info_sp = m_thread.GetStopInfo(); -if (stop_info_sp) { - switch (stop_info_sp->GetStopReason()) { - case eStopReasonWatchpoint: - case eStopReasonException: - case eStopReasonExec: - case eStopReasonSignal: -// In all these cases we want to stop in the deepest most -// frame. -m_current_inlined_pc = curr_pc; -m_current_inlined_depth = 0; -break; - case eStopReasonBreakpoint: { -// FIXME: Figure out what this break point is doing, and set the -// inline depth -// appropriately. Be careful to take into account breakpoints -// that implement step over prologue, since that should do the -// default calculation. For now, if the breakpoints -// corresponding to this hit are all internal, -// I set the stop location to the top of the inlined stack, -// since that will make -// things like stepping over
[Lldb-commits] [PATCH] D50025: Don't ignore byte_order in Stream::PutMaxHex64
teemperor updated this revision to Diff 158563. teemperor added a comment. - Updated patch to reflect code changes in the parent commit. https://reviews.llvm.org/D50025 Files: source/Utility/Stream.cpp unittests/Utility/StreamTest.cpp Index: unittests/Utility/StreamTest.cpp === --- unittests/Utility/StreamTest.cpp +++ unittests/Utility/StreamTest.cpp @@ -187,6 +187,32 @@ EXPECT_EQ("", TakeValue()); } +TEST_F(StreamTest, PutMaxHex64ByteOrderBig) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderBig); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderBig); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderBig); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderBig); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("121234123456781234567890abcdef", TakeValue()); +} + +TEST_F(StreamTest, PutMaxHex64ByteOrderLittle) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderLittle); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderLittle); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderLittle); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderLittle); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("12341278563412efcdab9078563412", TakeValue()); +} + //-- // Shift operator tests. //-- Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -427,11 +427,11 @@ case 1: return PutHex8((uint8_t)uvalue); case 2: -return PutHex16((uint16_t)uvalue); +return PutHex16((uint16_t)uvalue, byte_order); case 4: -return PutHex32((uint32_t)uvalue); +return PutHex32((uint32_t)uvalue, byte_order); case 8: -return PutHex64(uvalue); +return PutHex64(uvalue, byte_order); } return 0; } Index: unittests/Utility/StreamTest.cpp === --- unittests/Utility/StreamTest.cpp +++ unittests/Utility/StreamTest.cpp @@ -187,6 +187,32 @@ EXPECT_EQ("", TakeValue()); } +TEST_F(StreamTest, PutMaxHex64ByteOrderBig) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderBig); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderBig); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderBig); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderBig); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("121234123456781234567890abcdef", TakeValue()); +} + +TEST_F(StreamTest, PutMaxHex64ByteOrderLittle) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderLittle); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderLittle); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderLittle); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderLittle); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("12341278563412efcdab9078563412", TakeValue()); +} + //-- // Shift operator tests. //-- Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -427,11 +427,11 @@ case 1: return PutHex8((uint8_t)uvalue); case 2: -return PutHex16((uint16_t)uvalue); +return PutHex16((uint16_t)uvalue, byte_order); case 4: -return PutHex32((uint32_t)uvalue); +return PutHex32((uint32_t)uvalue, byte_order); case 8: -return PutHex64(uvalue); +return PutHex64(uvalue, byte_order); } return 0; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338591 - Don't ignore byte_order in Stream::PutMaxHex64
Author: teemperor Date: Wed Aug 1 10:12:58 2018 New Revision: 338591 URL: http://llvm.org/viewvc/llvm-project?rev=338591&view=rev Log: Don't ignore byte_order in Stream::PutMaxHex64 Reviewers: labath Reviewed By: labath Subscribers: zturner, lldb-commits Differential Revision: https://reviews.llvm.org/D50025 Modified: lldb/trunk/source/Utility/Stream.cpp lldb/trunk/unittests/Utility/StreamTest.cpp Modified: lldb/trunk/source/Utility/Stream.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Stream.cpp?rev=338591&r1=338590&r2=338591&view=diff == --- lldb/trunk/source/Utility/Stream.cpp (original) +++ lldb/trunk/source/Utility/Stream.cpp Wed Aug 1 10:12:58 2018 @@ -427,11 +427,11 @@ size_t Stream::PutMaxHex64(uint64_t uval case 1: return PutHex8((uint8_t)uvalue); case 2: -return PutHex16((uint16_t)uvalue); +return PutHex16((uint16_t)uvalue, byte_order); case 4: -return PutHex32((uint32_t)uvalue); +return PutHex32((uint32_t)uvalue, byte_order); case 8: -return PutHex64(uvalue); +return PutHex64(uvalue, byte_order); } return 0; } Modified: lldb/trunk/unittests/Utility/StreamTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/StreamTest.cpp?rev=338591&r1=338590&r2=338591&view=diff == --- lldb/trunk/unittests/Utility/StreamTest.cpp (original) +++ lldb/trunk/unittests/Utility/StreamTest.cpp Wed Aug 1 10:12:58 2018 @@ -187,6 +187,32 @@ TEST_F(StreamTest, PutHex64ByteOrderBig) EXPECT_EQ("", TakeValue()); } +TEST_F(StreamTest, PutMaxHex64ByteOrderBig) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderBig); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderBig); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderBig); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderBig); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("121234123456781234567890abcdef", TakeValue()); +} + +TEST_F(StreamTest, PutMaxHex64ByteOrderLittle) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderLittle); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderLittle); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderLittle); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderLittle); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("12341278563412efcdab9078563412", TakeValue()); +} + //-- // Shift operator tests. //-- ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50025: Don't ignore byte_order in Stream::PutMaxHex64
This revision was automatically updated to reflect the committed changes. Closed by commit rL338591: Don't ignore byte_order in Stream::PutMaxHex64 (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50025?vs=158563&id=158564#toc Repository: rL LLVM https://reviews.llvm.org/D50025 Files: lldb/trunk/source/Utility/Stream.cpp lldb/trunk/unittests/Utility/StreamTest.cpp Index: lldb/trunk/unittests/Utility/StreamTest.cpp === --- lldb/trunk/unittests/Utility/StreamTest.cpp +++ lldb/trunk/unittests/Utility/StreamTest.cpp @@ -187,6 +187,32 @@ EXPECT_EQ("", TakeValue()); } +TEST_F(StreamTest, PutMaxHex64ByteOrderBig) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderBig); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderBig); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderBig); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderBig); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("121234123456781234567890abcdef", TakeValue()); +} + +TEST_F(StreamTest, PutMaxHex64ByteOrderLittle) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderLittle); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderLittle); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderLittle); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderLittle); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("12341278563412efcdab9078563412", TakeValue()); +} + //-- // Shift operator tests. //-- Index: lldb/trunk/source/Utility/Stream.cpp === --- lldb/trunk/source/Utility/Stream.cpp +++ lldb/trunk/source/Utility/Stream.cpp @@ -427,11 +427,11 @@ case 1: return PutHex8((uint8_t)uvalue); case 2: -return PutHex16((uint16_t)uvalue); +return PutHex16((uint16_t)uvalue, byte_order); case 4: -return PutHex32((uint32_t)uvalue); +return PutHex32((uint32_t)uvalue, byte_order); case 8: -return PutHex64(uvalue); +return PutHex64(uvalue, byte_order); } return 0; } Index: lldb/trunk/unittests/Utility/StreamTest.cpp === --- lldb/trunk/unittests/Utility/StreamTest.cpp +++ lldb/trunk/unittests/Utility/StreamTest.cpp @@ -187,6 +187,32 @@ EXPECT_EQ("", TakeValue()); } +TEST_F(StreamTest, PutMaxHex64ByteOrderBig) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderBig); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderBig); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderBig); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderBig); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("121234123456781234567890abcdef", TakeValue()); +} + +TEST_F(StreamTest, PutMaxHex64ByteOrderLittle) { + std::size_t bytes; + bytes = s.PutMaxHex64(0x12U, 1, lldb::eByteOrderLittle); + EXPECT_EQ(2U, bytes); + bytes = s.PutMaxHex64(0x1234U, 2, lldb::eByteOrderLittle); + EXPECT_EQ(4U, bytes); + bytes = s.PutMaxHex64(0x12345678U, 4, lldb::eByteOrderLittle); + EXPECT_EQ(8U, bytes); + bytes = s.PutMaxHex64(0x1234567890ABCDEFU, 8, lldb::eByteOrderLittle); + EXPECT_EQ(16U, bytes); + EXPECT_EQ("12341278563412efcdab9078563412", TakeValue()); +} + //-- // Shift operator tests. //-- Index: lldb/trunk/source/Utility/Stream.cpp === --- lldb/trunk/source/Utility/Stream.cpp +++ lldb/trunk/source/Utility/Stream.cpp @@ -427,11 +427,11 @@ case 1: return PutHex8((uint8_t)uvalue); case 2: -return PutHex16((uint16_t)uvalue); +return PutHex16((uint16_t)uvalue, byte_order); case 4: -return PutHex32((uint32_t)uvalue); +return PutHex32((uint32_t)uvalue, byte_order); case 8: -return PutHex64(uvalue); +return PutHex64(uvalue, byte_order); } return 0; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50038: Introduce install-lldb-framework target
This revision was automatically updated to reflect the committed changes. Closed by commit rL338594: Introduce install-lldb-framework target (authored by xiaobai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D50038 Files: lldb/trunk/CMakeLists.txt lldb/trunk/cmake/modules/AddLLDB.cmake lldb/trunk/cmake/modules/LLDBFramework.cmake lldb/trunk/source/API/CMakeLists.txt Index: lldb/trunk/CMakeLists.txt === --- lldb/trunk/CMakeLists.txt +++ lldb/trunk/CMakeLists.txt @@ -51,6 +51,7 @@ message(FATAL_ERROR "LLDB.framework can only be generated when targeting Apple platforms") endif() + add_custom_target(lldb-framework) # These are used to fill out LLDB-Info.plist. These are relevant when building # the framework, and must be defined before building liblldb. set(PRODUCT_NAME "LLDB") @@ -60,6 +61,7 @@ set(LLDB_FRAMEWORK_DIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}) + include(LLDBFramework) endif() add_subdirectory(docs) @@ -162,10 +164,6 @@ add_subdirectory(utils/lldb-dotest) endif() -if (LLDB_BUILD_FRAMEWORK) - add_custom_target(lldb-framework) - include(LLDBFramework) -endif() if (NOT LLDB_DISABLE_PYTHON) # Add a Post-Build Event to copy over Python files and create the symlink Index: lldb/trunk/source/API/CMakeLists.txt === --- lldb/trunk/source/API/CMakeLists.txt +++ lldb/trunk/source/API/CMakeLists.txt @@ -143,6 +143,17 @@ ) endif() +if (LLDB_BUILD_FRAMEWORK) + set_target_properties(liblldb +PROPERTIES +OUTPUT_NAME LLDB +FRAMEWORK On +FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION} +MACOSX_FRAMEWORK_INFO_PLIST ${LLDB_SOURCE_DIR}/resources/LLDB-Info.plist +LIBRARY_OUTPUT_DIRECTORY ${LLDB_FRAMEWORK_DIR} + ) +endif() + if (LLDB_WRAP_PYTHON) add_dependencies(liblldb swig_wrapper) endif() Index: lldb/trunk/cmake/modules/LLDBFramework.cmake === --- lldb/trunk/cmake/modules/LLDBFramework.cmake +++ lldb/trunk/cmake/modules/LLDBFramework.cmake @@ -31,14 +31,9 @@ ) endif() -set_target_properties(liblldb PROPERTIES - OUTPUT_NAME LLDB - FRAMEWORK On - FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION} - MACOSX_FRAMEWORK_INFO_PLIST ${LLDB_SOURCE_DIR}/resources/LLDB-Info.plist - LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR} - PUBLIC_HEADER "${framework_headers}") - add_dependencies(lldb-framework lldb-framework-headers lldb-suite) + +add_custom_target(install-lldb-framework) +add_custom_target(install-lldb-framework-stripped) Index: lldb/trunk/cmake/modules/AddLLDB.cmake === --- lldb/trunk/cmake/modules/AddLLDB.cmake +++ lldb/trunk/cmake/modules/AddLLDB.cmake @@ -53,6 +53,11 @@ set(out_dir lib${LLVM_LIBDIR_SUFFIX}) if(${name} STREQUAL "liblldb" AND LLDB_BUILD_FRAMEWORK) set(out_dir ${LLDB_FRAMEWORK_INSTALL_DIR}) + # The framework that is generated will install with install-liblldb + # because we enable CMake's framework support. CMake will copy all the + # headers and resources for us. + add_dependencies(install-lldb-framework install-${name}) + add_dependencies(install-lldb-framework-stripped install-${name}-stripped) endif() install(TARGETS ${name} COMPONENT ${name} @@ -67,12 +72,20 @@ endif() if (NOT CMAKE_CONFIGURATION_TYPES) add_llvm_install_targets(install-${name} - DEPENDS ${name} + DEPENDS $ COMPONENT ${name}) + +# install-liblldb{,-stripped} is the actual target that will install the +# framework, so it must rely on the framework being fully built first. +if (LLDB_BUILD_FRAMEWORK AND ${name} STREQUAL "liblldb") + add_dependencies(install-${name} lldb-framework) + add_dependencies(install-lldb-framework-stripped lldb-framework) +endif() endif() endif() endif() + # Hack: only some LLDB libraries depend on the clang autogenerated headers, # but it is simple enough to make all of LLDB depend on some of those # headers without negatively impacting much of anything. @@ -124,6 +137,10 @@ set(out_dir "bin") if (LLDB_BUILD_FRAMEWORK AND ARG_INCLUDE_IN_SUITE) set(out_dir ${LLDB_FRAMEWORK_INSTALL_DIR}/${LLDB_FRAMEWORK_RESOURCE_DIR}) + # While install-liblldb-stripped will handle copying the tools, it will + # not strip them. We depend on this target to guarantee a stripped version + # will get installed in the framework. + add_dependencies(install-lldb-framework-stripped install-${name}-stripped) endif() install(T
[Lldb-commits] [PATCH] D50038: Introduce install-lldb-framework target
xiaobai added a comment. In https://reviews.llvm.org/D50038#1184445, @clayborg wrote: > Might be nice to put a blurb in the build page about this in the MacOS > section? Yep, I think that wouldn't be a bad idea. I can handle that in a separate commit. Might be nice for one of the buildbots to use this method of building as well. :P https://reviews.llvm.org/D50038 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338594 - Introduce install-lldb-framework target
Author: xiaobai Date: Wed Aug 1 10:21:18 2018 New Revision: 338594 URL: http://llvm.org/viewvc/llvm-project?rev=338594&view=rev Log: Introduce install-lldb-framework target Summary: Previously, I thought that install-liblldb would fail because CMake had a bug related to installing frameworks. In actuality, I misunderstood the semantics of `add_custom_target`: the DEPENDS option refers to specific files, not targets. Therefore `install-liblldb` should rely on the actual liblldb getting generated rather than the target. This means that the previous patch I committed (to stop relying on CMake's framework support) is no longer needed and has been reverted. Using CMake's framework support greatly simplifies the implementation. `install-lldb-framework` (and the stripped variant) is as simple as depending on `install-liblldb` because CMake knows that liblldb was built as a framework and will install the whole framework for you. The stripped variant will depend on the stripped variants of individual tools only to ensure they actually are stripped as well. Reviewers: labath, sas Subscribers: mgorny, lldb-commits Differential Revision: https://reviews.llvm.org/D50038 Modified: lldb/trunk/CMakeLists.txt lldb/trunk/cmake/modules/AddLLDB.cmake lldb/trunk/cmake/modules/LLDBFramework.cmake lldb/trunk/source/API/CMakeLists.txt Modified: lldb/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=338594&r1=338593&r2=338594&view=diff == --- lldb/trunk/CMakeLists.txt (original) +++ lldb/trunk/CMakeLists.txt Wed Aug 1 10:21:18 2018 @@ -51,6 +51,7 @@ if(LLDB_BUILD_FRAMEWORK) message(FATAL_ERROR "LLDB.framework can only be generated when targeting Apple platforms") endif() + add_custom_target(lldb-framework) # These are used to fill out LLDB-Info.plist. These are relevant when building # the framework, and must be defined before building liblldb. set(PRODUCT_NAME "LLDB") @@ -60,6 +61,7 @@ if(LLDB_BUILD_FRAMEWORK) set(LLDB_FRAMEWORK_DIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}) + include(LLDBFramework) endif() add_subdirectory(docs) @@ -162,10 +164,6 @@ if(LLDB_INCLUDE_TESTS) add_subdirectory(utils/lldb-dotest) endif() -if (LLDB_BUILD_FRAMEWORK) - add_custom_target(lldb-framework) - include(LLDBFramework) -endif() if (NOT LLDB_DISABLE_PYTHON) # Add a Post-Build Event to copy over Python files and create the symlink Modified: lldb/trunk/cmake/modules/AddLLDB.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=338594&r1=338593&r2=338594&view=diff == --- lldb/trunk/cmake/modules/AddLLDB.cmake (original) +++ lldb/trunk/cmake/modules/AddLLDB.cmake Wed Aug 1 10:21:18 2018 @@ -53,6 +53,11 @@ function(add_lldb_library name) set(out_dir lib${LLVM_LIBDIR_SUFFIX}) if(${name} STREQUAL "liblldb" AND LLDB_BUILD_FRAMEWORK) set(out_dir ${LLDB_FRAMEWORK_INSTALL_DIR}) + # The framework that is generated will install with install-liblldb + # because we enable CMake's framework support. CMake will copy all the + # headers and resources for us. + add_dependencies(install-lldb-framework install-${name}) + add_dependencies(install-lldb-framework-stripped install-${name}-stripped) endif() install(TARGETS ${name} COMPONENT ${name} @@ -67,12 +72,20 @@ function(add_lldb_library name) endif() if (NOT CMAKE_CONFIGURATION_TYPES) add_llvm_install_targets(install-${name} - DEPENDS ${name} + DEPENDS $ COMPONENT ${name}) + +# install-liblldb{,-stripped} is the actual target that will install the +# framework, so it must rely on the framework being fully built first. +if (LLDB_BUILD_FRAMEWORK AND ${name} STREQUAL "liblldb") + add_dependencies(install-${name} lldb-framework) + add_dependencies(install-lldb-framework-stripped lldb-framework) +endif() endif() endif() endif() + # Hack: only some LLDB libraries depend on the clang autogenerated headers, # but it is simple enough to make all of LLDB depend on some of those # headers without negatively impacting much of anything. @@ -124,6 +137,10 @@ function(add_lldb_executable name) set(out_dir "bin") if (LLDB_BUILD_FRAMEWORK AND ARG_INCLUDE_IN_SUITE) set(out_dir ${LLDB_FRAMEWORK_INSTALL_DIR}/${LLDB_FRAMEWORK_RESOURCE_DIR}) + # While install-liblldb-stripped will handle copying the tools, it will + # not strip them. We depend on this target to guarantee a stripped version + # will get installed in the framework. + add_dependencies(install-lldb-framew
[Lldb-commits] [PATCH] D50149: Fix out-of-bounds read in Stream::PutCStringAsRawHex8
teemperor created this revision. When I added the Stream unit test (r338488), the build bots failed due to an out-of- bound reads when passing an empty string to the PutCStringAsRawHex8 method. In r338491 I removed the test case to fix the bots. This patch fixes this in PutCStringAsRawHex8 by always checking for the terminating null character in the given string (instead of skipping it the first time). It also re-adds the test case I removed. https://reviews.llvm.org/D50149 Files: source/Utility/Stream.cpp unittests/Utility/StreamTest.cpp Index: unittests/Utility/StreamTest.cpp === --- unittests/Utility/StreamTest.cpp +++ unittests/Utility/StreamTest.cpp @@ -106,6 +106,9 @@ } TEST_F(StreamTest, PutCStringAsRawHex8) { + s.PutCStringAsRawHex8(""); + EXPECT_EQ("", TakeValue()); + s.PutCStringAsRawHex8("foobar"); EXPECT_EQ("666f6f626172", TakeValue()); Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -518,10 +518,10 @@ size_t bytes_written = 0; bool binary_is_set = m_flags.Test(eBinary); m_flags.Clear(eBinary); - do { + while(*s) { bytes_written += _PutHex8(*s, false); ++s; - } while (*s); + } if (binary_is_set) m_flags.Set(eBinary); return bytes_written; Index: unittests/Utility/StreamTest.cpp === --- unittests/Utility/StreamTest.cpp +++ unittests/Utility/StreamTest.cpp @@ -106,6 +106,9 @@ } TEST_F(StreamTest, PutCStringAsRawHex8) { + s.PutCStringAsRawHex8(""); + EXPECT_EQ("", TakeValue()); + s.PutCStringAsRawHex8("foobar"); EXPECT_EQ("666f6f626172", TakeValue()); Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -518,10 +518,10 @@ size_t bytes_written = 0; bool binary_is_set = m_flags.Test(eBinary); m_flags.Clear(eBinary); - do { + while(*s) { bytes_written += _PutHex8(*s, false); ++s; - } while (*s); + } if (binary_is_set) m_flags.Set(eBinary); return bytes_written; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338605 - Remove outdated documentation for Stream's LEB128 methods
Author: teemperor Date: Wed Aug 1 11:28:54 2018 New Revision: 338605 URL: http://llvm.org/viewvc/llvm-project?rev=338605&view=rev Log: Remove outdated documentation for Stream's LEB128 methods There is no format parameter for any of these methods. Modified: lldb/trunk/include/lldb/Utility/Stream.h Modified: lldb/trunk/include/lldb/Utility/Stream.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Stream.h?rev=338605&r1=338604&r2=338605&view=diff == --- lldb/trunk/include/lldb/Utility/Stream.h (original) +++ lldb/trunk/include/lldb/Utility/Stream.h Wed Aug 1 11:28:54 2018 @@ -504,9 +504,6 @@ public: /// /// @param[in] uval /// A uint64_t value that was extracted as a SLEB128 value. - /// - /// @param[in] format - /// The optional printf format that can be overridden. //-- size_t PutSLEB128(int64_t uval); @@ -518,9 +515,6 @@ public: /// /// @param[in] uval /// A uint64_t value that was extracted as a ULEB128 value. - /// - /// @param[in] format - /// The optional printf format that can be overridden. //-- size_t PutULEB128(uint64_t uval); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338611 - Fixed documentation for PutHex8 [NFC]
Author: teemperor Date: Wed Aug 1 11:38:19 2018 New Revision: 338611 URL: http://llvm.org/viewvc/llvm-project?rev=338611&view=rev Log: Fixed documentation for PutHex8 [NFC] The previous documentation was just copied from PrintfAsRawHex8 but doesn't actually fit to the PutHex8 method. Modified: lldb/trunk/include/lldb/Utility/Stream.h Modified: lldb/trunk/include/lldb/Utility/Stream.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Stream.h?rev=338611&r1=338610&r2=338611&view=diff == --- lldb/trunk/include/lldb/Utility/Stream.h (original) +++ lldb/trunk/include/lldb/Utility/Stream.h Wed Aug 1 11:38:19 2018 @@ -121,14 +121,10 @@ public: __attribute__((__format__(__printf__, 2, 3))); //-- - /// Format a C string from a printf style format and variable arguments and - /// encode and append the resulting C string as hex bytes. + /// Append an uint8_t value in the hexadecimal format to the stream. /// - /// @param[in] format - /// A printf style format string. - /// - /// @param[in] ... - /// Any additional arguments needed for the printf format string. + /// @param[in] uvalue + /// The value to append. /// /// @return /// The number of bytes that were appended to the stream. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49632: [lldb-mi] Re-implement MI HandleProcessEventStateSuspended.
apolyakov added a comment. @clayborg can you explain what are you worry about because I don't completely understand you? As I said, all lldb-mi commands print their result to STDOUT, AFAIK, an IDE should parse it and then show in a GUI. https://reviews.llvm.org/D49632 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)
vsk created this revision. vsk added reviewers: aprantl, jasonmolenda. Herald added subscribers: chrib, krytarowski, mgorny, srhines. This code looks like a good reference for building a new unwinder, but is currently unused, so there's no need to keep it. https://reviews.llvm.org/D50155 Files: lldb/lldb.xcodeproj/project.pbxproj lldb/source/Plugins/Process/Utility/CMakeLists.txt lldb/source/Plugins/Process/Utility/RegisterContextMacOSXFrameBackchain.cpp lldb/source/Plugins/Process/Utility/RegisterContextMacOSXFrameBackchain.h lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h lldb/source/Target/Thread.cpp Index: lldb/source/Target/Thread.cpp === --- lldb/source/Target/Thread.cpp +++ lldb/source/Target/Thread.cpp @@ -13,7 +13,6 @@ // Project includes #include "lldb/Target/Thread.h" #include "Plugins/Process/Utility/UnwindLLDB.h" -#include "Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h" #include "lldb/Breakpoint/BreakpointLocation.h" #include "lldb/Core/Debugger.h" #include "lldb/Core/FormatEntity.h" @@ -2055,10 +2054,7 @@ case llvm::Triple::hexagon: m_unwinder_ap.reset(new UnwindLLDB(*this)); break; - default: - if (target_arch.GetTriple().getVendor() == llvm::Triple::Apple) -m_unwinder_ap.reset(new UnwindMacOSXFrameBackchain(*this)); break; } } Index: lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h === --- lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h +++ /dev/null @@ -1,60 +0,0 @@ -//===-- UnwindMacOSXFrameBackchain.h *- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// - -#ifndef lldb_UnwindMacOSXFrameBackchain_h_ -#define lldb_UnwindMacOSXFrameBackchain_h_ - -// C Includes -// C++ Includes -#include - -// Other libraries and framework includes -// Project includes -#include "lldb/Target/Unwind.h" -#include "lldb/lldb-private.h" - -class UnwindMacOSXFrameBackchain : public lldb_private::Unwind { -public: - UnwindMacOSXFrameBackchain(lldb_private::Thread &thread); - - ~UnwindMacOSXFrameBackchain() override = default; - -protected: - void DoClear() override { m_cursors.clear(); } - - uint32_t DoGetFrameCount() override; - - bool DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa, - lldb::addr_t &pc) override; - - lldb::RegisterContextSP - DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) override; - - friend class RegisterContextMacOSXFrameBackchain; - - struct Cursor { -lldb::addr_t pc; // Program counter -lldb::addr_t fp; // Frame pointer for us with backchain - }; - -private: - std::vector m_cursors; - - size_t GetStackFrameData_i386(const lldb_private::ExecutionContext &exe_ctx); - - size_t - GetStackFrameData_x86_64(const lldb_private::ExecutionContext &exe_ctx); - - //-- - // For UnwindMacOSXFrameBackchain only - //-- - DISALLOW_COPY_AND_ASSIGN(UnwindMacOSXFrameBackchain); -}; - -#endif // lldb_UnwindMacOSXFrameBackchain_h_ Index: lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp === --- lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp +++ /dev/null @@ -1,246 +0,0 @@ -//===-- UnwindMacOSXFrameBackchain.cpp --*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// - -#include "lldb/Symbol/Function.h" -#include "lldb/Symbol/ObjectFile.h" -#include "lldb/Symbol/Symbol.h" -#include "lldb/Target/ExecutionContext.h" -#include "lldb/Target/Process.h" -#include "lldb/Target/Target.h" -#include "lldb/Target/Thread.h" -#include "lldb/Utility/ArchSpec.h" - -#include "RegisterContextMacOSXFrameBackchain.h" - -using namespace lldb; -using namespace lldb_private; - -UnwindMacOSXFrameBackchain::UnwindMacOSXFrameBackchain(Thread &thread) -: Unwind(thread), m_cursors() {} - -uint32_t UnwindMacOSXFrameBackchain::DoGetFrameCount() { - if (m_cursors.empty()) { -ExecutionContext exe_ctx(m_thread.shared_from_this()); -Target *target = exe_ctx.GetTargetPtr(); -if (target) { - const ArchSpec &target_arch = target->GetArchitecture(); - // Frame zero should always be supplied
[Lldb-commits] [PATCH] D50149: Fix out-of-bounds read in Stream::PutCStringAsRawHex8
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM. https://reviews.llvm.org/D50149 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)
aprantl added a comment. Deleting dead code is always good; I'll let Jason sign this off though. https://reviews.llvm.org/D50155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.
clayborg updated this revision to Diff 158631. clayborg added a comment. - run clang-format - fix doxygen parameter names https://reviews.llvm.org/D49750 Files: include/lldb/Target/Target.h lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm-linux.dmp packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm-macos.dmp packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.dmp source/Plugins/Process/minidump/MinidumpParser.cpp source/Plugins/Process/minidump/MinidumpParser.h source/Plugins/Process/minidump/ProcessMinidump.cpp source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h source/Plugins/Process/minidump/ThreadMinidump.cpp source/Target/Target.cpp Index: source/Target/Target.cpp === --- source/Target/Target.cpp +++ source/Target/Target.cpp @@ -1426,13 +1426,33 @@ } } -bool Target::SetArchitecture(const ArchSpec &arch_spec) { +bool Target::SetArchitecture(const ArchSpec &arch_spec, bool set_platform) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TARGET)); bool missing_local_arch = !m_arch.GetSpec().IsValid(); bool replace_local_arch = true; bool compatible_local_arch = false; ArchSpec other(arch_spec); + // Changing the architecture might mean that the currently selected platform + // isn't compatible. Set the platform correctly if we are asked to do so, + // otherwise assume the user will set the platform manually. + if (set_platform) { +if (other.IsValid()) { + auto platform_sp = GetPlatform(); + if (!platform_sp || + !platform_sp->IsCompatibleArchitecture(other, false, nullptr)) { +ArchSpec platform_arch; +auto arch_platform_sp = +Platform::GetPlatformForArchitecture(other, &platform_arch); +if (arch_platform_sp) { + SetPlatform(arch_platform_sp); + if (platform_arch.IsValid()) +other = platform_arch; +} + } +} + } + if (!missing_local_arch) { if (m_arch.GetSpec().IsCompatibleMatch(arch_spec)) { other.MergeFrom(m_arch.GetSpec()); Index: source/Plugins/Process/minidump/ThreadMinidump.cpp === --- source/Plugins/Process/minidump/ThreadMinidump.cpp +++ source/Plugins/Process/minidump/ThreadMinidump.cpp @@ -11,6 +11,8 @@ #include "ThreadMinidump.h" #include "ProcessMinidump.h" +#include "RegisterContextMinidump_ARM.h" +#include "RegisterContextMinidump_ARM64.h" #include "RegisterContextMinidump_x86_32.h" #include "RegisterContextMinidump_x86_64.h" @@ -88,17 +90,24 @@ *this, reg_interface, gpregset, {})); break; } +case llvm::Triple::aarch64: { + DataExtractor data(m_gpregset_data.data(), m_gpregset_data.size(), + lldb::eByteOrderLittle, 8); + m_thread_reg_ctx_sp.reset(new RegisterContextMinidump_ARM64(*this, data)); + break; +} +case llvm::Triple::arm: { + DataExtractor data(m_gpregset_data.data(), m_gpregset_data.size(), + lldb::eByteOrderLittle, 8); + const bool apple = arch.GetTriple().getVendor() == llvm::Triple::Apple; + m_thread_reg_ctx_sp.reset( + new RegisterContextMinidump_ARM(*this, data, apple)); + break; +} default: break; } -if (!reg_interface) { - if (log) -log->Printf("elf-core::%s:: Architecture(%d) not supported", -__FUNCTION__, arch.GetMachine()); - assert(false && "Architecture not supported"); -} - reg_ctx_sp = m_thread_reg_ctx_sp; } else if (m_unwinder_ap) { reg_ctx_sp = m_unwinder_ap->CreateRegisterContextForFrame(frame); Index: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h === --- source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h +++ source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h @@ -0,0 +1,85 @@ +//===-- RegisterContextMinidump_ARM64.h -*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef liblldb_RegisterContextMinidump_ARM64_h_ +#define liblldb_RegisterContextMinidump_ARM64_h_ + +// Project includes +#include "MinidumpTypes.h" + +// Other libraries and framework includes +#include "Plugi
[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.
clayborg marked 2 inline comments as done. clayborg added inline comments. Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp:57 +nullptr, nullptr, nullptr, 0} + +// Zero based LLDB register numbers for this register context No need to change the x86. Just seems like more work to map one set of registers to another when/if the actual context is in another format. The ARM registers differ from any other ARM register context as they have 8 "extra" registers, so it didn't make sense to try and map it to another register context. So the "don't like the x86" part is just because it was remapping from one context to another and these register contexts are simple. https://reviews.llvm.org/D49750 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338637 - Fix out-of-bounds read in Stream::PutCStringAsRawHex8
Author: teemperor Date: Wed Aug 1 14:07:18 2018 New Revision: 338637 URL: http://llvm.org/viewvc/llvm-project?rev=338637&view=rev Log: Fix out-of-bounds read in Stream::PutCStringAsRawHex8 Summary: When I added the Stream unit test (r338488), the build bots failed due to an out-of- bound reads when passing an empty string to the PutCStringAsRawHex8 method. In r338491 I removed the test case to fix the bots. This patch fixes this in PutCStringAsRawHex8 by always checking for the terminating null character in the given string (instead of skipping it the first time). It also re-adds the test case I removed. Reviewers: vsk Reviewed By: vsk Subscribers: vsk, lldb-commits Differential Revision: https://reviews.llvm.org/D50149 Modified: lldb/trunk/source/Utility/Stream.cpp lldb/trunk/unittests/Utility/StreamTest.cpp Modified: lldb/trunk/source/Utility/Stream.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Stream.cpp?rev=338637&r1=338636&r2=338637&view=diff == --- lldb/trunk/source/Utility/Stream.cpp (original) +++ lldb/trunk/source/Utility/Stream.cpp Wed Aug 1 14:07:18 2018 @@ -518,10 +518,10 @@ size_t Stream::PutCStringAsRawHex8(const size_t bytes_written = 0; bool binary_is_set = m_flags.Test(eBinary); m_flags.Clear(eBinary); - do { + while(*s) { bytes_written += _PutHex8(*s, false); ++s; - } while (*s); + } if (binary_is_set) m_flags.Set(eBinary); return bytes_written; Modified: lldb/trunk/unittests/Utility/StreamTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/StreamTest.cpp?rev=338637&r1=338636&r2=338637&view=diff == --- lldb/trunk/unittests/Utility/StreamTest.cpp (original) +++ lldb/trunk/unittests/Utility/StreamTest.cpp Wed Aug 1 14:07:18 2018 @@ -106,6 +106,9 @@ TEST_F(StreamTest, PutCharNull) { } TEST_F(StreamTest, PutCStringAsRawHex8) { + s.PutCStringAsRawHex8(""); + EXPECT_EQ("", TakeValue()); + s.PutCStringAsRawHex8("foobar"); EXPECT_EQ("666f6f626172", TakeValue()); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50149: Fix out-of-bounds read in Stream::PutCStringAsRawHex8
This revision was automatically updated to reflect the committed changes. Closed by commit rL338637: Fix out-of-bounds read in Stream::PutCStringAsRawHex8 (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50149?vs=158569&id=158632#toc Repository: rL LLVM https://reviews.llvm.org/D50149 Files: lldb/trunk/source/Utility/Stream.cpp lldb/trunk/unittests/Utility/StreamTest.cpp Index: lldb/trunk/unittests/Utility/StreamTest.cpp === --- lldb/trunk/unittests/Utility/StreamTest.cpp +++ lldb/trunk/unittests/Utility/StreamTest.cpp @@ -106,6 +106,9 @@ } TEST_F(StreamTest, PutCStringAsRawHex8) { + s.PutCStringAsRawHex8(""); + EXPECT_EQ("", TakeValue()); + s.PutCStringAsRawHex8("foobar"); EXPECT_EQ("666f6f626172", TakeValue()); Index: lldb/trunk/source/Utility/Stream.cpp === --- lldb/trunk/source/Utility/Stream.cpp +++ lldb/trunk/source/Utility/Stream.cpp @@ -518,10 +518,10 @@ size_t bytes_written = 0; bool binary_is_set = m_flags.Test(eBinary); m_flags.Clear(eBinary); - do { + while(*s) { bytes_written += _PutHex8(*s, false); ++s; - } while (*s); + } if (binary_is_set) m_flags.Set(eBinary); return bytes_written; Index: lldb/trunk/unittests/Utility/StreamTest.cpp === --- lldb/trunk/unittests/Utility/StreamTest.cpp +++ lldb/trunk/unittests/Utility/StreamTest.cpp @@ -106,6 +106,9 @@ } TEST_F(StreamTest, PutCStringAsRawHex8) { + s.PutCStringAsRawHex8(""); + EXPECT_EQ("", TakeValue()); + s.PutCStringAsRawHex8("foobar"); EXPECT_EQ("666f6f626172", TakeValue()); Index: lldb/trunk/source/Utility/Stream.cpp === --- lldb/trunk/source/Utility/Stream.cpp +++ lldb/trunk/source/Utility/Stream.cpp @@ -518,10 +518,10 @@ size_t bytes_written = 0; bool binary_is_set = m_flags.Test(eBinary); m_flags.Clear(eBinary); - do { + while(*s) { bytes_written += _PutHex8(*s, false); ++s; - } while (*s); + } if (binary_is_set) m_flags.Set(eBinary); return bytes_written; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338638 - [DWARFASTParser] Remove special cases for `llvm-gcc`
Author: davide Date: Wed Aug 1 14:13:45 2018 New Revision: 338638 URL: http://llvm.org/viewvc/llvm-project?rev=338638&view=rev Log: [DWARFASTParser] Remove special cases for `llvm-gcc` Reviewed by: aprantl, labath. Differential Revision: https://reviews.llvm.org/D48500 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=338638&r1=338637&r2=338638&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Wed Aug 1 14:13:45 2018 @@ -307,14 +307,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro decl.SetColumn(form_value.Unsigned()); break; case DW_AT_name: - type_name_cstr = form_value.AsCString(); -// Work around a bug in llvm-gcc where they give a name to a -// reference type which doesn't include the "&"... -if (tag == DW_TAG_reference_type) { - if (strchr(type_name_cstr, '&') == NULL) -type_name_cstr = NULL; -} if (type_name_cstr) type_name_const_str.SetCString(type_name_cstr); break; @@ -558,16 +551,9 @@ TypeSP DWARFASTParserClang::ParseTypeFro if (attributes.ExtractFormValueAtIndex(i, form_value)) { switch (attr) { case DW_AT_decl_file: -if (die.GetCU()->DW_AT_decl_file_attributes_are_invalid()) { - // llvm-gcc outputs invalid DW_AT_decl_file attributes that - // always point to the compile unit file, so we clear this - // invalid value so that we can still unique types - // efficiently. - decl.SetFile(FileSpec("", false)); -} else - decl.SetFile( - sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex( - form_value.Unsigned())); +decl.SetFile( + sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex( + form_value.Unsigned())); break; case DW_AT_decl_line: @@ -2977,15 +2963,6 @@ bool DWARFASTParserClang::ParseChildMemb class_language == eLanguageTypeObjC_plus_plus) accessibility = eAccessNone; -if (member_idx == 0 && !is_artificial && name && -(strstr(name, "_vptr$") == name)) { - // Not all compilers will mark the vtable pointer member as - // artificial (llvm-gcc). We can't have the virtual members in our - // classes otherwise it throws off all child offsets since we end up - // having and extra pointer sized member in our class layouts. - is_artificial = true; -} - // Handle static members if (is_external && member_byte_offset == UINT32_MAX) { Type *var_type = die.ResolveTypeUID(DIERef(encoding_form)); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48500: [DWARFASTParser] Remove special cases for `llvm-gcc`
This revision was automatically updated to reflect the committed changes. Closed by commit rL338638: [DWARFASTParser] Remove special cases for `llvm-gcc` (authored by davide, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D48500?vs=152523&id=158633#toc Repository: rL LLVM https://reviews.llvm.org/D48500 Files: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp === --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -307,14 +307,7 @@ decl.SetColumn(form_value.Unsigned()); break; case DW_AT_name: - type_name_cstr = form_value.AsCString(); -// Work around a bug in llvm-gcc where they give a name to a -// reference type which doesn't include the "&"... -if (tag == DW_TAG_reference_type) { - if (strchr(type_name_cstr, '&') == NULL) -type_name_cstr = NULL; -} if (type_name_cstr) type_name_const_str.SetCString(type_name_cstr); break; @@ -558,16 +551,9 @@ if (attributes.ExtractFormValueAtIndex(i, form_value)) { switch (attr) { case DW_AT_decl_file: -if (die.GetCU()->DW_AT_decl_file_attributes_are_invalid()) { - // llvm-gcc outputs invalid DW_AT_decl_file attributes that - // always point to the compile unit file, so we clear this - // invalid value so that we can still unique types - // efficiently. - decl.SetFile(FileSpec("", false)); -} else - decl.SetFile( - sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex( - form_value.Unsigned())); +decl.SetFile( + sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex( + form_value.Unsigned())); break; case DW_AT_decl_line: @@ -2977,15 +2963,6 @@ class_language == eLanguageTypeObjC_plus_plus) accessibility = eAccessNone; -if (member_idx == 0 && !is_artificial && name && -(strstr(name, "_vptr$") == name)) { - // Not all compilers will mark the vtable pointer member as - // artificial (llvm-gcc). We can't have the virtual members in our - // classes otherwise it throws off all child offsets since we end up - // having and extra pointer sized member in our class layouts. - is_artificial = true; -} - // Handle static members if (is_external && member_byte_offset == UINT32_MAX) { Type *var_type = die.ResolveTypeUID(DIERef(encoding_form)); Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp === --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -307,14 +307,7 @@ decl.SetColumn(form_value.Unsigned()); break; case DW_AT_name: - type_name_cstr = form_value.AsCString(); -// Work around a bug in llvm-gcc where they give a name to a -// reference type which doesn't include the "&"... -if (tag == DW_TAG_reference_type) { - if (strchr(type_name_cstr, '&') == NULL) -type_name_cstr = NULL; -} if (type_name_cstr) type_name_const_str.SetCString(type_name_cstr); break; @@ -558,16 +551,9 @@ if (attributes.ExtractFormValueAtIndex(i, form_value)) { switch (attr) { case DW_AT_decl_file: -if (die.GetCU()->DW_AT_decl_file_attributes_are_invalid()) { - // llvm-gcc outputs invalid DW_AT_decl_file attributes that - // always point to the compile unit file, so we clear this - // invalid value so that we can still unique types - // efficiently. - decl.SetFile(FileSpec("", false)); -} else - decl.SetFile( - sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex( - form_value.Unsigned())); +decl.SetFile( + sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex( + form_value.Unsigned())); break; case DW_AT_decl_line: @@ -2977,15 +2963,6 @@ cla
[Lldb-commits] [PATCH] D50071: Use rich mangling information in Symtab::InitNameIndexes()
sgraenitz updated this revision to Diff 158638. sgraenitz added a comment. Minor improvements. https://reviews.llvm.org/D50071 Files: include/lldb/Core/Mangled.h include/lldb/Core/RichManglingInfo.h include/lldb/Symbol/Symtab.h include/lldb/Utility/ConstString.h include/lldb/lldb-forward.h source/Core/CMakeLists.txt source/Core/Mangled.cpp source/Core/RichManglingInfo.cpp source/Symbol/Symtab.cpp Index: source/Symbol/Symtab.cpp === --- source/Symbol/Symtab.cpp +++ source/Symbol/Symtab.cpp @@ -10,9 +10,10 @@ #include #include -#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" #include "Plugins/Language/ObjC/ObjCLanguage.h" + #include "lldb/Core/Module.h" +#include "lldb/Core/RichManglingInfo.h" #include "lldb/Core/STLUtils.h" #include "lldb/Core/Section.h" #include "lldb/Symbol/ObjectFile.h" @@ -215,6 +216,39 @@ //-- // InitNameIndexes //-- +static bool +lldb_skip_name(llvm::StringRef mangled, Mangled::ManglingScheme scheme) { + switch (scheme) { + case Mangled::eManglingSchemeItanium: { +if (mangled.size() < 3 || !mangled.startswith("_Z")) + return true; + +// Avoid the following types of symbols in the index. +switch (mangled[2]) { +case 'G': // guard variables +case 'T': // virtual tables, VTT structures, typeinfo structures + names +case 'Z': // named local entities (if we eventually handle + // eSymbolTypeData, we will want this back) + return true; + +default: + break; +} + +// Include this name in the index. +return false; + } + + // No filters for this scheme yet. Include all names in indexing. + case Mangled::eManglingSchemeMSVC: +return false; + + // Don't try and demangle things we can't categorize. + case Mangled::eManglingSchemeNone: +return true; + } +} + void Symtab::InitNameIndexes() { // Protected function, no need to lock mutex... if (!m_name_indexes_computed) { @@ -243,25 +277,30 @@ m_name_to_index.Reserve(actual_count); #endif -NameToIndexMap::Entry entry; - // The "const char *" in "class_contexts" must come from a // ConstString::GetCString() std::set class_contexts; UniqueCStringMap mangled_name_to_index; std::vector symbol_contexts(num_symbols, nullptr); +// Instantiation of the demangler is expensive, so better use a single one +// for all entries during batch processing. +RichManglingContext MC; +NameToIndexMap::Entry entry; + for (entry.value = 0; entry.value < num_symbols; ++entry.value) { - const Symbol *symbol = &m_symbols[entry.value]; + Symbol *symbol = &m_symbols[entry.value]; // Don't let trampolines get into the lookup by name map If we ever need // the trampoline symbols to be searchable by name we can remove this and // then possibly add a new bool to any of the Symtab functions that // lookup symbols by name to indicate if they want trampolines. if (symbol->IsTrampoline()) continue; - const Mangled &mangled = symbol->GetMangled(); + // If the symbol's name string matched a Mangled::ManglingScheme, it is + // stored in the mangled field. + Mangled &mangled = symbol->GetMangled(); entry.cstring = mangled.GetMangledName(); if (entry.cstring) { m_name_to_index.Append(entry); @@ -274,70 +313,18 @@ m_name_to_index.Append(entry); } -const SymbolType symbol_type = symbol->GetType(); -if (symbol_type == eSymbolTypeCode || -symbol_type == eSymbolTypeResolver) { - llvm::StringRef entry_ref(entry.cstring.GetStringRef()); - if (entry_ref[0] == '_' && entry_ref[1] == 'Z' && - (entry_ref[2] != 'T' && // avoid virtual table, VTT structure, - // typeinfo structure, and typeinfo - // name - entry_ref[2] != 'G' && // avoid guard variables - entry_ref[2] != 'Z')) // named local entities (if we - // eventually handle eSymbolTypeData, - // we will want this back) - { -CPlusPlusLanguage::MethodName cxx_method( -mangled.GetDemangledName(lldb::eLanguageTypeC_plus_plus)); -entry.cstring = ConstString(cxx_method.GetBasename()); -if (entry.cstring) { - // ConstString objects permanently store the string in the pool - // so calling GetCString() on the value gets us a const char * - // that will never go away - const char *const_context = - ConstString(cxx_method.GetContext()).GetCString(); - - if (!const_conte
[Lldb-commits] [PATCH] D50071: Use rich mangling information in Symtab::InitNameIndexes()
sgraenitz marked 2 inline comments as done. sgraenitz added inline comments. Comment at: include/lldb/Core/RichManglingInfo.h:83-84 +public: + RichManglingInfo *SetItaniumInfo(); + RichManglingInfo *SetLegacyCxxParserInfo(const ConstString &mangled); + labath wrote: > I find it odd that one of these functions is stateless > (`SetLegacyCxxParserInfo` takes the string which initializes the object as an > argument), while the other receives the state magically via the IPD accessor. > > Would it be possible to change it so that the other option is stateless too > (`SetItaniumInfo(ConstString mangled)`)? > > We could change `RichManglingInfo` to return the full demangled name too, so > that you have access to the demangled name via this API too (which /I think/ > is the reason why you created a separate `GetItaniumRichDemangleInfo` > function). > Would it be possible to change it so that the other option is stateless too > (`SetItaniumInfo(ConstString mangled)`)? I think that would be misleading for the reader. Symmetry is always nice to indicate congruent behavior, but this is not the case here. The fallback C++ parser version mimics the previous behavior: demangle the name in step 1 and if that succeeds, parse it with `CPlusPlusLanguage::MethodName` to decode its properties. The IPD does that in a single step and so there is nothing to do in `SetItaniumInfo()`. `SetLegacyCxxParserInfo()` on the other hand has to create a `CPlusPlusLanguage::MethodName` from the mangled string. > We could change `RichManglingInfo` to return the full demangled name too, so > that you have access to the demangled name via this API too (which /I think/ > is the reason why you created a separate GetItaniumRichDemangleInfo function). The interface wants to provide access to the name's encoded properties. That doesn't really include the demangled "human readable" string representation (it's also not needed in `Symtab::RegisterMangledNameEntry()`). Of course `Symtab::InitNameIndexes()` will ask for `GetDemangledName()` just after processing the mangled one. I think chances are good that for symbols that are not filtered out anyway, we reached `DemangleWithRichManglingInfo()` earlier and it will be in the string pool already (see Mangled.cpp:322). Otherwise, yes it will be demangled in with a new IPD instance. I think it's not bad to keep these cases as they are. I would propose to investigate the performance of the current implementation in more detail and think about the benefits before mixing them. Not sure at all, if it would bring measurable gain. Comment at: source/Core/RichManglingInfo.cpp:36-40 +RichManglingContext::SetLegacyCxxParserInfo(const ConstString &mangled) { + m_info.ResetProvider(); + m_info.m_provider = RichManglingInfo::PluginCxxLanguage; + m_info.m_legacy_parser = new CPlusPlusLanguage::MethodName(mangled); + return &m_info; labath wrote: > Is this really the **mangled** name? Yes, it is. https://reviews.llvm.org/D50071 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50159: Add byte counting mechanism to stream.
teemperor created this revision. teemperor added a reviewer: labath. This patch allows LLDB's Stream class to count the bytes it has written to so far. There are two major motivations for this patch: The first one is that this will allow us to get rid of all the handwritten byte counting code we have in LLDB so far. Examples for this are pretty much all functions in LLDB that take a Stream to write to and return a size_t, which usually represents the bytes written. By moving to this centralized byte counting mechanism, we hopefully can avoid some tricky errors that happen when some code forgets to count the written bytes while writing something to a stream. The second motivation is that this is needed for the migration away from LLDB's `Stream` and towards LLVM's `raw_ostream`. My current plan is to start offering a fake raw_ostream class that just forwards to a LLDB Stream. However, for this raw_ostream wrapper we need to fulfill the raw_ostream interface with LLDB's Stream, which currently lacks the ability to count the bytes written so far (which raw_ostream exposes by it's `tell()` method). By adding this functionality it is trivial to start rolling out our raw_ostream wrapper (and then eventually completely move to raw_ostream). Also, once this fake raw_ostream is available, we can start replacing our own code writing to LLDB's Stream by LLVM code writing to raw_ostream. The best example for this is the LEB128 encoding we currently ship, which can be replaced with by LLVM's version which accepts an raw_ostream. https://reviews.llvm.org/D50159 Files: include/lldb/Core/StreamAsynchronousIO.h include/lldb/Core/StreamBuffer.h include/lldb/Core/StreamFile.h include/lldb/Utility/Stream.h include/lldb/Utility/StreamString.h include/lldb/Utility/StreamTee.h source/Core/StreamAsynchronousIO.cpp source/Core/StreamFile.cpp source/Utility/StreamString.cpp unittests/Utility/StreamTeeTest.cpp unittests/Utility/StreamTest.cpp Index: unittests/Utility/StreamTest.cpp === --- unittests/Utility/StreamTest.cpp +++ unittests/Utility/StreamTest.cpp @@ -44,171 +44,238 @@ TEST_F(StreamTest, PutChar) { s.PutChar('a'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("a", TakeValue()); s.PutChar('1'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("1", TakeValue()); } TEST_F(StreamTest, PutCharWhitespace) { s.PutChar(' '); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(" ", TakeValue()); s.PutChar('\n'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("\n", TakeValue()); s.PutChar('\r'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("\r", TakeValue()); s.PutChar('\t'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("\t", TakeValue()); } TEST_F(StreamTest, PutCString) { s.PutCString(""); + EXPECT_EQ(0U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); s.PutCString("foobar"); + EXPECT_EQ(6U, s.GetWrittenBytes()); EXPECT_EQ("foobar", TakeValue()); s.PutCString(" "); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(" ", TakeValue()); } TEST_F(StreamTest, PutCStringWithStringRef) { s.PutCString(llvm::StringRef("")); + EXPECT_EQ(0U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); s.PutCString(llvm::StringRef("foobar")); + EXPECT_EQ(6U, s.GetWrittenBytes()); EXPECT_EQ("foobar", TakeValue()); s.PutCString(llvm::StringRef(" ")); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(" ", TakeValue()); } TEST_F(StreamTest, QuotedCString) { s.QuotedCString("foo"); + EXPECT_EQ(5U, s.GetWrittenBytes()); EXPECT_EQ(R"("foo")", TakeValue()); s.QuotedCString("ba r"); + EXPECT_EQ(6U, s.GetWrittenBytes()); EXPECT_EQ(R"("ba r")", TakeValue()); s.QuotedCString(" "); + EXPECT_EQ(3U, s.GetWrittenBytes()); EXPECT_EQ(R"(" ")", TakeValue()); } TEST_F(StreamTest, PutCharNull) { s.PutChar('\0'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(std::string("\0", 1), TakeValue()); s.PutChar('a'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(std::string("a", 1), TakeValue()); } TEST_F(StreamTest, PutCStringAsRawHex8) { s.PutCStringAsRawHex8("foobar"); + EXPECT_EQ(12U, s.GetWrittenBytes()); EXPECT_EQ("666f6f626172", TakeValue()); s.PutCStringAsRawHex8(" "); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("20", TakeValue()); } TEST_F(StreamTest, PutHex8) { s.PutHex8((uint8_t)55); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("37", TakeValue()); s.PutHex8(std::numeric_limits::max()); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("ff", TakeValue()); s.PutHex8((uint8_t)0); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("00", TakeValue()); } TEST_F(StreamTest, PutNHex8) { s.PutNHex8(0, (uint8_t)55); + EXPECT_EQ(0U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); + s.PutNHex8(1, (uint8_t)55); + EXPECT_EQ(2U, s.GetWrittenBytes(
[Lldb-commits] [PATCH] D50159: Add byte counting mechanism to LLDB's Stream class.
teemperor updated this revision to Diff 158642. teemperor edited the summary of this revision. teemperor added a comment. - Added some more documentation. https://reviews.llvm.org/D50159 Files: include/lldb/Core/StreamAsynchronousIO.h include/lldb/Core/StreamBuffer.h include/lldb/Core/StreamFile.h include/lldb/Utility/Stream.h include/lldb/Utility/StreamString.h include/lldb/Utility/StreamTee.h source/Core/StreamAsynchronousIO.cpp source/Core/StreamFile.cpp source/Utility/StreamString.cpp unittests/Utility/StreamTeeTest.cpp unittests/Utility/StreamTest.cpp Index: unittests/Utility/StreamTest.cpp === --- unittests/Utility/StreamTest.cpp +++ unittests/Utility/StreamTest.cpp @@ -44,171 +44,238 @@ TEST_F(StreamTest, PutChar) { s.PutChar('a'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("a", TakeValue()); s.PutChar('1'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("1", TakeValue()); } TEST_F(StreamTest, PutCharWhitespace) { s.PutChar(' '); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(" ", TakeValue()); s.PutChar('\n'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("\n", TakeValue()); s.PutChar('\r'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("\r", TakeValue()); s.PutChar('\t'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ("\t", TakeValue()); } TEST_F(StreamTest, PutCString) { s.PutCString(""); + EXPECT_EQ(0U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); s.PutCString("foobar"); + EXPECT_EQ(6U, s.GetWrittenBytes()); EXPECT_EQ("foobar", TakeValue()); s.PutCString(" "); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(" ", TakeValue()); } TEST_F(StreamTest, PutCStringWithStringRef) { s.PutCString(llvm::StringRef("")); + EXPECT_EQ(0U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); s.PutCString(llvm::StringRef("foobar")); + EXPECT_EQ(6U, s.GetWrittenBytes()); EXPECT_EQ("foobar", TakeValue()); s.PutCString(llvm::StringRef(" ")); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(" ", TakeValue()); } TEST_F(StreamTest, QuotedCString) { s.QuotedCString("foo"); + EXPECT_EQ(5U, s.GetWrittenBytes()); EXPECT_EQ(R"("foo")", TakeValue()); s.QuotedCString("ba r"); + EXPECT_EQ(6U, s.GetWrittenBytes()); EXPECT_EQ(R"("ba r")", TakeValue()); s.QuotedCString(" "); + EXPECT_EQ(3U, s.GetWrittenBytes()); EXPECT_EQ(R"(" ")", TakeValue()); } TEST_F(StreamTest, PutCharNull) { s.PutChar('\0'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(std::string("\0", 1), TakeValue()); s.PutChar('a'); + EXPECT_EQ(1U, s.GetWrittenBytes()); EXPECT_EQ(std::string("a", 1), TakeValue()); } TEST_F(StreamTest, PutCStringAsRawHex8) { s.PutCStringAsRawHex8("foobar"); + EXPECT_EQ(12U, s.GetWrittenBytes()); EXPECT_EQ("666f6f626172", TakeValue()); s.PutCStringAsRawHex8(" "); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("20", TakeValue()); } TEST_F(StreamTest, PutHex8) { s.PutHex8((uint8_t)55); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("37", TakeValue()); s.PutHex8(std::numeric_limits::max()); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("ff", TakeValue()); s.PutHex8((uint8_t)0); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("00", TakeValue()); } TEST_F(StreamTest, PutNHex8) { s.PutNHex8(0, (uint8_t)55); + EXPECT_EQ(0U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); + s.PutNHex8(1, (uint8_t)55); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("37", TakeValue()); + s.PutNHex8(2, (uint8_t)55); + EXPECT_EQ(4U, s.GetWrittenBytes()); EXPECT_EQ("3737", TakeValue()); + s.PutNHex8(1, (uint8_t)56); + EXPECT_EQ(2U, s.GetWrittenBytes()); EXPECT_EQ("38", TakeValue()); } TEST_F(StreamTest, PutHex16ByteOrderLittle) { s.PutHex16(0x1234U, lldb::eByteOrderLittle); + EXPECT_EQ(4U, s.GetWrittenBytes()); EXPECT_EQ("3412", TakeValue()); + s.PutHex16(std::numeric_limits::max(), lldb::eByteOrderLittle); + EXPECT_EQ(4U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); + s.PutHex16(0U, lldb::eByteOrderLittle); + EXPECT_EQ(4U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); } TEST_F(StreamTest, PutHex16ByteOrderBig) { s.PutHex16(0x1234U, lldb::eByteOrderBig); + EXPECT_EQ(4U, s.GetWrittenBytes()); EXPECT_EQ("1234", TakeValue()); + s.PutHex16(std::numeric_limits::max(), lldb::eByteOrderBig); + EXPECT_EQ(4U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); + s.PutHex16(0U, lldb::eByteOrderBig); + EXPECT_EQ(4U, s.GetWrittenBytes()); EXPECT_EQ("", TakeValue()); } TEST_F(StreamTest, PutHex32ByteOrderLittle) { s.PutHex32(0x12345678U, lldb::eByteOrderLittle); + EXPECT_EQ(8U, s.GetWrittenBytes()); EXPECT_EQ("78563412", TakeValue()); + s.PutHex32(std::numeric_limits::max(), lldb::eByteOrderLittle); + EXPECT_EQ(8U, s.GetWrittenBytes()); EXPECT
[Lldb-commits] [PATCH] D50071: Use rich mangling information in Symtab::InitNameIndexes()
sgraenitz added a comment. > If I understand things correctly, we could avoid circular deps and untyped > pointers (or llvm::Any, which is essentially the same thing), by moving > CPlusPlusLanguage::MethodName to a separate file. Banning public nested classes in general is a good practice as long as C++ is lacking rich forward declarations. This is the issue here. If we could forward declare the class, all was great. The problem is, of course, that there is a pattern behind `Language::MethodName` within the language plugins. So changing all of them? Hmm.. > moving CPlusPlusLanguage::MethodName to a separate file. You mean putting it in a separate file and include that one in the `RichManglingInfo.h`? I am not so familiar with the code yet, but it looks like there are reasons for having `CPlusPlusLanguage` under `Plugins` and separate these from other things like `Core` or `Symbol`. I don't think we should include a `Plugins` header from a `Core` header.. > Could we do that as a preparatory step for this patch? Well, now I spent the time to make the hack nice haha. To be honest, I am happy to talk about a follow-up task here, but doing this now and before finishing the demangling sounds like a big unrelated piece of work. https://reviews.llvm.org/D50071 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50161: Add raw_ostream wrapper to the Stream class
teemperor created this revision. This wrapper will allow us in the future to reuse LLVM methods from within the Stream class. Currently no test as this is intended to be an internal class that shouldn't have any NFC. The test for this change will be the follow up patch that migrates LLDB's LEB128 implementation to the one from LLVM. https://reviews.llvm.org/D50161 Files: include/lldb/Utility/Stream.h source/Utility/Stream.cpp Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -23,11 +23,11 @@ Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order) : m_flags(flags), m_addr_size(addr_size), m_byte_order(byte_order), - m_indent_level(0) {} + m_indent_level(0), m_forward(*this) {} Stream::Stream() : m_flags(0), m_addr_size(4), m_byte_order(endian::InlHostByteOrder()), - m_indent_level(0) {} + m_indent_level(0), m_forward(*this) {} //-- // Destructor Index: include/lldb/Utility/Stream.h === --- include/lldb/Utility/Stream.h +++ include/lldb/Utility/Stream.h @@ -15,6 +15,7 @@ #include "lldb/lldb-enumerations.h" // for ByteOrder::eByteOrderInvalid #include "llvm/ADT/StringRef.h" // for StringRef #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/raw_ostream.h" #include #include // for size_t @@ -52,6 +53,17 @@ //-- Stream(); + // FIXME: Streams should not be copyable. + Stream(const Stream &other) : m_forward(*this) { (*this) = other; } + + Stream &operator=(const Stream &rhs) { +m_flags = rhs.m_flags; +m_addr_size = rhs.m_addr_size; +m_byte_order = rhs.m_byte_order; +m_indent_level = rhs.m_indent_level; +return *this; + } + //-- /// Destructor //-- @@ -548,6 +560,34 @@ /// The number of bytes that were appended to the stream. //-- virtual size_t WriteImpl(const void *src, size_t src_len) = 0; + + //-- + /// @class RawOstreamForward Stream.h "lldb/Utility/Stream.h" + /// This is a wrapper class that exposes a raw_ostream interface that just + /// forwards to an LLDB stream, allowing to reuse LLVM algorithms that take + /// a raw_ostream within the LLDB code base. + //-- + class RawOstreamForward : public llvm::raw_ostream { +// Note: This stream must *not* maintain its own buffer, but instead +// directly write everything to the internal Stream class. Without this, +// we would run into the problem that the Stream written byte count would +// differ from the actually written bytes by the size of the internal +// raw_ostream buffer. + +Stream &m_forward_to; +void write_impl(const char *Ptr, size_t Size) override { + m_forward_to.Write(Ptr, Size); +} + +uint64_t current_pos() const override { + return m_forward_to.GetWrittenBytes(); +} + + public: +RawOstreamForward(Stream &forward_to) +: llvm::raw_ostream(/*unbuffered*/ true), m_forward_to(forward_to) {} + }; + RawOstreamForward m_forward; }; } // namespace lldb_private Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -23,11 +23,11 @@ Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order) : m_flags(flags), m_addr_size(addr_size), m_byte_order(byte_order), - m_indent_level(0) {} + m_indent_level(0), m_forward(*this) {} Stream::Stream() : m_flags(0), m_addr_size(4), m_byte_order(endian::InlHostByteOrder()), - m_indent_level(0) {} + m_indent_level(0), m_forward(*this) {} //-- // Destructor Index: include/lldb/Utility/Stream.h === --- include/lldb/Utility/Stream.h +++ include/lldb/Utility/Stream.h @@ -15,6 +15,7 @@ #include "lldb/lldb-enumerations.h" // for ByteOrder::eByteOrderInvalid #include "llvm/ADT/StringRef.h" // for StringRef #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/raw_ostream.h" #include #include // for size_t @@ -52,6 +53,17 @@ //-- Stream(); + // FIXME: Streams should not be copyable. + Stream(const Stream &other) : m_forward(*this) { (*this) = other; } + + Stream &operator=(const Stream &rhs) { +
[Lldb-commits] [PATCH] D50162: Replace LLDB's LEB128 implementation with the one from LLVM
teemperor created this revision. https://reviews.llvm.org/D50162 Files: source/Utility/Stream.cpp Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -12,6 +12,7 @@ #include "lldb/Utility/Endian.h" #include "lldb/Utility/VASPrintf.h" #include "llvm/ADT/SmallString.h" // for SmallString +#include "llvm/Support/LEB128.h" #include @@ -49,47 +50,20 @@ // Put an SLEB128 "uval" out to the stream using the printf format in "format". //-- size_t Stream::PutSLEB128(int64_t sval) { - size_t bytes_written = 0; - if (m_flags.Test(eBinary)) { -bool more = true; -while (more) { - uint8_t byte = sval & 0x7fu; - sval >>= 7; - /* sign bit of byte is 2nd high order bit (0x40) */ - if ((sval == 0 && !(byte & 0x40)) || (sval == -1 && (byte & 0x40))) -more = false; - else -// more bytes to come -byte |= 0x80u; - bytes_written += Write(&byte, 1); -} - } else { -bytes_written = Printf("0x%" PRIi64, sval); - } - - return bytes_written; + if (m_flags.Test(eBinary)) +return llvm::encodeSLEB128(sval, m_forward); + else +return Printf("0x%" PRIi64, sval); } //-- // Put an ULEB128 "uval" out to the stream using the printf format in "format". //-- size_t Stream::PutULEB128(uint64_t uval) { - size_t bytes_written = 0; - if (m_flags.Test(eBinary)) { -do { - - uint8_t byte = uval & 0x7fu; - uval >>= 7; - if (uval != 0) { -// more bytes to come -byte |= 0x80u; - } - bytes_written += Write(&byte, 1); -} while (uval != 0); - } else { -bytes_written = Printf("0x%" PRIx64, uval); - } - return bytes_written; + if (m_flags.Test(eBinary)) +return llvm::encodeULEB128(uval, m_forward); + else +return Printf("0x%" PRIx64, uval); } //-- Index: source/Utility/Stream.cpp === --- source/Utility/Stream.cpp +++ source/Utility/Stream.cpp @@ -12,6 +12,7 @@ #include "lldb/Utility/Endian.h" #include "lldb/Utility/VASPrintf.h" #include "llvm/ADT/SmallString.h" // for SmallString +#include "llvm/Support/LEB128.h" #include @@ -49,47 +50,20 @@ // Put an SLEB128 "uval" out to the stream using the printf format in "format". //-- size_t Stream::PutSLEB128(int64_t sval) { - size_t bytes_written = 0; - if (m_flags.Test(eBinary)) { -bool more = true; -while (more) { - uint8_t byte = sval & 0x7fu; - sval >>= 7; - /* sign bit of byte is 2nd high order bit (0x40) */ - if ((sval == 0 && !(byte & 0x40)) || (sval == -1 && (byte & 0x40))) -more = false; - else -// more bytes to come -byte |= 0x80u; - bytes_written += Write(&byte, 1); -} - } else { -bytes_written = Printf("0x%" PRIi64, sval); - } - - return bytes_written; + if (m_flags.Test(eBinary)) +return llvm::encodeSLEB128(sval, m_forward); + else +return Printf("0x%" PRIi64, sval); } //-- // Put an ULEB128 "uval" out to the stream using the printf format in "format". //-- size_t Stream::PutULEB128(uint64_t uval) { - size_t bytes_written = 0; - if (m_flags.Test(eBinary)) { -do { - - uint8_t byte = uval & 0x7fu; - uval >>= 7; - if (uval != 0) { -// more bytes to come -byte |= 0x80u; - } - bytes_written += Write(&byte, 1); -} while (uval != 0); - } else { -bytes_written = Printf("0x%" PRIx64, uval); - } - return bytes_written; + if (m_flags.Test(eBinary)) +return llvm::encodeULEB128(uval, m_forward); + else +return Printf("0x%" PRIx64, uval); } //-- ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49949: Add missing boundary checks to variable completion.
jingham added a comment. One trivial fix to the test case. Repository: rL LLVM https://reviews.llvm.org/D49949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
shafik updated this revision to Diff 158662. shafik added a comment. Simplifying initialization of has_optional in test. https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp source/Plugins/Language/CPlusPlus/CMakeLists.txt source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp source/Plugins/Language/CPlusPlus/LibCxx.cpp source/Plugins/Language/CPlusPlus/LibCxx.h source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp === --- /dev/null +++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp @@ -0,0 +1,85 @@ +//===-- LibCxxOptional.cpp --*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "LibCxx.h" +#include "lldb/DataFormatters/FormattersHelpers.h" + +using namespace lldb; +using namespace lldb_private; + +namespace { + +class OptionalFrontEnd : public SyntheticChildrenFrontEnd { +public: + OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) { +Update(); + } + + size_t GetIndexOfChildWithName(const ConstString &name) override { +return formatters::ExtractIndexFromString(name.GetCString()); + } + + bool MightHaveChildren() override { return true; } + bool Update() override; + size_t CalculateNumChildren() override { return m_size; } + ValueObjectSP GetChildAtIndex(size_t idx) override; + +private: + size_t m_size = 0; + ValueObjectSP m_base_sp; +}; +} // namespace + +bool OptionalFrontEnd::Update() { + ValueObjectSP engaged_sp( + m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)); + + if (!engaged_sp) +return false; + + // __engaged_ is a bool flag and is true if the optional contains a value. + // Converting it to unsigned gives us a size of 1 if it contains a value + // and 0 if not. + m_size = engaged_sp->GetValueAsUnsigned(0); + + return false; +} + +ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) { + if (idx >= m_size) +return ValueObjectSP(); + + // __val_ contains the underlying value of an optional if it has one. + // Currently because it is part of an anonymous union GetChildMemberWithName() + // does not peer through and find it unless we are at the parent itself. + // We can obtain the parent through __engaged_. + ValueObjectSP val_sp( + m_backend.GetChildMemberWithName(ConstString("__engaged_"), true) + ->GetParent() + ->GetChildAtIndex(0, true) + ->GetChildMemberWithName(ConstString("__val_"), true)); + + if (!val_sp) +return ValueObjectSP(); + + CompilerType holder_type = val_sp->GetCompilerType(); + + if (!holder_type) +return ValueObjectSP(); + + return val_sp->Clone(ConstString(llvm::formatv("Value").str())); +} + +SyntheticChildrenFrontEnd * +formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *, + lldb::ValueObjectSP valobj_sp) { + if (valobj_sp) +return new OptionalFrontEnd(*valobj_sp); + return nullptr; +} Index: source/Plugins/Language/CPlusPlus/LibCxx.h === --- source/Plugins/Language/CPlusPlus/LibCxx.h +++ source/Plugins/Language/CPlusPlus/LibCxx.h @@ -27,6 +27,10 @@ ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options); // libc++ std::wstring +bool LibcxxOptionalSummaryProvider( +ValueObject &valobj, Stream &stream, +const TypeSummaryOptions &options); // libc++ std::optional<> + bool LibcxxSmartPointerSummaryProvider( ValueObject &valobj, Stream &stream, const TypeSummaryOptions @@ -133,6 +137,10 @@ SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *, lldb::ValueObjectSP); +SyntheticChildrenFrontEnd * +LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *, + lldb::ValueObjectSP valobj_sp); + } // namespace formatters } // namespace lldb_private Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp === --- source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -33,6 +33,28 @@ using namespace lldb_private; using namespace lldb_private::formatters; +bool lldb_private::formatters::LibcxxOp
[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional
shafik marked 4 inline comments as done. shafik added a comment. @labath Addressed comment, thank you for helping out. Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21 +@add_test_categories(["libc++"]) +@skipIf(oslist=no_match(["macosx"]), compiler="clang", compiler_version=['<', '5.0']) + labath wrote: > shafik wrote: > > labath wrote: > > > Could you add another line for `gcc` here? The -std=c++17 flag seems to > > > be supported starting with gcc-5.1. > > > > > > Also a comment that this is due to the -std flag would be helpful to > > > people looking at this in the future. > > Adding a comment makes sense. > > > > So `@add_test_categories(["libc++"])` won't skip for gcc bots then? > It won't because it doesn't make sense to do that. Gcc is perfectly capable > of using libc++. The only tricky thing is that it doesn't have the magic > `-stdlib` argument, so you have to specify the include paths and libraries > explicitly (which our makefiles do). Good point I did not realize this before! Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp:17-20 +bool has_optional = false ; +#if HAVE_OPTIONAL == 1 +has_optional = true ; +#endif labath wrote: > This could be simplified to `bool has_optional = HAVE_OPTIONAL;` Good catch! https://reviews.llvm.org/D49271 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338657 - Remove unnecessary target from TestCompletion patch
Author: teemperor Date: Wed Aug 1 16:54:37 2018 New Revision: 338657 URL: http://llvm.org/viewvc/llvm-project?rev=338657&view=rev Log: Remove unnecessary target from TestCompletion patch As Jim pointed out, we don't need to manually create a target here because we already create a target implicitly in the very next line (which means we just created a target and don't use it). This patch just removes the line that creates the first unused target. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=338657&r1=338656&r2=338657&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py Wed Aug 1 16:54:37 2018 @@ -43,7 +43,6 @@ class CommandLineCompletionTestCase(Test self.build() self.main_source = "main.cpp" self.main_source_spec = lldb.SBFileSpec(self.main_source) -self.dbg.CreateTarget(self.getBuildArtifact("a.out")) (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, '// Break here', self.main_source_spec) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r338662 - [LLDB] Added syntax highlighting support
Author: teemperor Date: Wed Aug 1 17:30:15 2018 New Revision: 338662 URL: http://llvm.org/viewvc/llvm-project?rev=338662&view=rev Log: [LLDB] Added syntax highlighting support Summary: This patch adds syntax highlighting support to LLDB. When enabled (and lldb is allowed to use colors), printed source code is annotated with the ANSI color escape sequences. So far we have only one highlighter which is based on Clang and is responsible for all languages that are supported by Clang. It essentially just runs the raw lexer over the input and then surrounds the specific tokens with the configured escape sequences. Reviewers: zturner, davide Reviewed By: davide Subscribers: labath, teemperor, llvm-commits, mgorny, lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D49334 Added: lldb/trunk/include/lldb/Core/Highlighter.h lldb/trunk/source/Core/Highlighter.cpp lldb/trunk/source/Plugins/Language/ClangCommon/ lldb/trunk/source/Plugins/Language/ClangCommon/CMakeLists.txt lldb/trunk/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp lldb/trunk/source/Plugins/Language/ClangCommon/ClangHighlighter.h lldb/trunk/unittests/Language/Highlighting/ lldb/trunk/unittests/Language/Highlighting/CMakeLists.txt lldb/trunk/unittests/Language/Highlighting/HighlighterTest.cpp Modified: lldb/trunk/include/lldb/Core/Debugger.h lldb/trunk/include/lldb/Target/Language.h lldb/trunk/packages/Python/lldbsuite/test/source-manager/TestSourceManager.py lldb/trunk/source/Core/CMakeLists.txt lldb/trunk/source/Core/Debugger.cpp lldb/trunk/source/Core/SourceManager.cpp lldb/trunk/source/Plugins/Language/CMakeLists.txt lldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h lldb/trunk/source/Plugins/Language/Go/GoLanguage.cpp lldb/trunk/source/Plugins/Language/Go/GoLanguage.h lldb/trunk/source/Plugins/Language/Java/JavaLanguage.cpp lldb/trunk/source/Plugins/Language/Java/JavaLanguage.h lldb/trunk/source/Plugins/Language/OCaml/OCamlLanguage.cpp lldb/trunk/source/Plugins/Language/OCaml/OCamlLanguage.h lldb/trunk/source/Plugins/Language/ObjC/CMakeLists.txt lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h lldb/trunk/source/Plugins/Language/ObjCPlusPlus/CMakeLists.txt lldb/trunk/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp lldb/trunk/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h lldb/trunk/source/Target/Language.cpp lldb/trunk/unittests/Language/CMakeLists.txt Modified: lldb/trunk/include/lldb/Core/Debugger.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=338662&r1=338661&r2=338662&view=diff == --- lldb/trunk/include/lldb/Core/Debugger.h (original) +++ lldb/trunk/include/lldb/Core/Debugger.h Wed Aug 1 17:30:15 2018 @@ -272,6 +272,8 @@ public: bool SetUseColor(bool use_color); + bool GetHighlightSource() const; + lldb::StopShowColumn GetStopShowColumn() const; const FormatEntity::Entry *GetStopShowColumnAnsiPrefix() const; Added: lldb/trunk/include/lldb/Core/Highlighter.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Highlighter.h?rev=338662&view=auto == --- lldb/trunk/include/lldb/Core/Highlighter.h (added) +++ lldb/trunk/include/lldb/Core/Highlighter.h Wed Aug 1 17:30:15 2018 @@ -0,0 +1,159 @@ +//===-- Highlighter.h ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef liblldb_Highlighter_h_ +#define liblldb_Highlighter_h_ + +#include +#include + +#include "lldb/Utility/Stream.h" +#include "lldb/lldb-enumerations.h" +#include "llvm/ADT/StringRef.h" + +namespace lldb_private { + +//-- +/// Represents style that the highlighter should apply to the given source code. +/// Stores information about how every kind of token should be annotated. +//-- +struct HighlightStyle { + + //-- + /// A pair of strings that should be placed around a certain token. Usually + /// stores color codes in these strings (the suffix string is often used for + /// resetting the terminal attributes back to normal). + //-
[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support
This revision was automatically updated to reflect the committed changes. Closed by commit rL338662: [LLDB] Added syntax highlighting support (authored by teemperor, committed by ). Changed prior to commit: https://reviews.llvm.org/D49334?vs=157702&id=158671#toc Repository: rL LLVM https://reviews.llvm.org/D49334 Files: lldb/trunk/include/lldb/Core/Debugger.h lldb/trunk/include/lldb/Core/Highlighter.h lldb/trunk/include/lldb/Target/Language.h lldb/trunk/packages/Python/lldbsuite/test/source-manager/TestSourceManager.py lldb/trunk/source/Core/CMakeLists.txt lldb/trunk/source/Core/Debugger.cpp lldb/trunk/source/Core/Highlighter.cpp lldb/trunk/source/Core/SourceManager.cpp lldb/trunk/source/Plugins/Language/CMakeLists.txt lldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h lldb/trunk/source/Plugins/Language/ClangCommon/CMakeLists.txt lldb/trunk/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp lldb/trunk/source/Plugins/Language/ClangCommon/ClangHighlighter.h lldb/trunk/source/Plugins/Language/Go/GoLanguage.cpp lldb/trunk/source/Plugins/Language/Go/GoLanguage.h lldb/trunk/source/Plugins/Language/Java/JavaLanguage.cpp lldb/trunk/source/Plugins/Language/Java/JavaLanguage.h lldb/trunk/source/Plugins/Language/OCaml/OCamlLanguage.cpp lldb/trunk/source/Plugins/Language/OCaml/OCamlLanguage.h lldb/trunk/source/Plugins/Language/ObjC/CMakeLists.txt lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h lldb/trunk/source/Plugins/Language/ObjCPlusPlus/CMakeLists.txt lldb/trunk/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp lldb/trunk/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h lldb/trunk/source/Target/Language.cpp lldb/trunk/unittests/Language/CMakeLists.txt lldb/trunk/unittests/Language/Highlighting/CMakeLists.txt lldb/trunk/unittests/Language/Highlighting/HighlighterTest.cpp Index: lldb/trunk/unittests/Language/Highlighting/HighlighterTest.cpp === --- lldb/trunk/unittests/Language/Highlighting/HighlighterTest.cpp +++ lldb/trunk/unittests/Language/Highlighting/HighlighterTest.cpp @@ -0,0 +1,221 @@ +//===-- HighlighterTest.cpp -*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "gtest/gtest.h" + +#include "lldb/Core/Highlighter.h" + +#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" +#include "Plugins/Language/Go/GoLanguage.h" +#include "Plugins/Language/Java/JavaLanguage.h" +#include "Plugins/Language/OCaml/OCamlLanguage.h" +#include "Plugins/Language/ObjC/ObjCLanguage.h" +#include "Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h" + +using namespace lldb_private; + +namespace { +class HighlighterTest : public testing::Test { +public: + static void SetUpTestCase(); + static void TearDownTestCase(); +}; +} // namespace + +void HighlighterTest::SetUpTestCase() { + // The HighlighterManager uses the language plugins under the hood, so we + // have to initialize them here for our test process. + CPlusPlusLanguage::Initialize(); + GoLanguage::Initialize(); + JavaLanguage::Initialize(); + ObjCLanguage::Initialize(); + ObjCPlusPlusLanguage::Initialize(); + OCamlLanguage::Initialize(); +} + +void HighlighterTest::TearDownTestCase() { + CPlusPlusLanguage::Terminate(); + GoLanguage::Terminate(); + JavaLanguage::Terminate(); + ObjCLanguage::Terminate(); + ObjCPlusPlusLanguage::Terminate(); + OCamlLanguage::Terminate(); +} + +static std::string getName(lldb::LanguageType type) { + HighlighterManager m; + return m.getHighlighterFor(type, "").GetName().str(); +} + +static std::string getName(llvm::StringRef path) { + HighlighterManager m; + return m.getHighlighterFor(lldb::eLanguageTypeUnknown, path).GetName().str(); +} + +TEST_F(HighlighterTest, HighlighterSelectionType) { + EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus), "clang"); + EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_03), "clang"); + EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_11), "clang"); + EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_14), "clang"); + EXPECT_EQ(getName(lldb::eLanguageTypeObjC), "clang"); + EXPECT_EQ(getName(lldb::eLanguageTypeObjC_plus_plus), "clang"); + + EXPECT_EQ(getName(lldb::eLanguageTypeUnknown), "none"); + EXPECT_EQ(getName(lldb::eLanguageTypeJulia), "none"); + EXPECT_EQ(getName(lldb::eLanguageTypeJava), "none"); + EXPECT_EQ(getName(lldb::eLanguageTypeHaskell), "none"); +} + +TEST_F(HighlighterTest, HighlighterSelectionPath) { + EXPECT_EQ(getName("my
[Lldb-commits] [lldb] r338669 - Added missing highlighter files to XCode project
Author: teemperor Date: Wed Aug 1 20:01:09 2018 New Revision: 338669 URL: http://llvm.org/viewvc/llvm-project?rev=338669&view=rev Log: Added missing highlighter files to XCode project Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=338669&r1=338668&r2=338669&view=diff == --- lldb/trunk/lldb.xcodeproj/project.pbxproj (original) +++ lldb/trunk/lldb.xcodeproj/project.pbxproj Wed Aug 1 20:01:09 2018 @@ -158,6 +158,8 @@ 268900D313353E6F00698AC0 /* ClangExternalASTSourceCallbacks.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26E69030129C6BEF00DDECD9 /* ClangExternalASTSourceCallbacks.cpp */; }; 4966DCC4148978A10028481B /* ClangExternalASTSourceCommon.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4966DCC3148978A10028481B /* ClangExternalASTSourceCommon.cpp */; }; 2689005F13353E0E00698AC0 /* ClangFunctionCaller.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4C98D3DA118FB96F00E575D0 /* ClangFunctionCaller.cpp */; }; + 58A080AC2112AABB00D5580F /* ClangHighlighter.cpp in CopyFiles */ = {isa = PBXBuildFile; fileRef = 58A080AB2112AABB00D5580F /* ClangHighlighter.cpp */; }; + 58A080AE2112AAC500D5580F /* ClangHighlighter.h in CopyFiles */ = {isa = PBXBuildFile; fileRef = 58A080AD2112AAC500D5580F /* ClangHighlighter.h */; }; 4CD44D5820C603CB0003557C /* ClangHost.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4CD44D5620C603A80003557C /* ClangHost.cpp */; }; 4959511F1A1BC4BC00F6F8FC /* ClangModulesDeclVendor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4959511E1A1BC4BC00F6F8FC /* ClangModulesDeclVendor.cpp */; }; 2689006313353E0E00698AC0 /* ClangPersistentVariables.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 49D4FE871210B61C00CDB854 /* ClangPersistentVariables.cpp */; }; @@ -336,6 +338,8 @@ AE44FB301BB07EB20033EB62 /* GoUserExpression.cpp in Sources */ = {isa = PBXBuildFile; fileRef = AE44FB2C1BB07DD80033EB62 /* GoUserExpression.cpp */; }; 6D95DC011B9DC057000E318A /* HashedNameToDIE.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6D95DBFE1B9DC057000E318A /* HashedNameToDIE.cpp */; }; 2666ADC81B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2666ADC31B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp */; }; + 58A080B42112AB3800D5580F /* Highlighter.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 58A080A92112AA9400D5580F /* Highlighter.cpp */; }; + 58A080B32112AB2900D5580F /* HighlighterTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 58A080B12112AB2200D5580F /* HighlighterTest.cpp */; }; AF1729D6182C907200E0AB97 /* HistoryThread.cpp in Sources */ = {isa = PBXBuildFile; fileRef = AF1729D4182C907200E0AB97 /* HistoryThread.cpp */; }; AF1729D7182C907200E0AB97 /* HistoryUnwind.cpp in Sources */ = {isa = PBXBuildFile; fileRef = AF1729D5182C907200E0AB97 /* HistoryUnwind.cpp */; }; 2689007113353E1A00698AC0 /* Host.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 69A01E1C1236C5D400C660B5 /* Host.cpp */; }; @@ -1279,6 +1283,8 @@ dstPath = "$(DEVELOPER_INSTALL_DIR)/usr/share/man/man1"; dstSubfolderSpec = 0; files = ( + 58A080AE2112AAC500D5580F /* ClangHighlighter.h in CopyFiles */, + 58A080AC2112AABB00D5580F /* ClangHighlighter.cpp in CopyFiles */, AF90106515AB7D3600FF120D /* lldb.1 in CopyFiles */, ); runOnlyForDeploymentPostprocessing = 1; @@ -1518,6 +1524,8 @@ 26BC7D5510F1B77400F91463 /* ClangForward.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ClangForward.h; path = include/lldb/Core/ClangForward.h; sourceTree = ""; }; 4C98D3DA118FB96F00E575D0 /* ClangFunctionCaller.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ClangFunctionCaller.cpp; path = ExpressionParser/Clang/ClangFunctionCaller.cpp; sourceTree = ""; }; 4C98D3E0118FB98F00E575D0 /* ClangFunctionCaller.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ClangFunctionCaller.h; path = ExpressionParser/Clang/ClangFunctionCaller.h; sourceTree = ""; }; + 58A080AB2112AABB00D5580F /* ClangHighlighter.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ClangHighlighter.cpp; path = Language/ClangCommon/ClangHighlighter.cpp; sourceTree = ""