[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.
JDevlieghere updated this revision to Diff 173112.
JDevlieghere marked 18 inline comments as done.
JDevlieghere added a comment.
Feedback from Jim
https://reviews.llvm.org/D54221
Files:
include/lldb/API/SBBreakpoint.h
include/lldb/API/SBThreadPlan.h
include/lldb/Breakpoint/Breakpoint.h
include/lldb/Target/Target.h
include/lldb/Target/Thread.h
include/lldb/Target/ThreadPlan.h
include/lldb/Target/ThreadPlanPython.h
include/lldb/Target/ThreadPlanShouldStopHere.h
include/lldb/Target/ThreadPlanStepInRange.h
include/lldb/Target/ThreadPlanStepInstruction.h
include/lldb/Target/ThreadPlanStepOut.h
include/lldb/Target/ThreadPlanStepThrough.h
include/lldb/Target/ThreadPlanStepUntil.h
include/lldb/lldb-private-interfaces.h
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
scripts/interface/SBBreakpoint.i
source/API/SBBreakpoint.cpp
source/API/SBThread.cpp
source/API/SBThreadPlan.cpp
source/Breakpoint/Breakpoint.cpp
source/Commands/CommandObjectThread.cpp
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
source/Target/Process.cpp
source/Target/StopInfo.cpp
source/Target/Target.cpp
source/Target/Thread.cpp
source/Target/ThreadPlan.cpp
source/Target/ThreadPlanCallOnFunctionExit.cpp
source/Target/ThreadPlanPython.cpp
source/Target/ThreadPlanRunToAddress.cpp
source/Target/ThreadPlanShouldStopHere.cpp
source/Target/ThreadPlanStepInRange.cpp
source/Target/ThreadPlanStepInstruction.cpp
source/Target/ThreadPlanStepOut.cpp
source/Target/ThreadPlanStepOverRange.cpp
source/Target/ThreadPlanStepRange.cpp
source/Target/ThreadPlanStepThrough.cpp
source/Target/ThreadPlanStepUntil.cpp
Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -57,7 +57,10 @@
m_return_addr = return_frame_sp->GetStackID().GetPC();
Breakpoint *return_bp =
target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+ m_could_not_resolve_hw_bp = true;
return_bp->SetThreadID(thread_id);
m_return_bp_id = return_bp->GetID();
return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +100,7 @@
}
}
m_until_points.clear();
+ m_could_not_resolve_hw_bp = false;
}
void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +131,16 @@
}
bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
- if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+ if (m_could_not_resolve_hw_bp) {
+if (error)
+ error->PutCString(
+ "Could not create hardware breakpoint for thread plan.");
+return false;
+ } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+ error->PutCString("Could not create return breakpoint.");
return false;
- else {
+ } else {
until_collection::iterator pos, end = m_until_points.end();
for (pos = m_until_points.begin(); pos != end; pos++) {
if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -62,7 +62,10 @@
->GetTarget()
.CreateBreakpoint(m_backstop_addr, true, false)
.get();
+
if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+ m_could_not_resolve_hw_bp = true;
return_bp->SetThreadID(m_thread.GetID());
m_backstop_bkpt_id = return_bp->GetID();
return_bp->SetBreakpointKind("step-through-backstop");
@@ -139,7 +142,26 @@
}
bool ThreadPlanStepThrough::ValidatePlan(Stream *error) {
- return m_sub_plan_sp.get() != nullptr;
+ if (m_could_not_resolve_hw_bp) {
+if (error)
+ error->PutCString(
+ "Could not create hardware breakpoint for thread plan.");
+return false;
+ }
+
+ if (m_backstop_bkpt_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+ error->PutCString("Could not create backstop breakpoint.");
+return false;
+ }
+
+ if (!m_sub_plan_sp.get()) {
+if (error)
+ error->PutCString("Does not have a subplan.");
+return false;
+ }
+
+ return true;
}
bool ThreadPlanStepThrough::DoPlanExplainsStop(Event *event_ptr) {
@@ -215,6 +237,7 @@
if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID) {
[Lldb-commits] [PATCH] D54241: Fix bug in printing ValueObjects and in PE/COFF Plugin
zturner updated this revision to Diff 173103.
zturner added a comment.
I actually found an even better fix for this. LLDB sections already have the
notion of "zero fill sections", we just need to mark the `.data` section as a
zero fill section in this condition. No need to override `ReadSectionData`
https://reviews.llvm.org/D54241
Files:
lldb/lit/SymbolFile/NativePDB/Inputs/globals-bss.lldbinit
lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
lldb/lit/SymbolFile/NativePDB/globals-bss.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -710,7 +710,10 @@
llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA &&
((const_sect_name == g_data_sect_name) ||
(const_sect_name == g_DATA_sect_name))) {
- section_type = eSectionTypeData;
+ if (m_sect_headers[idx].size == 0 && m_sect_headers[idx].offset == 0)
+section_type = eSectionTypeZeroFill;
+ else
+section_type = eSectionTypeData;
} else if (m_sect_headers[idx].flags &
llvm::COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA &&
((const_sect_name == g_bss_sect_name) ||
@@ -1053,6 +1056,7 @@
}
ObjectFile::Strata ObjectFilePECOFF::CalculateStrata() { return eStrataUser; }
+
//--
// PluginInterface protocol
//--
Index: lldb/source/Core/ValueObjectVariable.cpp
===
--- lldb/source/Core/ValueObjectVariable.cpp
+++ lldb/source/Core/ValueObjectVariable.cpp
@@ -67,7 +67,7 @@
CompilerType ValueObjectVariable::GetCompilerTypeImpl() {
Type *var_type = m_variable_sp->GetType();
if (var_type)
-return var_type->GetForwardCompilerType();
+return var_type->GetFullCompilerType();
return CompilerType();
}
Index: lldb/lit/SymbolFile/NativePDB/globals-bss.cpp
===
--- /dev/null
+++ lldb/lit/SymbolFile/NativePDB/globals-bss.cpp
@@ -0,0 +1,35 @@
+// clang-format off
+// REQUIRES: lld
+
+// Make sure we can read variables from BSS
+// RUN: clang-cl /Z7 /GS- /GR- /c /Fo%t.obj -- %s
+// RUN: lld-link /DEBUG /nodefaultlib /entry:main /OUT:%t.exe /PDB:%t.pdb -- %t.obj
+// RUN: llvm-readobj -s %t.exe | FileCheck --check-prefix=BSS %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/globals-bss.lldbinit 2>&1 | FileCheck %s
+
+int GlobalVariable = 0;
+
+int main(int argc, char **argv) {
+ return 0;
+}
+
+// BSS: Section {
+// BSS: Number: 3
+// BSS: Name: .data
+// BSS-NEXT:VirtualSize: 0x4
+// BSS-NEXT:VirtualAddress:
+// BSS-NEXT:RawDataSize: 0
+// BSS-NEXT:PointerToRawData: 0x0
+// BSS-NEXT:PointerToRelocations: 0x0
+// BSS-NEXT:PointerToLineNumbers: 0x0
+// BSS-NEXT:RelocationCount: 0
+// BSS-NEXT:LineNumberCount: 0
+// BSS-NEXT:Characteristics [ (0xC040)
+// BSS-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA (0x40)
+// BSS-NEXT: IMAGE_SCN_MEM_READ (0x4000)
+// BSS-NEXT: IMAGE_SCN_MEM_WRITE (0x8000)
+// BSS-NEXT:]
+// BSS-NEXT: }
+
+// CHECK: (int) GlobalVariable = 0
Index: lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
===
--- lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
+++ lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
@@ -89,22 +89,23 @@
// CHECK: (TrivialC) TC = {}
// CHECK: (TrivialS) TS = {}
// CHECK: (TrivialU) TU = {}
-// CHECK: (TrivialE) TE =
-// CHECK: (A::B::C) ABCInt = (ABCMember = )
-// CHECK: (A::B::C) ABCFloat = (ABCMember = )
-// CHECK: (A::B::C) ABCVoid = (ABCSpecializationMember = )
+// CHECK: (TrivialE) TE = TE_A
+// CHECK: (A::B::C) ABCInt = (ABCMember = 0)
+// CHECK: (A::B::C) ABCFloat = (ABCMember = 0)
+// CHECK: (A::B::C) ABCVoid = (ABCSpecializationMember = 0x)
// CHECK: (A::C<0>) AC0 = {}
// CHECK: (A::C<-1>) ACNeg1 = {}
-// CHECK: (A::C<0>::D) AC0D = (ACDMember = , CPtr = )
-// CHECK: (A::C<-1>::D) ACNeg1D = (ACDMember = , CPtr = )
+// CHECK: (A::C<0>::D) AC0D = (ACDMember = 0, CPtr = 0x)
+// CHECK: (A::C<-1>::D) ACNeg1D = (ACDMember = 0, CPtr = 0x)
// CHECK: (A::D) AD = {}
-// CHECK: (A::D::E) ADE = (ADDMember = )
+// CHECK: (A::D::E) ADE = (ADDMember = 0)
// CHECK: Dumping clang ast for 1 modules.
// CHECK: TranslationUnitDecl {{.*}}
// CHECK: |-CXXRecordDecl {{.*}} class TrivialC definition
// CHECK: |-CXXRecordDecl {{.*}} struct TrivialS definition
//
[Lldb-commits] [PATCH] D54241: Fix bug in printing ValueObjects and in PE/COFF Plugin
lemo added a comment. Shouldn't we also handle the general case where raw size < virtual size? (which would naturally subsume this fix) Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:1073 + + if (section_offset >= section->GetByteSize()) +return 0; shouldn't this be a hard error instead of returning 0? Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:1077 + size_t bytes_avail = section->GetByteSize() - section_offset; + size_t read_size = std::min(dst_len, bytes_avail); + ::memset(dst, 0, read_size); do we really need to return the available "bytes" if the full read size can't be fullfiled? (ie. shouldn't that be a hard error instead?) https://reviews.llvm.org/D54241 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54241: Fix bug in printing ValueObjects and in PE/COFF Plugin
zturner updated this revision to Diff 173095.
zturner added a comment.
I added a test specifically for the bss bug. Even though one of my existing
broken tests became fixed, someone could come down the line and adjust the test
and set one of the variables to 1 which would cause the data to go into a
regular section. To make sure this doesn't happen, I made a specific test just
for this, and used `llvm-readobj` to also check that that the size of the
section is in fact 0. So this test will always fail if the section stops being
a compressed bss section for whatever reason.
https://reviews.llvm.org/D54241
Files:
lldb/lit/SymbolFile/NativePDB/Inputs/globals-bss.lldbinit
lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
lldb/lit/SymbolFile/NativePDB/globals-bss.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -125,6 +125,10 @@
ObjectFile::Strata CalculateStrata() override;
+ size_t ReadSectionData(lldb_private::Section *section,
+ lldb::offset_t section_offset, void *dst,
+ size_t dst_len) override;
+
//--
// PluginInterface protocol
//--
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1053,6 +1053,32 @@
}
ObjectFile::Strata ObjectFilePECOFF::CalculateStrata() { return eStrataUser; }
+
+static bool IsCompressedBss(const Section ) {
+ if (section.GetName() != ConstString(".data"))
+return false;
+ if (section.GetFileSize() != 0)
+return false;
+ if (section.GetFileOffset() != 0)
+return false;
+ return true;
+}
+
+size_t ObjectFilePECOFF::ReadSectionData(Section *section,
+ lldb::offset_t section_offset,
+ void *dst, size_t dst_len) {
+ if (!IsCompressedBss(*section))
+return ObjectFile::ReadSectionData(section, section_offset, dst, dst_len);
+
+ if (section_offset >= section->GetByteSize())
+return 0;
+
+ size_t bytes_avail = section->GetByteSize() - section_offset;
+ size_t read_size = std::min(dst_len, bytes_avail);
+ ::memset(dst, 0, read_size);
+ return read_size;
+}
+
//--
// PluginInterface protocol
//--
Index: lldb/source/Core/ValueObjectVariable.cpp
===
--- lldb/source/Core/ValueObjectVariable.cpp
+++ lldb/source/Core/ValueObjectVariable.cpp
@@ -67,7 +67,7 @@
CompilerType ValueObjectVariable::GetCompilerTypeImpl() {
Type *var_type = m_variable_sp->GetType();
if (var_type)
-return var_type->GetForwardCompilerType();
+return var_type->GetFullCompilerType();
return CompilerType();
}
Index: lldb/lit/SymbolFile/NativePDB/globals-bss.cpp
===
--- /dev/null
+++ lldb/lit/SymbolFile/NativePDB/globals-bss.cpp
@@ -0,0 +1,35 @@
+// clang-format off
+// REQUIRES: lld
+
+// Make sure we can read variables from BSS
+// RUN: clang-cl /Z7 /GS- /GR- /c /Fo%t.obj -- %s
+// RUN: lld-link /DEBUG /nodefaultlib /entry:main /OUT:%t.exe /PDB:%t.pdb -- %t.obj
+// RUN: llvm-readobj -s %t.exe | FileCheck --check-prefix=BSS %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb -f %t.exe -s \
+// RUN: %p/Inputs/globals-bss.lldbinit 2>&1 | FileCheck %s
+
+int GlobalVariable = 0;
+
+int main(int argc, char **argv) {
+ return 0;
+}
+
+// BSS: Section {
+// BSS-NEXT:Number: 3
+// BSS-NEXT:Name: .data
+// BSS-NEXT:VirtualSize: 0x4
+// BSS-NEXT:VirtualAddress:
+// BSS-NEXT:RawDataSize: 0
+// BSS-NEXT:PointerToRawData: 0x0
+// BSS-NEXT:PointerToRelocations: 0x0
+// BSS-NEXT:PointerToLineNumbers: 0x0
+// BSS-NEXT:RelocationCount: 0
+// BSS-NEXT:LineNumberCount: 0
+// BSS-NEXT:Characteristics [ (0xC040)
+// BSS-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA (0x40)
+// BSS-NEXT: IMAGE_SCN_MEM_READ (0x4000)
+// BSS-NEXT: IMAGE_SCN_MEM_WRITE (0x8000)
+// BSS-NEXT:]
+// BSS-NEXT: }
+
+// CHECK: (int) GlobalVariable = 0
Index: lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
===
---
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54241: Fix bug in printing ValueObjects and in PE/COFF Plugin
zturner created this revision.
zturner added a reviewer: clayborg.
Herald added subscribers: JDevlieghere, aprantl.
The first bug is that the implementation of `GetCompilerType` for
`ValueObjectVariable` only requests the forward type. This is insufficient for
printing enum decls. I suspect this works with the DWARF plugin because the
DWARF plugin probably completes enums immediately. The PDB plugin implements
enums similarly to other tag records though, and only completes them lazily.
`ValueObjectVariable` then requests the forward compiler type, and makes no
attempt to complete it, resulting in insufficient information to print out the
value of the enum.
Note that in general, you need a full compiler type to print out the variable
anyway. If it's a class type you need it to print out its children, and this
already happens when the `ValueObject` calls `GetNumChildren`. So this
particular change isn't actually invoking any additional work or parsing that
wasn't already going to happen anyway, it just allows the case where a symbol
file plugin parses enums lazily.
The second bug fixed here is in `ObjectFilePECOFF`. Both MSVC and clang-cl
emit their bss section as a magic section called `.data` with a raw size of 0
and a file offset of 0.These 3 properties together are sufficient to
indicate "this is actually a BSS section", but we had no mechanism of reading
from such sections. So we would just try the default read algorithm, which
looks at the size, sees that it's too small (0 bytes), and gives up. So we
need to add support for this in `ObjectFilePECOFF`.
This fixes all of the outstanding printing bugs in the native pdb reading tests.
https://reviews.llvm.org/D54241
Files:
lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -125,6 +125,10 @@
ObjectFile::Strata CalculateStrata() override;
+ size_t ReadSectionData(lldb_private::Section *section,
+ lldb::offset_t section_offset, void *dst,
+ size_t dst_len) override;
+
//--
// PluginInterface protocol
//--
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -1053,6 +1053,32 @@
}
ObjectFile::Strata ObjectFilePECOFF::CalculateStrata() { return eStrataUser; }
+
+static bool IsCompressedBss(const Section ) {
+ if (section.GetName() != ConstString(".data"))
+return false;
+ if (section.GetFileSize() != 0)
+return false;
+ if (section.GetFileOffset() != 0)
+return false;
+ return true;
+}
+
+size_t ObjectFilePECOFF::ReadSectionData(Section *section,
+ lldb::offset_t section_offset,
+ void *dst, size_t dst_len) {
+ if (!IsCompressedBss(*section))
+return ObjectFile::ReadSectionData(section, section_offset, dst, dst_len);
+
+ if (section_offset >= section->GetByteSize())
+return 0;
+
+ size_t bytes_avail = section->GetByteSize() - section_offset;
+ size_t read_size = std::min(dst_len, bytes_avail);
+ ::memset(dst, 0, read_size);
+ return read_size;
+}
+
//--
// PluginInterface protocol
//--
Index: lldb/source/Core/ValueObjectVariable.cpp
===
--- lldb/source/Core/ValueObjectVariable.cpp
+++ lldb/source/Core/ValueObjectVariable.cpp
@@ -67,7 +67,7 @@
CompilerType ValueObjectVariable::GetCompilerTypeImpl() {
Type *var_type = m_variable_sp->GetType();
if (var_type)
-return var_type->GetForwardCompilerType();
+return var_type->GetFullCompilerType();
return CompilerType();
}
Index: lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
===
--- lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
+++ lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
@@ -89,16 +89,16 @@
// CHECK: (TrivialC) TC = {}
// CHECK: (TrivialS) TS = {}
// CHECK: (TrivialU) TU = {}
-// CHECK: (TrivialE) TE =
-// CHECK: (A::B::C) ABCInt = (ABCMember = )
-// CHECK: (A::B::C) ABCFloat = (ABCMember = )
-// CHECK: (A::B::C) ABCVoid = (ABCSpecializationMember = )
[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
This is pretty good, but in all the places where some plan tries to find a
sub-plan to do its job, you are losing the text of the error when that job
fails. So the controlling plan can't present a good error message. You need
to hold onto the Status object and return that, either as the error from
ValidatePlan if it happens when the plan is getting created, or in the plan's
stop description so that StopInfoThreadPlan::GetDescription can print it
properly.
Comment at: include/lldb/Target/Thread.h:643
//--
- virtual lldb::ThreadPlanSP QueueFundamentalPlan(bool abort_other_plans);
+ virtual lldb::ThreadPlanSP QueueFundamentalPlan(Status ,
+ bool abort_other_plans);
You need to change the HeaderDoc to describe the status parameter. Looks like
you have to do this for all the QueueThreadPlan... functions.
Comment at: include/lldb/Target/ThreadPlanBase.h:55
friend lldb::ThreadPlanSP
- Thread::QueueFundamentalPlan(bool abort_other_plans);
+ Thread::QueueFundamentalPlan(Status , bool abort_other_plans);
I don't think it is useful to pass an error in this case. The "Fundamental
plan" just fields unhandled responses from other plans, and queuing it can
never fail.
Comment at: include/lldb/Target/ThreadPlanRunToAddress.h:66
// using to stop us at m_address.
+ bool m_could_not_resolve_hw_bp;
Looks like you are adding this to most of the plans. Would it make sense to
add it to the ThreadPlan base class? This is just an error flag, so it would
stay false except is a derived plan wants to set it.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py:33-43
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(exe)
+
+breakpoint = target.BreakpointCreateByLocation("main.c", 1)
+
+self.runCmd("run")
+
This can all be done with:
(target, process, stepping_thread) =
lldbutil.run_to_line_breakpoint(SBFileSpec("main.c"), 1)
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py:47
+
+self.expect("thread step-in")
+self.expect("thread step-in", error=True)
Can you do this using "SBThread.StepInto" and check the error return in the
case where it fails? Since we are treating the possibility of step's failing,
we need to make sure that the SB API's return errors everywhere and that the
errors look right. So it would be good to test that.
Ditto for the other stepping tests.
Comment at: source/API/SBThread.cpp:733
new_plan_sp = thread->QueueThreadPlanForStepSingleInstruction(
-false, abort_other_plans, stop_other_threads);
+new_plan_status, false, abort_other_plans, stop_other_threads);
}
Shouldn't new_plan_status get reflected in the "error" parameter passed into
SBThread::StepInto?
Comment at: source/API/SBThread.cpp:767
+ new_plan_status, abort_other_plans, NULL, false, stop_other_threads,
+ eVoteYes, eVoteNoOpinion, 0, avoid_no_debug));
Same here. If new_plan_status comes back with an error, we probably don't want
to call ResumeNewPlan, and we want to report the error from queueing the plan.
Comment at: source/API/SBThread.cpp:818
+ Status new_plan_status;
ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut(
Same comment here.
Comment at: source/API/SBThread.cpp:847
Thread *thread = exe_ctx.GetThreadPtr();
- ThreadPlanSP new_plan_sp(
- thread->QueueThreadPlanForStepSingleInstruction(step_over, true, true));
+ Status new_plan_status;
+ ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepSingleInstruction(
And here.
Comment at: source/API/SBThread.cpp:881
+ Status new_plan_status;
ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForRunToAddress(
And here.
Comment at: source/API/SBThreadPlan.cpp:155
start_address->CalculateSymbolContext();
+Status plan_status;
return SBThreadPlan(
Can you add a variant of these calls that takes an SBError &? The scripted
thread plans will need to have a way to report this error when they try to
queue a plan and it fails. And below as well.
Comment at: source/Commands/CommandObjectThread.cpp:802
} else {
+ result.SetError(new_plan_status);
result.AppendError("Couldn't find thread
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
zturner added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:909 clang::EnumConstantDecl *AddEnumerationValueToEnumerationType( - lldb::opaque_compiler_type_t type, - const CompilerType _qual_type, const Declaration , - const char *name, int64_t enum_value, uint32_t enum_value_bit_size); + const CompilerType enum_type, const Declaration , const char *name, + int64_t enum_value, uint32_t enum_value_bit_size); shafik wrote: > jingham wrote: > > clayborg wrote: > > > teemperor wrote: > > > > Can we pass `enum_type` as const ref? > > > CompilerType instances are two pointers. Pretty cheap to copy. > > We're not entirely consistent, but a quick glance through headers show us > > almost always choosing to pass "const CompilerType &" over "const > > CompilerType". > That was a good catch, thank you! Definitely passing by const value doesn't make sense, but I also think passing by value is what we should do going forward. It doesn't necessarily have to be in this patch, but I don't see any reason to pass by reference in general, and it just adds an extra level of pointer indirection for no real benefit IMO. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
shafik updated this revision to Diff 173083.
shafik marked 8 inline comments as done.
shafik added a comment.
Made changes based on comments.
https://reviews.llvm.org/D54003
Files:
include/lldb/Symbol/ClangASTContext.h
packages/Python/lldbsuite/test/expression_command/radar_43822994/Makefile
packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py
packages/Python/lldbsuite/test/expression_command/radar_43822994/main.cpp
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -8876,43 +8876,55 @@
}
clang::EnumConstantDecl *ClangASTContext::AddEnumerationValueToEnumerationType(
-lldb::opaque_compiler_type_t type,
-const CompilerType _clang_type, const Declaration ,
-const char *name, int64_t enum_value, uint32_t enum_value_bit_size) {
- if (type && enumerator_clang_type.IsValid() && name && name[0]) {
-clang::QualType enum_qual_type(GetCanonicalQualType(type));
-
-bool is_signed = false;
-enumerator_clang_type.IsIntegerType(is_signed);
-const clang::Type *clang_type = enum_qual_type.getTypePtr();
-if (clang_type) {
- const clang::EnumType *enutype =
- llvm::dyn_cast(clang_type);
-
- if (enutype) {
-llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
-enum_llvm_apsint = enum_value;
-clang::EnumConstantDecl *enumerator_decl =
-clang::EnumConstantDecl::Create(
-*getASTContext(), enutype->getDecl(), clang::SourceLocation(),
-name ? ()->Idents.get(name)
- : nullptr, // Identifier
-ClangUtil::GetQualType(enumerator_clang_type),
-nullptr, enum_llvm_apsint);
-
-if (enumerator_decl) {
- enutype->getDecl()->addDecl(enumerator_decl);
+const CompilerType _type, const Declaration , const char *name,
+int64_t enum_value, uint32_t enum_value_bit_size) {
+
+ if (!enum_type || ConstString(name).IsEmpty())
+return nullptr;
+
+ lldbassert(enum_type.GetTypeSystem() == static_cast(this));
+
+ lldb::opaque_compiler_type_t enum_opaque_compiler_type =
+ enum_type.GetOpaqueQualType();
+
+ if (!enum_opaque_compiler_type)
+return nullptr;
+
+ CompilerType underlying_type =
+ GetEnumerationIntegerType(enum_type.GetOpaqueQualType());
+
+ clang::QualType enum_qual_type(
+ GetCanonicalQualType(enum_opaque_compiler_type));
+
+ bool is_signed = false;
+ underlying_type.IsIntegerType(is_signed);
+ const clang::Type *clang_type = enum_qual_type.getTypePtr();
+
+ if (!clang_type)
+return nullptr;
+
+ const clang::EnumType *enutype = llvm::dyn_cast(clang_type);
+
+ if (!enutype)
+return nullptr;
+
+ llvm::APSInt enum_llvm_apsint(enum_value_bit_size, is_signed);
+ enum_llvm_apsint = enum_value;
+ clang::EnumConstantDecl *enumerator_decl = clang::EnumConstantDecl::Create(
+ *getASTContext(), enutype->getDecl(), clang::SourceLocation(),
+ name ? ()->Idents.get(name) : nullptr, // Identifier
+ clang::QualType(enutype, 0), nullptr, enum_llvm_apsint);
+
+ if (!enumerator_decl)
+return nullptr;
+
+ enutype->getDecl()->addDecl(enumerator_decl);
#ifdef LLDB_CONFIGURATION_DEBUG
- VerifyDecl(enumerator_decl);
+ VerifyDecl(enumerator_decl);
#endif
- return enumerator_decl;
-}
- }
-}
- }
- return nullptr;
+ return enumerator_decl;
}
CompilerType
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1122,8 +1122,7 @@
uint32_t byte_size = m_ast.getASTContext()->getTypeSize(
ClangUtil::GetQualType(underlying_type));
auto enum_constant_decl = m_ast.AddEnumerationValueToEnumerationType(
- enum_type.GetOpaqueQualType(), underlying_type, decl, name.c_str(),
- raw_value, byte_size * 8);
+ enum_type, decl, name.c_str(), raw_value, byte_size * 8);
if (!enum_constant_decl)
return false;
Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -159,13 +159,9 @@
TypeSP underlying_type =
m_symbol_file.GetOrCreateType(m_cvr.er.getUnderlyingType());
- lldb::opaque_compiler_type_t enum_qt = m_derived_ct.GetOpaqueQualType();
-
- CompilerType enumerator_type = clang.GetEnumerationIntegerType(enum_qt);
uint64_t byte_size = underlying_type->GetByteSize();
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
shafik added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:909 clang::EnumConstantDecl *AddEnumerationValueToEnumerationType( - lldb::opaque_compiler_type_t type, - const CompilerType _qual_type, const Declaration , - const char *name, int64_t enum_value, uint32_t enum_value_bit_size); + const CompilerType enum_type, const Declaration , const char *name, + int64_t enum_value, uint32_t enum_value_bit_size); jingham wrote: > clayborg wrote: > > teemperor wrote: > > > Can we pass `enum_type` as const ref? > > CompilerType instances are two pointers. Pretty cheap to copy. > We're not entirely consistent, but a quick glance through headers show us > almost always choosing to pass "const CompilerType &" over "const > CompilerType". That was a good catch, thank you! https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
shafik added a comment. @jingham @zturner @teemperor @clayborg I believe I have addressed all the comments. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54135: Add convenience method in FileSystem to check if a path/filespec is a directory.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346375: [FileSystem] Add convenience method to check for
directories. (authored by JDevlieghere, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D54135?vs=172703=173075#toc
Repository:
rL LLVM
https://reviews.llvm.org/D54135
Files:
lldb/trunk/include/lldb/Host/FileSystem.h
lldb/trunk/source/API/SBPlatform.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Host/common/FileSystem.cpp
lldb/trunk/source/Host/common/Symbols.cpp
lldb/trunk/source/Host/macosx/Symbols.cpp
lldb/trunk/source/Host/macosx/objcxx/Host.mm
lldb/trunk/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/trunk/source/Target/TargetList.cpp
Index: lldb/trunk/source/Host/macosx/objcxx/Host.mm
===
--- lldb/trunk/source/Host/macosx/objcxx/Host.mm
+++ lldb/trunk/source/Host/macosx/objcxx/Host.mm
@@ -101,7 +101,7 @@
bool Host::GetBundleDirectory(const FileSpec ,
FileSpec _directory) {
#if defined(__APPLE__)
- if (llvm::sys::fs::is_directory(file.GetPath())) {
+ if (FileSystem::Instance().IsDirectory(file)) {
char path[PATH_MAX];
if (file.GetPath(path, sizeof(path))) {
CFCBundle bundle(path);
@@ -118,7 +118,7 @@
bool Host::ResolveExecutableInBundle(FileSpec ) {
#if defined(__APPLE__)
- if (llvm::sys::fs::is_directory(file.GetPath())) {
+ if (FileSystem::Instance().IsDirectory(file)) {
char path[PATH_MAX];
if (file.GetPath(path, sizeof(path))) {
CFCBundle bundle(path);
Index: lldb/trunk/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/trunk/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/trunk/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -141,7 +141,7 @@
raw_path.append("/../bin");
FileSpec support_dir_spec(raw_path);
FileSystem::Instance().Resolve(support_dir_spec);
-if (!llvm::sys::fs::is_directory(support_dir_spec.GetPath())) {
+if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
if (log)
log->Printf("HostInfoMacOSX::%s(): failed to find support directory",
Index: lldb/trunk/source/Host/macosx/Symbols.cpp
===
--- lldb/trunk/source/Host/macosx/Symbols.cpp
+++ lldb/trunk/source/Host/macosx/Symbols.cpp
@@ -109,7 +109,7 @@
if (path[0] == '~')
FileSystem::Instance().Resolve(dsym_filespec);
-if (llvm::sys::fs::is_directory(dsym_filespec.GetPath())) {
+if (FileSystem::Instance().IsDirectory(dsym_filespec)) {
dsym_filespec =
Symbols::FindSymbolFileInBundle(dsym_filespec, uuid, arch);
++items_found;
Index: lldb/trunk/source/Host/common/Symbols.cpp
===
--- lldb/trunk/source/Host/common/Symbols.cpp
+++ lldb/trunk/source/Host/common/Symbols.cpp
@@ -311,7 +311,7 @@
for (size_t idx = 0; idx < num_directories; ++idx) {
FileSpec dirspec = debug_file_search_paths.GetFileSpecAtIndex(idx);
FileSystem::Instance().Resolve(dirspec);
- if (!llvm::sys::fs::is_directory(dirspec.GetPath()))
+ if (!FileSystem::Instance().IsDirectory(dirspec))
continue;
std::vector files;
Index: lldb/trunk/source/Host/common/FileSystem.cpp
===
--- lldb/trunk/source/Host/common/FileSystem.cpp
+++ lldb/trunk/source/Host/common/FileSystem.cpp
@@ -124,6 +124,17 @@
return Readable(file_spec.GetPath());
}
+bool FileSystem::IsDirectory(const Twine ) const {
+ ErrorOr status = m_fs->status(path);
+ if (!status)
+return false;
+ return status->isDirectory();
+}
+
+bool FileSystem::IsDirectory(const FileSpec _spec) const {
+ return IsDirectory(file_spec.GetPath());
+}
+
void FileSystem::EnumerateDirectory(Twine path, bool find_directories,
bool find_files, bool find_other,
EnumerateDirectoryCallbackType callback,
Index: lldb/trunk/source/API/SBPlatform.cpp
===
---
[Lldb-commits] [lldb] r346375 - [FileSystem] Add convenience method to check for directories.
Author: jdevlieghere
Date: Wed Nov 7 16:14:50 2018
New Revision: 346375
URL: http://llvm.org/viewvc/llvm-project?rev=346375=rev
Log:
[FileSystem] Add convenience method to check for directories.
Replace calls to LLVM's is_directory with calls to LLDB's FileSytem
class. For this I introduced a new convenience method that, like the
other methods, takes either a path or filespec. This still uses the LLVM
functions under the hood.
Differential revision: https://reviews.llvm.org/D54135
Modified:
lldb/trunk/include/lldb/Host/FileSystem.h
lldb/trunk/source/API/SBPlatform.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Host/common/FileSystem.cpp
lldb/trunk/source/Host/common/Symbols.cpp
lldb/trunk/source/Host/macosx/Symbols.cpp
lldb/trunk/source/Host/macosx/objcxx/Host.mm
lldb/trunk/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/trunk/source/Target/TargetList.cpp
Modified: lldb/trunk/include/lldb/Host/FileSystem.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSystem.h?rev=346375=346374=346375=diff
==
--- lldb/trunk/include/lldb/Host/FileSystem.h (original)
+++ lldb/trunk/include/lldb/Host/FileSystem.h Wed Nov 7 16:14:50 2018
@@ -88,6 +88,12 @@ public:
bool Readable(const llvm::Twine ) const;
/// @}
+ /// Returns whether the given path is a directory.
+ /// @{
+ bool IsDirectory(const FileSpec _spec) const;
+ bool IsDirectory(const llvm::Twine ) const;
+ /// @}
+
/// Make the given file path absolute.
/// @{
std::error_code MakeAbsolute(llvm::SmallVectorImpl ) const;
Modified: lldb/trunk/source/API/SBPlatform.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBPlatform.cpp?rev=346375=346374=346375=diff
==
--- lldb/trunk/source/API/SBPlatform.cpp (original)
+++ lldb/trunk/source/API/SBPlatform.cpp Wed Nov 7 16:14:50 2018
@@ -366,7 +366,7 @@ SBError SBPlatform::Put(SBFileSpec ,
if (src.Exists()) {
uint32_t permissions = FileSystem::Instance().GetPermissions(src.ref());
if (permissions == 0) {
-if (llvm::sys::fs::is_directory(src.ref().GetPath()))
+if (FileSystem::Instance().IsDirectory(src.ref()))
permissions = eFilePermissionsDirectoryDefault;
else
permissions = eFilePermissionsFileDefault;
Modified: lldb/trunk/source/Core/Module.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=346375=346374=346375=diff
==
--- lldb/trunk/source/Core/Module.cpp (original)
+++ lldb/trunk/source/Core/Module.cpp Wed Nov 7 16:14:50 2018
@@ -1448,7 +1448,7 @@ void Module::SetSymbolFileFileSpec(const
// ("/tmp/a.out.dSYM/Contents/Resources/DWARF/a.out"). So we need to
// check this
-if (llvm::sys::fs::is_directory(file.GetPath())) {
+if (FileSystem::Instance().IsDirectory(file)) {
std::string new_path(file.GetPath());
std::string old_path(obj_file->GetFileSpec().GetPath());
if (old_path.find(new_path) == 0) {
Modified: lldb/trunk/source/Core/ModuleList.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ModuleList.cpp?rev=346375=346374=346375=diff
==
--- lldb/trunk/source/Core/ModuleList.cpp (original)
+++ lldb/trunk/source/Core/ModuleList.cpp Wed Nov 7 16:14:50 2018
@@ -859,7 +859,7 @@ Status ModuleList::GetSharedModule(const
auto search_path_spec = module_search_paths_ptr->GetFileSpecAtIndex(idx);
FileSystem::Instance().Resolve(search_path_spec);
namespace fs = llvm::sys::fs;
- if (!fs::is_directory(search_path_spec.GetPath()))
+ if (!FileSystem::Instance().IsDirectory(search_path_spec))
continue;
search_path_spec.AppendPathComponent(
module_spec.GetFileSpec().GetFilename().AsCString());
Modified: lldb/trunk/source/Host/common/FileSystem.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSystem.cpp?rev=346375=346374=346375=diff
==
--- lldb/trunk/source/Host/common/FileSystem.cpp (original)
+++ lldb/trunk/source/Host/common/FileSystem.cpp Wed Nov 7
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
Looking at the extant API's we do seem to prefer "const CompilerType &" over
"const CompilerType" so it seems more consistent to choose that. Other than
that, this looks fine to me.
Comment at: include/lldb/Symbol/ClangASTContext.h:909
clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
- lldb::opaque_compiler_type_t type,
- const CompilerType _qual_type, const Declaration ,
- const char *name, int64_t enum_value, uint32_t enum_value_bit_size);
+ const CompilerType enum_type, const Declaration , const char *name,
+ int64_t enum_value, uint32_t enum_value_bit_size);
clayborg wrote:
> teemperor wrote:
> > Can we pass `enum_type` as const ref?
> CompilerType instances are two pointers. Pretty cheap to copy.
We're not entirely consistent, but a quick glance through headers show us
almost always choosing to pass "const CompilerType &" over "const CompilerType".
Comment at:
packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py:42
+## integral is not implicitly convertible to a scoped enum
+value = frame.EvaluateExpression("1 == Foo::FooBar")
+self.assertTrue(value.IsValid())
davide wrote:
> This clearly looks like can be an inline test (and probably would make the
> thing more readable, IMHO).
The form of test to use is up to the test writer, seems to me. This test is
not at all hard to read.
https://reviews.llvm.org/D54003
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types
For dotest style tests, the debug format to test is chosen by the test driver.
All the tests should run with any format, but sometimes there are bugs in one
reader or another (or in one version of DWARF or another) so you can skip or
xfail a test based on format. Sounds like this test should be skipped when the
debug format is PDB, since it seems like either the PDB format doesn't express
a way to find the actual size of the vla (or the current PDB reader doesn't
process those records).
Jim
> On Nov 7, 2018, at 2:00 PM, Stella Stamenova via Phabricator
> wrote:
>
> stella.stamenova added a comment.
>
> In https://reviews.llvm.org/D53530#1290680, @aprantl wrote:
>
>> In https://reviews.llvm.org/D53530#1290664, @stella.stamenova wrote:
>>
>>> TestVla fails on Windows:
>>> http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio
>>>
>>> AssertionError: False is not True : 'frame var vla' returns expected
>>> result, got '(int []) vla = {}'
>>>
>>>
>>> I will have time to look in more detail later this week, but if you have
>>> any ideas in the mean time, that would be great.
>>
>>
>> Is your bot using DWARF or CodeView as a debug info format? Because this
>> test definitely requires DWARF.
>
>
> I was under the impression that each test can control how to build the
> necessary payloads. Is that not the case?
>
>
> Repository:
> rL LLVM
>
> https://reviews.llvm.org/D53530
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
shafik added a comment. @zturner I was out of date when I posted this diff but I have that file updated locally and it will show up when I update the diff. https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
I meant the folder. It’s the first result in your search results. Just curious, why does your build successfully complete without fixing that instance? On Wed, Nov 7, 2018 at 2:20 PM Shafik Yaghmour via Phabricator < revi...@reviews.llvm.org> wrote: > shafik added a comment. > > @zturner I don't see `AddEnumerationValueToEnumerationType` being called > in `SymbolFile/NativePDB.cpp` > https://github.com/llvm-mirror/lldb/search?q=AddEnumerationValueToEnumerationType_q=AddEnumerationValueToEnumerationType > > > https://reviews.llvm.org/D54003 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
zturner added a subscriber: shafik. zturner added a comment. I meant the folder. It’s the first result in your search results. Just curious, why does your build successfully complete without fixing that instance? https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
stella.stamenova added a comment. The builds don't specify the two options for LLDB_TEST_C/CXX_COMPILER, so they should be picking up the freshly build compilers. We don't have an option for clang-cl though - so it's never been explicitly specified. Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter
shafik added a comment. @zturner I don't see `AddEnumerationValueToEnumerationType` being called in `SymbolFile/NativePDB.cpp` https://github.com/llvm-mirror/lldb/search?q=AddEnumerationValueToEnumerationType_q=AddEnumerationValueToEnumerationType https://reviews.llvm.org/D54003 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
zturner added a comment. Actually maybe it’s the other way around. Are you specifying LLDB_TEST_COMPILER? If it’s picking up VS’s version, it would definitely be able to find link.exe Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
Actually maybe it’s the other way around. Are you specifying LLDB_TEST_COMPILER? If it’s picking up VS’s version, it would definitely be able to find link.exe On Wed, Nov 7, 2018 at 2:07 PM Stella Stamenova via Phabricator < revi...@reviews.llvm.org> wrote: > stella.stamenova added a comment. > > I haven't verified this yet - but I suspect it is now picking up the > clang-cl that comes with VS rather than the one that was just built. > > > Repository: > rLLDB LLDB > > https://reviews.llvm.org/D54009 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
stella.stamenova added a comment. I haven't verified this yet - but I suspect it is now picking up the clang-cl that comes with VS rather than the one that was just built. Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
zturner added a comment. It’s possible we lost some environment variable propagation, that would do it. But I’m curious how it was finding the visual studio installation before my patch. It also looks like it’s failing finding link.exe (we really should make lld-link the default). Another fix is to pass -fuse-ld=lld or split the clang-cl line to separate compiler and linker invocations Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
It’s possible we lost some environment variable propagation, that would do it. But I’m curious how it was finding the visual studio installation before my patch. It also looks like it’s failing finding link.exe (we really should make lld-link the default). Another fix is to pass -fuse-ld=lld or split the clang-cl line to separate compiler and linker invocations On Wed, Nov 7, 2018 at 1:44 PM Stella Stamenova via Phabricator < revi...@reviews.llvm.org> wrote: > stella.stamenova added a comment. > > In https://reviews.llvm.org/D54009#1290644, @zturner wrote: > > > I have not run the dotest suite recently, is that where you’re seeing the > > failures? I successfully ran the lit suite and unit tests though > > > Yes, they are primarily in the lldb-suite but not only: > > > http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio > > There are some other failures, but what I suspect happened after this > change is that the tests are now not picking up clang-cl correctly: > > $ "clang-cl" "/Zi" > "E:\build_slave\lldb-x64-windows-ninja\llvm\tools\lldb\lit\Expr/Inputs/call-function.cpp" > "-o" > "E:\build_slave\lldb-x64-windows-ninja\build\tools\lldb\lit\Expr\Output\TestIRMemoryMapWindows.test.tmp" > # command stderr: > clang-cl: warning: unable to find a Visual Studio installation; try > running Clang from a developer command prompt [-Wmsvc-not-found] > > clang-cl: error: unable to execute command: program not executable > > clang-cl: error: linker command failed with exit code 1 (use -v to see > invocation) > > I will have some time to have a look in more detail tomorrow. > > > Repository: > rLLDB LLDB > > https://reviews.llvm.org/D54009 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types
stella.stamenova added a comment.
In https://reviews.llvm.org/D53530#1290680, @aprantl wrote:
> In https://reviews.llvm.org/D53530#1290664, @stella.stamenova wrote:
>
> > TestVla fails on Windows:
> > http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio
> >
> > AssertionError: False is not True : 'frame var vla' returns expected
> > result, got '(int []) vla = {}'
> >
> >
> > I will have time to look in more detail later this week, but if you have
> > any ideas in the mean time, that would be great.
>
>
> Is your bot using DWARF or CodeView as a debug info format? Because this test
> definitely requires DWARF.
I was under the impression that each test can control how to build the
necessary payloads. Is that not the case?
Repository:
rL LLVM
https://reviews.llvm.org/D53530
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types
aprantl added a comment.
In https://reviews.llvm.org/D53530#1290664, @stella.stamenova wrote:
> TestVla fails on Windows:
> http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio
>
> AssertionError: False is not True : 'frame var vla' returns expected
> result, got '(int []) vla = {}'
>
>
> I will have time to look in more detail later this week, but if you have any
> ideas in the mean time, that would be great.
Is your bot using DWARF or CodeView as a debug info format? Because this test
definitely requires DWARF.
Repository:
rL LLVM
https://reviews.llvm.org/D53530
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.
JDevlieghere updated this revision to Diff 173021.
JDevlieghere added a comment.
Remove `ValidatePlan` implementation for `ThreadPlanPython` as I believe it's
bogus.
https://reviews.llvm.org/D54221
Files:
include/lldb/API/SBBreakpoint.h
include/lldb/Breakpoint/Breakpoint.h
include/lldb/Target/Target.h
include/lldb/Target/Thread.h
include/lldb/Target/ThreadPlanBase.h
include/lldb/Target/ThreadPlanRunToAddress.h
include/lldb/Target/ThreadPlanStepInRange.h
include/lldb/Target/ThreadPlanStepInstruction.h
include/lldb/Target/ThreadPlanStepOut.h
include/lldb/Target/ThreadPlanStepRange.h
include/lldb/Target/ThreadPlanStepThrough.h
include/lldb/Target/ThreadPlanStepUntil.h
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
scripts/interface/SBBreakpoint.i
source/API/SBBreakpoint.cpp
source/API/SBThread.cpp
source/API/SBThreadPlan.cpp
source/Breakpoint/Breakpoint.cpp
source/Commands/CommandObjectThread.cpp
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
source/Target/Process.cpp
source/Target/StopInfo.cpp
source/Target/Target.cpp
source/Target/Thread.cpp
source/Target/ThreadPlanCallOnFunctionExit.cpp
source/Target/ThreadPlanPython.cpp
source/Target/ThreadPlanRunToAddress.cpp
source/Target/ThreadPlanShouldStopHere.cpp
source/Target/ThreadPlanStepInRange.cpp
source/Target/ThreadPlanStepInstruction.cpp
source/Target/ThreadPlanStepOut.cpp
source/Target/ThreadPlanStepOverRange.cpp
source/Target/ThreadPlanStepRange.cpp
source/Target/ThreadPlanStepThrough.cpp
source/Target/ThreadPlanStepUntil.cpp
Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -39,7 +39,8 @@
m_return_bp_id(LLDB_INVALID_BREAK_ID),
m_return_addr(LLDB_INVALID_ADDRESS), m_stepped_out(false),
m_should_stop(false), m_ran_analyze(false), m_explains_stop(false),
- m_until_points(), m_stop_others(stop_others) {
+ m_until_points(), m_stop_others(stop_others),
+ m_could_not_resolve_hw_bp(false) {
// Stash away our "until" addresses:
TargetSP target_sp(m_thread.CalculateTarget());
@@ -57,7 +58,10 @@
m_return_addr = return_frame_sp->GetStackID().GetPC();
Breakpoint *return_bp =
target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+ m_could_not_resolve_hw_bp = true;
return_bp->SetThreadID(thread_id);
m_return_bp_id = return_bp->GetID();
return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +101,7 @@
}
}
m_until_points.clear();
+ m_could_not_resolve_hw_bp = false;
}
void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +132,16 @@
}
bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
- if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+ if (m_could_not_resolve_hw_bp) {
+if (error)
+ error->PutCString(
+ "Could not create hardware breakpoint for thread plan.");
+return false;
+ } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+ error->PutCString("Could not create return breakpoint.");
return false;
- else {
+ } else {
until_collection::iterator pos, end = m_until_points.end();
for (pos = m_until_points.begin(); pos != end; pos++) {
if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -40,7 +40,7 @@
eVoteNoOpinion, eVoteNoOpinion),
m_start_address(0), m_backstop_bkpt_id(LLDB_INVALID_BREAK_ID),
m_backstop_addr(LLDB_INVALID_ADDRESS), m_return_stack_id(m_stack_id),
- m_stop_others(stop_others) {
+ m_stop_others(stop_others), m_could_not_resolve_hw_bp(false) {
LookForPlanToStepThroughFromCurrentPC();
// If we don't get a valid step through plan, don't bother to set up a
@@ -62,7 +62,10 @@
->GetTarget()
.CreateBreakpoint(m_backstop_addr, true, false)
.get();
+
if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+ m_could_not_resolve_hw_bp = true;
return_bp->SetThreadID(m_thread.GetID());
m_backstop_bkpt_id = return_bp->GetID();
[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types
stella.stamenova added a comment.
TestVla fails on Windows:
http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio
AssertionError: False is not True : 'frame var vla' returns expected result,
got '(int []) vla = {}'
I will have time to look in more detail later this week, but if you have any
ideas in the mean time, that would be great.
Repository:
rL LLVM
https://reviews.llvm.org/D53530
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
stella.stamenova added a comment. In https://reviews.llvm.org/D54009#1290644, @zturner wrote: > I have not run the dotest suite recently, is that where you’re seeing the > failures? I successfully ran the lit suite and unit tests though Yes, they are primarily in the lldb-suite but not only: http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1129/steps/test/logs/stdio There are some other failures, but what I suspect happened after this change is that the tests are now not picking up clang-cl correctly: $ "clang-cl" "/Zi" "E:\build_slave\lldb-x64-windows-ninja\llvm\tools\lldb\lit\Expr/Inputs/call-function.cpp" "-o" "E:\build_slave\lldb-x64-windows-ninja\build\tools\lldb\lit\Expr\Output\TestIRMemoryMapWindows.test.tmp" # command stderr: clang-cl: warning: unable to find a Visual Studio installation; try running Clang from a developer command prompt [-Wmsvc-not-found] clang-cl: error: unable to execute command: program not executable clang-cl: error: linker command failed with exit code 1 (use -v to see invocation) I will have some time to have a look in more detail tomorrow. Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
zturner added a comment. I have not run the dotest suite recently, is that where you’re seeing the failures? I successfully ran the lit suite and unit tests though Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
I have not run the dotest suite recently, is that where you’re seeing the failures? I successfully ran the lit suite and unit tests though On Wed, Nov 7, 2018 at 1:32 PM Stella Stamenova via Phabricator < revi...@reviews.llvm.org> wrote: > stella.stamenova added a comment. > > Several of the windows tests that invoke clang-cl have started failing > recently (I am not sure exactly when) and I suspect this change is the > culprit. Were you able to run the tests successfully with this change? > > > Repository: > rLLDB LLDB > > https://reviews.llvm.org/D54009 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54009: Refactor LLDB lit configuration files
stella.stamenova added a comment. Several of the windows tests that invoke clang-cl have started failing recently (I am not sure exactly when) and I suspect this change is the culprit. Were you able to run the tests successfully with this change? Repository: rLLDB LLDB https://reviews.llvm.org/D54009 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53915: [FileSystem] Move resolve logic out of FileSpec
stella.stamenova added a comment. Some of the file spec changes caused a bunch of failures on Windows. We know have a silent Buildbot that you can see the failures on as well: http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1128/steps/test/logs/stdio Let me know if you need anything else. Once all the tests are green again we're going to make the Buildbot no longer silent, so this should be easier to catch. Repository: rL LLVM https://reviews.llvm.org/D53915 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.
JDevlieghere updated this revision to Diff 173015.
JDevlieghere added a comment.
Fix bug for `thread until` and add test case.
https://reviews.llvm.org/D54221
Files:
include/lldb/API/SBBreakpoint.h
include/lldb/Breakpoint/Breakpoint.h
include/lldb/Target/Target.h
include/lldb/Target/Thread.h
include/lldb/Target/ThreadPlanBase.h
include/lldb/Target/ThreadPlanRunToAddress.h
include/lldb/Target/ThreadPlanStepInRange.h
include/lldb/Target/ThreadPlanStepInstruction.h
include/lldb/Target/ThreadPlanStepOut.h
include/lldb/Target/ThreadPlanStepRange.h
include/lldb/Target/ThreadPlanStepThrough.h
include/lldb/Target/ThreadPlanStepUntil.h
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
scripts/interface/SBBreakpoint.i
source/API/SBBreakpoint.cpp
source/API/SBThread.cpp
source/API/SBThreadPlan.cpp
source/Breakpoint/Breakpoint.cpp
source/Commands/CommandObjectThread.cpp
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
source/Target/Process.cpp
source/Target/StopInfo.cpp
source/Target/Target.cpp
source/Target/Thread.cpp
source/Target/ThreadPlanCallOnFunctionExit.cpp
source/Target/ThreadPlanPython.cpp
source/Target/ThreadPlanRunToAddress.cpp
source/Target/ThreadPlanShouldStopHere.cpp
source/Target/ThreadPlanStepInRange.cpp
source/Target/ThreadPlanStepInstruction.cpp
source/Target/ThreadPlanStepOut.cpp
source/Target/ThreadPlanStepOverRange.cpp
source/Target/ThreadPlanStepRange.cpp
source/Target/ThreadPlanStepThrough.cpp
source/Target/ThreadPlanStepUntil.cpp
Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -39,7 +39,8 @@
m_return_bp_id(LLDB_INVALID_BREAK_ID),
m_return_addr(LLDB_INVALID_ADDRESS), m_stepped_out(false),
m_should_stop(false), m_ran_analyze(false), m_explains_stop(false),
- m_until_points(), m_stop_others(stop_others) {
+ m_until_points(), m_stop_others(stop_others),
+ m_could_not_resolve_hw_bp(false) {
// Stash away our "until" addresses:
TargetSP target_sp(m_thread.CalculateTarget());
@@ -57,7 +58,10 @@
m_return_addr = return_frame_sp->GetStackID().GetPC();
Breakpoint *return_bp =
target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+ m_could_not_resolve_hw_bp = true;
return_bp->SetThreadID(thread_id);
m_return_bp_id = return_bp->GetID();
return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +101,7 @@
}
}
m_until_points.clear();
+ m_could_not_resolve_hw_bp = false;
}
void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +132,16 @@
}
bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
- if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+ if (m_could_not_resolve_hw_bp) {
+if (error)
+ error->PutCString(
+ "Could not create hardware breakpoint for thread plan.");
+return false;
+ } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+ error->PutCString("Could not create return breakpoint.");
return false;
- else {
+ } else {
until_collection::iterator pos, end = m_until_points.end();
for (pos = m_until_points.begin(); pos != end; pos++) {
if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -40,7 +40,7 @@
eVoteNoOpinion, eVoteNoOpinion),
m_start_address(0), m_backstop_bkpt_id(LLDB_INVALID_BREAK_ID),
m_backstop_addr(LLDB_INVALID_ADDRESS), m_return_stack_id(m_stack_id),
- m_stop_others(stop_others) {
+ m_stop_others(stop_others), m_could_not_resolve_hw_bp(false) {
LookForPlanToStepThroughFromCurrentPC();
// If we don't get a valid step through plan, don't bother to set up a
@@ -62,7 +62,10 @@
->GetTarget()
.CreateBreakpoint(m_backstop_addr, true, false)
.get();
+
if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+ m_could_not_resolve_hw_bp = true;
return_bp->SetThreadID(m_thread.GetID());
m_backstop_bkpt_id = return_bp->GetID();
return_bp->SetBreakpointKind("step-through-backstop");
@@ -139,7 +142,26 @@
}
[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.
krytarowski added a comment.
On NetBSD one has to check PaX MPROTECT property of a traced process.
Something like:
bool IsMPROTECT(pid_t pid) {
#if defined(__NetBSD__)
int mib[3];
int paxflags;
size_t len = sizeof(paxflags);
mib[0] = CTL_PROC;
mib[1] = pid;
mib[2] = PROC_PID_PAXFLAGS;
if (sysctl(mib, 3, , , NULL, 0) != 0)
err(EXIT_FAILURE, "sysctl"); /* or return true */
return !!(paxflags & CTL_PROC_PAXFLAGS_MPROTECT);
#else
return false;
#endif
}
If IsMPROTECT is true, then we must use hardware assisted/emulated breakpoints.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D54221
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.
labath added a comment. I recall something about linux on arm having a magic unmodifiable (even by ptrace) page of memory, so this could be useful there too. However, it's not clear to me how a user is going to figure out that he needs to enable this setting. Would it make sense to automatically try setting a hardware breakpoint if software breakpoint fails? Repository: rLLDB LLDB https://reviews.llvm.org/D54221 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.
JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, jasonmolenda.
JDevlieghere added a project: LLDB.
Herald added subscribers: teemperor, abidh.
When debugging read-only memory we cannot use software breakpoint. We already
have support for hardware breakpoints and users can specify them with `-H`.
However, there's no option to force hardware breakpoints even while stepping.
This patch adds a setting `target.require-hardware-breakpoint` that forces LLDB
to always use hardware breakpoints. Because hardware breakpoints are a limited
resource and therefore can fail to resolve, this patch also extends error
handling in thread plans, where breakpoints are used for stepping.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D54221
Files:
include/lldb/API/SBBreakpoint.h
include/lldb/Breakpoint/Breakpoint.h
include/lldb/Target/Target.h
include/lldb/Target/Thread.h
include/lldb/Target/ThreadPlanBase.h
include/lldb/Target/ThreadPlanStepInRange.h
include/lldb/Target/ThreadPlanStepInstruction.h
include/lldb/Target/ThreadPlanStepOut.h
include/lldb/Target/ThreadPlanStepRange.h
include/lldb/Target/ThreadPlanStepThrough.h
include/lldb/Target/ThreadPlanStepUntil.h
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
scripts/interface/SBBreakpoint.i
source/API/SBBreakpoint.cpp
source/API/SBThread.cpp
source/API/SBThreadPlan.cpp
source/Breakpoint/Breakpoint.cpp
source/Commands/CommandObjectThread.cpp
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
source/Target/Process.cpp
source/Target/StopInfo.cpp
source/Target/Target.cpp
source/Target/Thread.cpp
source/Target/ThreadPlanCallOnFunctionExit.cpp
source/Target/ThreadPlanShouldStopHere.cpp
source/Target/ThreadPlanStepInRange.cpp
source/Target/ThreadPlanStepInstruction.cpp
source/Target/ThreadPlanStepOut.cpp
source/Target/ThreadPlanStepOverRange.cpp
source/Target/ThreadPlanStepRange.cpp
source/Target/ThreadPlanStepThrough.cpp
source/Target/ThreadPlanStepUntil.cpp
Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -39,7 +39,8 @@
m_return_bp_id(LLDB_INVALID_BREAK_ID),
m_return_addr(LLDB_INVALID_ADDRESS), m_stepped_out(false),
m_should_stop(false), m_ran_analyze(false), m_explains_stop(false),
- m_until_points(), m_stop_others(stop_others) {
+ m_until_points(), m_stop_others(stop_others),
+ m_could_not_resolve_hw_bp(false) {
// Stash away our "until" addresses:
TargetSP target_sp(m_thread.CalculateTarget());
@@ -57,7 +58,10 @@
m_return_addr = return_frame_sp->GetStackID().GetPC();
Breakpoint *return_bp =
target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+ m_could_not_resolve_hw_bp = true;
return_bp->SetThreadID(thread_id);
m_return_bp_id = return_bp->GetID();
return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +101,7 @@
}
}
m_until_points.clear();
+ m_could_not_resolve_hw_bp = false;
}
void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +132,16 @@
}
bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
- if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+ if (m_could_not_resolve_hw_bp) {
+if (error)
+ error->PutCString(
+ "Could not create hardware breakpoint for thread plan.");
+return false;
+ } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+ error->PutCString("Could not create return breakpoint.");
return false;
- else {
+ } else {
until_collection::iterator pos, end = m_until_points.end();
for (pos = m_until_points.begin(); pos != end; pos++) {
if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -40,7 +40,7 @@
eVoteNoOpinion, eVoteNoOpinion),
m_start_address(0), m_backstop_bkpt_id(LLDB_INVALID_BREAK_ID),
m_backstop_addr(LLDB_INVALID_ADDRESS), m_return_stack_id(m_stack_id),
- m_stop_others(stop_others) {
+ m_stop_others(stop_others), m_could_not_resolve_hw_bp(false) {
LookForPlanToStepThroughFromCurrentPC();
// If we don't get a valid step through plan, don't bother to set up a
@@ -62,7 +62,10 @@
->GetTarget()
[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()
labath added a comment. Could we reverse the dependencies between the two? I.e., first add the necessary functionality to lldb-test and then write the test using that? Or just merge that into a single patch? Some of the information tested here is already available there (list of sections) other can be added easily (list of symbols). https://reviews.llvm.org/D53094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r346347 - Re-commit regularization of the lldb-gtest-build target.
Author: jmolenda Date: Wed Nov 7 11:28:03 2018 New Revision: 346347 URL: http://llvm.org/viewvc/llvm-project?rev=346347=rev Log: Re-commit regularization of the lldb-gtest-build target. Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=346347=346346=346347=diff == --- lldb/trunk/lldb.xcodeproj/project.pbxproj (original) +++ lldb/trunk/lldb.xcodeproj/project.pbxproj Wed Nov 7 11:28:03 2018 @@ -9036,6 +9036,8 @@ "-framework", ApplicationServices, "$(LLDB_GTESTS_LDFLAGS)", + "$(LLDB_ZLIB_LDFLAGS)", + "$(LLDB_COMPRESSION_LDFLAGS)", ); PRODUCT_NAME = "lldb-gtest"; }; @@ -9078,6 +9080,8 @@ "-framework", ApplicationServices, "$(LLDB_GTESTS_LDFLAGS)", + "$(LLDB_ZLIB_LDFLAGS)", + "$(LLDB_COMPRESSION_LDFLAGS)", ); PRODUCT_NAME = "lldb-gtest"; }; @@ -9120,6 +9124,8 @@ "-framework", ApplicationServices, "$(LLDB_GTESTS_LDFLAGS)", + "$(LLDB_ZLIB_LDFLAGS)", + "$(LLDB_COMPRESSION_LDFLAGS)", ); PRODUCT_NAME = "lldb-gtest"; }; @@ -9162,6 +9168,8 @@ "-framework", ApplicationServices, "$(LLDB_GTESTS_LDFLAGS)", + "$(LLDB_ZLIB_LDFLAGS)", + "$(LLDB_COMPRESSION_LDFLAGS)", ); PRODUCT_NAME = "lldb-gtest"; }; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54216: [NativePDB] Improve support for reconstructing a clang AST from PDB debug info
zturner created this revision.
zturner added reviewers: aleksandr.urakov, labath, lemo.
Herald added subscribers: erik.pilkington, JDevlieghere, aprantl.
This is an alternative to https://reviews.llvm.org/D54053 which uses a
different approach. The goal of both is the same - to be able to improve the
quality of the AST that we reconstruct when parsing the debug info.
https://reviews.llvm.org/D54053 attempts to address this by demangling the
unique name of each type, and using the structure of the demangler's AST to try
to reconstruct a clang AST.
However, there are some complications with this approach. The two biggest ones
are:
a) The mangling does not always provide enough information to disambiguate
between two types, depending on where it occurs in the mangling.
b) The mangling provides no way to differentiate outer classes from outer
namespaces, so in `A::B::C`, we don't know if `A` and `B` are (class, class),
(namespace, namespace), or (namespace, class).
b) sounds like it could be an unimportant distinction, but since LLDB works by
gradually building up an AST over time that grows as more and more debug info
is parsed, you can very quickly end up in a situation where there are
ambiguities in your AST. For example, you may decide that `B` is probably a
namespace, so you create a `NamespaceDecl` for it in the AST, and then later
someone instatiates a variable of type `A::B` and you have precise debug info
telling you it's a class. This will create two decls at the same scope in the
AST hierarchy with the same name, causing ambiguities and these will slowly
build up over time leading to instability.
The approach here is based off of the observation that the PDB contains
information about nested classes in the parent -> child direction, just not the
other way around. That is to say, if you have code such as: `struct A { struct
B {}; };` Then the debug info record for `A` will tell you that it contains a
nested type call `B`, along with an index for the full definition of `B` in the
debug info. The problem we are facing all along is that if someone declares a
variable of type `A::B`, they need the reverse mapping, and PDB doesn't offer
that.
So, the simple solution employed here is to simply pre-process all types up
front and build the reverse mapping. This gives us perfect information about
class hierarchy, and allows us to precisely determine if a part of a scope is a
namespace (specifically, it will have no parent in the reverse mapping).
But we can even re-purpose this pre-processing step for other things down the
line. For example, we may wish to find all types name `Foo`, but maybe `Foo`
is a template and the instantion is `Foo`. We could use this
pre-processing step to build this kind of hash table. And many other things as
well.
Note that the idea of demangling a type and using the structured demangler AST
is not totally abandoned. For example, if you have a template instantiation
named `Foo`, the patch here will simply create a class with the name
`Foo`. In other words, we make no attempt to parse template parameters
and create the appropriate instantiations in the AST.
We also do not yet handle scoped classes (i.e. classes that are defined inside
the body of a funtion). But we can handle those later.
Note that I started adding a new kind of test, an ast test. I even retrofitted
existing tests with ast testing functionality. I think this is a useful
testing strategy to ensure we are generating correct ASTs from debug info.
https://reviews.llvm.org/D54216
Files:
lldb/lit/SymbolFile/NativePDB/Inputs/ast-reconstruction.lldbinit
lldb/lit/SymbolFile/NativePDB/Inputs/function-types-classes.lldbinit
lldb/lit/SymbolFile/NativePDB/Inputs/globals-classes.lldbinit
lldb/lit/SymbolFile/NativePDB/ast-reconstruction.cpp
lldb/lit/SymbolFile/NativePDB/function-types-classes.cpp
lldb/lit/SymbolFile/NativePDB/global-classes.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
Index: llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
===
--- llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
+++ llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
@@ -429,6 +429,10 @@
return (Options & ClassOptions::ForwardReference) != ClassOptions::None;
}
+ bool containsNestedClass() const {
+return (Options & ClassOptions::ContainsNestedClass) != ClassOptions::None;
+ }
+
bool isScoped() const {
return (Options & ClassOptions::Scoped) != ClassOptions::None;
}
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===
---
[Lldb-commits] [lldb] r346342 - Revert r346285 until I can make it work correctly
Author: jmolenda Date: Wed Nov 7 10:38:15 2018 New Revision: 346342 URL: http://llvm.org/viewvc/llvm-project?rev=346342=rev Log: Revert r346285 until I can make it work correctly the way the bots build lldb. Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=346342=346341=346342=diff == --- lldb/trunk/lldb.xcodeproj/project.pbxproj (original) +++ lldb/trunk/lldb.xcodeproj/project.pbxproj Wed Nov 7 10:38:15 2018 @@ -8623,22 +8623,9 @@ "-Wimplicit-fallthrough", ); OTHER_LDFLAGS = ( - "$(inherited)", - "-filelist", - "$(LLVM_BUILD_DIR)/archives.txt", - "-framework", - Foundation, - "-framework", - DebugSymbols, - "-framework", - Security, - "-framework", - CoreServices, - "-framework", - ApplicationServices, - "$(LLDB_GTESTS_LDFLAGS)", - "$(LLDB_ZLIB_LDFLAGS)", "$(LLDB_COMPRESSION_LDFLAGS)", + "$(LLDB_ZLIB_LDFLAGS)", + "$(LLDB_COVERAGE_LDFLAGS)", ); PYTHON_FRAMEWORK_PATH = /System/Library/Frameworks/Python.framework/; PYTHON_VERSION_MAJOR = 2; @@ -8726,22 +8713,9 @@ "-Wimplicit-fallthrough", ); OTHER_LDFLAGS = ( - "$(inherited)", - "-filelist", - "$(LLVM_BUILD_DIR)/archives.txt", - "-framework", - Foundation, - "-framework", - DebugSymbols, - "-framework", - Security, - "-framework", - CoreServices, - "-framework", - ApplicationServices, - "$(LLDB_GTESTS_LDFLAGS)", - "$(LLDB_ZLIB_LDFLAGS)", "$(LLDB_COMPRESSION_LDFLAGS)", + "$(LLDB_ZLIB_LDFLAGS)", + "$(LLDB_COVERAGE_LDFLAGS)", ); PYTHON_FRAMEWORK_PATH = /System/Library/Frameworks/Python.framework/; PYTHON_VERSION_MAJOR = 2; @@ -9062,8 +9036,6 @@ "-framework", ApplicationServices, "$(LLDB_GTESTS_LDFLAGS)", - "$(LLDB_ZLIB_LDFLAGS)", - "$(LLDB_COMPRESSION_LDFLAGS)", ); PRODUCT_NAME = "lldb-gtest"; }; @@ -9106,8 +9078,6 @@ "-framework", ApplicationServices, "$(LLDB_GTESTS_LDFLAGS)", - "$(LLDB_ZLIB_LDFLAGS)", - "$(LLDB_COMPRESSION_LDFLAGS)", ); PRODUCT_NAME = "lldb-gtest"; }; @@ -9150,8 +9120,6 @@ "-framework", ApplicationServices, "$(LLDB_GTESTS_LDFLAGS)", - "$(LLDB_ZLIB_LDFLAGS)", - "$(LLDB_COMPRESSION_LDFLAGS)", ); PRODUCT_NAME = "lldb-gtest"; }; @@ -9194,8 +9162,6 @@ "-framework",
