[Lldb-commits] [PATCH] D25592: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
enlight updated this revision to Diff 74622.
enlight added a comment.
Renamed placeholder c-strings.
Repository:
rL LLVM
https://reviews.llvm.org/D25592
Files:
tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
tools/lldb-mi/MICmnLLDBUtilSBValue.h
Index: tools/lldb-mi/MICmnLLDBUtilSBValue.h
===
--- tools/lldb-mi/MICmnLLDBUtilSBValue.h
+++ tools/lldb-mi/MICmnLLDBUtilSBValue.h
@@ -70,8 +70,6 @@
// Attributes:
private:
lldb::SBValue &m_rValue;
- const char *m_pUnkwn;
- const char *m_pComposite;
bool m_bValidSBValue; // True = SBValue is a valid object, false = not valid.
bool m_bHandleCharType; // True = Yes return text molding to char type, false
// = just return data.
Index: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
===
--- tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
+++ tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
@@ -18,6 +18,9 @@
#include "MICmnMIValueTuple.h"
#include "MIUtilString.h"
+static const char *kUnresolvedValue = "??";
+static const char *kUnresolvedCompositeValue = "{...}";
+
//++
//
// Details: CMICmnLLDBUtilSBValue constructor.
@@ -32,8 +35,8 @@
CMICmnLLDBUtilSBValue::CMICmnLLDBUtilSBValue(
const lldb::SBValue &vrValue, const bool vbHandleCharType /* = false */,
const bool vbHandleArrayType /* = true */)
-: m_rValue(const_cast(vrValue)), m_pUnkwn("??"),
- m_pComposite("{...}"), m_bHandleCharType(vbHandleCharType),
+: m_rValue(const_cast(vrValue)),
+ m_bHandleCharType(vbHandleCharType),
m_bHandleArrayType(vbHandleArrayType) {
m_bValidSBValue = m_rValue.IsValid();
}
@@ -80,7 +83,7 @@
CMIUtilString CMICmnLLDBUtilSBValue::GetValue(
const bool vbExpandAggregates /* = false */) const {
if (!m_bValidSBValue)
-return m_pUnkwn;
+return kUnresolvedValue;
CMICmnLLDBDebugSessionInfo &rSessionInfo(
CMICmnLLDBDebugSessionInfo::Instance());
@@ -98,7 +101,7 @@
return value;
if (!vbExpandAggregates && !bPrintExpandAggregates)
-return m_pComposite;
+return kUnresolvedCompositeValue;
bool bPrintAggregateFieldNames = false;
bPrintAggregateFieldNames =
@@ -110,7 +113,7 @@
CMICmnMIValueTuple miValueTuple;
const bool bOk = GetCompositeValue(bPrintAggregateFieldNames, miValueTuple);
if (!bOk)
-return m_pUnkwn;
+return kUnresolvedValue;
value = miValueTuple.GetString();
return value;
@@ -131,11 +134,11 @@
CMIUtilString &vwrValue) const {
const MIuint nChildren = m_rValue.GetNumChildren();
if (nChildren == 0) {
-vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(), m_pUnkwn);
+vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(), kUnresolvedValue);
return MIstatus::success;
} else if (IsPointerType()) {
vwrValue =
-GetValueSummary(!m_bHandleCharType && IsPointeeCharType(), m_pUnkwn);
+GetValueSummary(!m_bHandleCharType && IsPointeeCharType(), kUnresolvedValue);
return MIstatus::success;
} else if (IsArrayType()) {
CMICmnLLDBDebugSessionInfo &rSessionInfo(
@@ -187,13 +190,13 @@
miValueTuple, vnDepth + 1);
if (!bOk)
// Can't obtain composite type
-value = m_pUnkwn;
+value = kUnresolvedValue;
else
// OK. Value is composite and was successfully got
value = miValueTuple.GetString();
} else {
// Need to get value from composite type, but vnMaxDepth is reached
- value = m_pComposite;
+ value = kUnresolvedCompositeValue;
}
const bool bNoQuotes = true;
const CMICmnMIValueConst miValueConst(value, bNoQuotes);
@@ -404,7 +407,7 @@
const MIuint64 nReadBytes =
process.ReadMemory(addr, &ch, sizeof(ch), error);
if (error.Fail() || nReadBytes != sizeof(ch))
- return m_pUnkwn;
+ return kUnresolvedValue;
else if (ch == 0)
break;
result.append(
@@ -425,7 +428,7 @@
//--
bool CMICmnLLDBUtilSBValue::IsNameUnknown() const {
const CMIUtilString name(GetName());
- return (name == m_pUnkwn);
+ return (name == kUnresolvedValue);
}
//++
@@ -438,7 +441,7 @@
//--
bool CMICmnLLDBUtilSBValue::IsValueUnknown() const {
const CMIUtilString value(GetValue());
- return (value == m_pUnkwn);
+ return (value == kUnresolvedValue);
}
//++
@@ -451,7 +454,7 @@
//--
CMIUtilString CMICmnLLDBUtilSBValue::GetTypeName() const {
const char *pName = m_bValidSBValue ? m_rValue.GetTypeName() : nullptr;
- const CMIUtilString text((pName != nullptr) ? pName : m_pUnkwn);
+ const CMIUtilString text((pName != nullptr) ? pName : kUnresolvedValue);
return text;
}
@@ -466,7 +469,7 @@
//--
CMIUtilString CMICmnLLDBUtilSBValue::GetTypeNameDisplay() const {
const char *pName
[Lldb-commits] [PATCH] D25592: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
ki.stfu accepted this revision.
ki.stfu added inline comments.
This revision is now accepted and ready to land.
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:21-22
+static const char *kUnknownValue = "??";
+static const char *kCompositeValuePlaceholder = "{...}";
+
enlight wrote:
> ki.stfu wrote:
> > use unnamed namespace here
> I would like to use an anonymous namespace but the [[
> http://llvm.org/docs/CodingStandards.html#anonymous-namespaces | LLVM coding
> standards ]] say
>
> > make anonymous namespaces as small as possible, and only use them for class
> > declarations
>
> I don't really buy their reasoning, but we are supposed to be following their
> style now aren't we?
>
> kUnresolvedValue/kUnresolvedCompositeValue does sound better, I'll rename
> them.
Okay, we have to follow the coding style.
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:22
+static const char *kUnknownValue = "??";
+static const char *kCompositeValuePlaceholder = "{...}";
+
ki.stfu wrote:
> I dislike Placeholder suffix because the previous one is also a placeholder
> for unknown values. How about renaming to
> kUnresolvedValue/kUnresolvedCompositeValue?
Actually I like kUnknownValue and I meant to rename the second one only. But it
looks good to me also.
Repository:
rL LLVM
https://reviews.llvm.org/D25592
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D25592: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
ki.stfu requested changes to this revision. ki.stfu added inline comments. This revision now requires changes to proceed. Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:116 if (!bOk) -return m_pUnkwn; +return kUnresolvedValue; After couple of minutes I see the lack of logic there: the value is still composite but we return kUnresolvedValue instead of kUnresolvedCompositeValue. I think we have to rename kUnresolvedValue back to kUnknownValue. Repository: rL LLVM https://reviews.llvm.org/D25592 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D25592: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
enlight updated this revision to Diff 74628.
enlight marked an inline comment as done.
enlight added a comment.
Rename `kUnresolvedValue` back to `kUknownValue`.
Repository:
rL LLVM
https://reviews.llvm.org/D25592
Files:
tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
tools/lldb-mi/MICmnLLDBUtilSBValue.h
Index: tools/lldb-mi/MICmnLLDBUtilSBValue.h
===
--- tools/lldb-mi/MICmnLLDBUtilSBValue.h
+++ tools/lldb-mi/MICmnLLDBUtilSBValue.h
@@ -70,8 +70,6 @@
// Attributes:
private:
lldb::SBValue &m_rValue;
- const char *m_pUnkwn;
- const char *m_pComposite;
bool m_bValidSBValue; // True = SBValue is a valid object, false = not valid.
bool m_bHandleCharType; // True = Yes return text molding to char type, false
// = just return data.
Index: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
===
--- tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
+++ tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
@@ -18,6 +18,9 @@
#include "MICmnMIValueTuple.h"
#include "MIUtilString.h"
+static const char *kUnknownValue = "??";
+static const char *kUnresolvedCompositeValue = "{...}";
+
//++
//
// Details: CMICmnLLDBUtilSBValue constructor.
@@ -32,8 +35,8 @@
CMICmnLLDBUtilSBValue::CMICmnLLDBUtilSBValue(
const lldb::SBValue &vrValue, const bool vbHandleCharType /* = false */,
const bool vbHandleArrayType /* = true */)
-: m_rValue(const_cast(vrValue)), m_pUnkwn("??"),
- m_pComposite("{...}"), m_bHandleCharType(vbHandleCharType),
+: m_rValue(const_cast(vrValue)),
+ m_bHandleCharType(vbHandleCharType),
m_bHandleArrayType(vbHandleArrayType) {
m_bValidSBValue = m_rValue.IsValid();
}
@@ -80,7 +83,7 @@
CMIUtilString CMICmnLLDBUtilSBValue::GetValue(
const bool vbExpandAggregates /* = false */) const {
if (!m_bValidSBValue)
-return m_pUnkwn;
+return kUnknownValue;
CMICmnLLDBDebugSessionInfo &rSessionInfo(
CMICmnLLDBDebugSessionInfo::Instance());
@@ -98,7 +101,7 @@
return value;
if (!vbExpandAggregates && !bPrintExpandAggregates)
-return m_pComposite;
+return kUnresolvedCompositeValue;
bool bPrintAggregateFieldNames = false;
bPrintAggregateFieldNames =
@@ -110,7 +113,7 @@
CMICmnMIValueTuple miValueTuple;
const bool bOk = GetCompositeValue(bPrintAggregateFieldNames, miValueTuple);
if (!bOk)
-return m_pUnkwn;
+return kUnknownValue;
value = miValueTuple.GetString();
return value;
@@ -131,11 +134,11 @@
CMIUtilString &vwrValue) const {
const MIuint nChildren = m_rValue.GetNumChildren();
if (nChildren == 0) {
-vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(), m_pUnkwn);
+vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(), kUnknownValue);
return MIstatus::success;
} else if (IsPointerType()) {
vwrValue =
-GetValueSummary(!m_bHandleCharType && IsPointeeCharType(), m_pUnkwn);
+GetValueSummary(!m_bHandleCharType && IsPointeeCharType(), kUnknownValue);
return MIstatus::success;
} else if (IsArrayType()) {
CMICmnLLDBDebugSessionInfo &rSessionInfo(
@@ -187,13 +190,13 @@
miValueTuple, vnDepth + 1);
if (!bOk)
// Can't obtain composite type
-value = m_pUnkwn;
+value = kUnknownValue;
else
// OK. Value is composite and was successfully got
value = miValueTuple.GetString();
} else {
// Need to get value from composite type, but vnMaxDepth is reached
- value = m_pComposite;
+ value = kUnresolvedCompositeValue;
}
const bool bNoQuotes = true;
const CMICmnMIValueConst miValueConst(value, bNoQuotes);
@@ -404,7 +407,7 @@
const MIuint64 nReadBytes =
process.ReadMemory(addr, &ch, sizeof(ch), error);
if (error.Fail() || nReadBytes != sizeof(ch))
- return m_pUnkwn;
+ return kUnknownValue;
else if (ch == 0)
break;
result.append(
@@ -425,7 +428,7 @@
//--
bool CMICmnLLDBUtilSBValue::IsNameUnknown() const {
const CMIUtilString name(GetName());
- return (name == m_pUnkwn);
+ return (name == kUnknownValue);
}
//++
@@ -438,7 +441,7 @@
//--
bool CMICmnLLDBUtilSBValue::IsValueUnknown() const {
const CMIUtilString value(GetValue());
- return (value == m_pUnkwn);
+ return (value == kUnknownValue);
}
//++
@@ -451,7 +454,7 @@
//--
CMIUtilString CMICmnLLDBUtilSBValue::GetTypeName() const {
const char *pName = m_bValidSBValue ? m_rValue.GetTypeName() : nullptr;
- const CMIUtilString text((pName != nullptr) ? pName : m_pUnkwn);
+ const CMIUtilString text((pName != nullptr) ? pName : kUnknownValue);
return text;
}
@@ -466,7 +469,7 @@
//--
CMIUtilString CMICmnLLDBUtilSBValue::GetTypeNameDisplay
[Lldb-commits] [PATCH] D25592: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
enlight marked an inline comment as done. enlight added inline comments. Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:116 if (!bOk) -return m_pUnkwn; +return kUnresolvedValue; ki.stfu wrote: > After couple of minutes I see the lack of logic there: the value is still > composite but we return kUnresolvedValue instead of kUnresolvedCompositeValue. > > I think we have to rename kUnresolvedValue back to kUnknownValue. Yep, good point, I've renamed it back. Repository: rL LLVM https://reviews.llvm.org/D25592 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D25569: Minidump plugin: functions parsing memory structures and filtering module list
labath added a comment.
First round of comments from me :).
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:248
+
+if (lowest_addr.find(module_name) == lowest_addr.end()) {
+ lowest_addr[module_name] =
If you use the `emplace` function, and then check the returned value, you will
avoid doing two lookups into the table.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:256
+
+ for (auto module : lowest_addr) {
+filtered_modules.push_back(module.second.second);
`const auto &`, to avoid a copy. You can also call reserve() on the vector to
avoid reallocations.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:387
+ info.SetReadable((entry->protect & PageNoAccess) == 0
+ ? MemoryRegionInfo::eYes
+ : MemoryRegionInfo::eNo);
these would probably fit on one line if you defined helper constants "yes" and
"no" like you did in the tests.
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:79
+ std::vector
+ FilterModuleList(llvm::ArrayRef &modules);
+
This would be easier to use if was declared like this:
```
... GetFilteredModuleList();
```
and it called GetModuleList under the hood. (Unless you actually anticipate
needing to filter random module lists, in which case it should probably be
static as it does not touch the local vars). And in any case, it should not
take a reference to the ArrayRef.
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:85
+
+ size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size, Error &error);
+
We use `Do` prefix is used for internal helper functions, when you cannot think
of a better name. This functions is not that, so it should be just
`ReadMemory`. That said, I don't think this function should be here at all.
An interface more consistent with the rest of the class would be:
```
llvm::ArrayRef GetMemory(lldb::addr_t addr, size_t size);
```
It involves no copying (but the caller can still easily memcpy() it if he
really needs to. As for the corner cases (callee asks for a region which is
only partially available, you can define whichever behaviour makes more sense
for your use case. If after this, you find that the function is too small to be
useful, you can keep it as a private function inside the register context class.
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:87
+
+ Error GetMemoryRegionInfo(lldb::addr_t load_addr,
+lldb_private::MemoryRegionInfo &info);
```
llvm::Optional GetMemoryRegionInfo(lldb::addr_t)
```
seems more consistent.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:144
+ parser->FilterModuleList(modules);
+ EXPECT_GT(modules.size(), filtered_modules.size());
+ bool found = false;
Why not expect the exact numbers here? They're not going to change between runs.
https://reviews.llvm.org/D25569
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r284231 - [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
Author: enlight
Date: Fri Oct 14 07:58:02 2016
New Revision: 284231
URL: http://llvm.org/viewvc/llvm-project?rev=284231&view=rev
Log:
[LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
Summary:
Placeholder c-strings don't need to be instance variables.
Reviewers: ki.stfu, abidh
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D25592
Modified:
lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.h
Modified: lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp?rev=284231&r1=284230&r2=284231&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp Fri Oct 14 07:58:02 2016
@@ -18,6 +18,9 @@
#include "MICmnMIValueTuple.h"
#include "MIUtilString.h"
+static const char *kUnknownValue = "??";
+static const char *kUnresolvedCompositeValue = "{...}";
+
//++
//
// Details: CMICmnLLDBUtilSBValue constructor.
@@ -32,8 +35,8 @@
CMICmnLLDBUtilSBValue::CMICmnLLDBUtilSBValue(
const lldb::SBValue &vrValue, const bool vbHandleCharType /* = false */,
const bool vbHandleArrayType /* = true */)
-: m_rValue(const_cast(vrValue)), m_pUnkwn("??"),
- m_pComposite("{...}"), m_bHandleCharType(vbHandleCharType),
+: m_rValue(const_cast(vrValue)),
+ m_bHandleCharType(vbHandleCharType),
m_bHandleArrayType(vbHandleArrayType) {
m_bValidSBValue = m_rValue.IsValid();
}
@@ -80,7 +83,7 @@ CMIUtilString CMICmnLLDBUtilSBValue::Get
CMIUtilString CMICmnLLDBUtilSBValue::GetValue(
const bool vbExpandAggregates /* = false */) const {
if (!m_bValidSBValue)
-return m_pUnkwn;
+return kUnknownValue;
CMICmnLLDBDebugSessionInfo &rSessionInfo(
CMICmnLLDBDebugSessionInfo::Instance());
@@ -98,7 +101,7 @@ CMIUtilString CMICmnLLDBUtilSBValue::Get
return value;
if (!vbExpandAggregates && !bPrintExpandAggregates)
-return m_pComposite;
+return kUnresolvedCompositeValue;
bool bPrintAggregateFieldNames = false;
bPrintAggregateFieldNames =
@@ -110,7 +113,7 @@ CMIUtilString CMICmnLLDBUtilSBValue::Get
CMICmnMIValueTuple miValueTuple;
const bool bOk = GetCompositeValue(bPrintAggregateFieldNames, miValueTuple);
if (!bOk)
-return m_pUnkwn;
+return kUnknownValue;
value = miValueTuple.GetString();
return value;
@@ -131,11 +134,11 @@ bool CMICmnLLDBUtilSBValue::GetSimpleVal
CMIUtilString &vwrValue) const {
const MIuint nChildren = m_rValue.GetNumChildren();
if (nChildren == 0) {
-vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(), m_pUnkwn);
+vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(),
kUnknownValue);
return MIstatus::success;
} else if (IsPointerType()) {
vwrValue =
-GetValueSummary(!m_bHandleCharType && IsPointeeCharType(), m_pUnkwn);
+GetValueSummary(!m_bHandleCharType && IsPointeeCharType(),
kUnknownValue);
return MIstatus::success;
} else if (IsArrayType()) {
CMICmnLLDBDebugSessionInfo &rSessionInfo(
@@ -187,13 +190,13 @@ bool CMICmnLLDBUtilSBValue::GetComposite
miValueTuple, vnDepth + 1);
if (!bOk)
// Can't obtain composite type
-value = m_pUnkwn;
+value = kUnknownValue;
else
// OK. Value is composite and was successfully got
value = miValueTuple.GetString();
} else {
// Need to get value from composite type, but vnMaxDepth is reached
- value = m_pComposite;
+ value = kUnresolvedCompositeValue;
}
const bool bNoQuotes = true;
const CMICmnMIValueConst miValueConst(value, bNoQuotes);
@@ -404,7 +407,7 @@ CMICmnLLDBUtilSBValue::ReadCStringFromHo
const MIuint64 nReadBytes =
process.ReadMemory(addr, &ch, sizeof(ch), error);
if (error.Fail() || nReadBytes != sizeof(ch))
- return m_pUnkwn;
+ return kUnknownValue;
else if (ch == 0)
break;
result.append(
@@ -425,7 +428,7 @@ CMICmnLLDBUtilSBValue::ReadCStringFromHo
//--
bool CMICmnLLDBUtilSBValue::IsNameUnknown() const {
const CMIUtilString name(GetName());
- return (name == m_pUnkwn);
+ return (name == kUnknownValue);
}
//++
@@ -438,7 +441,7 @@ bool CMICmnLLDBUtilSBValue::IsNameUnknow
//--
bool CMICmnLLDBUtilSBValue::IsValueUnknown() const {
const CMIUtilString value(GetValue());
- return (value == m_pUnkwn);
+ return (value == kUnknownValue);
}
//++
@@ -451,7 +454,7 @@ bool CMICmnLLDBUtilSBValue::IsValueUnkno
//--
CMIUtilString CMICmnLLDBUtilSBValue::GetTypeName() const {
const char *pName = m_bValidSBValue ? m_rValue.GetTypeName() : nullptr;
- const CMIUt
[Lldb-commits] [PATCH] D25592: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue class
This revision was automatically updated to reflect the committed changes.
enlight marked an inline comment as done.
Closed by commit rL284231: [LLDB-MI] Minor cleanup of CMICmnLLDBUtilSBValue
class (authored by enlight).
Changed prior to commit:
https://reviews.llvm.org/D25592?vs=74628&id=74667#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25592
Files:
lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.h
Index: lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
+++ lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.cpp
@@ -18,6 +18,9 @@
#include "MICmnMIValueTuple.h"
#include "MIUtilString.h"
+static const char *kUnknownValue = "??";
+static const char *kUnresolvedCompositeValue = "{...}";
+
//++
//
// Details: CMICmnLLDBUtilSBValue constructor.
@@ -32,8 +35,8 @@
CMICmnLLDBUtilSBValue::CMICmnLLDBUtilSBValue(
const lldb::SBValue &vrValue, const bool vbHandleCharType /* = false */,
const bool vbHandleArrayType /* = true */)
-: m_rValue(const_cast(vrValue)), m_pUnkwn("??"),
- m_pComposite("{...}"), m_bHandleCharType(vbHandleCharType),
+: m_rValue(const_cast(vrValue)),
+ m_bHandleCharType(vbHandleCharType),
m_bHandleArrayType(vbHandleArrayType) {
m_bValidSBValue = m_rValue.IsValid();
}
@@ -80,7 +83,7 @@
CMIUtilString CMICmnLLDBUtilSBValue::GetValue(
const bool vbExpandAggregates /* = false */) const {
if (!m_bValidSBValue)
-return m_pUnkwn;
+return kUnknownValue;
CMICmnLLDBDebugSessionInfo &rSessionInfo(
CMICmnLLDBDebugSessionInfo::Instance());
@@ -98,7 +101,7 @@
return value;
if (!vbExpandAggregates && !bPrintExpandAggregates)
-return m_pComposite;
+return kUnresolvedCompositeValue;
bool bPrintAggregateFieldNames = false;
bPrintAggregateFieldNames =
@@ -110,7 +113,7 @@
CMICmnMIValueTuple miValueTuple;
const bool bOk = GetCompositeValue(bPrintAggregateFieldNames, miValueTuple);
if (!bOk)
-return m_pUnkwn;
+return kUnknownValue;
value = miValueTuple.GetString();
return value;
@@ -131,11 +134,11 @@
CMIUtilString &vwrValue) const {
const MIuint nChildren = m_rValue.GetNumChildren();
if (nChildren == 0) {
-vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(), m_pUnkwn);
+vwrValue = GetValueSummary(!m_bHandleCharType && IsCharType(), kUnknownValue);
return MIstatus::success;
} else if (IsPointerType()) {
vwrValue =
-GetValueSummary(!m_bHandleCharType && IsPointeeCharType(), m_pUnkwn);
+GetValueSummary(!m_bHandleCharType && IsPointeeCharType(), kUnknownValue);
return MIstatus::success;
} else if (IsArrayType()) {
CMICmnLLDBDebugSessionInfo &rSessionInfo(
@@ -187,13 +190,13 @@
miValueTuple, vnDepth + 1);
if (!bOk)
// Can't obtain composite type
-value = m_pUnkwn;
+value = kUnknownValue;
else
// OK. Value is composite and was successfully got
value = miValueTuple.GetString();
} else {
// Need to get value from composite type, but vnMaxDepth is reached
- value = m_pComposite;
+ value = kUnresolvedCompositeValue;
}
const bool bNoQuotes = true;
const CMICmnMIValueConst miValueConst(value, bNoQuotes);
@@ -404,7 +407,7 @@
const MIuint64 nReadBytes =
process.ReadMemory(addr, &ch, sizeof(ch), error);
if (error.Fail() || nReadBytes != sizeof(ch))
- return m_pUnkwn;
+ return kUnknownValue;
else if (ch == 0)
break;
result.append(
@@ -425,7 +428,7 @@
//--
bool CMICmnLLDBUtilSBValue::IsNameUnknown() const {
const CMIUtilString name(GetName());
- return (name == m_pUnkwn);
+ return (name == kUnknownValue);
}
//++
@@ -438,7 +441,7 @@
//--
bool CMICmnLLDBUtilSBValue::IsValueUnknown() const {
const CMIUtilString value(GetValue());
- return (value == m_pUnkwn);
+ return (value == kUnknownValue);
}
//++
@@ -451,7 +454,7 @@
//--
CMIUtilString CMICmnLLDBUtilSBValue::GetTypeName() const {
const char *pName = m_bValidSBValue ? m_rValue.GetTypeName() : nullptr;
- const CMIUtilString text((pName != nullptr) ? pName : m_pUnkwn);
+ const CMIUtilString text((pName != nullptr) ? pName : kUnknownValue);
return text;
}
@@ -466,7 +469,7 @@
//--
CMIUtilString CMICmnLLDBUtilSBValue::GetTypeNameDisplay() const {
const char *pName = m_bValidSBValue ? m_rValue.GetDisplayTypeName() : nullptr;
- const CMIUtilString text((pName != nullptr) ? pName : m_pUnkwn);
+ const CMIUtilString text((pName != nullptr) ? pName : kUnknownValue);
return text;
}
Index: lldb/trunk/tools/lldb-mi/MICmnLLDBUtilSBValue.h
===
[Lldb-commits] [PATCH] D25569: Minidump plugin: functions parsing memory structures and filtering module list
dvlahovski updated this revision to Diff 74694.
dvlahovski marked 7 inline comments as done.
dvlahovski added a comment.
Resolved Pavel's remarks.
Also added a unit test for the GetMemory function.
https://reviews.llvm.org/D25569
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/MinidumpTypes.h
unittests/Process/minidump/CMakeLists.txt
unittests/Process/minidump/Inputs/fizzbuzz_wow64.dmp
unittests/Process/minidump/Inputs/linux-x86_64_not_crashed.dmp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -19,6 +19,7 @@
#include "lldb/Core/ArchSpec.h"
#include "lldb/Core/DataExtractor.h"
#include "lldb/Host/FileSpec.h"
+#include "lldb/Target/MemoryRegionInfo.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Optional.h"
@@ -68,7 +69,11 @@
ASSERT_EQ(1UL, thread_list.size());
const MinidumpThread thread = thread_list[0];
- ASSERT_EQ(16001UL, thread.thread_id);
+
+ EXPECT_EQ(16001UL, thread.thread_id);
+
+ llvm::ArrayRef context = parser->GetThreadContext(thread);
+ EXPECT_EQ(1232UL, context.size());
}
TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) {
@@ -131,14 +136,111 @@
}
}
+TEST_F(MinidumpParserTest, GetFilteredModuleList) {
+ SetUpData("linux-x86_64_not_crashed.dmp");
+ llvm::ArrayRef modules = parser->GetModuleList();
+ std::vector filtered_modules =
+ parser->GetFilteredModuleList();
+ EXPECT_EQ(10UL, modules.size());
+ EXPECT_EQ(9UL, filtered_modules.size());
+ // EXPECT_GT(modules.size(), filtered_modules.size());
+ bool found = false;
+ for (size_t i = 0; i < filtered_modules.size(); ++i) {
+llvm::Optional name =
+parser->GetMinidumpString(filtered_modules[i]->module_name_rva);
+ASSERT_TRUE(name.hasValue());
+if (name.getValue() == "/tmp/test/linux-x86_64_not_crashed") {
+ ASSERT_FALSE(found) << "There should be only one module with this name "
+ "in the filtered module list";
+ found = true;
+ ASSERT_EQ(0x40UL, filtered_modules[i]->base_of_image);
+}
+ }
+}
+
TEST_F(MinidumpParserTest, GetExceptionStream) {
SetUpData("linux-x86_64.dmp");
const MinidumpExceptionStream *exception_stream =
parser->GetExceptionStream();
ASSERT_NE(nullptr, exception_stream);
ASSERT_EQ(11UL, exception_stream->exception_record.exception_code);
}
+void check_mem_range_exists(std::unique_ptr &parser,
+const uint64_t range_start,
+const uint64_t range_size) {
+ llvm::Optional range = parser->FindMemoryRange(range_start);
+ ASSERT_TRUE(range.hasValue()) << "There is no range containing this address";
+ EXPECT_EQ(range_start, range->start);
+ EXPECT_EQ(range_start + range_size, range->start + range->range_ref.size());
+}
+
+TEST_F(MinidumpParserTest, FindMemoryRange) {
+ SetUpData("linux-x86_64.dmp");
+ // There are two memory ranges in the file (size is in bytes, decimal):
+ // 1) 0x401d46 256
+ // 2) 0x7ffceb34a000 12288
+ EXPECT_FALSE(parser->FindMemoryRange(0x00).hasValue());
+ EXPECT_FALSE(parser->FindMemoryRange(0x2a).hasValue());
+
+ check_mem_range_exists(parser, 0x401d46, 256);
+ EXPECT_FALSE(parser->FindMemoryRange(0x401d46 + 256).hasValue());
+
+ check_mem_range_exists(parser, 0x7ffceb34a000, 12288);
+ EXPECT_FALSE(parser->FindMemoryRange(0x7ffceb34a000 + 12288).hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetMemory) {
+ SetUpData("linux-x86_64.dmp");
+
+ EXPECT_EQ(128UL, parser->GetMemory(0x401d46, 128).size());
+ EXPECT_EQ(256UL, parser->GetMemory(0x401d46, 512).size());
+
+ EXPECT_EQ(12288UL, parser->GetMemory(0x7ffceb34a000, 12288).size());
+ EXPECT_EQ(1024UL, parser->GetMemory(0x7ffceb34a000, 1024).size());
+
+ EXPECT_EQ(0UL, parser->GetMemory(0x50, 512).size());
+}
+
+TEST_F(MinidumpParserTest, FindMemoryRangeWithFullMemoryMinidump) {
+ SetUpData("fizzbuzz_wow64.dmp");
+
+ // There are a lot of ranges in the file, just testing with some of them
+ EXPECT_FALSE(parser->FindMemoryRange(0x00).hasValue());
+ EXPECT_FALSE(parser->FindMemoryRange(0x2a).hasValue());
+ check_mem_range_exists(parser, 0x1, 65536); // first range
+ check_mem_range_exists(parser, 0x4, 4096);
+ EXPECT_FALSE(parser->FindMemoryRange(0x4 + 4096).hasValue());
+ check_mem_range_exists(parser, 0x77c12000, 8192);
+ check_mem_range_exists(parser, 0x7ffe, 4096); // last range
+ EXPECT_FALSE(parser->FindMemoryRange(0x7ffe + 4096).hasValue());
+}
+
+void check_region_info(std::unique_ptr &parser,
+ const uint64_t addr, MemoryRegionInfo::OptionalBool read,
+ MemoryRegionInfo::
[Lldb-commits] [lldb] r284250 - [CMake] Populate LLDB.framework's headers directory
Author: cbieneman
Date: Fri Oct 14 12:09:55 2016
New Revision: 284250
URL: http://llvm.org/viewvc/llvm-project?rev=284250&view=rev
Log:
[CMake] Populate LLDB.framework's headers directory
Summary:
This patch adds support for installing public headers in LLDB.framework, and
symlinking the headers into the build directory.
While writing the patch I discovered a bug in CMake that prevents applying
POST_BUILD steps to framework targets
(https://gitlab.kitware.com/cmake/cmake/issues/16363).
I've implemented the support using POST_BUILD steps wrapped under a CMake
version check with a TODO so that we can track the fix.
Reviewers: tfiala, zturner, spyffe
Subscribers: lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D25570
Modified:
lldb/trunk/source/API/CMakeLists.txt
Modified: lldb/trunk/source/API/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/CMakeLists.txt?rev=284250&r1=284249&r2=284250&view=diff
==
--- lldb/trunk/source/API/CMakeLists.txt (original)
+++ lldb/trunk/source/API/CMakeLists.txt Fri Oct 14 12:09:55 2016
@@ -135,10 +135,26 @@ endif()
target_link_libraries(liblldb PRIVATE ${LLDB_SYSTEM_LIBS})
if(LLDB_BUILD_FRAMEWORK)
+ file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
set_target_properties(liblldb PROPERTIES
OUTPUT_NAME LLDB
FRAMEWORK On
FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
-LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR})
+LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}
+PUBLIC_HEADER "${public_headers}")
+
+ # This works around a CMake bug where POST_BUILD steps are not applied to
+ # framework targets. This fix is merged into the CMake release branch and
+ # should be available with CMake 3.7 rc2:
+ # https://gitlab.kitware.com/cmake/cmake/issues/16363
+ if(CMAKE_VERSION VERSION_GREATER 3.6.99)
+add_custom_command(TARGET liblldb POST_BUILD
+ COMMAND ${CMAKE_COMMAND} -E create_symlink
${LLDB_SOURCE_DIR}/include/lldb/API $/Headers)
+ else()
+add_custom_command(OUTPUT
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Headers
+ COMMAND ${CMAKE_COMMAND} -E create_symlink
${LLDB_SOURCE_DIR}/include/lldb/API $/Headers)
+add_custom_target(lldb_header_symlink
+ DEPENDS
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Headers)
+ endif()
endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D25570: [CMake] Populate LLDB.framework's headers directory
This revision was automatically updated to reflect the committed changes.
Closed by commit rL284250: [CMake] Populate LLDB.framework's headers directory
(authored by cbieneman).
Changed prior to commit:
https://reviews.llvm.org/D25570?vs=74543&id=74708#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25570
Files:
lldb/trunk/source/API/CMakeLists.txt
Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -135,10 +135,26 @@
target_link_libraries(liblldb PRIVATE ${LLDB_SYSTEM_LIBS})
if(LLDB_BUILD_FRAMEWORK)
+ file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
set_target_properties(liblldb PROPERTIES
OUTPUT_NAME LLDB
FRAMEWORK On
FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
-LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR})
+LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}
+PUBLIC_HEADER "${public_headers}")
+
+ # This works around a CMake bug where POST_BUILD steps are not applied to
+ # framework targets. This fix is merged into the CMake release branch and
+ # should be available with CMake 3.7 rc2:
+ # https://gitlab.kitware.com/cmake/cmake/issues/16363
+ if(CMAKE_VERSION VERSION_GREATER 3.6.99)
+add_custom_command(TARGET liblldb POST_BUILD
+ COMMAND ${CMAKE_COMMAND} -E create_symlink
${LLDB_SOURCE_DIR}/include/lldb/API $/Headers)
+ else()
+add_custom_command(OUTPUT
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Headers
+ COMMAND ${CMAKE_COMMAND} -E create_symlink
${LLDB_SOURCE_DIR}/include/lldb/API $/Headers)
+add_custom_target(lldb_header_symlink
+ DEPENDS
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Headers)
+ endif()
endif()
Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -135,10 +135,26 @@
target_link_libraries(liblldb PRIVATE ${LLDB_SYSTEM_LIBS})
if(LLDB_BUILD_FRAMEWORK)
+ file(GLOB public_headers ${LLDB_SOURCE_DIR}/include/lldb/API/*.h)
set_target_properties(liblldb PROPERTIES
OUTPUT_NAME LLDB
FRAMEWORK On
FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
-LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR})
+LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}
+PUBLIC_HEADER "${public_headers}")
+
+ # This works around a CMake bug where POST_BUILD steps are not applied to
+ # framework targets. This fix is merged into the CMake release branch and
+ # should be available with CMake 3.7 rc2:
+ # https://gitlab.kitware.com/cmake/cmake/issues/16363
+ if(CMAKE_VERSION VERSION_GREATER 3.6.99)
+add_custom_command(TARGET liblldb POST_BUILD
+ COMMAND ${CMAKE_COMMAND} -E create_symlink ${LLDB_SOURCE_DIR}/include/lldb/API $/Headers)
+ else()
+add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Headers
+ COMMAND ${CMAKE_COMMAND} -E create_symlink ${LLDB_SOURCE_DIR}/include/lldb/API $/Headers)
+add_custom_target(lldb_header_symlink
+ DEPENDS ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Headers)
+ endif()
endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D25569: Minidump plugin: functions parsing memory structures and filtering module list
zturner added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:242
+
+ for (const auto &module : modules) {
+name = GetMinidumpString(module.module_name_rva);
I don't know how big the minidumps you're working with are or if performance is
a concern, but `std::map<>` is more or less a piece of junk. The only time it
should really be used is if you need a deterministic iteration order.
`std::unordered_map<>` is slightly better. Better still is `llvm::StringMap<>`
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:281
+
+ if (data.size() == 0 && data64.size() == 0)
+return llvm::None;
`data.empty()`?
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:284
+
+ if (data.size() > 0) {
+llvm::ArrayRef memory_list =
`!data.empty()`?
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:291
+
+for (auto memory_desc : memory_list) {
+ const MinidumpLocationDescriptor &loc_desc = memory_desc.memory;
`auto &` to avoid a copy.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:320
+
+for (auto memory_desc64 : memory64_list) {
+ const lldb::addr_t range_start = memory_desc64.start_of_memory_range;
`auto &`
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:222
+
+ if (header->size_of_header > sizeof(MinidumpMemoryInfoListHeader)) {
+data = data.drop_front(header->size_of_header -
I don't think you need the conditional here do you? Just `data =
data.drop_front(sizeof_MinidumpMemoryInfoListHeader)`.
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:268
+ MemFree = 0x1,
+ MemReserve = 0x2000,
+};
Can you use `llvm/ADT/BitmaskEnum.h` here to give this enum bitwise operators?
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:274
+ MemMapped = 0x4,
+ MemPrivate = 0x2,
+};
Same
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:294
+ PageExecutable = PageExecute | PageExecuteRead | PageExecuteReadWrite |
+ PageExecuteWriteCopy,
+};
Same
https://reviews.llvm.org/D25569
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] LLVM buildmaster will be updated and restarted tonight
Hello everyone, LLVM buildmaster will be updated and restarted after 5 PM Pacific time today. Thanks Galina ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r284296 - This test is no longer failing for gmodules.
Author: jingham
Date: Fri Oct 14 19:04:38 2016
New Revision: 284296
URL: http://llvm.org/viewvc/llvm-project?rev=284296&view=rev
Log:
This test is no longer failing for gmodules.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods2.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods2.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods2.py?rev=284296&r1=284295&r2=284296&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods2.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods2.py
Fri Oct 14 19:04:38 2016
@@ -108,10 +108,6 @@ class FoundationTestCase2(TestBase):
patterns=["\(int\) \$.* = 3"])
self.runCmd("process continue")
-@expectedFailureAll(
-oslist=["macosx"],
-debug_info="gmodules",
-bugnumber="llvm.org/pr27861")
def test_NSString_expr_commands(self):
"""Test expression commands for NSString."""
self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
