Re: [Lldb-commits] [PATCH] D24514: Add support for DW_AT_ranges_base attribute
labath added a comment. Looks straight-forward enough. However, it could use a test-case. https://reviews.llvm.org/D24514 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D24549: [LLDB][MIPS] Skip some test case which were causing LLDB to go into infinite loop
nitesh.jain created this revision. nitesh.jain added reviewers: clayborg, labath. nitesh.jain added subscribers: jaydeep, bhushan, slthakur, lldb-commits. These test cases tries to insert breakpoint in atomic sequence and cause atomic sequence to restart when breakpoint is hit . https://reviews.llvm.org/D24549 Files: packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoint_delay_breakpoint_one_signal/TestConcurrentBreakpointDelayBreakpointOneSignal.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoint_one_delay_breakpoint_threads/TestConcurrentBreakpointOneDelayBreakpointThreads.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/breakpoints_delayed_breakpoint_one_watchpoint/TestConcurrentBreakpointsDelayedBreakpointOneWatchpoint.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/crash_with_break/TestConcurrentCrashWithBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/crash_with_signal/TestConcurrentCrashWithSignal.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/crash_with_watchpoint/TestConcurrentCrashWithWatchpoint.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/crash_with_watchpoint_breakpoint_signal/TestConcurrentCrashWithWatchpointBreakpointSignal.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delay_signal_break/TestConcurrentDelaySignalBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delay_signal_watch/TestConcurrentDelaySignalWatch.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delay_watch_break/TestConcurrentDelayWatchBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delayed_crash_with_breakpoint_signal/TestConcurrentDelayedCrashWithBreakpointSignal.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/delayed_crash_with_breakpoint_watchpoint/TestConcurrentDelayedCrashWithBreakpointWatchpoint.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/many_breakpoints/TestConcurrentManyBreakpoints.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/many_crash/TestConcurrentManyCrash.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/many_signals/TestConcurrentManySignals.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/many_watchpoints/TestConcurrentManyWatchpoints.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/n_watch_n_break/TestConcurrentNWatchNBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_break/TestConcurrentSignalBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_delay_break/TestConcurrentSignalDelayBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_delay_watch/TestConcurrentSignalDelayWatch.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_n_watch_n_break/TestConcurrentSignalNWatchNBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_watch/TestConcurrentSignalWatch.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/signal_watch_break/TestConcurrentSignalWatchBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_breakpoint_threads/TestConcurrentTwoBreakpointThreads.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_breakpoints_one_delay_signal/TestConcurrentTwoBreakpointsOneDelaySignal.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_breakpoints_one_signal/TestConcurrentTwoBreakpointsOneSignal.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_breakpoints_one_watchpoint/TestConcurrentTwoBreakpointsOneWatchpoint.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoint_threads/TestConcurrentTwoWatchpointThreads.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_breakpoint/TestConcurrentTwoWatchpointsOneBreakpoint.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_delay_breakpoint/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_signal/TestConcurrentTwoWatchpointsOneSignal.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/watch_break/TestConcurrentWatchBreak.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/watch_break_delay/TestConcurrentWatchBreakDelay.py packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/watchpoint_delay_watchpoint_one_breakpoint/TestCon
Re: [Lldb-commits] [PATCH] D24549: [LLDB][MIPS] Skip some test case which were causing LLDB to go into infinite loop
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. lgtm. In general, feel free to manage mips decorators as you see fit. https://reviews.llvm.org/D24549 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24514: Add support for DW_AT_ranges_base attribute
tberghammer updated this revision to Diff 71316.
tberghammer added a comment.
Adding a new test case
https://reviews.llvm.org/D24514
Files:
packages/Python/lldbsuite/test/python_api/symbol-context/two-files/TestSymbolContextTwoFiles.py
packages/Python/lldbsuite/test/python_api/symbol-context/two-files/decls.h
packages/Python/lldbsuite/test/python_api/symbol-context/two-files/file1.cpp
packages/Python/lldbsuite/test/python_api/symbol-context/two-files/file2.cpp
source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3810,7 +3810,8 @@
if (form_value.Form() == DW_FORM_sec_offset) {
DWARFRangeList dwarf_scope_ranges;
const DWARFDebugRanges *debug_ranges = DebugRanges();
- debug_ranges->FindRanges(form_value.Unsigned(),
+ debug_ranges->FindRanges(die.GetCU()->GetRangesBase(),
+ form_value.Unsigned(),
dwarf_scope_ranges);
// All DW_AT_start_scope are relative to the base address of the
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
@@ -23,7 +23,8 @@
static void Dump(lldb_private::Stream &s,
const lldb_private::DWARFDataExtractor &debug_ranges_data,
lldb::offset_t *offset_ptr, dw_addr_t cu_base_addr);
- bool FindRanges(dw_offset_t debug_ranges_offset,
+ bool FindRanges(dw_addr_t debug_ranges_base,
+ dw_offset_t debug_ranges_offset,
DWARFRangeList &range_list) const;
protected:
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
@@ -118,9 +118,11 @@
}
}
-bool DWARFDebugRanges::FindRanges(dw_offset_t debug_ranges_offset,
+bool DWARFDebugRanges::FindRanges(dw_addr_t debug_ranges_base,
+ dw_offset_t debug_ranges_offset,
DWARFRangeList &range_list) const {
- range_map_const_iterator pos = m_range_map.find(debug_ranges_offset);
+ dw_addr_t debug_ranges_address = debug_ranges_base + debug_ranges_offset;
+ range_map_const_iterator pos = m_range_map.find(debug_ranges_address);
if (pos != m_range_map.end()) {
range_list = pos->second;
return true;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -450,7 +450,7 @@
case DW_AT_ranges: {
const DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges();
if (debug_ranges) {
-debug_ranges->FindRanges(form_value.Unsigned(), ranges);
+debug_ranges->FindRanges(cu->GetRangesBase(), form_value.Unsigned(), ranges);
// All DW_AT_ranges are relative to the base address of the
// compile unit. We add the compile unit base address to make
// sure all the addresses are properly fixed up.
@@ -1108,7 +1108,7 @@
if (debug_ranges_offset != DW_INVALID_OFFSET) {
DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges();
-debug_ranges->FindRanges(debug_ranges_offset, ranges);
+debug_ranges->FindRanges(cu->GetRangesBase(), debug_ranges_offset, ranges);
ranges.Slide(cu->GetBaseAddress());
} else if (check_hi_lo_pc) {
dw_addr_t lo_pc = LLDB_INVALID_ADDRESS;
@@ -1772,7 +1772,7 @@
if (debug_ranges_offset != DW_INVALID_OFFSET) {
DWARFRangeList ranges;
DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges();
- debug_ranges->FindRanges(debug_ranges_offset, ranges);
+ debug_ranges->FindRanges(cu->GetRangesBase(), debug_ranges_offset, ranges);
// All DW_AT_ranges are relative to the base address of the
// compile unit. We add the compile unit base address to make
// sure all the addresses are properly fixed up.
Index: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFCom
Re: [Lldb-commits] [PATCH] D24514: Add support for DW_AT_ranges_base attribute
labath added a comment. Thanks for adding the test case. LGTM from my side. https://reviews.llvm.org/D24514 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
tfiala added a comment. In https://reviews.llvm.org/D20835#541901, @jingham wrote: > See inlined comments. Otherwise this is great. Great, I'll fix those up. I like the command-line option idea. https://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24559: Use Intel CPU flags to determine target supported features.
labath added a comment.
I have to admit I have very little knowledge of this part of the code. Could
you provide a bit of a high-level overview of this change? Does this fix an
existing problem ? (If so, should it have a test?) Or is it just a refactor?
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:12
@@ -11,1 +11,3 @@
+#include
+
Please include the system header last.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:187
@@ +186,3 @@
+// Codes for register sets.
+enum { gpr, fpu, avx, mpx };
+
I like the short names, but let's make this an `enum class`, so the names don't
leak into the global namespace.
https://reviews.llvm.org/D24559
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24559: Use Intel CPU flags to determine target supported features.
valentinagiusti marked 2 inline comments as done. valentinagiusti added a comment. This fixes the fact that there is no proper check that the kernel or the hardware are actually supporting either AVX or MPX. Before this patch, the code only relied on a "hack" that checks if it's possible to do a ptrace to retrieve the XSAVE or FXSAVE areas: the assumption was that if XSAVE is there, then there must be also AVX and MPX, which obviously is not the correct thing to do. The 'cpuid' calls (wrappers for the CPUID instruction) get the info directly from the hardware, and then the ptrace call is made to actually get either FXSAVE or XSAVE. If XSAVE is there, then 'cpuid' is used again to check the hardware for AVX and MPX, and then if this step is also successful, the XSAVE memory region is further checked to verify that the kernel is properly handling these features. Basically it's both a refactoring and a fix, and it doesn't require a dedicated test: the fact that the current register tests succeed is proof enough. https://reviews.llvm.org/D24559 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24514: Add support for DW_AT_ranges_base attribute
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D24514 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24559: Use Intel CPU flags to determine target supported features.
valentinagiusti updated this revision to Diff 71371.
valentinagiusti added a comment.
moved header to the bottom and moved enum into header file
https://reviews.llvm.org/D24559
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
source/Plugins/Process/Utility/RegisterContext_x86.h
Index: source/Plugins/Process/Utility/RegisterContext_x86.h
===
--- source/Plugins/Process/Utility/RegisterContext_x86.h
+++ source/Plugins/Process/Utility/RegisterContext_x86.h
@@ -277,7 +277,9 @@
uint32_t mxcsrmask; // MXCSR Mask
MMSReg stmm[8]; // 8*16 bytes for each FP-reg = 128 bytes
XMMReg xmm[16]; // 16*16 bytes for each XMM-reg = 256 bytes
- uint32_t padding[24];
+ uint8_t padding1[48];
+ uint64_t xcr0;
+ uint8_t padding2[40];
};
//---
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
@@ -77,7 +77,8 @@
private:
// Private member types.
- enum FPRType { eFPRTypeNotValid = 0, eFPRTypeFXSAVE, eFPRTypeXSAVE };
+ enum XStateType { eXStateTypeNotValid = 0, eXStateTypeFXSAVE, eXStateTypeXSAVE };
+ enum RegSet { gpr, fpu, avx, mpx };
// Info about register ranges.
struct RegInfo {
@@ -106,26 +107,30 @@
};
// Private member variables.
- mutable FPRType m_fpr_type;
- FPR m_fpr;
+ mutable XStateType m_xstate_type;
+ FPR m_fpr; // Extended States Area, named FPR for historical reasons.
IOVEC m_iovec;
YMM m_ymm_set;
MPX m_mpx_set;
RegInfo m_reg_info;
uint64_t m_gpr_x86_64[k_num_gpr_registers_x86_64];
uint32_t m_fctrl_offset_in_userarea;
// Private member methods.
+ bool HasFXSAVE() const;
+
+ bool HasXSAVE() const;
+
+ bool IsCPUFeatureAvailable(RegSet feature_code) const;
+
bool IsRegisterSetAvailable(uint32_t set_index) const;
bool IsGPR(uint32_t reg_index) const;
- FPRType GetFPRType() const;
+ XStateType GetXStateType() const;
bool IsFPR(uint32_t reg_index) const;
- bool IsFPR(uint32_t reg_index, FPRType fpr_type) const;
-
bool CopyXSTATEtoYMM(uint32_t reg_index, lldb::ByteOrder byte_order);
bool CopyYMMtoXSTATE(uint32_t reg, lldb::ByteOrder byte_order);
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -20,6 +20,8 @@
#include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
+#include
+
using namespace lldb_private;
using namespace lldb_private::process_linux;
@@ -218,6 +220,23 @@
#define NT_PRXFPREG 0x46e62b7f
#endif
+//
+// Required MPX define.
+//
+
+// Support MPX extensions also if compiled with compiler without MPX support.
+#ifndef bit_MPX
+#define bit_MPX 0x4000
+#endif
+
+//
+// XCR0 extended register sets masks.
+//
+#define mask_XSTATE_AVX (1ULL << 2)
+#define mask_XSTATE_BNDREGS (1ULL << 3)
+#define mask_XSTATE_BNDCFG (1ULL << 4)
+#define mask_XSTATE_MPX (mask_XSTATE_BNDREGS | mask_XSTATE_BNDCFG)
+
NativeRegisterContextLinux *
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
const ArchSpec &target_arch, NativeThreadProtocol &native_thread,
@@ -249,7 +268,7 @@
uint32_t concrete_frame_idx)
: NativeRegisterContextLinux(native_thread, concrete_frame_idx,
CreateRegisterInfoInterface(target_arch)),
- m_fpr_type(eFPRTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
+ m_xstate_type(eXStateTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
// Set up data about ranges of valid registers.
switch (target_arch.GetMachine()) {
@@ -379,7 +398,7 @@
return error;
}
- if (IsFPR(reg, GetFPRType())) {
+ if (IsFPR(reg) || IsAVX(reg) || IsMPX(reg)) {
error = ReadFPR();
if (error.Fail())
return error;
@@ -428,25 +447,25 @@
reg_info->byte_size, byte_order);
if (reg >= m_reg_info.first_ymm && reg <= m_reg_info.last_ymm) {
// Concatenate ymm using the register halves in xmm.bytes and ymmh.bytes
-if (GetFPRType() == eFPRTypeXSAVE && CopyXSTATEtoYMM(reg, byte_order))
+if
Re: [Lldb-commits] [PATCH] D24559: Use Intel CPU flags to determine target supported features.
Can you still make the enum an enum class? On Wed, Sep 14, 2016 at 9:07 AM Valentina Giusti via lldb-commits < lldb-commits@lists.llvm.org> wrote: > valentinagiusti updated this revision to Diff 71371. > valentinagiusti added a comment. > > moved header to the bottom and moved enum into header file > > > https://reviews.llvm.org/D24559 > > Files: > source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp > source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h > source/Plugins/Process/Utility/RegisterContext_x86.h > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
tfiala added a comment. Greg had a few pieces of feedback as well that we just discussed: - Change the on/off switch for column marking to support the following states: - terminal-code-only (i.e. only do terminal code highlighting, not the text caret) - terminal-code + text caret - text caret only - off - Add a pre and post terminal-code-format format string option for the marker. This allows the user to modify what kind of highlighting the user can do. It will default to the way I'm doing it now, which is ansi underline. https://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.
tfiala added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py:163 @@ +162,3 @@ +# mixed in with stop locations. +self.dbg.SetUseColor(False) + Upon further reflection, there is a logically more appropriate approach. The better one is to explicitly turn off column marking and not count on the fact that the column marker happens on the line below the stop match text. (This test initially failed because the stop text that is scraped was getting confused by the newly-added ansi escape sequences around the first character). https://reviews.llvm.org/D20835 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281507 - Use Intel CPU flags to determine target supported features.
Author: valentinagiusti
Date: Wed Sep 14 12:27:48 2016
New Revision: 281507
URL: http://llvm.org/viewvc/llvm-project?rev=281507&view=rev
Log:
Use Intel CPU flags to determine target supported features.
Summary:
This patch uses the instruction CPUID to verify that FXSAVE, XSAVE, AVX
and MPX are supported by the target hardware. In case the HW supports XSAVE,
and at least one of the extended register sets, it further checks if the
target software has the kernel support for such features, by verifying that
their XSAVE part is correctly managed.
Differential Revision: https://reviews.llvm.org/D24559
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
lldb/trunk/source/Plugins/Process/Utility/RegisterContext_x86.h
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp?rev=281507&r1=281506&r2=281507&view=diff
==
---
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
(original)
+++
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
Wed Sep 14 12:27:48 2016
@@ -20,6 +20,8 @@
#include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
+#include
+
using namespace lldb_private;
using namespace lldb_private::process_linux;
@@ -218,6 +220,23 @@ static const RegisterSet g_reg_sets_x86_
#define NT_PRXFPREG 0x46e62b7f
#endif
+//
+// Required MPX define.
+//
+
+// Support MPX extensions also if compiled with compiler without MPX support.
+#ifndef bit_MPX
+#define bit_MPX 0x4000
+#endif
+
+//
+// XCR0 extended register sets masks.
+//
+#define mask_XSTATE_AVX (1ULL << 2)
+#define mask_XSTATE_BNDREGS (1ULL << 3)
+#define mask_XSTATE_BNDCFG (1ULL << 4)
+#define mask_XSTATE_MPX (mask_XSTATE_BNDREGS | mask_XSTATE_BNDCFG)
+
NativeRegisterContextLinux *
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
const ArchSpec &target_arch, NativeThreadProtocol &native_thread,
@@ -249,7 +268,7 @@ NativeRegisterContextLinux_x86_64::Nativ
uint32_t concrete_frame_idx)
: NativeRegisterContextLinux(native_thread, concrete_frame_idx,
CreateRegisterInfoInterface(target_arch)),
- m_fpr_type(eFPRTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
+ m_xstate_type(eXStateTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
// Set up data about ranges of valid registers.
switch (target_arch.GetMachine()) {
@@ -379,7 +398,7 @@ Error NativeRegisterContextLinux_x86_64:
return error;
}
- if (IsFPR(reg, GetFPRType())) {
+ if (IsFPR(reg) || IsAVX(reg) || IsMPX(reg)) {
error = ReadFPR();
if (error.Fail())
return error;
@@ -428,7 +447,7 @@ Error NativeRegisterContextLinux_x86_64:
reg_info->byte_size, byte_order);
if (reg >= m_reg_info.first_ymm && reg <= m_reg_info.last_ymm) {
// Concatenate ymm using the register halves in xmm.bytes and
ymmh.bytes
-if (GetFPRType() == eFPRTypeXSAVE && CopyXSTATEtoYMM(reg, byte_order))
+if (CopyXSTATEtoYMM(reg, byte_order))
reg_value.SetBytes(m_ymm_set.ymm[reg - m_reg_info.first_ymm].bytes,
reg_info->byte_size, byte_order);
else {
@@ -437,7 +456,7 @@ Error NativeRegisterContextLinux_x86_64:
}
}
if (reg >= m_reg_info.first_mpxr && reg <= m_reg_info.last_mpxr) {
-if (GetFPRType() == eFPRTypeXSAVE && CopyXSTATEtoMPX(reg))
+if (CopyXSTATEtoMPX(reg))
reg_value.SetBytes(m_mpx_set.mpxr[reg - m_reg_info.first_mpxr].bytes,
reg_info->byte_size, byte_order);
else {
@@ -446,7 +465,7 @@ Error NativeRegisterContextLinux_x86_64:
}
}
if (reg >= m_reg_info.first_mpxc && reg <= m_reg_info.last_mpxc) {
-if (GetFPRType() == eFPRTypeXSAVE && CopyXSTATEtoMPX(reg))
+if (CopyXSTATEtoMPX(reg))
reg_value.SetBytes(m_mpx_set.mpxc[reg - m_reg_info.first_mpxc].bytes,
reg_info->byte_size, byte_order);
else {
@@ -517,7 +536,7 @@ Error NativeRegisterContextLinux_x86_64:
if (IsGPR(reg_index))
return WriteRegisterRaw(reg_index, reg_value);
- if (IsFPR(reg_index, GetFPRType())) {
+ if (IsFPR(reg_index) || IsAVX(reg_index) || IsMPX(reg_ind
Re: [Lldb-commits] [PATCH] D24559: Use Intel CPU flags to determine target supported features.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281507: Use Intel CPU flags to determine target supported
features. (authored by valentinagiusti).
Changed prior to commit:
https://reviews.llvm.org/D24559?vs=71371&id=71390#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24559
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
lldb/trunk/source/Plugins/Process/Utility/RegisterContext_x86.h
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
@@ -77,7 +77,8 @@
private:
// Private member types.
- enum FPRType { eFPRTypeNotValid = 0, eFPRTypeFXSAVE, eFPRTypeXSAVE };
+ enum XStateType { eXStateTypeNotValid = 0, eXStateTypeFXSAVE, eXStateTypeXSAVE };
+ enum RegSet { gpr, fpu, avx, mpx };
// Info about register ranges.
struct RegInfo {
@@ -106,26 +107,30 @@
};
// Private member variables.
- mutable FPRType m_fpr_type;
- FPR m_fpr;
+ mutable XStateType m_xstate_type;
+ FPR m_fpr; // Extended States Area, named FPR for historical reasons.
IOVEC m_iovec;
YMM m_ymm_set;
MPX m_mpx_set;
RegInfo m_reg_info;
uint64_t m_gpr_x86_64[k_num_gpr_registers_x86_64];
uint32_t m_fctrl_offset_in_userarea;
// Private member methods.
+ bool HasFXSAVE() const;
+
+ bool HasXSAVE() const;
+
+ bool IsCPUFeatureAvailable(RegSet feature_code) const;
+
bool IsRegisterSetAvailable(uint32_t set_index) const;
bool IsGPR(uint32_t reg_index) const;
- FPRType GetFPRType() const;
+ XStateType GetXStateType() const;
bool IsFPR(uint32_t reg_index) const;
- bool IsFPR(uint32_t reg_index, FPRType fpr_type) const;
-
bool CopyXSTATEtoYMM(uint32_t reg_index, lldb::ByteOrder byte_order);
bool CopyYMMtoXSTATE(uint32_t reg, lldb::ByteOrder byte_order);
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -20,6 +20,8 @@
#include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
+#include
+
using namespace lldb_private;
using namespace lldb_private::process_linux;
@@ -218,6 +220,23 @@
#define NT_PRXFPREG 0x46e62b7f
#endif
+//
+// Required MPX define.
+//
+
+// Support MPX extensions also if compiled with compiler without MPX support.
+#ifndef bit_MPX
+#define bit_MPX 0x4000
+#endif
+
+//
+// XCR0 extended register sets masks.
+//
+#define mask_XSTATE_AVX (1ULL << 2)
+#define mask_XSTATE_BNDREGS (1ULL << 3)
+#define mask_XSTATE_BNDCFG (1ULL << 4)
+#define mask_XSTATE_MPX (mask_XSTATE_BNDREGS | mask_XSTATE_BNDCFG)
+
NativeRegisterContextLinux *
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
const ArchSpec &target_arch, NativeThreadProtocol &native_thread,
@@ -249,7 +268,7 @@
uint32_t concrete_frame_idx)
: NativeRegisterContextLinux(native_thread, concrete_frame_idx,
CreateRegisterInfoInterface(target_arch)),
- m_fpr_type(eFPRTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
+ m_xstate_type(eXStateTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
// Set up data about ranges of valid registers.
switch (target_arch.GetMachine()) {
@@ -379,7 +398,7 @@
return error;
}
- if (IsFPR(reg, GetFPRType())) {
+ if (IsFPR(reg) || IsAVX(reg) || IsMPX(reg)) {
error = ReadFPR();
if (error.Fail())
return error;
@@ -428,25 +447,25 @@
reg_info->byte_size, byte_order);
if (reg >= m_reg_info.first_ymm && reg <= m_reg_info.last_ymm) {
// Concatenate ymm using the register halves in xmm.bytes and ymmh.bytes
-if (GetFPRType() == eFPRTypeXSAVE && CopyXSTATEtoYMM(reg, byte_order))
+if (CopyXSTATEtoYMM(reg, byte_order))
reg_value.SetBytes(m_ymm_set.ymm[reg - m_reg_info.first_ymm].bytes,
reg_info->byte_size, byte_order);
else {
error.SetErrorString("failed to copy ymm register value");
return error;
}
}
if (reg >= m_reg_info.first_mpxr && reg <=
Re: [Lldb-commits] [lldb] r281507 - Use Intel CPU flags to determine target supported features.
On Wed, Sep 14, 2016 at 10:36 AM Valentina Giusti via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
>
> ==
> ---
> lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
> (original)
> +++
> lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
> Wed Sep 14 12:27:48 2016
> @@ -77,7 +77,8 @@ protected:
>
> private:
>// Private member types.
> - enum FPRType { eFPRTypeNotValid = 0, eFPRTypeFXSAVE, eFPRTypeXSAVE };
> + enum XStateType { eXStateTypeNotValid = 0, eXStateTypeFXSAVE,
> eXStateTypeXSAVE };
> + enum RegSet { gpr, fpu, avx, mpx };
>
This still isn't an enum class. You need to write:
enum class RegSet { gpr, fpu, avx, mpx };
Can you make this change and submit it as a followup please?
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r281507 - Use Intel CPU flags to determine target supported features.
Can probably do the same for XStateType as well:
enum class XStateType { Invalid, FXSAVE, XSAVE };
On Wed, Sep 14, 2016 at 10:46 AM Zachary Turner wrote:
> On Wed, Sep 14, 2016 at 10:36 AM Valentina Giusti via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>
> ==
> ---
> lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
> (original)
> +++
> lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
> Wed Sep 14 12:27:48 2016
> @@ -77,7 +77,8 @@ protected:
>
> private:
>// Private member types.
> - enum FPRType { eFPRTypeNotValid = 0, eFPRTypeFXSAVE, eFPRTypeXSAVE };
> + enum XStateType { eXStateTypeNotValid = 0, eXStateTypeFXSAVE,
> eXStateTypeXSAVE };
> + enum RegSet { gpr, fpu, avx, mpx };
>
>
> This still isn't an enum class. You need to write:
>
> enum class RegSet { gpr, fpu, avx, mpx };
>
> Can you make this change and submit it as a followup please?
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available
wallace updated this revision to Diff 71407.
wallace added a comment.
fix pointers
https://reviews.llvm.org/D24284
Files:
source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -50,6 +50,10 @@
const lldb_private::FileSpec *file,
lldb::offset_t file_offset, lldb::offset_t length);
+ ObjectFilePECOFF(const lldb::ModuleSP &module_sp,
+ lldb::DataBufferSP &header_data_sp,
+ const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
+
~ObjectFilePECOFF() override;
//--
@@ -131,29 +135,31 @@
bool
IsThumb();
-protected:
- bool NeedsEndianSwap() const;
-
- typedef struct dos_header { // DOS .EXE header
-uint16_t e_magic; // Magic number
-uint16_t e_cblp; // Bytes on last page of file
-uint16_t e_cp;// Pages in file
-uint16_t e_crlc; // Relocations
-uint16_t e_cparhdr; // Size of header in paragraphs
-uint16_t e_minalloc; // Minimum extra paragraphs needed
-uint16_t e_maxalloc; // Maximum extra paragraphs needed
-uint16_t e_ss;// Initial (relative) SS value
-uint16_t e_sp;// Initial SP value
-uint16_t e_csum; // Checksum
-uint16_t e_ip;// Initial IP value
-uint16_t e_cs;// Initial (relative) CS value
-uint16_t e_lfarlc;// File address of relocation table
-uint16_t e_ovno; // Overlay number
-uint16_t e_res[4];// Reserved words
-uint16_t e_oemid; // OEM identifier (for e_oeminfo)
-uint16_t e_oeminfo; // OEM information; e_oemid specific
-uint16_t e_res2[10]; // Reserved words
-uint32_t e_lfanew;// File address of new exe header
+lldb_private::DataExtractor ReadImageData(uint32_t offset, size_t size);
+
+ protected:
+bool NeedsEndianSwap() const;
+
+typedef struct dos_header { // DOS .EXE header
+ uint16_t e_magic; // Magic number
+ uint16_t e_cblp; // Bytes on last page of file
+ uint16_t e_cp;// Pages in file
+ uint16_t e_crlc; // Relocations
+ uint16_t e_cparhdr; // Size of header in paragraphs
+ uint16_t e_minalloc; // Minimum extra paragraphs needed
+ uint16_t e_maxalloc; // Maximum extra paragraphs needed
+ uint16_t e_ss;// Initial (relative) SS value
+ uint16_t e_sp;// Initial SP value
+ uint16_t e_csum; // Checksum
+ uint16_t e_ip;// Initial IP value
+ uint16_t e_cs;// Initial (relative) CS value
+ uint16_t e_lfarlc;// File address of relocation table
+ uint16_t e_ovno; // Overlay number
+ uint16_t e_res[4];// Reserved words
+ uint16_t e_oemid; // OEM identifier (for e_oeminfo)
+ uint16_t e_oeminfo; // OEM information; e_oemid specific
+ uint16_t e_res2[10]; // Reserved words
+ uint32_t e_lfanew;// File address of new exe header
} dos_header_t;
typedef struct coff_header {
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -10,10 +10,12 @@
#include "ObjectFilePECOFF.h"
#include "WindowsMiniDump.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/COFF.h"
#include "lldb/Core/ArchSpec.h"
#include "lldb/Core/DataBuffer.h"
+#include "lldb/Core/DataBufferHeap.h"
#include "lldb/Core/FileSpecList.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
@@ -83,6 +85,14 @@
ObjectFile *ObjectFilePECOFF::CreateMemoryInstance(
const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {
+ if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp)) {
+return NULL;
+ }
+ auto objfile_ap = llvm::make_unique(
+ module_sp, data_sp, process_sp, header_addr);
+ if (objfile_ap.get() && objfile_ap->ParseHeader()) {
+return objfile_ap.release();
+ }
return NULL;
}
@@ -164,6 +174,18 @@
::memset(&m_coff_header_opt, 0, sizeof(m_coff_header_opt));
}
+ObjectFilePECOFF::ObjectFilePECOFF(const lldb::ModuleSP &module_sp,
+ DataBufferSP &header_data_sp,
+ const lldb::ProcessSP &process_sp,
+ addr_t header_addr)
+: ObjectFile(module_sp, process
Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available
wallace updated this revision to Diff 71409.
wallace added a comment.
rebase
https://reviews.llvm.org/D24284
Files:
source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -50,6 +50,10 @@
const lldb_private::FileSpec *file,
lldb::offset_t file_offset, lldb::offset_t length);
+ ObjectFilePECOFF(const lldb::ModuleSP &module_sp,
+ lldb::DataBufferSP &header_data_sp,
+ const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
+
~ObjectFilePECOFF() override;
//--
@@ -128,29 +132,31 @@
uint32_t GetPluginVersion() override;
-protected:
- bool NeedsEndianSwap() const;
-
- typedef struct dos_header { // DOS .EXE header
-uint16_t e_magic; // Magic number
-uint16_t e_cblp; // Bytes on last page of file
-uint16_t e_cp;// Pages in file
-uint16_t e_crlc; // Relocations
-uint16_t e_cparhdr; // Size of header in paragraphs
-uint16_t e_minalloc; // Minimum extra paragraphs needed
-uint16_t e_maxalloc; // Maximum extra paragraphs needed
-uint16_t e_ss;// Initial (relative) SS value
-uint16_t e_sp;// Initial SP value
-uint16_t e_csum; // Checksum
-uint16_t e_ip;// Initial IP value
-uint16_t e_cs;// Initial (relative) CS value
-uint16_t e_lfarlc;// File address of relocation table
-uint16_t e_ovno; // Overlay number
-uint16_t e_res[4];// Reserved words
-uint16_t e_oemid; // OEM identifier (for e_oeminfo)
-uint16_t e_oeminfo; // OEM information; e_oemid specific
-uint16_t e_res2[10]; // Reserved words
-uint32_t e_lfanew;// File address of new exe header
+lldb_private::DataExtractor ReadImageData(uint32_t offset, size_t size);
+
+ protected:
+bool NeedsEndianSwap() const;
+
+typedef struct dos_header { // DOS .EXE header
+ uint16_t e_magic; // Magic number
+ uint16_t e_cblp; // Bytes on last page of file
+ uint16_t e_cp;// Pages in file
+ uint16_t e_crlc; // Relocations
+ uint16_t e_cparhdr; // Size of header in paragraphs
+ uint16_t e_minalloc; // Minimum extra paragraphs needed
+ uint16_t e_maxalloc; // Maximum extra paragraphs needed
+ uint16_t e_ss;// Initial (relative) SS value
+ uint16_t e_sp;// Initial SP value
+ uint16_t e_csum; // Checksum
+ uint16_t e_ip;// Initial IP value
+ uint16_t e_cs;// Initial (relative) CS value
+ uint16_t e_lfarlc;// File address of relocation table
+ uint16_t e_ovno; // Overlay number
+ uint16_t e_res[4];// Reserved words
+ uint16_t e_oemid; // OEM identifier (for e_oeminfo)
+ uint16_t e_oeminfo; // OEM information; e_oemid specific
+ uint16_t e_res2[10]; // Reserved words
+ uint32_t e_lfanew;// File address of new exe header
} dos_header_t;
typedef struct coff_header {
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -10,10 +10,12 @@
#include "ObjectFilePECOFF.h"
#include "WindowsMiniDump.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/COFF.h"
#include "lldb/Core/ArchSpec.h"
#include "lldb/Core/DataBuffer.h"
+#include "lldb/Core/DataBufferHeap.h"
#include "lldb/Core/FileSpecList.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
@@ -83,6 +85,14 @@
ObjectFile *ObjectFilePECOFF::CreateMemoryInstance(
const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {
+ if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp)) {
+return NULL;
+ }
+ auto objfile_ap = llvm::make_unique(
+ module_sp, data_sp, process_sp, header_addr);
+ if (objfile_ap.get() && objfile_ap->ParseHeader()) {
+return objfile_ap.release();
+ }
return NULL;
}
@@ -161,6 +171,18 @@
::memset(&m_coff_header_opt, 0, sizeof(m_coff_header_opt));
}
+ObjectFilePECOFF::ObjectFilePECOFF(const lldb::ModuleSP &module_sp,
+ DataBufferSP &header_data_sp,
+ const lldb::ProcessSP &process_sp,
+ addr_t header_addr)
+: ObjectFile(module_
[Lldb-commits] [lldb] r281519 - Fix some const-ness issues with BreakpointID & BreakpointIDList.
Author: jingham
Date: Wed Sep 14 14:05:27 2016
New Revision: 281519
URL: http://llvm.org/viewvc/llvm-project?rev=281519&view=rev
Log:
Fix some const-ness issues with BreakpointID & BreakpointIDList.
Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointID.h
lldb/trunk/include/lldb/Breakpoint/BreakpointIDList.h
lldb/trunk/source/Breakpoint/BreakpointIDList.cpp
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointID.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointID.h?rev=281519&r1=281518&r2=281519&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointID.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointID.h Wed Sep 14 14:05:27 2016
@@ -30,9 +30,9 @@ public:
virtual ~BreakpointID();
- lldb::break_id_t GetBreakpointID() { return m_break_id; }
+ lldb::break_id_t GetBreakpointID() const { return m_break_id; }
- lldb::break_id_t GetLocationID() { return m_location_id; }
+ lldb::break_id_t GetLocationID() const { return m_location_id; }
void SetID(lldb::break_id_t bp_id, lldb::break_id_t loc_id) {
m_break_id = bp_id;
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointIDList.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointIDList.h?rev=281519&r1=281518&r2=281519&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointIDList.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointIDList.h Wed Sep 14 14:05:27
2016
@@ -34,9 +34,9 @@ public:
virtual ~BreakpointIDList();
- size_t GetSize();
+ size_t GetSize() const;
- BreakpointID &GetBreakpointIDAtIndex(size_t index);
+ const BreakpointID &GetBreakpointIDAtIndex(size_t index) const;
bool RemoveBreakpointIDAtIndex(size_t index);
@@ -46,9 +46,9 @@ public:
bool AddBreakpointID(const char *bp_id);
- bool FindBreakpointID(BreakpointID &bp_id, size_t *position);
+ bool FindBreakpointID(BreakpointID &bp_id, size_t *position) const;
- bool FindBreakpointID(const char *bp_id, size_t *position);
+ bool FindBreakpointID(const char *bp_id, size_t *position) const;
void InsertStringArray(const char **string_array, size_t array_size,
CommandReturnObject &result);
Modified: lldb/trunk/source/Breakpoint/BreakpointIDList.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointIDList.cpp?rev=281519&r1=281518&r2=281519&view=diff
==
--- lldb/trunk/source/Breakpoint/BreakpointIDList.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointIDList.cpp Wed Sep 14 14:05:27 2016
@@ -31,9 +31,10 @@ BreakpointIDList::BreakpointIDList()
BreakpointIDList::~BreakpointIDList() = default;
-size_t BreakpointIDList::GetSize() { return m_breakpoint_ids.size(); }
+size_t BreakpointIDList::GetSize() const { return m_breakpoint_ids.size(); }
-BreakpointID &BreakpointIDList::GetBreakpointIDAtIndex(size_t index) {
+const BreakpointID &
+BreakpointIDList::GetBreakpointIDAtIndex(size_t index) const {
return ((index < m_breakpoint_ids.size()) ? m_breakpoint_ids[index]
: m_invalid_id);
}
@@ -71,7 +72,8 @@ bool BreakpointIDList::AddBreakpointID(c
return success;
}
-bool BreakpointIDList::FindBreakpointID(BreakpointID &bp_id, size_t *position)
{
+bool BreakpointIDList::FindBreakpointID(BreakpointID &bp_id,
+size_t *position) const {
for (size_t i = 0; i < m_breakpoint_ids.size(); ++i) {
BreakpointID tmp_id = m_breakpoint_ids[i];
if (tmp_id.GetBreakpointID() == bp_id.GetBreakpointID() &&
@@ -85,7 +87,7 @@ bool BreakpointIDList::FindBreakpointID(
}
bool BreakpointIDList::FindBreakpointID(const char *bp_id_str,
-size_t *position) {
+size_t *position) const {
BreakpointID temp_bp_id;
break_id_t bp_id;
break_id_t loc_id;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281520 - Add SB API's for writing breakpoints to & creating the from a file.
Author: jingham
Date: Wed Sep 14 14:07:35 2016
New Revision: 281520
URL: http://llvm.org/viewvc/llvm-project?rev=281520&view=rev
Log:
Add SB API's for writing breakpoints to & creating the from a file.
Moved the guts of the code from CommandObjectBreakpoint to Target (should
have done it that way in the first place.) Added an SBBreakpointList class
so there's a way to specify which breakpoints to serialize and to report the
deserialized breakpoints.
Modified:
lldb/trunk/include/lldb/API/SBBreakpoint.h
lldb/trunk/include/lldb/API/SBTarget.h
lldb/trunk/include/lldb/Core/StructuredData.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/scripts/interface/SBBreakpoint.i
lldb/trunk/scripts/interface/SBTarget.i
lldb/trunk/source/API/SBBreakpoint.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Core/StructuredData.cpp
lldb/trunk/source/Target/Target.cpp
Modified: lldb/trunk/include/lldb/API/SBBreakpoint.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBBreakpoint.h?rev=281520&r1=281519&r2=281520&view=diff
==
--- lldb/trunk/include/lldb/API/SBBreakpoint.h (original)
+++ lldb/trunk/include/lldb/API/SBBreakpoint.h Wed Sep 14 14:07:35 2016
@@ -119,6 +119,7 @@ public:
GetNumBreakpointLocationsFromEvent(const lldb::SBEvent &event_sp);
private:
+ friend class SBBreakpointList;
friend class SBBreakpointLocation;
friend class SBTarget;
@@ -139,6 +140,35 @@ private:
lldb::BreakpointSP m_opaque_sp;
};
+class SBBreakpointListImpl;
+
+class LLDB_API SBBreakpointList {
+public:
+ SBBreakpointList(SBTarget &target);
+
+ ~SBBreakpointList();
+
+ size_t GetSize() const;
+
+ SBBreakpoint GetBreakpointAtIndex(size_t idx);
+
+ void Append(const SBBreakpoint &sb_file);
+
+ bool AppendIfUnique(const SBBreakpoint &sb_file);
+
+ void AppendByID(lldb::break_id_t id);
+
+ void Clear();
+
+protected:
+ friend class SBTarget;
+
+ void CopyToBreakpointIDList(lldb_private::BreakpointIDList &bp_id_list);
+
+private:
+ std::shared_ptr m_opaque_sp;
+};
+
} // namespace lldb
#endif // LLDB_SBBreakpoint_h_
Modified: lldb/trunk/include/lldb/API/SBTarget.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBTarget.h?rev=281520&r1=281519&r2=281520&view=diff
==
--- lldb/trunk/include/lldb/API/SBTarget.h (original)
+++ lldb/trunk/include/lldb/API/SBTarget.h Wed Sep 14 14:07:35 2016
@@ -16,6 +16,7 @@
// Project includes
#include "lldb/API/SBAddress.h"
#include "lldb/API/SBAttachInfo.h"
+#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBBroadcaster.h"
#include "lldb/API/SBDefines.h"
#include "lldb/API/SBFileSpec.h"
@@ -641,6 +642,18 @@ public:
lldb::SBBreakpoint BreakpointCreateBySBAddress(SBAddress &address);
+ // Reads in breakpoints from source_file, returning the newly created
+ // breakpoints in new_bps.
+ lldb::SBError BreakpointsCreateFromFile(SBFileSpec &source_file,
+ SBBreakpointList &new_bps);
+
+ // Writes all breakpoints to dest_file.
+ lldb::SBError BreakpointsWriteToFile(SBFileSpec &dest_file);
+
+ // Writes the breakpoints in bkpt_list to dest_file
+ lldb::SBError BreakpointsWriteToFile(SBFileSpec &dest_file,
+ SBBreakpointList &bkpt_list);
+
uint32_t GetNumBreakpoints() const;
lldb::SBBreakpoint GetBreakpointAtIndex(uint32_t idx) const;
@@ -741,6 +754,7 @@ public:
protected:
friend class SBAddress;
friend class SBBlock;
+ friend class SBBreakpointListImpl;
friend class SBDebugger;
friend class SBExecutionContext;
friend class SBFunction;
Modified: lldb/trunk/include/lldb/Core/StructuredData.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/StructuredData.h?rev=281520&r1=281519&r2=281520&view=diff
==
--- lldb/trunk/include/lldb/Core/StructuredData.h (original)
+++ lldb/trunk/include/lldb/Core/StructuredData.h Wed Sep 14 14:07:35 2016
@@ -552,7 +552,7 @@ public:
static ObjectSP ParseJSON(std::string json_text);
- static ObjectSP ParseJSONFromFile(FileSpec &file, Error &error);
+ static ObjectSP ParseJSONFromFile(const FileSpec &file, Error &error);
};
} // namespace lldb_private
Modified: lldb/trunk/include/lldb/Target/Target.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=281520&r1=281519&r2=281520&view=diff
==
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Wed Sep 14 14:07:35 2016
@@ -678,6 +678,12 @@ public:
bool IgnoreWatchpointByID(lldb::watch_id_t watch_id, uint32_t ignore_count);
Re: [Lldb-commits] [lldb] r281507 - Use Intel CPU flags to determine target supported features.
Sorry I didn’t see your comments on the web interface, I’ll submit a followup
patch with these changes.
From: Zachary Turner [mailto:ztur...@google.com]
Sent: Wednesday, September 14, 2016 7:47 PM
To: Giusti, Valentina ; lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] [lldb] r281507 - Use Intel CPU flags to determine
target supported features.
Can probably do the same for XStateType as well:
enum class XStateType { Invalid, FXSAVE, XSAVE };
On Wed, Sep 14, 2016 at 10:46 AM Zachary Turner
mailto:ztur...@google.com>> wrote:
On Wed, Sep 14, 2016 at 10:36 AM Valentina Giusti via lldb-commits
mailto:lldb-commits@lists.llvm.org>> wrote:
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
(original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
Wed Sep 14 12:27:48 2016
@@ -77,7 +77,8 @@ protected:
private:
// Private member types.
- enum FPRType { eFPRTypeNotValid = 0, eFPRTypeFXSAVE, eFPRTypeXSAVE };
+ enum XStateType { eXStateTypeNotValid = 0, eXStateTypeFXSAVE,
eXStateTypeXSAVE };
+ enum RegSet { gpr, fpu, avx, mpx };
This still isn't an enum class. You need to write:
enum class RegSet { gpr, fpu, avx, mpx };
Can you make this change and submit it as a followup please?
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24578: Use 'enum class' instead of 'enum' in NativeRegisterContextLinux_x86_x64.
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. Thanks! https://reviews.llvm.org/D24578 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24578: Use 'enum class' instead of 'enum' in NativeRegisterContextLinux_x86_x64.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281528: Use 'enum class' instead of 'enum' in
NativeRegisterContextLinux_x86_x64. (authored by valentinagiusti).
Changed prior to commit:
https://reviews.llvm.org/D24578?vs=71412&id=71418#toc
Repository:
rL LLVM
https://reviews.llvm.org/D24578
Files:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
@@ -77,8 +77,8 @@
private:
// Private member types.
- enum XStateType { eXStateTypeNotValid = 0, eXStateTypeFXSAVE, eXStateTypeXSAVE };
- enum RegSet { gpr, fpu, avx, mpx };
+ enum class XStateType { Invalid, FXSAVE, XSAVE };
+ enum class RegSet { gpr, fpu, avx, mpx };
// Info about register ranges.
struct RegInfo {
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -268,7 +268,7 @@
uint32_t concrete_frame_idx)
: NativeRegisterContextLinux(native_thread, concrete_frame_idx,
CreateRegisterInfoInterface(target_arch)),
- m_xstate_type(eXStateTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
+ m_xstate_type(XStateType::Invalid), m_fpr(), m_iovec(), m_ymm_set(),
m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
// Set up data about ranges of valid registers.
switch (target_arch.GetMachine()) {
@@ -664,12 +664,12 @@
::memcpy(dst, &m_gpr_x86_64, GetRegisterInfoInterface().GetGPRSize());
dst += GetRegisterInfoInterface().GetGPRSize();
- if (GetXStateType() == eXStateTypeFXSAVE)
+ if (GetXStateType() == XStateType::FXSAVE)
::memcpy(dst, &m_fpr.xstate.fxsave, sizeof(m_fpr.xstate.fxsave));
- else if (GetXStateType() == eXStateTypeXSAVE) {
+ else if (GetXStateType() == XStateType::XSAVE) {
lldb::ByteOrder byte_order = GetByteOrder();
-if (IsCPUFeatureAvailable(avx)) {
+if (IsCPUFeatureAvailable(RegSet::avx)) {
// Assemble the YMM register content from the register halves.
for (uint32_t reg = m_reg_info.first_ymm; reg <= m_reg_info.last_ymm;
++reg) {
@@ -684,7 +684,7 @@
}
}
-if (IsCPUFeatureAvailable(mpx)) {
+if (IsCPUFeatureAvailable(RegSet::mpx)) {
for (uint32_t reg = m_reg_info.first_mpxr; reg <= m_reg_info.last_mpxc;
++reg) {
if (!CopyXSTATEtoMPX(reg)) {
@@ -756,19 +756,19 @@
return error;
src += GetRegisterInfoInterface().GetGPRSize();
- if (GetXStateType() == eXStateTypeFXSAVE)
+ if (GetXStateType() == XStateType::FXSAVE)
::memcpy(&m_fpr.xstate.fxsave, src, sizeof(m_fpr.xstate.fxsave));
- else if (GetXStateType() == eXStateTypeXSAVE)
+ else if (GetXStateType() == XStateType::XSAVE)
::memcpy(&m_fpr.xstate.xsave, src, sizeof(m_fpr.xstate.xsave));
error = WriteFPR();
if (error.Fail())
return error;
- if (GetXStateType() == eXStateTypeXSAVE) {
+ if (GetXStateType() == XStateType::XSAVE) {
lldb::ByteOrder byte_order = GetByteOrder();
-if (IsCPUFeatureAvailable(avx)) {
+if (IsCPUFeatureAvailable(RegSet::avx)) {
// Parse the YMM register content from the register halves.
for (uint32_t reg = m_reg_info.first_ymm; reg <= m_reg_info.last_ymm;
++reg) {
@@ -783,7 +783,7 @@
}
}
-if (IsCPUFeatureAvailable(mpx)) {
+if (IsCPUFeatureAvailable(RegSet::mpx)) {
for (uint32_t reg = m_reg_info.first_mpxr; reg <= m_reg_info.last_mpxc;
++reg) {
if (!CopyMPXtoXSTATE(reg)) {
@@ -808,7 +808,7 @@
if (!__get_cpuid(1, &rax, &rbx, &rcx, &rdx))
return false;
if ((rdx & bit_FXSAVE) == bit_FXSAVE) {
-m_xstate_type = eXStateTypeFXSAVE;
+m_xstate_type = XStateType::FXSAVE;
if (const_cast(this)->ReadFPR().Fail())
return false;
return true;
@@ -823,7 +823,7 @@
if (!__get_cpuid(1, &rax, &rbx, &rcx, &rdx))
return false;
if ((rcx & bit_OSXSAVE) == bit_OSXSAVE) {
-m_xstate_type = eXStateTypeXSAVE;
+m_xstate_type = XStateType::XSAVE;
if (const_cast(this)->ReadFPR().Fail())
return false;
return true;
@@ -841,10 +841,10 @@
__get_cpuid(1, &rax, &rbx, &rcx, &rdx);
switch (feature_code) {
- case avx: // Check if CPU has AVX and if there is kernel support, by reading in the XCR0 area of XSAVE.
+ case RegSet::avx: // Check if CPU has AVX and if there is kernel support, by reading in the XCR0 area of
[Lldb-commits] [lldb] r281528 - Use 'enum class' instead of 'enum' in NativeRegisterContextLinux_x86_x64.
Author: valentinagiusti
Date: Wed Sep 14 15:12:12 2016
New Revision: 281528
URL: http://llvm.org/viewvc/llvm-project?rev=281528&view=rev
Log:
Use 'enum class' instead of 'enum' in NativeRegisterContextLinux_x86_x64.
Reviewers: labath, clayborg, zturner
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D24578
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp?rev=281528&r1=281527&r2=281528&view=diff
==
---
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
(original)
+++
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
Wed Sep 14 15:12:12 2016
@@ -268,7 +268,7 @@ NativeRegisterContextLinux_x86_64::Nativ
uint32_t concrete_frame_idx)
: NativeRegisterContextLinux(native_thread, concrete_frame_idx,
CreateRegisterInfoInterface(target_arch)),
- m_xstate_type(eXStateTypeNotValid), m_fpr(), m_iovec(), m_ymm_set(),
+ m_xstate_type(XStateType::Invalid), m_fpr(), m_iovec(), m_ymm_set(),
m_mpx_set(), m_reg_info(), m_gpr_x86_64() {
// Set up data about ranges of valid registers.
switch (target_arch.GetMachine()) {
@@ -664,12 +664,12 @@ Error NativeRegisterContextLinux_x86_64:
::memcpy(dst, &m_gpr_x86_64, GetRegisterInfoInterface().GetGPRSize());
dst += GetRegisterInfoInterface().GetGPRSize();
- if (GetXStateType() == eXStateTypeFXSAVE)
+ if (GetXStateType() == XStateType::FXSAVE)
::memcpy(dst, &m_fpr.xstate.fxsave, sizeof(m_fpr.xstate.fxsave));
- else if (GetXStateType() == eXStateTypeXSAVE) {
+ else if (GetXStateType() == XStateType::XSAVE) {
lldb::ByteOrder byte_order = GetByteOrder();
-if (IsCPUFeatureAvailable(avx)) {
+if (IsCPUFeatureAvailable(RegSet::avx)) {
// Assemble the YMM register content from the register halves.
for (uint32_t reg = m_reg_info.first_ymm; reg <= m_reg_info.last_ymm;
++reg) {
@@ -684,7 +684,7 @@ Error NativeRegisterContextLinux_x86_64:
}
}
-if (IsCPUFeatureAvailable(mpx)) {
+if (IsCPUFeatureAvailable(RegSet::mpx)) {
for (uint32_t reg = m_reg_info.first_mpxr; reg <= m_reg_info.last_mpxc;
++reg) {
if (!CopyXSTATEtoMPX(reg)) {
@@ -756,19 +756,19 @@ Error NativeRegisterContextLinux_x86_64:
return error;
src += GetRegisterInfoInterface().GetGPRSize();
- if (GetXStateType() == eXStateTypeFXSAVE)
+ if (GetXStateType() == XStateType::FXSAVE)
::memcpy(&m_fpr.xstate.fxsave, src, sizeof(m_fpr.xstate.fxsave));
- else if (GetXStateType() == eXStateTypeXSAVE)
+ else if (GetXStateType() == XStateType::XSAVE)
::memcpy(&m_fpr.xstate.xsave, src, sizeof(m_fpr.xstate.xsave));
error = WriteFPR();
if (error.Fail())
return error;
- if (GetXStateType() == eXStateTypeXSAVE) {
+ if (GetXStateType() == XStateType::XSAVE) {
lldb::ByteOrder byte_order = GetByteOrder();
-if (IsCPUFeatureAvailable(avx)) {
+if (IsCPUFeatureAvailable(RegSet::avx)) {
// Parse the YMM register content from the register halves.
for (uint32_t reg = m_reg_info.first_ymm; reg <= m_reg_info.last_ymm;
++reg) {
@@ -783,7 +783,7 @@ Error NativeRegisterContextLinux_x86_64:
}
}
-if (IsCPUFeatureAvailable(mpx)) {
+if (IsCPUFeatureAvailable(RegSet::mpx)) {
for (uint32_t reg = m_reg_info.first_mpxr; reg <= m_reg_info.last_mpxc;
++reg) {
if (!CopyMPXtoXSTATE(reg)) {
@@ -808,7 +808,7 @@ bool NativeRegisterContextLinux_x86_64::
if (!__get_cpuid(1, &rax, &rbx, &rcx, &rdx))
return false;
if ((rdx & bit_FXSAVE) == bit_FXSAVE) {
-m_xstate_type = eXStateTypeFXSAVE;
+m_xstate_type = XStateType::FXSAVE;
if (const_cast(this)->ReadFPR().Fail())
return false;
return true;
@@ -823,7 +823,7 @@ bool NativeRegisterContextLinux_x86_64::
if (!__get_cpuid(1, &rax, &rbx, &rcx, &rdx))
return false;
if ((rcx & bit_OSXSAVE) == bit_OSXSAVE) {
-m_xstate_type = eXStateTypeXSAVE;
+m_xstate_type = XStateType::XSAVE;
if (const_cast(this)->ReadFPR().Fail())
return false;
return true;
@@ -841,10 +841,10 @@ bool NativeRegisterContextLinux_x86_64::
__get_cpuid(1, &rax, &rbx, &rcx, &rdx);
switch (feature_code) {
- case avx: // Check if CPU has AVX and if there is kernel support, by reading
in the XCR0 area of XSAVE.
+ case RegSet::avx: // Check if CPU has AVX and if there is kernel support, by
reading in the XCR0 area of XSAVE.
if (((rcx & bit_AVX) != 0) && ((m_fpr.xstate.xsave.i387.xcr0 &
mask_XSTATE_AVX) == mask
[Lldb-commits] [lldb] r281534 - Cleaned up a little bit of redundant code in 'frame diagnose.`
Author: spyffe
Date: Wed Sep 14 15:29:57 2016
New Revision: 281534
URL: http://llvm.org/viewvc/llvm-project?rev=281534&view=rev
Log:
Cleaned up a little bit of redundant code in 'frame diagnose.`
Modified:
lldb/trunk/source/Target/StackFrame.cpp
Modified: lldb/trunk/source/Target/StackFrame.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?rev=281534&r1=281533&r2=281534&view=diff
==
--- lldb/trunk/source/Target/StackFrame.cpp (original)
+++ lldb/trunk/source/Target/StackFrame.cpp Wed Sep 14 15:29:57 2016
@@ -1492,6 +1492,12 @@ lldb::ValueObjectSP DoGuessValueAt(Stack
using namespace OperandMatchers;
+ const RegisterInfo *reg_info =
+ frame.GetRegisterContext()->GetRegisterInfoByName(reg.AsCString());
+ if (!reg_info) {
+return ValueObjectSP();
+ }
+
Instruction::Operand op =
offset ? Instruction::Operand::BuildDereference(
Instruction::Operand::BuildSum(
@@ -1517,9 +1523,9 @@ lldb::ValueObjectSP DoGuessValueAt(Stack
for (uint32_t ii = current_inst - 1; ii != (uint32_t)-1; --ii) {
// This is not an exact algorithm, and it sacrifices accuracy for
-// generality.
-// Recognizing "mov" and "ld" instructions –– and which are their
source and
-// destination operands -- is something the disassembler should do for us.
+// generality. Recognizing "mov" and "ld" instructions –– and which
are
+// their source and destination operands -- is something the disassembler
+// should do for us.
InstructionSP instruction_sp =
disassembler.GetInstructionList().GetInstructionAtIndex(ii);
@@ -1602,16 +1608,18 @@ lldb::ValueObjectSP DoGuessValueAt(Stack
}
Instruction::Operand *origin_operand = nullptr;
-if (operands[0].m_type == Instruction::Operand::Type::Register &&
-operands[0].m_clobbered == true && operands[0].m_register == reg) {
- // operands[0] is a register operand
+std::function clobbered_reg_matcher =
+[reg_info](const Instruction::Operand &op) {
+ return MatchRegOp(*reg_info)(op) && op.m_clobbered;
+};
+
+if (clobbered_reg_matcher(operands[0])) {
origin_operand = &operands[1];
-} else if (operands[1].m_type == Instruction::Operand::Type::Register &&
- operands[1].m_clobbered == true &&
- operands[1].m_register == reg) {
+}
+else if (clobbered_reg_matcher(operands[1])) {
origin_operand = &operands[0];
- // operands[1] is a register operand
-} else {
+}
+else {
continue;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r281534 - Cleaned up a little bit of redundant code in 'frame diagnose.`
On Wed, Sep 14, 2016 at 1:38 PM Sean Callanan via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
>
> Instruction::Operand *origin_operand = nullptr;
> -if (operands[0].m_type == Instruction::Operand::Type::Register &&
> -operands[0].m_clobbered == true && operands[0].m_register == reg)
> {
> - // operands[0] is a register operand
> +std::function
> clobbered_reg_matcher =
> +[reg_info](const Instruction::Operand &op) {
> + return MatchRegOp(*reg_info)(op) && op.m_clobbered;
> +};
>
You should use `auto` here instead of `std::function`. Putting it in a
std::function is actually less efficient due to the way it's implemented,
but if you use `auto` you get the actual lambda type itself.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r281534 - Cleaned up a little bit of redundant code in 'frame diagnose.`
Okay, one sec.
Sean
> On Sep 14, 2016, at 1:40 PM, Zachary Turner wrote:
>
>
>
> On Wed, Sep 14, 2016 at 1:38 PM Sean Callanan via lldb-commits
> mailto:lldb-commits@lists.llvm.org>> wrote:
>
> Instruction::Operand *origin_operand = nullptr;
> -if (operands[0].m_type == Instruction::Operand::Type::Register &&
> -operands[0].m_clobbered == true && operands[0].m_register == reg) {
> - // operands[0] is a register operand
> +std::function clobbered_reg_matcher =
> +[reg_info](const Instruction::Operand &op) {
> + return MatchRegOp(*reg_info)(op) && op.m_clobbered;
> +};
> You should use `auto` here instead of `std::function`. Putting it in a
> std::function is actually less efficient due to the way it's implemented, but
> if you use `auto` you get the actual lambda type itself.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281536 - Replaced two instances of std::function with auto.
Author: spyffe
Date: Wed Sep 14 15:58:31 2016
New Revision: 281536
URL: http://llvm.org/viewvc/llvm-project?rev=281536&view=rev
Log:
Replaced two instances of std::function with auto.
Thanks to Zachary Turner for the suggestion. It's distasteful that the actual
type of the lambda can't be spelled out, but it should be evident from the
definition of the lambda body.
Modified:
lldb/trunk/source/Expression/DWARFExpression.cpp
lldb/trunk/source/Target/StackFrame.cpp
Modified: lldb/trunk/source/Expression/DWARFExpression.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DWARFExpression.cpp?rev=281536&r1=281535&r2=281536&view=diff
==
--- lldb/trunk/source/Expression/DWARFExpression.cpp (original)
+++ lldb/trunk/source/Expression/DWARFExpression.cpp Wed Sep 14 15:58:31 2016
@@ -3335,10 +3335,9 @@ bool DWARFExpression::MatchesOperand(Sta
return false;
}
-std::function recurse =
-[&frame, fb_expr](const Instruction::Operand &child) {
- return fb_expr->MatchesOperand(frame, child);
-};
+auto recurse = [&frame, fb_expr](const Instruction::Operand &child) {
+ return fb_expr->MatchesOperand(frame, child);
+};
if (!offset &&
MatchUnaryOp(MatchOpType(Instruction::Operand::Type::Dereference),
Modified: lldb/trunk/source/Target/StackFrame.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?rev=281536&r1=281535&r2=281536&view=diff
==
--- lldb/trunk/source/Target/StackFrame.cpp (original)
+++ lldb/trunk/source/Target/StackFrame.cpp Wed Sep 14 15:58:31 2016
@@ -1608,10 +1608,9 @@ lldb::ValueObjectSP DoGuessValueAt(Stack
}
Instruction::Operand *origin_operand = nullptr;
-std::function clobbered_reg_matcher =
-[reg_info](const Instruction::Operand &op) {
- return MatchRegOp(*reg_info)(op) && op.m_clobbered;
-};
+auto clobbered_reg_matcher = [reg_info](const Instruction::Operand &op) {
+ return MatchRegOp(*reg_info)(op) && op.m_clobbered;
+};
if (clobbered_reg_matcher(operands[0])) {
origin_operand = &operands[1];
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r281536 - Replaced two instances of std::function with auto.
Yea, it's too bad about the type. This is one case where auto isn't just
syntactic sugar, it's actually necessary.
On Wed, Sep 14, 2016 at 2:07 PM Sean Callanan via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> Author: spyffe
> Date: Wed Sep 14 15:58:31 2016
> New Revision: 281536
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281536&view=rev
> Log:
> Replaced two instances of std::function with auto.
>
> Thanks to Zachary Turner for the suggestion. It's distasteful that the
> actual
> type of the lambda can't be spelled out, but it should be evident from the
> definition of the lambda body.
>
> Modified:
> lldb/trunk/source/Expression/DWARFExpression.cpp
> lldb/trunk/source/Target/StackFrame.cpp
>
> Modified: lldb/trunk/source/Expression/DWARFExpression.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DWARFExpression.cpp?rev=281536&r1=281535&r2=281536&view=diff
>
> ==
> --- lldb/trunk/source/Expression/DWARFExpression.cpp (original)
> +++ lldb/trunk/source/Expression/DWARFExpression.cpp Wed Sep 14 15:58:31
> 2016
> @@ -3335,10 +3335,9 @@ bool DWARFExpression::MatchesOperand(Sta
>return false;
> }
>
> -std::function recurse =
> -[&frame, fb_expr](const Instruction::Operand &child) {
> - return fb_expr->MatchesOperand(frame, child);
> -};
> +auto recurse = [&frame, fb_expr](const Instruction::Operand &child) {
> + return fb_expr->MatchesOperand(frame, child);
> +};
>
> if (!offset &&
> MatchUnaryOp(MatchOpType(Instruction::Operand::Type::Dereference),
>
> Modified: lldb/trunk/source/Target/StackFrame.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?rev=281536&r1=281535&r2=281536&view=diff
>
> ==
> --- lldb/trunk/source/Target/StackFrame.cpp (original)
> +++ lldb/trunk/source/Target/StackFrame.cpp Wed Sep 14 15:58:31 2016
> @@ -1608,10 +1608,9 @@ lldb::ValueObjectSP DoGuessValueAt(Stack
> }
>
> Instruction::Operand *origin_operand = nullptr;
> -std::function
> clobbered_reg_matcher =
> -[reg_info](const Instruction::Operand &op) {
> - return MatchRegOp(*reg_info)(op) && op.m_clobbered;
> -};
> +auto clobbered_reg_matcher = [reg_info](const Instruction::Operand
> &op) {
> + return MatchRegOp(*reg_info)(op) && op.m_clobbered;
> +};
>
> if (clobbered_reg_matcher(operands[0])) {
>origin_operand = &operands[1];
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281545 - More cleanup in `frame diagnose, ` eliminating a bunch of messy cases.
Author: spyffe
Date: Wed Sep 14 16:54:28 2016
New Revision: 281545
URL: http://llvm.org/viewvc/llvm-project?rev=281545&view=rev
Log:
More cleanup in `frame diagnose,` eliminating a bunch of messy cases.
Modified:
lldb/trunk/include/lldb/Core/Disassembler.h
lldb/trunk/source/Core/Disassembler.cpp
lldb/trunk/source/Target/StackFrame.cpp
Modified: lldb/trunk/include/lldb/Core/Disassembler.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Disassembler.h?rev=281545&r1=281544&r2=281545&view=diff
==
--- lldb/trunk/include/lldb/Core/Disassembler.h (original)
+++ lldb/trunk/include/lldb/Core/Disassembler.h Wed Sep 14 16:54:28 2016
@@ -224,6 +224,8 @@ MatchUnaryOp(std::function
MatchRegOp(const RegisterInfo &info);
+std::function FetchRegOp(ConstString ®);
+
std::function MatchImmOp(int64_t imm);
std::function FetchImmOp(int64_t &imm);
Modified: lldb/trunk/source/Core/Disassembler.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Disassembler.cpp?rev=281545&r1=281544&r2=281545&view=diff
==
--- lldb/trunk/source/Core/Disassembler.cpp (original)
+++ lldb/trunk/source/Core/Disassembler.cpp Wed Sep 14 16:54:28 2016
@@ -1405,6 +1405,17 @@ lldb_private::OperandMatchers::MatchRegO
}
std::function
+lldb_private::OperandMatchers::FetchRegOp(ConstString ®) {
+ return [®](const Instruction::Operand &op) {
+if (op.m_type != Instruction::Operand::Type::Register) {
+ return false;
+}
+reg = op.m_register;
+return true;
+ };
+}
+
+std::function
lldb_private::OperandMatchers::MatchImmOp(int64_t imm) {
return [imm](const Instruction::Operand &op) {
return (op.m_type == Instruction::Operand::Type::Immediate &&
Modified: lldb/trunk/source/Target/StackFrame.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?rev=281545&r1=281544&r2=281545&view=diff
==
--- lldb/trunk/source/Target/StackFrame.cpp (original)
+++ lldb/trunk/source/Target/StackFrame.cpp Wed Sep 14 16:54:28 2016
@@ -1519,8 +1519,6 @@ lldb::ValueObjectSP DoGuessValueAt(Stack
return ValueObjectSP();
}
- ValueObjectSP source_path;
-
for (uint32_t ii = current_inst - 1; ii != (uint32_t)-1; --ii) {
// This is not an exact algorithm, and it sacrifices accuracy for
// generality. Recognizing "mov" and "ld" instructions –– and which
are
@@ -1623,67 +1621,29 @@ lldb::ValueObjectSP DoGuessValueAt(Stack
}
// We have an origin operand. Can we track its value down?
-switch (origin_operand->m_type) {
-default:
- break;
-case Instruction::Operand::Type::Register:
+ValueObjectSP source_path;
+ConstString origin_register;
+int64_t origin_offset = 0;
+
+if (FetchRegOp(origin_register)(*origin_operand)) {
+ source_path = DoGuessValueAt(frame, origin_register, 0, disassembler,
+ variables, instruction_sp->GetAddress());
+} else if (MatchUnaryOp(
+ MatchOpType(Instruction::Operand::Type::Dereference),
+ FetchRegOp(origin_register))(*origin_operand) ||
+ MatchUnaryOp(
+ MatchOpType(Instruction::Operand::Type::Dereference),
+ MatchBinaryOp(MatchOpType(Instruction::Operand::Type::Sum),
+ FetchRegOp(origin_register),
+ FetchImmOp(origin_offset)))(*origin_operand))
{
source_path =
- DoGuessValueAt(frame, origin_operand->m_register, 0, disassembler,
+ DoGuessValueAt(frame, origin_register, origin_offset, disassembler,
variables, instruction_sp->GetAddress());
- break;
-case Instruction::Operand::Type::Dereference: {
- const Instruction::Operand &pointer = origin_operand->m_children[0];
- switch (pointer.m_type) {
- default:
-break;
- case Instruction::Operand::Type::Register:
-source_path = DoGuessValueAt(frame, pointer.m_register, 0,
disassembler,
- variables, instruction_sp->GetAddress());
-if (source_path) {
- Error err;
- source_path = source_path->Dereference(err);
- if (!err.Success()) {
-source_path.reset();
- }
-}
-break;
- case Instruction::Operand::Type::Sum: {
-const Instruction::Operand *origin_register = nullptr;
-const Instruction::Operand *origin_offset = nullptr;
-if (pointer.m_children.size() != 2) {
- break;
-}
-if (pointer.m_children[0].m_type ==
-Instruction::Operand::Type::Register &&
-pointer.m_children[1].m_type ==
-Instruction::Operand::Type::Immediate) {
-
[Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
beanz created this revision.
beanz added reviewers: zturner, labath, jingham, tfiala.
beanz added a subscriber: lldb-commits.
Herald added subscribers: mgorny, beanz.
This patch supplies basic infrastructure for LLDB to use LIT, and ports a few
basic test cases from the LLDB test suite into LIT.
With this patch the LLDB lit system is not capable or intended to fully replace
the existing LLDB test suite, but this first patch enables people to write lit
tests for LLDB.
The lit substitution for %cc and %cxx default to the host compiler unless the
CMake option LLDB_TEST_CLANG is On, in which case the in-tree clang will be
used.
The target check-lldb-lit will run all lit tests including the lit-based
executor for the unit tests. Alternatively there is a target generated for each
subdirectory under the lit directory, so check-lldb-unit and check-lldb-expr
will run just the tests under their respective directories.
The ported tests are not removed from the existing suite, and should not be
until such a time when the lit runner is mature and in use by bots and
workflows.
https://reviews.llvm.org/D24591
Files:
lit/CMakeLists.txt
lit/Expr/Inputs/anonymous-struct.cpp
lit/Expr/Inputs/call-function.cpp
lit/Expr/TestCallStdStringFunction.test
lit/Expr/TestCallStopAndContinue.test
lit/Expr/TestCallUserAnonTypedef.test
lit/Expr/TestCallUserDefinedFunction.test
lit/Expr/lit.local.cfg
lit/Unit/lit.cfg
lit/Unit/lit.site.cfg.in
lit/lit.cfg
lit/lit.site.cfg.in
Index: lit/lit.site.cfg.in
===
--- lit/lit.site.cfg.in
+++ lit/lit.site.cfg.in
@@ -8,6 +8,12 @@
config.lldb_obj_root = "@LLDB_BINARY_DIR@"
config.target_triple = "@TARGET_TRIPLE@"
config.python_executable = "@PYTHON_EXECUTABLE@"
+config.host_cc = "@CMAKE_C_COMPILER@"
+config.host_cxx = "@CMAKE_CXX_COMPILER@"
+
+test_clang = "@LLDB_TEST_CLANG@".lower()
+
+config.test_clang = test_clang == "on" or test_clang == "true" or test_clang == "1"
# Support substitution of the tools and libs dirs with user parameters. This is
# used when we can't determine the tool dir at configuration time.
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -112,17 +112,82 @@
lit_config.load_config(config, site_cfg)
raise SystemExit
+# Register substitutions
+config.substitutions.append(('%python', config.python_executable))
+config.substitutions.append(('%host_cc', config.host_cc))
+config.substitutions.append(('%host_cxx', config.host_cxx))
+
+cc = config.host_cc
+cxx = config.host_cxx
+debugserver = '%s/debugserver' % llvm_tools_dir
+
+if config.test_clang:
+cc = '%s/clang' % llvm_tools_dir
+cxx = '%s/clang++' % llvm_tools_dir
+
+if platform.system() in ['Darwin']:
+try:
+out = lit.util.capture(['xcrun', '--show-sdk-path']).strip()
+res = 0
+except OSError:
+res = -1
+if res == 0 and out:
+sdk_path = out
+lit_config.note('using SDKROOT: %r' % sdk_path)
+cxx += " -isysroot %s" % sdk_path
+debugserver = '%s/Library/Frameworks/LLDB.frameworks/Resources/debugserver' % config.llvm_obj_root
+
+
+config.substitutions.append(('%cc', cc))
+config.substitutions.append(('%cxx', cxx))
+
+config.substitutions.append(('%lldb', '%s/lldb' % llvm_tools_dir))
+config.substitutions.append(('%debugserver', debugserver))
+
+for pattern in [r"\bFileCheck\b",
+r"\| \bnot\b"]:
+tool_match = re.match(r"^(\\)?((\| )?)\W+b([0-9A-Za-z-_]+)\\b\W*$",
+ pattern)
+tool_pipe = tool_match.group(2)
+tool_name = tool_match.group(4)
+tool_path = config.llvm_tools_dir + '/' + tool_name
+if not tool_path:
+# Warn, but still provide a substitution.
+lit_config.note(
+'Did not find ' + tool_name + ' in ' + config.llvm_tools_dir)
+config.substitutions.append((pattern, tool_pipe + tool_path))
+
# Shell execution
if platform.system() not in ['Windows'] or lit_config.getBashPath() != '':
config.available_features.add('shell')
# Running on Darwin OS
if platform.system() in ['Darwin']:
+config.available_features.add('darwin')
config.available_features.add('system-linker-mach-o')
# Running on ELF based *nix
if platform.system() in ['FreeBSD', 'Linux']:
config.available_features.add('system-linker-elf')
+if platform.system() in ['FreeBSD']:
+config.available_features.add('freebsd')
+else:
+config.available_features.add('linux')
+
+if platform.system() in ['Windows']:
+config.available_features.add('windows')
+
+if re.match(r'^arm(hf.*-linux)|(.*-linux-gnuabihf)', config.target_triple):
+config.available_features.add("armhf-linux")
+
+if re.match(r'icc', cc):
+config.available_features.add("compiler-icc")
+elif re.match(r'clang', cc):
+config.available_features.add("compiler-clang")
+elif re.match(r'gcc', cc):
+config
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added a comment. Huh... This is a surprise. Looking forward to reviewing this. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
tfiala added a comment.
Looks like a nice start. Thanks for pulling this together, Chris!
Comment at: lit/CMakeLists.txt:14
@@ -13,1 +13,3 @@
+option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off)
+
The macOS bots (and all the Swift ones) build with in-tree clang. I'm glad to
have this option.
Comment at: lit/CMakeLists.txt:26
@@ -23,1 +25,3 @@
+ FileCheck
+ debugserver
LLDBUnitTests
Not entirely sure what exactly this is checking, but for non-Apple platforms
(and maybe Apple in the future), we'll be using an lldb-server binary rather
than debugserver. Anything we need to do here to handle that?
Comment at: lit/lit.cfg:190
@@ -126,1 +189,3 @@
+elif re.match(r'cl', cc):
+config.available_features.add("compiler-msvc")
As we discussed earlier, it'll be helpful to be able to parse out the compiler
versions so we can do some kind of conditional check on specific version or
version ranges for known bugs (or features).
https://reviews.llvm.org/D24591
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
tfiala added inline comments. Comment at: lit/CMakeLists.txt:14 @@ -13,1 +13,3 @@ +option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off) + tfiala wrote: > The macOS bots (and all the Swift ones) build with in-tree clang. I'm glad > to have this option. > The macOS bots (and all the Swift ones) build with in-tree clang. I'm glad to > have this option. ... test with the in-tree clang, rather. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
tfiala accepted this revision. tfiala added a comment. This revision is now accepted and ready to land. Assuming this doesn't break anybody, this LGTM. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
spyffe added a subscriber: spyffe. spyffe added a comment. I like this concept a lot, and I think it's great for testcases that actually need to interact with the command line. I'm concerned, though, that the separation of input from command files makes tests more complex to write. One thing that's very nice about the `inline` tests in LLDB is that the test and the source for the debugged program are both in the same file. If the goal is to port compiler tests across (or just to have tests be more understandable for compiler engineers), I would argue for choosing that model as the default. It would mean a slightly customized LLDB driver but that wouldn't be insanely hard. (We've already done it in Python!) https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added a comment.
Couple questions:
1. What is the status of lit and Python 3? Running the test suite on Windows
**requires** Python 3.5+, so if we want this to work on Windows, we will need
to make sure the lit infrastructure is compatible with Python 3.
Comment at: lit/CMakeLists.txt:14
@@ -13,1 +13,3 @@
+option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off)
+
LLDB's CMake already has an option called `LLDB_TEST_COMPILER`. Is it possible
to re-use that?
We have situations where we want to run the test suite using neither the
in-tree clang nor the host compiler, so I think we need to retain this
flexibility to specify a path to the compiler.
Comment at: lit/CMakeLists.txt:33
@@ +32,3 @@
+if(LLDB_TEST_CLANG)
+ list(APPELND LLDB_TEST_DEPS clang)
+endif()
`s/APPELND/APPEND/`
Comment at: lit/lit.cfg:125-126
@@ +124,4 @@
+if config.test_clang:
+cc = '%s/clang' % llvm_tools_dir
+cxx = '%s/clang++' % llvm_tools_dir
+
Can you use `os.path.join` instead of hardcoding a forward slash? This might
not apply though given my earlier comment about `LLDB_TEST_COMPILER`.
Comment at: lit/lit.cfg:144
@@ +143,3 @@
+
+config.substitutions.append(('%lldb', '%s/lldb' % llvm_tools_dir))
+config.substitutions.append(('%debugserver', debugserver))
`os.path.join` here, and on Windows you will need to add `.exe`
Comment at: lit/lit.cfg:189-190
@@ -126,1 +188,4 @@
+config.available_features.add("compiler-gcc")
+elif re.match(r'cl', cc):
+config.available_features.add("compiler-msvc")
I'm ok with removing this branch. Currently on Windows we require `clang.exe`
as the test compiler, and we use it in cl driver mode, so command lines are
mostly interchangeable across platforms. Having to support an entirely
different command line syntax would fragment the tests too much.
https://reviews.llvm.org/D24591
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
tfiala added a comment. One other items we discussed that is sure to come up: - Right now this is geared towards one compile per .test file (similar to something Zachary brought up before). One way we could get the multiple debug info formats handled is to have a .test for each of the formats, but have each of them pull in the common test checks from a shared file in the same directory. We'll have to play with that. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
beanz updated this revision to Diff 71452.
beanz added a comment.
Added lldb-server as a test dependency based on feedback from @tfiala.
https://reviews.llvm.org/D24591
Files:
lit/CMakeLists.txt
lit/Expr/Inputs/anonymous-struct.cpp
lit/Expr/Inputs/call-function.cpp
lit/Expr/TestCallStdStringFunction.test
lit/Expr/TestCallStopAndContinue.test
lit/Expr/TestCallUserAnonTypedef.test
lit/Expr/TestCallUserDefinedFunction.test
lit/Expr/lit.local.cfg
lit/Unit/lit.cfg
lit/Unit/lit.site.cfg.in
lit/lit.cfg
lit/lit.site.cfg.in
Index: lit/lit.site.cfg.in
===
--- lit/lit.site.cfg.in
+++ lit/lit.site.cfg.in
@@ -8,6 +8,12 @@
config.lldb_obj_root = "@LLDB_BINARY_DIR@"
config.target_triple = "@TARGET_TRIPLE@"
config.python_executable = "@PYTHON_EXECUTABLE@"
+config.host_cc = "@CMAKE_C_COMPILER@"
+config.host_cxx = "@CMAKE_CXX_COMPILER@"
+
+test_clang = "@LLDB_TEST_CLANG@".lower()
+
+config.test_clang = test_clang == "on" or test_clang == "true" or test_clang == "1"
# Support substitution of the tools and libs dirs with user parameters. This is
# used when we can't determine the tool dir at configuration time.
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -112,17 +112,82 @@
lit_config.load_config(config, site_cfg)
raise SystemExit
+# Register substitutions
+config.substitutions.append(('%python', config.python_executable))
+config.substitutions.append(('%host_cc', config.host_cc))
+config.substitutions.append(('%host_cxx', config.host_cxx))
+
+cc = config.host_cc
+cxx = config.host_cxx
+debugserver = '%s/debugserver' % llvm_tools_dir
+
+if config.test_clang:
+cc = '%s/clang' % llvm_tools_dir
+cxx = '%s/clang++' % llvm_tools_dir
+
+if platform.system() in ['Darwin']:
+try:
+out = lit.util.capture(['xcrun', '--show-sdk-path']).strip()
+res = 0
+except OSError:
+res = -1
+if res == 0 and out:
+sdk_path = out
+lit_config.note('using SDKROOT: %r' % sdk_path)
+cxx += " -isysroot %s" % sdk_path
+debugserver = '%s/Library/Frameworks/LLDB.frameworks/Resources/debugserver' % config.llvm_obj_root
+
+
+config.substitutions.append(('%cc', cc))
+config.substitutions.append(('%cxx', cxx))
+
+config.substitutions.append(('%lldb', '%s/lldb' % llvm_tools_dir))
+config.substitutions.append(('%debugserver', debugserver))
+
+for pattern in [r"\bFileCheck\b",
+r"\| \bnot\b"]:
+tool_match = re.match(r"^(\\)?((\| )?)\W+b([0-9A-Za-z-_]+)\\b\W*$",
+ pattern)
+tool_pipe = tool_match.group(2)
+tool_name = tool_match.group(4)
+tool_path = config.llvm_tools_dir + '/' + tool_name
+if not tool_path:
+# Warn, but still provide a substitution.
+lit_config.note(
+'Did not find ' + tool_name + ' in ' + config.llvm_tools_dir)
+config.substitutions.append((pattern, tool_pipe + tool_path))
+
# Shell execution
if platform.system() not in ['Windows'] or lit_config.getBashPath() != '':
config.available_features.add('shell')
# Running on Darwin OS
if platform.system() in ['Darwin']:
+config.available_features.add('darwin')
config.available_features.add('system-linker-mach-o')
# Running on ELF based *nix
if platform.system() in ['FreeBSD', 'Linux']:
config.available_features.add('system-linker-elf')
+if platform.system() in ['FreeBSD']:
+config.available_features.add('freebsd')
+else:
+config.available_features.add('linux')
+
+if platform.system() in ['Windows']:
+config.available_features.add('windows')
+
+if re.match(r'^arm(hf.*-linux)|(.*-linux-gnuabihf)', config.target_triple):
+config.available_features.add("armhf-linux")
+
+if re.match(r'icc', cc):
+config.available_features.add("compiler-icc")
+elif re.match(r'clang', cc):
+config.available_features.add("compiler-clang")
+elif re.match(r'gcc', cc):
+config.available_features.add("compiler-gcc")
+elif re.match(r'cl', cc):
+config.available_features.add("compiler-msvc")
# llvm-config knows whether it is compiled with asserts (and)
# whether we are operating in release/debug mode.
Index: lit/Unit/lit.site.cfg.in
===
--- lit/Unit/lit.site.cfg.in
+++ lit/Unit/lit.site.cfg.in
@@ -1,5 +1,6 @@
@LIT_SITE_CFG_IN_HEADER@
+config.test_exec_root = "@LLVM_BINARY_DIR@"
config.llvm_src_root = "@LLVM_SOURCE_DIR@"
config.llvm_obj_root = "@LLVM_BINARY_DIR@"
config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
Index: lit/Unit/lit.cfg
===
--- lit/Unit/lit.cfg
+++ lit/Unit/lit.cfg
@@ -6,6 +6,19 @@
import lit.formats
+# Check that the object root is known.
+if config.test_exec_root is None:
+# Otherwise, we haven't loaded the site specific configuration (the user is
+# pro
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
granata.enrico added a subscriber: granata.enrico. granata.enrico added a comment. One piece of concern that I have is the multiplication of things that "testing LLDB" means. We have unit tests, Python tests, and now LIT tests. That means, for each change I want to commit, if I want confidence that I am not breaking anything (which seems like a thing one could conceivably want :-) now I need to know and perform three unrelated incantations instead of one. If that is the temporary state of affairs while we transition to a better new world, then so be it. But I would definitely want for there to eventually be one true incantation that runs all tests, no matter their flavor. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
jingham added a comment. Writing tests this way means we're going back to testing the command line commands. That was what gdb did for the most part, and you ended up terrified of making changes that might effect command output, not because the change was hard but because knew you would have to go waste a day or your life making annoying fixes to a whole bunch of tests. This happened in lldb with the command line tests we did have, specifically with the breakpoint reporting. So I had to go make functions for setting breakpoints and making sure they got set correctly - and change all the tests to use them - so I could change just one place when I added output to breakpoint reporting. We made a choice a while ago to favor tests using the SB API's for this reason. Is the virtue of lit sufficient that we really want to go back on this decision? https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added a comment. In https://reviews.llvm.org/D24591#543242, @jingham wrote: > Writing tests this way means we're going back to testing the command line > commands. That was what gdb did for the most part, and you ended up > terrified of making changes that might effect command output, not because the > change was hard but because knew you would have to go waste a day or your > life making annoying fixes to a whole bunch of tests. This happened in lldb > with the command line tests we did have, specifically with the breakpoint > reporting. So I had to go make functions for setting breakpoints and making > sure they got set correctly - and change all the tests to use them - so I > could change just one place when I added output to breakpoint reporting. > > We made a choice a while ago to favor tests using the SB API's for this > reason. Is the virtue of lit sufficient that we really want to go back on > this decision? I had been thinking about this the other day as well. An example that springs to mind that I personally encountered is when a test on Windows was trying to verify that the value of argc was 3. So it was trying to match "argc=3" in stack trace. Seems reasonable, except that the code was actually completely broken on Windows, and argc was pointing to junk memory, which was something like "argc=3239082340982", and the test was passing because this value happened to start with a 3. One idea might be to add a developer command to LLDB that is able to drill down much deeper to return specific values of interest instead of large blocks of text. For example, imagine commands like this: (lldb) print-dev stack-frame[0].params[argc] 3 (lldb) print-dev threads.count 7 (lldb) print-dev --hex threads[6].id 0x1234 Now imagine you can get every little nook and cranny of the process's state through a syntax such as this. As you know I've been a big supporter of testing the API since the beginning for the same reason you mention -- that it makes people too afraid to change the output format -- but to answer your question: I do think the virtue of lit is high enough that we find a way to solve the problem. The above idea is one possibility for reducing the fragility of command line tests, but if we brainstorm we can probably come up with others as well. It's certainly a lot of work, but I do think the benefits are worth it. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
How different is that really from
(lldb) script lldb.frame.FindVariable("argc").GetValue()
'1'
(lldb) script lldb.process.GetNumThreads()
1
(lldb) script lldb.process.GetThreadAtIndex(0).GetThreadID()
3514809
? If it's developer-only, then this is even fairly well-documented using e.g.
"script dir(lldb.process)"
> On Sep 14, 2016, at 3:43 PM, Zachary Turner wrote:
>
> (lldb) print-dev stack-frame[0].params[argc]
> 3
> (lldb) print-dev threads.count
> 7
> (lldb) print-dev --hex threads[6].id
> 0x1234
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added a comment. Also, it occurred to me that if all tests were like this (and yes, that's a tall order to imagine a world where not a single test was written using the Python API), we could probably actually drop the Python 3.5 requirement on Windows. Another thing that's nice about tests like this is that it makes it trivial to see how to reproduce a failure. It's currently very hard to debug failures because you have to first figure out where in the test it's failing (i.e. what line of python), then attach to the python executable and try to get a breakpoint on the native code side in the right SB API call matching up with the place where you determined the test is failing. This is really a pain without a debugger that supports mixed<->native transitions between python and c++, and even with a debugger that does support it like we have on Windows, it often doesn't work very well or exhibits flakiness. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
beanz added a comment. @zturner, on many of your comments I need to do some research and get back to you. Particularly I need to understand how lit works on Windows better. I do have one inline response. @granata.enrico, we could migrate the existing tests into being executed by lit even if they aren't using lit's features, so if that direction is desired we could get everything in lit. That said, you shouldn't ever really have multiple incantations. Once we have reliable lit testing that is useful it should be connected to the "check-lldb" and "check-all" targets appropriately. Just because it runs more than one type of test doesn't mean you need multiple incantations, and more and varied testing is generally better for the quality of the product. @jingham and @zturner, we can also take advantage of FileCheck's use of regular expressions to write robust matchers. In general LLVM has managed to change text output formats many times in radical ways, and LIT's testing has still suited the project well. And to echo @zturner's last comment, one huge benefit to LIT is that reproducing failures is very simple. LIT failures log simple shell commands that can be executed to reproduce. Comment at: lit/CMakeLists.txt:14 @@ -13,1 +13,3 @@ +option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off) + The `LLDB_TEST_COMPILER` option doesn't signify that it is using an in-tree or out-of-tree compiler which is significant if we're going to tie the test target to depending on the clang target. We could support that option in addition to this one, but I see them as distinctly different. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
granata.enrico added a comment. In https://reviews.llvm.org/D24591#543277, @beanz wrote: > @granata.enrico, we could migrate the existing tests into being executed by > lit even if they aren't using lit's features, so if that direction is desired > we could get everything in lit. That said, you shouldn't ever really have > multiple incantations. Once we have reliable lit testing that is useful it > should be connected to the "check-lldb" and "check-all" targets > appropriately. Just because it runs more than one type of test doesn't mean > you need multiple incantations, and more and varied testing is generally > better for the quality of the product. The problem is that some of us at Apple build LLDB in Xcode, and then test by saying $ ./dotest.py which means no amount of CMake magic will do anything for us. If Xcode builds are supposed to be deprecated and not-to-be-used, that's a possible path forward (but one I am hearing about for the first time...) Mind you, I would not be opposed to having dotest.py also run lit tests and unit tests - or lit run Python and unit tests as well if that's the preferred direction I am just worried about the multiplication of solutions we see in LLDB (Xcode build vs. Cmake build, unit tests, vs lit tests vs Python tests, ...) - it would be nice to streamline those at some point. Your patch is just getting friendly fire because it adds one more axis to this space is all :) https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
On Wed, Sep 14, 2016 at 3:51 PM Sean Callanan wrote:
> How different is that really from
>
> (lldb) script lldb.frame.FindVariable("argc").GetValue()
> '1'
> (lldb) script lldb.process.GetNumThreads()
> 1
> (lldb) script lldb.process.GetThreadAtIndex(0).GetThreadID()
> 3514809
>
> ? If it's developer-only, then this is even fairly well-documented using
> e.g. "script dir(lldb.process)"
>
Admittedly I hadn't thought of doing that. That said I can think of a
couple of differences:
1. Writing it using script commands means you have to be familiar with the
python api, somewhat raising the barrier to entry for writing new tests.
2. It depends on proper behavior of the script command, which means that if
the script command itself is broken, every single test will be broken.
3. It means that building the lldb python module is required to run tests.
Wouldn't it be awesome if there was a way to run tests even with
LLDB_DISABLE_PYTHON=1? Then script could just be a "feature" (as in, you
could limit tests which use the script command to tests whose purpose is
explicitly to test where the scripting system works, and for those you
could just add "REQUIRES: python" at the top of the lit file. And I
imagine that not invoking python during the running of the test suite would
drastically speed up its execution.
4. You can test a LOT more things when you are able to use an api that
doesn't have to be stable.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
On Wed, Sep 14, 2016 at 4:00 PM Zachary Turner wrote: > > 4. You can test a LOT more things when you are able to use an api that > doesn't have to be stable. > Regarding this point specifically, I think in the past Jim has even floated the idea of having a "private API" that doesn't have to be stable and coudl be used for testing. Only after seeing this review though did I have the idea that such an API could actually just be a single LLDB command. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
beanz updated this revision to Diff 71460.
beanz added a comment.
- Use lit.util.which instead of constructing paths, which adds exe suffixes and
uses proper path seperators
- Fixed spelling error in list(APPEND ...) command
https://reviews.llvm.org/D24591
Files:
lit/CMakeLists.txt
lit/Expr/Inputs/anonymous-struct.cpp
lit/Expr/Inputs/call-function.cpp
lit/Expr/TestCallStdStringFunction.test
lit/Expr/TestCallStopAndContinue.test
lit/Expr/TestCallUserAnonTypedef.test
lit/Expr/TestCallUserDefinedFunction.test
lit/Expr/lit.local.cfg
lit/Unit/lit.cfg
lit/Unit/lit.site.cfg.in
lit/lit.cfg
lit/lit.site.cfg.in
Index: lit/lit.site.cfg.in
===
--- lit/lit.site.cfg.in
+++ lit/lit.site.cfg.in
@@ -8,6 +8,12 @@
config.lldb_obj_root = "@LLDB_BINARY_DIR@"
config.target_triple = "@TARGET_TRIPLE@"
config.python_executable = "@PYTHON_EXECUTABLE@"
+config.host_cc = "@CMAKE_C_COMPILER@"
+config.host_cxx = "@CMAKE_CXX_COMPILER@"
+
+test_clang = "@LLDB_TEST_CLANG@".lower()
+
+config.test_clang = test_clang == "on" or test_clang == "true" or test_clang == "1"
# Support substitution of the tools and libs dirs with user parameters. This is
# used when we can't determine the tool dir at configuration time.
Index: lit/lit.cfg
===
--- lit/lit.cfg
+++ lit/lit.cfg
@@ -112,17 +112,83 @@
lit_config.load_config(config, site_cfg)
raise SystemExit
+# Register substitutions
+config.substitutions.append(('%python', config.python_executable))
+config.substitutions.append(('%host_cc', config.host_cc))
+config.substitutions.append(('%host_cxx', config.host_cxx))
+
+cc = config.host_cc
+cxx = config.host_cxx
+debugserver = lit.util.which('debugserver', llvm_tools_dir)
+lldb = lit.util.which('lldb', llvm_tools_dir)
+
+if config.test_clang:
+cc = lit.util.which('clang', llvm_tools_dir)
+cxx = lit.util.which('clang++', llvm_tools_dir)
+
+if platform.system() in ['Darwin']:
+try:
+out = lit.util.capture(['xcrun', '--show-sdk-path']).strip()
+res = 0
+except OSError:
+res = -1
+if res == 0 and out:
+sdk_path = out
+lit_config.note('using SDKROOT: %r' % sdk_path)
+cxx += " -isysroot %s" % sdk_path
+debugserver = '%s/Library/Frameworks/LLDB.frameworks/Resources/debugserver' % config.llvm_obj_root
+
+
+config.substitutions.append(('%cc', cc))
+config.substitutions.append(('%cxx', cxx))
+
+config.substitutions.append(('%lldb', lldb))
+config.substitutions.append(('%debugserver', debugserver))
+
+for pattern in [r"\bFileCheck\b",
+r"\| \bnot\b"]:
+tool_match = re.match(r"^(\\)?((\| )?)\W+b([0-9A-Za-z-_]+)\\b\W*$",
+ pattern)
+tool_pipe = tool_match.group(2)
+tool_name = tool_match.group(4)
+tool_path = lit.util.which(tool_name, config.llvm_tools_dir)
+if not tool_path:
+# Warn, but still provide a substitution.
+lit_config.note(
+'Did not find ' + tool_name + ' in ' + config.llvm_tools_dir)
+config.substitutions.append((pattern, tool_pipe + tool_path))
+
# Shell execution
if platform.system() not in ['Windows'] or lit_config.getBashPath() != '':
config.available_features.add('shell')
# Running on Darwin OS
if platform.system() in ['Darwin']:
+config.available_features.add('darwin')
config.available_features.add('system-linker-mach-o')
# Running on ELF based *nix
if platform.system() in ['FreeBSD', 'Linux']:
config.available_features.add('system-linker-elf')
+if platform.system() in ['FreeBSD']:
+config.available_features.add('freebsd')
+else:
+config.available_features.add('linux')
+
+if platform.system() in ['Windows']:
+config.available_features.add('windows')
+
+if re.match(r'^arm(hf.*-linux)|(.*-linux-gnuabihf)', config.target_triple):
+config.available_features.add("armhf-linux")
+
+if re.match(r'icc', cc):
+config.available_features.add("compiler-icc")
+elif re.match(r'clang', cc):
+config.available_features.add("compiler-clang")
+elif re.match(r'gcc', cc):
+config.available_features.add("compiler-gcc")
+elif re.match(r'cl', cc):
+config.available_features.add("compiler-msvc")
# llvm-config knows whether it is compiled with asserts (and)
# whether we are operating in release/debug mode.
Index: lit/Unit/lit.site.cfg.in
===
--- lit/Unit/lit.site.cfg.in
+++ lit/Unit/lit.site.cfg.in
@@ -1,5 +1,6 @@
@LIT_SITE_CFG_IN_HEADER@
+config.test_exec_root = "@LLVM_BINARY_DIR@"
config.llvm_src_root = "@LLVM_SOURCE_DIR@"
config.llvm_obj_root = "@LLVM_BINARY_DIR@"
config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
Index: lit/Unit/lit.cfg
===
--- lit/Unit/lit.cfg
+++ lit/Unit/lit.cfg
@@ -6,6 +6,19 @@
import lit.formats
+# Chec
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
beanz added a comment. @granata.enrico, it seems to me that this is a problem the people maintaining the Xcode project should address. I know it is possible to create targets in Xcode projects that call shell commands or makefiles (CMake does this). It is therefore possible that the Xcode project could have a "check" target that runs all the tests, and I really don't think we should limit our testing options based on solvable problems. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added a comment. Todd probably knows about this since he added the gtest target to the Xcode project. I imagine there's a python test suite target and a gtest target. Isn't it possible to have a third target which when you build it, builds the other two targets (which would run the test suites)? If that's not possible, then having a target which invokes a command line would work as beanz mentioned. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added inline comments. Comment at: lit/CMakeLists.txt:14 @@ -13,1 +13,3 @@ +option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off) + beanz wrote: > The `LLDB_TEST_COMPILER` option doesn't signify that it is using an in-tree > or out-of-tree compiler which is significant if we're going to tie the test > target to depending on the clang target. We could support that option in > addition to this one, but I see them as distinctly different. At the same time, setting `LLDB_TEST_COMPILER` to something and `LLDB_TEST_CLANG` seem to be incompatible with each other. Should we error if both of them are set? Another possibility is to allow one to set `LLDB_TEST_COMPILER` to be set to a magic value like `` https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
beanz added inline comments. Comment at: lit/CMakeLists.txt:14 @@ -13,1 +13,3 @@ +option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off) + Disallowing setting both seems reasonable to me. I'm not entirely sure how to connect `LLDB_TEST_COMPILER` up into the lit suite because we really want something that more matches the CMake style of `...__COMPILER` so that we could override multiple compilers. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
I have to concur with Jim's point -- writing & maintaining the gdb testsuite for years, which was based on commands & expected output like these lit tests, was a huge drag on everyone's productivity as the debugger changed over time. This style of test looks wonderfully easy to read & write & debug, but that's only if you ignore long-term maintainability, then you quickly find that it was a huge mistake. I have nothing against lit, this is a very nice way of writing commands & expected output tests, but having lived through the consequences of that style testsuite, I couldn't imagine making that choice again, especially when we've avoided it so far. It's a bit more work to read & write & debug the SB API testsuites that we have today, but it's vastly more maintainable long-term. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added a comment. One more question: Is there a way in lit that we can append command line flags to the run lines even if the user doesn't specify them? For example in the substitution? For example, if someone writes `# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t && %lldb -b -s %s -- %t | FileCheck %s` then this is always going to run `clang function.cpp -g -o`. But we need to manipulate the command line on different platforms. Like on Windows we will need to add `-fms-compatibility` `-fuse-ld=lld`, and various other things. And on other platforms there are other specific things that always have to be added. Is this possible somehow? (You don't have to address it in this patch, just curious) Comment at: lit/CMakeLists.txt:14 @@ -13,1 +13,3 @@ +option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off) + beanz wrote: > Disallowing setting both seems reasonable to me. I'm not entirely sure how to > connect `LLDB_TEST_COMPILER` up into the lit suite because we really want > something that more matches the CMake style of `...__COMPILER` so that > we could override multiple compilers. What if you added a parallel of `LLDB_TEST_COMPILER` directly in this file, that is used specifically for lit tests? Like `LLDB_LIT_TEST_COMPILER` or `LLDB_LIT_CLANG_PATH`? The only reason I'm harping on this is because as it stands, this won't work on windows. (The host compiler is MSVC, which uses a completely different command line syntax, and the in-tree clang is going to be a debug one when doing a debug build, which is going to be unacceptably slow). https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
On Wed, Sep 14, 2016 at 4:28 PM Jason Molenda wrote: > I have to concur with Jim's point -- writing & maintaining the gdb > testsuite for years, which was based on commands & expected output like > these lit tests, was a huge drag on everyone's productivity as the debugger > changed over time. This style of test looks wonderfully easy to read & > write & debug, but that's only if you ignore long-term maintainability, > then you quickly find that it was a huge mistake. I have nothing against > lit, this is a very nice way of writing commands & expected output tests, > but having lived through the consequences of that style testsuite, I > couldn't imagine making that choice again, especially when we've avoided it > so far. It's a bit more work to read & write & debug the SB API testsuites > that we have today, but it's vastly more maintainable long-term. Isn't that solvable though by having commands which return single values instead of large blocks of test, so that you don't have to worry about pattern matching and formatting issues? The only reason the SB API solves this problem is because it allows you to query for specific values instead of for formatted blocks of text. If you can query for the same specific values without the SB API, then the problem still seems to be solved. It's just a different API. I would say it's significantly more work to read, write, and debug the SB API tests, and I'm speculating that that's in part why there's so few. Granted I have no familiarity with GDB's test suite, but I can't imagine it not being possible to solve this. As long as you can query the debugger for data about a sufficiently granular state, I don't foresee this problem being an issue in practice. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
beanz added a comment. @zturner, we can absolutely add flags into the substitutions. Other projects do that too. Comment at: lit/CMakeLists.txt:14 @@ -13,1 +13,3 @@ +option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off) + zturner wrote: > beanz wrote: > > Disallowing setting both seems reasonable to me. I'm not entirely sure how > > to connect `LLDB_TEST_COMPILER` up into the lit suite because we really > > want something that more matches the CMake style of `...__COMPILER` > > so that we could override multiple compilers. > What if you added a parallel of `LLDB_TEST_COMPILER` directly in this file, > that is used specifically for lit tests? Like `LLDB_LIT_TEST_COMPILER` or > `LLDB_LIT_CLANG_PATH`? > > The only reason I'm harping on this is because as it stands, this won't work > on windows. (The host compiler is MSVC, which uses a completely different > command line syntax, and the in-tree clang is going to be a debug one when > doing a debug build, which is going to be unacceptably slow). We could add parallels. How about instead of trying to match the existing model though we go with something more like `LLDB_TEST_C_COMPILER` and `LLDB_TEST_CXX_COMPILER` as values that replace the `%cc` and `%cxx` substitutions respectively? https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner added inline comments. Comment at: lit/CMakeLists.txt:14 @@ -13,1 +13,3 @@ +option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off) + beanz wrote: > zturner wrote: > > beanz wrote: > > > Disallowing setting both seems reasonable to me. I'm not entirely sure > > > how to connect `LLDB_TEST_COMPILER` up into the lit suite because we > > > really want something that more matches the CMake style of > > > `...__COMPILER` so that we could override multiple compilers. > > What if you added a parallel of `LLDB_TEST_COMPILER` directly in this file, > > that is used specifically for lit tests? Like `LLDB_LIT_TEST_COMPILER` or > > `LLDB_LIT_CLANG_PATH`? > > > > The only reason I'm harping on this is because as it stands, this won't > > work on windows. (The host compiler is MSVC, which uses a completely > > different command line syntax, and the in-tree clang is going to be a debug > > one when doing a debug build, which is going to be unacceptably slow). > We could add parallels. How about instead of trying to match the existing > model though we go with something more like `LLDB_TEST_C_COMPILER` and > `LLDB_TEST_CXX_COMPILER` as values that replace the `%cc` and `%cxx` > substitutions respectively? Works for me. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
> On Sep 14, 2016, at 4:00 PM, Zachary Turner wrote:
>
> On Wed, Sep 14, 2016 at 3:51 PM Sean Callanan wrote:
> How different is that really from
>
> (lldb) script lldb.frame.FindVariable("argc").GetValue()
> '1'
> (lldb) script lldb.process.GetNumThreads()
> 1
> (lldb) script lldb.process.GetThreadAtIndex(0).GetThreadID()
> 3514809
>
> ? If it's developer-only, then this is even fairly well-documented using
> e.g. "script dir(lldb.process)"
>
> Admittedly I hadn't thought of doing that. That said I can think of a couple
> of differences:
>
> 1. Writing it using script commands means you have to be familiar with the
> python api, somewhat raising the barrier to entry for writing new tests.
The lldb api is pretty simple, especially for basic stuff like checking
variable values. Plus, if you wanted to write tests like "I have a funky new
type and I want to check that all its children are right" the bit of time you
spent looking at how SBValues works will pay off many-fold in how much more
accurate you can make these tests, and how much more easily you can get exactly
what you want to test, when you aren't trying to parse text output of the value
printing. I think this is really a false tradeoff.
And if you are testing something simple, you can just use Sean's inline tests,
they are not really any more difficult to write than the lit tests (except that
using the SB API's are the natural language, but I see that as a positive, not
a negative.)
>
> 2. It depends on proper behavior of the script command, which means that if
> the script command itself is broken, every single test will be broken.
> 3. It means that building the lldb python module is required to run tests.
> Wouldn't it be awesome if there was a way to run tests even with
> LLDB_DISABLE_PYTHON=1? Then script could just be a "feature" (as in, you
> could limit tests which use the script command to tests whose purpose is
> explicitly to test where the scripting system works, and for those you could
> just add "REQUIRES: python" at the top of the lit file. And I imagine that
> not invoking python during the running of the test suite would drastically
> speed up its execution.
>
I would find that very not awesome, because it would mean ALL our tests are
scraping text output, and I lived through that once already and would rather
not do it again.
> 4. You can test a LOT more things when you are able to use an api that
> doesn't have to be stable.
When I mentioned that API, I had in mind an internal Python module that you
could use to grub into internals. I'm all for that. I'm not so much in favor
of "I have an ad hoc command that prints out various bits of texts, and we'll
use that for testing."
Jim
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
On Wed, Sep 14, 2016 at 4:39 PM Jim Ingham wrote: > > > 4. You can test a LOT more things when you are able to use an api that > doesn't have to be stable. > > When I mentioned that API, I had in mind an internal Python module that > you could use to grub into internals. I'm all for that. I'm not so much > in favor of "I have an ad hoc command that prints out various bits of > texts, and we'll use that for testing." > Is it any different though? Take the API you're imagining to grub for internals. Now imagine the EXACT same API as an lldb command. What's the difference? What could you do with the Python API that you couldn't do with the command API? Aside from write imperative control flow constructs, which I see as a positive rather than a negative. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
If the internal thing is three things in a list, you get back thing one, thing two and thing three in declarative ways rather than "I got three space separated things, maybe they had spaces in them, maybe there were quotes, etc, and don't ever change this now or you'll have to go fix all the tests... I really don't see how going back to irregular text output grubbing is a step forward. Also, the SBInternal API's would be much simpler to write, (a) since you wouldn't have to concern yourself with how the other end was going to parse up what you were outputting and (b) because individual API calls are much easier to write than LLDB commands/sub-commands. You could have your special lldb command that produced JSON and then the test engine had some language to pick fields out, then you could only read this output with confidence, and you're back to the same fragility when testing the results of regular commands. You could change all the regular commands to optionally produce some structured output, but that's a lot of work to reproduce something that already works pretty well. Jim > On Sep 14, 2016, at 4:43 PM, Zachary Turner wrote: > > > > On Wed, Sep 14, 2016 at 4:39 PM Jim Ingham wrote: > > > 4. You can test a LOT more things when you are able to use an api that > > doesn't have to be stable. > > When I mentioned that API, I had in mind an internal Python module that you > could use to grub into internals. I'm all for that. I'm not so much in > favor of "I have an ad hoc command that prints out various bits of texts, and > we'll use that for testing." > > Is it any different though? Take the API you're imagining to grub for > internals. Now imagine the EXACT same API as an lldb command. What's the > difference? What could you do with the Python API that you couldn't do with > the command API? Aside from write imperative control flow constructs, which > I see as a positive rather than a negative. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
> On Sep 14, 2016, at 3:56 PM, Chris Bieneman wrote: > > @jingham and @zturner, we can also take advantage of FileCheck's use of > regular expressions to write robust matchers. In general LLVM has managed to > change text output formats many times in radical ways, and LIT's testing has > still suited the project well. > The gdb testsuite used expect for all its pattern matching. And yet it still ended up both being fragile and making it pretty much impossible to change some aspects of the command output because nobody would ever want to go fix all the tests. Jim ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
You can get thing 1, thing 2, and thing 3 in declarative ways with a command line api too. Like the example I gave earlier: (lldb) print-dev threads[3].id 0x1234 (lldb) print-dev threads[4].id 0x2345 You're getting one value at a time, not a space separated list. Although I think we could take it one step further and say that all developer commands like this must either return one value or a comma separated list with spaces. In any case, we're not talking about the output of arbitrary commands here, we're talking about the output of a very controlled command which we can restrict any way we like. And we can even have tests that validate that this special command's output always matches a particular simple regex to ensure that it's never something that will be confusing to parse. Furthermore, we're talking about a command independent of the normal user command api. There would be zero risk associated with changing the output format of a given command, because it would be completely independent of the command the tests would be using. On Wed, Sep 14, 2016 at 4:51 PM Jim Ingham wrote: > If the internal thing is three things in a list, you get back thing one, > thing two and thing three in declarative ways rather than "I got three > space separated things, maybe they had spaces in them, maybe there were > quotes, etc, and don't ever change this now or you'll have to go fix all > the tests... I really don't see how going back to irregular text output > grubbing is a step forward. > > Also, the SBInternal API's would be much simpler to write, (a) since you > wouldn't have to concern yourself with how the other end was going to parse > up what you were outputting and (b) because individual API calls are much > easier to write than LLDB commands/sub-commands. > > You could have your special lldb command that produced JSON and then the > test engine had some language to pick fields out, then you could only read > this output with confidence, and you're back to the same fragility when > testing the results of regular commands. You could change all the regular > commands to optionally produce some structured output, but that's a lot of > work to reproduce something that already works pretty well. > > Jim > > > > > On Sep 14, 2016, at 4:43 PM, Zachary Turner wrote: > > > > > > > > On Wed, Sep 14, 2016 at 4:39 PM Jim Ingham wrote: > > > > > 4. You can test a LOT more things when you are able to use an api that > doesn't have to be stable. > > > > When I mentioned that API, I had in mind an internal Python module that > you could use to grub into internals. I'm all for that. I'm not so much > in favor of "I have an ad hoc command that prints out various bits of > texts, and we'll use that for testing." > > > > Is it any different though? Take the API you're imagining to grub for > internals. Now imagine the EXACT same API as an lldb command. What's the > difference? What could you do with the Python API that you couldn't do > with the command API? Aside from write imperative control flow constructs, > which I see as a positive rather than a negative. > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
Also, w.r.t: > Aside from write imperative control flow constructs, which I see as a > positive rather than a negative. I wrote a bunch of tests to test that stepping behavior for swift and C was reasonable. When stepping through source code, there is not one correct way to write the line tables, and in fact clang & swiftc change how they describe the source through the line tables all the time. So you have to do: I stepped, and sometimes I'll get to A, sometimes to B, both are "right" but I have to do different things in either case. If A, step again before the next test, if B go to the next test. You could "fix" that by only doing one step per test, and taking each of these as a success. But then you wouldn't test that series of steps don't accumulate errors, you'd only test "run to a breakpoint and step once." That would not be good. So your positive would be very much a negative for this kind of test. Traditionally the answer to this has been: we know we have to keep the current testsuite around but we're just adding other new different ways to write tests. Now you are saying something very different. Do you really mean that? Jim ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
> On Sep 14, 2016, at 3:51 PM, Zachary Turner wrote: > > zturner added a comment. > > Also, it occurred to me that if all tests were like this (and yes, that's a > tall order to imagine a world where not a single test was written using the > Python API), we could probably actually drop the Python 3.5 requirement on > Windows. > > Another thing that's nice about tests like this is that it makes it trivial > to see how to reproduce a failure. It's currently very hard to debug > failures because you have to first figure out where in the test it's failing > (i.e. what line of python), then attach to the python executable and try to > get a breakpoint on the native code side in the right SB API call matching up > with the place where you determined the test is failing. This is really a > pain without a debugger that supports mixed<->native transitions between > python and c++, and even with a debugger that does support it like we have on > Windows, it often doesn't work very well or exhibits flakiness. I must admit, I have never found analyzing individual test failures to be particularly difficult. The test suite tells you exactly which line failed in the test suite. Most of the time looking at the failure line will tell you how to repro the problem with just the test binary. The rest of the time I can go put a few printf's around that line, and quickly figure out what is going wrong. Python is great for this sort of iterative exploration. Once you know what fails, run "dotest -d" attach, break at the SB API that is causing the problem and start debugging. Jim > > > https://reviews.llvm.org/D24591 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
I'm only saying that we should have an open mind. Obviously there are (valid!) concerns. If we can't solve them then we can't solve them. The goal (my goal anyway) is always to make things better, not to use X because it's X. There's value in consistency, but that doesn't mean that the value from consistency always outweighs the value you get from doing a custom thing. So what I'm saying is: IF we can find a way to have one test suite across all of LLVM and it's subprojects, AND it is sufficiently powerful to test all the things we need to test while remaining maintainable in the long term, we should absolutely jump on the opportunity. But this is one of those things that requires a lot of upfront time investment before you can actually know if it can work for 100% of things. Obviously some people don't want to invest their time that way when they're already satisfied, and I don't blame them. But for the people who do, and who think they can solve the problem, what's the harm? Obviously the burden is on those people to prove that their vision can be realized. But if it is successful, then there's no denying the benefits. 1) Tests become easier to write. 2) Tests become easier to debug. 3) Consistency encourages people who have traditionally stayed away from LLDB to contribute. 4) All the people pouring their effort into the custom thing can now pour it into the shared thing, so everybody benefits. I don't blame you for being scared of command tests. I don't support their use in the current LLDB test suite either, for exactly the same reasons you and Jason have expressed. But I do think it's possible to come up with something that a) doesn't suffer from the same problems, b) allows testing a ton of extra functionality that is not currently testable through the api, and c) doesn't rely on python at all. If I'm wrong I'll eat crow :) On Wed, Sep 14, 2016 at 5:00 PM Jim Ingham wrote: > Also, w.r.t: > > > Aside from write imperative control flow constructs, which I see as a > positive rather than a negative. > > I wrote a bunch of tests to test that stepping behavior for swift and C > was reasonable. When stepping through source code, there is not one > correct way to write the line tables, and in fact clang & swiftc change how > they describe the source through the line tables all the time. So you have > to do: I stepped, and sometimes I'll get to A, sometimes to B, both are > "right" but I have to do different things in either case. If A, step again > before the next test, if B go to the next test. > > You could "fix" that by only doing one step per test, and taking each of > these as a success. But then you wouldn't test that series of steps don't > accumulate errors, you'd only test "run to a breakpoint and step once." > That would not be good. So your positive would be very much a negative for > this kind of test. > > Traditionally the answer to this has been: we know we have to keep the > current testsuite around but we're just adding other new different ways to > write tests. Now you are saying something very different. Do you really > mean that? > > Jim > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
Also, you mentioned that this is very similar to the lldbinline tests. For that reason, I would actually propose dropping lldbinline tests in favor of this. If they are essentially the same, then it seems better to have 2 types of tests rather than 3, and it seems better for those 2 to be lit + python rather than lldbinline + python, solely for the reason that lit is shared and receives many performance and stability improvements from non LLDB developers. On Wed, Sep 14, 2016 at 5:13 PM Zachary Turner wrote: > I'm only saying that we should have an open mind. Obviously there are > (valid!) concerns. If we can't solve them then we can't solve them. The > goal (my goal anyway) is always to make things better, not to use X because > it's X. There's value in consistency, but that doesn't mean that the value > from consistency always outweighs the value you get from doing a custom > thing. > > So what I'm saying is: IF we can find a way to have one test suite across > all of LLVM and it's subprojects, AND it is sufficiently powerful to test > all the things we need to test while remaining maintainable in the long > term, we should absolutely jump on the opportunity. > > But this is one of those things that requires a lot of upfront time > investment before you can actually know if it can work for 100% of things. > Obviously some people don't want to invest their time that way when they're > already satisfied, and I don't blame them. > > But for the people who do, and who think they can solve the problem, > what's the harm? Obviously the burden is on those people to prove that > their vision can be realized. > > But if it is successful, then there's no denying the benefits. 1) Tests > become easier to write. 2) Tests become easier to debug. 3) Consistency > encourages people who have traditionally stayed away from LLDB to > contribute. 4) All the people pouring their effort into the custom thing > can now pour it into the shared thing, so everybody benefits. > > I don't blame you for being scared of command tests. I don't support > their use in the current LLDB test suite either, for exactly the same > reasons you and Jason have expressed. But I do think it's possible to come > up with something that a) doesn't suffer from the same problems, b) allows > testing a ton of extra functionality that is not currently testable through > the api, and c) doesn't rely on python at all. If I'm wrong I'll eat crow > :) > > On Wed, Sep 14, 2016 at 5:00 PM Jim Ingham wrote: > > Also, w.r.t: > > > Aside from write imperative control flow constructs, which I see as a > positive rather than a negative. > > I wrote a bunch of tests to test that stepping behavior for swift and C > was reasonable. When stepping through source code, there is not one > correct way to write the line tables, and in fact clang & swiftc change how > they describe the source through the line tables all the time. So you have > to do: I stepped, and sometimes I'll get to A, sometimes to B, both are > "right" but I have to do different things in either case. If A, step again > before the next test, if B go to the next test. > > You could "fix" that by only doing one step per test, and taking each of > these as a success. But then you wouldn't test that series of steps don't > accumulate errors, you'd only test "run to a breakpoint and step once." > That would not be good. So your positive would be very much a negative for > this kind of test. > > Traditionally the answer to this has been: we know we have to keep the > current testsuite around but we're just adding other new different ways to > write tests. Now you are saying something very different. Do you really > mean that? > > Jim > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
> On Sep 14, 2016, at 5:13 PM, Zachary Turner wrote: > > I'm only saying that we should have an open mind. Obviously there are > (valid!) concerns. If we can't solve them then we can't solve them. The > goal (my goal anyway) is always to make things better, not to use X because > it's X. There's value in consistency, but that doesn't mean that the value > from consistency always outweighs the value you get from doing a custom thing. > > So what I'm saying is: IF we can find a way to have one test suite across all > of LLVM and it's subprojects, AND it is sufficiently powerful to test all the > things we need to test while remaining maintainable in the long term, we > should absolutely jump on the opportunity. > > But this is one of those things that requires a lot of upfront time > investment before you can actually know if it can work for 100% of things. > Obviously some people don't want to invest their time that way when they're > already satisfied, and I don't blame them. > > But for the people who do, and who think they can solve the problem, what's > the harm? Obviously the burden is on those people to prove that their vision > can be realized. > > But if it is successful, then there's no denying the benefits. 1) Tests > become easier to write. 2) Tests become easier to debug. 3) Consistency > encourages people who have traditionally stayed away from LLDB to contribute. > 4) All the people pouring their effort into the custom thing can now pour it > into the shared thing, so everybody benefits. I disagree that one of the benefits will be #2. I worry that what will really happen is "easy tests will become slightly easier to write, and complex tests will be fragile when they aren't impossible" which will mean we end up only writing easy tests. Lots of easy tests, but we won't test complex multi-threaded stepping behavior, etc and so we'll end up breaking that sort of thing, which we won't find out about quickly because the failures are intermittent, and hard to write bugs about. Jim > > I don't blame you for being scared of command tests. I don't support their > use in the current LLDB test suite either, for exactly the same reasons you > and Jason have expressed. But I do think it's possible to come up with > something that a) doesn't suffer from the same problems, b) allows testing a > ton of extra functionality that is not currently testable through the api, > and c) doesn't rely on python at all. If I'm wrong I'll eat crow :) > > On Wed, Sep 14, 2016 at 5:00 PM Jim Ingham wrote: > Also, w.r.t: > > > Aside from write imperative control flow constructs, which I see as a > > positive rather than a negative. > > I wrote a bunch of tests to test that stepping behavior for swift and C was > reasonable. When stepping through source code, there is not one correct way > to write the line tables, and in fact clang & swiftc change how they describe > the source through the line tables all the time. So you have to do: I > stepped, and sometimes I'll get to A, sometimes to B, both are "right" but I > have to do different things in either case. If A, step again before the next > test, if B go to the next test. > > You could "fix" that by only doing one step per test, and taking each of > these as a success. But then you wouldn't test that series of steps don't > accumulate errors, you'd only test "run to a breakpoint and step once." That > would not be good. So your positive would be very much a negative for this > kind of test. > > Traditionally the answer to this has been: we know we have to keep the > current testsuite around but we're just adding other new different ways to > write tests. Now you are saying something very different. Do you really > mean that? > > Jim > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
I agree that if all the tests look like the ones in this review there will be no way to test 100% of the things we currently test. At the same time, I see this as just a beginning. A replacement for lldbinline but one which is more extensible. To test things like you said like multi-threaded stepping behavior, a simple sequential list of actions and pattern matches won't be sufficient. So I think a path forward is something like this: 1. get all the easy stuff out of the way first, such as the tests in this patch (and there are probably at least 100 more that are just as easy) 2. Once we have a reasonable number of all easy cases ported, we do some benchmarking (at this point the original versions of these tests are still in the tree). Is it faster? Slower? Just to give us actionable data. 3. Turn on these tests on the bots, let them run for a while and see how stable they are. 4. If all goes well after a week or two, delete the ones that have been ported over and starting using this style of test whenever possible and whenever one would have previously used an lldbinline test. 5. Analyze the remaining difficult tests, and find areas where we can make simple, incremental changes to LLDB's lit test runner to support new types of tests. Some will be easier than others. At every point we're just getting low hanging fruit, small, non-controversial improvements that enable writing new tests in this style. And then just see how far we can get. I could throw out crazy sledgehammer ideas right now, like "allow people to embed SB API code directly in the lit test file", but it seems unproductive since we don't even know exactly what problems we'll encounter. On Wed, Sep 14, 2016 at 5:38 PM Jim Ingham wrote: > > > On Sep 14, 2016, at 5:13 PM, Zachary Turner wrote: > > > > I'm only saying that we should have an open mind. Obviously there are > (valid!) concerns. If we can't solve them then we can't solve them. The > goal (my goal anyway) is always to make things better, not to use X because > it's X. There's value in consistency, but that doesn't mean that the value > from consistency always outweighs the value you get from doing a custom > thing. > > > > So what I'm saying is: IF we can find a way to have one test suite > across all of LLVM and it's subprojects, AND it is sufficiently powerful to > test all the things we need to test while remaining maintainable in the > long term, we should absolutely jump on the opportunity. > > > > But this is one of those things that requires a lot of upfront time > investment before you can actually know if it can work for 100% of things. > Obviously some people don't want to invest their time that way when they're > already satisfied, and I don't blame them. > > > > But for the people who do, and who think they can solve the problem, > what's the harm? Obviously the burden is on those people to prove that > their vision can be realized. > > > > But if it is successful, then there's no denying the benefits. 1) Tests > become easier to write. 2) Tests become easier to debug. 3) Consistency > encourages people who have traditionally stayed away from LLDB to > contribute. 4) All the people pouring their effort into the custom thing > can now pour it into the shared thing, so everybody benefits. > > I disagree that one of the benefits will be #2. I worry that what will > really happen is "easy tests will become slightly easier to write, and > complex tests will be fragile when they aren't impossible" which will mean > we end up only writing easy tests. Lots of easy tests, but we won't test > complex multi-threaded stepping behavior, etc and so we'll end up breaking > that sort of thing, which we won't find out about quickly because the > failures are intermittent, and hard to write bugs about. > > Jim > > > > > > I don't blame you for being scared of command tests. I don't support > their use in the current LLDB test suite either, for exactly the same > reasons you and Jason have expressed. But I do think it's possible to come > up with something that a) doesn't suffer from the same problems, b) allows > testing a ton of extra functionality that is not currently testable through > the api, and c) doesn't rely on python at all. If I'm wrong I'll eat crow > :) > > > > On Wed, Sep 14, 2016 at 5:00 PM Jim Ingham wrote: > > Also, w.r.t: > > > > > Aside from write imperative control flow constructs, which I see as a > positive rather than a negative. > > > > I wrote a bunch of tests to test that stepping behavior for swift and C > was reasonable. When stepping through source code, there is not one > correct way to write the line tables, and in fact clang & swiftc change how > they describe the source through the line tables all the time. So you have > to do: I stepped, and sometimes I'll get to A, sometimes to B, both are > "right" but I have to do different things in either case. If A, step again > before the next test, if B go to the next test. > > >
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
I agree completely with tackling the easy stuff first. That said, the easy stuff (probably >50% of the testsuite) doesn't even require command-line interaction and is just of the form "stop here, run this one expression, maybe print this variable using 'frame variab'e'" I would argue that we can find a simpler way to handle these cases than feeding lldb command lines to the command-line parser. To be clear, that simpler way is not the SB API, it's probably a special-purpose language that knows how to stop at breakpoints and inspect locals/run expressions. That's all it does. The SB API / command line parser argument is for what I'd argue are the complex cases, and I think we should deal with those later because they're both technically hairy and seem to attract more discussion. Sean > On Sep 14, 2016, at 5:57 PM, Zachary Turner wrote: > > 1. get all the easy stuff out of the way first, such as the tests in this > patch (and there are probably at least 100 more that are just as easy) > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
I actually like the idea of feeding command lines. Again, not when the command you're running prints a huge block of text and you try to match it. But for "get the debugger into the state where i can reproduce the bug", and sometimes the step to actually repro it, I think it's hands down the best. You can look at a test and tell in a few seconds how to repro it. You can grok the test almost immediately. More importantly, it gives you everything you need to repro it under a debugger. If you wanted to be fancy it could even spit out a command file so you could get the test case failing in a debugger in seconds On Wed, Sep 14, 2016 at 6:02 PM Sean Callanan wrote: > I agree completely with tackling the easy stuff first. > That said, the easy stuff (probably >50% of the testsuite) doesn't even > require command-line interaction and is just of the form "stop here, run > this one expression, maybe print this variable using 'frame variab'e'" > I would argue that we can find a simpler way to handle these cases than > feeding lldb command lines to the command-line parser. > To be clear, that simpler way is not the SB API, it's probably a > special-purpose language that knows how to stop at breakpoints and inspect > locals/run expressions. That's all it does. > The SB API / command line parser argument is for what I'd argue are the > complex cases, and I think we should deal with those later because they're > both technically hairy and seem to attract more discussion. > > Sean > > On Sep 14, 2016, at 5:57 PM, Zachary Turner wrote: > > 1. get all the easy stuff out of the way first, such as the tests in this > patch (and there are probably at least 100 more that are just as easy) > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r281569 - Make the keys enumerations for options and resolvers enum classes.
Author: jingham
Date: Wed Sep 14 20:47:22 2016
New Revision: 281569
URL: http://llvm.org/viewvc/llvm-project?rev=281569&view=rev
Log:
Make the keys enumerations for options and resolvers enum classes.
This keeps them from conflicting with other symbols names, so it's
worth their being less convenient to use for indexing.
Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h
lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
lldb/trunk/source/Breakpoint/BreakpointResolver.cpp
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h?rev=281569&r1=281568&r2=281569&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h Wed Sep 14 20:47:22
2016
@@ -51,17 +51,18 @@ public:
bool stop_on_error;
private:
-enum OptionNames {
+enum class OptionNames : uint32_t {
UserSource = 0,
ScriptSource,
StopOnError,
LastOptionName
};
-static const char *g_option_names[LastOptionName];
+static const char
+*g_option_names[static_cast(OptionNames::LastOptionName)];
static const char *GetKey(enum OptionNames enum_value) {
- return g_option_names[enum_value];
+ return g_option_names[static_cast(enum_value)];
}
};
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h?rev=281569&r1=281568&r2=281569&view=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h Wed Sep 14 20:47:22
2016
@@ -202,7 +202,7 @@ protected:
// Used for serializing resolver options:
// The options in this enum and the strings in the
// g_option_names must be kept in sync.
- enum OptionNames {
+ enum class OptionNames : uint32_t {
AddressOffset = 0,
ExactMatch,
FileName,
@@ -218,11 +218,12 @@ protected:
SymbolNameArray,
LastOptionName
};
- static const char *g_option_names[LastOptionName];
+ static const char
+ *g_option_names[static_cast(OptionNames::LastOptionName)];
public:
static const char *GetKey(enum OptionNames enum_value) {
-return g_option_names[enum_value];
+return g_option_names[static_cast(enum_value)];
}
protected:
Modified: lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointOptions.cpp?rev=281569&r1=281568&r2=281569&view=diff
==
--- lldb/trunk/source/Breakpoint/BreakpointOptions.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointOptions.cpp Wed Sep 14 20:47:22 2016
@@ -28,8 +28,9 @@
using namespace lldb;
using namespace lldb_private;
-const char *BreakpointOptions::CommandData::g_option_names
-[BreakpointOptions::CommandData::OptionNames::LastOptionName]{
+const char
+*BreakpointOptions::CommandData::g_option_names[static_cast(
+BreakpointOptions::CommandData::OptionNames::LastOptionName)]{
"UserSource", "ScriptSource", "StopOnError"};
StructuredData::ObjectSP
Modified: lldb/trunk/source/Breakpoint/BreakpointResolver.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolver.cpp?rev=281569&r1=281568&r2=281569&view=diff
==
--- lldb/trunk/source/Breakpoint/BreakpointResolver.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointResolver.cpp Wed Sep 14 20:47:22 2016
@@ -42,11 +42,11 @@ const char *BreakpointResolver::g_ty_to_
"SymbolName", "SourceRegex",
"Exception", "Unknown"};
-const char *BreakpointResolver::g_option_names
-[BreakpointResolver::OptionNames::LastOptionName] = {
-"AddressOffset", "Exact","FileName", "Inlines", "Language",
-"LineNumber","ModuleName", "NameMask", "Offset", "Regex",
-"SectionName", "SkipPrologue", "SymbolNames"};
+const char *BreakpointResolver::g_option_names[static_cast(
+BreakpointResolver::OptionNames::LastOptionName)] = {
+"AddressOffset", "Exact","FileName", "Inlines", "Language",
+"LineNumber","ModuleName", "NameMask", "Offset", "Regex",
+"SectionName", "SkipPrologue", "SymbolNames"};
const char *BreakpointResolver::ResolverTyToName(enum ResolverTy type) {
if (type > LastKnownResolverType)
___
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
zturner accepted this revision. zturner added a comment. lgtm from my point of view after `LLDB_TEST_CXX_COMPILER` and `LLDB_TEST_C_COMPILER` are added. One question: Currently the tests appear to run all commands until the end of the file, and then do 1 pass over the output through `FileCheck`. Is this correct? We have a lot of tests where the command to run next depends on the output of a previous command. If the test is running LLDB commands rather than using the python api, this means you need some pexpect-style solution that can read output and send input to the process as it's running. Does lit have anything like this? I'm pretty sure the answer is no, but just want to check. We can punt on supporting this type of test until later, just want to find out if it's already supported. https://reviews.llvm.org/D24591 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
On Sep 14, 2016, at 5:13 PM, Zachary Turner wrote: > I don't blame you for being scared of command tests. I don't support their > use in the current LLDB test suite either, for exactly the same reasons you > and Jason have expressed. But I do think it's possible to come up with > something that a) doesn't suffer from the same problems, b) allows testing a > ton of extra functionality that is not currently testable through the api, > and c) doesn't rely on python at all. If I'm wrong I'll eat crow :) I think your examples are reductive to "printing values is easy to keep consistent" -- but that's only a small part of what a debugger testsuite needs to do. If we want to use lit to drive tests written in terms of SB API, then I'm open to seeing how this tool can be used in lldb. If lit can only be used to write command output tests, then I believe, based on years and years of experience with exactly that kind of test suite in with gdb, that this is a very poor addition to lldb. It will only hamper future lldb development as we want to improve the lldb command line UI - as it did with gdb. Every change to the command line UI breaks hundreds of "easy to write" tests. And who is responsible for cleaning all those up? The people who wrote these easy-to-write command line output matching tests? No, it'll be the person trying to improve the debugger, which is a big disincentive to changing anything else you'll need to wade through the swamp of thousands of accumulated test cases. There's a cost to writing a test case. Either it is paid up front when you write the test in terms of SB API, or it is paid repeatedly over time as people change the UI to the command line tool. I have nothing against lit per se, I'm open to seeing what an SB API based test case in that harness looks like. I have nothing but objections to adding significant new additions to the testsuite that are locked to the command line UI behavior of lldb today. J ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
There's also a cost to *not* writing test cases, which is paid proportionally to how difficult it is to write test cases. That said, aren't the existing lldbinline tests an example of the behavior you're objecting to? And that is a relatively recent addition which I have never seen or heard of any objections to. On Wed, Sep 14, 2016 at 9:07 PM Jason Molenda wrote: > > On Sep 14, 2016, at 5:13 PM, Zachary Turner wrote: > > > I don't blame you for being scared of command tests. I don't support > their use in the current LLDB test suite either, for exactly the same > reasons you and Jason have expressed. But I do think it's possible to come > up with something that a) doesn't suffer from the same problems, b) allows > testing a ton of extra functionality that is not currently testable through > the api, and c) doesn't rely on python at all. If I'm wrong I'll eat crow > :) > > > I think your examples are reductive to "printing values is easy to keep > consistent" -- but that's only a small part of what a debugger testsuite > needs to do. > > If we want to use lit to drive tests written in terms of SB API, then I'm > open to seeing how this tool can be used in lldb. > > If lit can only be used to write command output tests, then I believe, > based on years and years of experience with exactly that kind of test suite > in with gdb, that this is a very poor addition to lldb. It will only > hamper future lldb development as we want to improve the lldb command line > UI - as it did with gdb. Every change to the command line UI breaks > hundreds of "easy to write" tests. And who is responsible for cleaning all > those up? The people who wrote these easy-to-write command line output > matching tests? No, it'll be the person trying to improve the debugger, > which is a big disincentive to changing anything else you'll need to wade > through the swamp of thousands of accumulated test cases. > > There's a cost to writing a test case. Either it is paid up front when > you write the test in terms of SB API, or it is paid repeatedly over time > as people change the UI to the command line tool. > > I have nothing against lit per se, I'm open to seeing what an SB API based > test case in that harness looks like. I have nothing but objections to > adding significant new additions to the testsuite that are locked to the > command line UI behavior of lldb today. > > > J ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
Also, sb api and command line test are neither mutually exclusive nor the only 2 options. Sean mentioned the idea of a dsl for example. I mentioned the idea of a command that is independent of the normal ui command line. There are lots of possibilities if you look. Also our current situation is far from perfect. There is flakiness on every platform. Python is slow. There's complexity in supporting both Python 2 and 3. The test suite is about 80% smaller than it should be since so much goes into writing one. I don't know if there's any buildbots at all running tests due to how fragile everything is. Using lit solves many of those problems immediately, and makes it possible to solve the rest. Maybe we've solved the problem of hard to maintain tests, which again I never experienced, but it's hard to argue that we're in a good state. On Wed, Sep 14, 2016 at 9:36 PM Zachary Turner wrote: > There's also a cost to *not* writing test cases, which is paid > proportionally to how difficult it is to write test cases. > > That said, aren't the existing lldbinline tests an example of the behavior > you're objecting to? And that is a relatively recent addition which I have > never seen or heard of any objections to. > > On Wed, Sep 14, 2016 at 9:07 PM Jason Molenda wrote: > > > On Sep 14, 2016, at 5:13 PM, Zachary Turner wrote: > > > I don't blame you for being scared of command tests. I don't support > their use in the current LLDB test suite either, for exactly the same > reasons you and Jason have expressed. But I do think it's possible to come > up with something that a) doesn't suffer from the same problems, b) allows > testing a ton of extra functionality that is not currently testable through > the api, and c) doesn't rely on python at all. If I'm wrong I'll eat crow > :) > > > I think your examples are reductive to "printing values is easy to keep > consistent" -- but that's only a small part of what a debugger testsuite > needs to do. > > If we want to use lit to drive tests written in terms of SB API, then I'm > open to seeing how this tool can be used in lldb. > > If lit can only be used to write command output tests, then I believe, > based on years and years of experience with exactly that kind of test suite > in with gdb, that this is a very poor addition to lldb. It will only > hamper future lldb development as we want to improve the lldb command line > UI - as it did with gdb. Every change to the command line UI breaks > hundreds of "easy to write" tests. And who is responsible for cleaning all > those up? The people who wrote these easy-to-write command line output > matching tests? No, it'll be the person trying to improve the debugger, > which is a big disincentive to changing anything else you'll need to wade > through the swamp of thousands of accumulated test cases. > > There's a cost to writing a test case. Either it is paid up front when > you write the test in terms of SB API, or it is paid repeatedly over time > as people change the UI to the command line tool. > > I have nothing against lit per se, I'm open to seeing what an SB API based > test case in that harness looks like. I have nothing but objections to > adding significant new additions to the testsuite that are locked to the > command line UI behavior of lldb today. > > > J > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
It's great to make writing tests easier. We'd all love to have more tests. If "writing tests easier" is "command line output scraping", that's only hurting the project in the long term. I'm telling you this from years of experience on a project where we did exactly this. It's great to have examples where we're testing that "p 5" prints 5. That's awesome and I'm sure it won't break. But that's not more than a tiny fraction of what you need to test in a debugger. It's important to separate "let's use lit" and "let's write command line scraping tests", unless lit means "command line scraping tests", in which case it's fine to conflate the two. No one is going to be up in arms about lit as a test harness -- but using that as a way to get lots of command line scraping tests added to lldb is disingenuous. We've had this discussion many times on the mailing lists, and all of us who had to work on a scraping testsuite have pushed back hard against the siren song of these tests because we've learned what a big mistake it is. Talking about python 2 v python 3 compatibility or speed of the testsuite -- these are implementation side issues that I don't have an opinion on. If we want to write our SB API in a different way to solve those issues -- want to write the tests in C++? Fine, whatever. I think that would just make it slower to write new tests. Can tests in lit use the SB API exclusively? Then I don't care about adding lit. I remain firmly against command line scraping tests, it only adds debt to the project long term. It's a lesson learned from gdb that bears not repeating. J > On Sep 14, 2016, at 9:36 PM, Zachary Turner wrote: > > There's also a cost to *not* writing test cases, which is paid proportionally > to how difficult it is to write test cases. > > That said, aren't the existing lldbinline tests an example of the behavior > you're objecting to? And that is a relatively recent addition which I have > never seen or heard of any objections to. > > On Wed, Sep 14, 2016 at 9:07 PM Jason Molenda wrote: > > On Sep 14, 2016, at 5:13 PM, Zachary Turner wrote: > > > I don't blame you for being scared of command tests. I don't support their > > use in the current LLDB test suite either, for exactly the same reasons you > > and Jason have expressed. But I do think it's possible to come up with > > something that a) doesn't suffer from the same problems, b) allows testing > > a ton of extra functionality that is not currently testable through the > > api, and c) doesn't rely on python at all. If I'm wrong I'll eat crow :) > > > I think your examples are reductive to "printing values is easy to keep > consistent" -- but that's only a small part of what a debugger testsuite > needs to do. > > If we want to use lit to drive tests written in terms of SB API, then I'm > open to seeing how this tool can be used in lldb. > > If lit can only be used to write command output tests, then I believe, based > on years and years of experience with exactly that kind of test suite in with > gdb, that this is a very poor addition to lldb. It will only hamper future > lldb development as we want to improve the lldb command line UI - as it did > with gdb. Every change to the command line UI breaks hundreds of "easy to > write" tests. And who is responsible for cleaning all those up? The people > who wrote these easy-to-write command line output matching tests? No, it'll > be the person trying to improve the debugger, which is a big disincentive to > changing anything else you'll need to wade through the swamp of thousands of > accumulated test cases. > > There's a cost to writing a test case. Either it is paid up front when you > write the test in terms of SB API, or it is paid repeatedly over time as > people change the UI to the command line tool. > > I have nothing against lit per se, I'm open to seeing what an SB API based > test case in that harness looks like. I have nothing but objections to > adding significant new additions to the testsuite that are locked to the > command line UI behavior of lldb today. > > > J ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24591: [LIT] First pass of LLDB LIT support
If we want to add a testsuite runner which takes a source file, a place to put a breakpoint, the name of a variable to examine, and it runs through those in SB API, I'm all in favor. I don't know if that's going to add a lot of test coverage to lldb, but I have no problem with such a thing. My goal isn't to make test case writing hard. My goal is to make the testsuite a benefit to the project, instead of a boat anchor. My non-goal is what testsuite harness is used to run the SB API tests - whether it's lit or whatever, I don't care. > On Sep 14, 2016, at 11:19 PM, Jason Molenda wrote: > > It's great to make writing tests easier. We'd all love to have more tests. > > If "writing tests easier" is "command line output scraping", that's only > hurting the project in the long term. I'm telling you this from years of > experience on a project where we did exactly this. It's great to have > examples where we're testing that "p 5" prints 5. That's awesome and I'm > sure it won't break. But that's not more than a tiny fraction of what you > need to test in a debugger. > > It's important to separate "let's use lit" and "let's write command line > scraping tests", unless lit means "command line scraping tests", in which > case it's fine to conflate the two. No one is going to be up in arms about > lit as a test harness -- but using that as a way to get lots of command line > scraping tests added to lldb is disingenuous. We've had this discussion many > times on the mailing lists, and all of us who had to work on a scraping > testsuite have pushed back hard against the siren song of these tests because > we've learned what a big mistake it is. > > Talking about python 2 v python 3 compatibility or speed of the testsuite -- > these are implementation side issues that I don't have an opinion on. If we > want to write our SB API in a different way to solve those issues -- want to > write the tests in C++? Fine, whatever. I think that would just make it > slower to write new tests. Can tests in lit use the SB API exclusively? > Then I don't care about adding lit. > > I remain firmly against command line scraping tests, it only adds debt to the > project long term. It's a lesson learned from gdb that bears not repeating. > > J > > >> On Sep 14, 2016, at 9:36 PM, Zachary Turner wrote: >> >> There's also a cost to *not* writing test cases, which is paid >> proportionally to how difficult it is to write test cases. >> >> That said, aren't the existing lldbinline tests an example of the behavior >> you're objecting to? And that is a relatively recent addition which I have >> never seen or heard of any objections to. >> >> On Wed, Sep 14, 2016 at 9:07 PM Jason Molenda wrote: >> >> On Sep 14, 2016, at 5:13 PM, Zachary Turner wrote: >> >>> I don't blame you for being scared of command tests. I don't support their >>> use in the current LLDB test suite either, for exactly the same reasons you >>> and Jason have expressed. But I do think it's possible to come up with >>> something that a) doesn't suffer from the same problems, b) allows testing >>> a ton of extra functionality that is not currently testable through the >>> api, and c) doesn't rely on python at all. If I'm wrong I'll eat crow :) >> >> >> I think your examples are reductive to "printing values is easy to keep >> consistent" -- but that's only a small part of what a debugger testsuite >> needs to do. >> >> If we want to use lit to drive tests written in terms of SB API, then I'm >> open to seeing how this tool can be used in lldb. >> >> If lit can only be used to write command output tests, then I believe, based >> on years and years of experience with exactly that kind of test suite in >> with gdb, that this is a very poor addition to lldb. It will only hamper >> future lldb development as we want to improve the lldb command line UI - as >> it did with gdb. Every change to the command line UI breaks hundreds of >> "easy to write" tests. And who is responsible for cleaning all those up? >> The people who wrote these easy-to-write command line output matching tests? >> No, it'll be the person trying to improve the debugger, which is a big >> disincentive to changing anything else you'll need to wade through the swamp >> of thousands of accumulated test cases. >> >> There's a cost to writing a test case. Either it is paid up front when you >> write the test in terms of SB API, or it is paid repeatedly over time as >> people change the UI to the command line tool. >> >> I have nothing against lit per se, I'm open to seeing what an SB API based >> test case in that harness looks like. I have nothing but objections to >> adding significant new additions to the testsuite that are locked to the >> command line UI behavior of lldb today. >> >> >> J > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.or
