[Lldb-commits] [PATCH] D42409: Fix memory leaks in GoParser
teemperor created this revision.
teemperor added reviewers: labath, davide.
The GoParser is leaking memory in the tests due to not freeing allocated nodes
when encountering some parsing errors. With this patch all GoParser tests are
passing with enabled memory sanitizers/ubsan.
https://reviews.llvm.org/D42409
Files:
source/Plugins/ExpressionParser/Go/GoParser.cpp
Index: source/Plugins/ExpressionParser/Go/GoParser.cpp
===
--- source/Plugins/ExpressionParser/Go/GoParser.cpp
+++ source/Plugins/ExpressionParser/Go/GoParser.cpp
@@ -439,8 +439,10 @@
if (!type)
return r.error();
GoASTCompositeLit *lit = LiteralValue();
- if (!lit)
+ if (!lit) {
+delete type;
return r.error();
+ }
lit->SetType(type);
return lit;
}
@@ -548,6 +550,7 @@
GoASTExpr *GoParser::Conversion() {
Rule r("Conversion", this);
if (GoASTExpr *t = Type2()) {
+std::unique_ptr owner(t);
if (match(GoLexer::OP_LPAREN)) {
GoASTExpr *v = Expression();
if (!v)
@@ -557,6 +560,7 @@
return r.error();
GoASTCallExpr *call = new GoASTCallExpr(false);
call->SetFun(t);
+ owner.release();
call->AddArgs(v);
return call;
}
Index: source/Plugins/ExpressionParser/Go/GoParser.cpp
===
--- source/Plugins/ExpressionParser/Go/GoParser.cpp
+++ source/Plugins/ExpressionParser/Go/GoParser.cpp
@@ -439,8 +439,10 @@
if (!type)
return r.error();
GoASTCompositeLit *lit = LiteralValue();
- if (!lit)
+ if (!lit) {
+delete type;
return r.error();
+ }
lit->SetType(type);
return lit;
}
@@ -548,6 +550,7 @@
GoASTExpr *GoParser::Conversion() {
Rule r("Conversion", this);
if (GoASTExpr *t = Type2()) {
+std::unique_ptr owner(t);
if (match(GoLexer::OP_LPAREN)) {
GoASTExpr *v = Expression();
if (!v)
@@ -557,6 +560,7 @@
return r.error();
GoASTCallExpr *call = new GoASTCallExpr(false);
call->SetFun(t);
+ owner.release();
call->AddArgs(v);
return call;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r323181 - Prevent unaligned memory read in parseMinidumpString
Author: teemperor
Date: Tue Jan 23 00:04:27 2018
New Revision: 323181
URL: http://llvm.org/viewvc/llvm-project?rev=323181&view=rev
Log:
Prevent unaligned memory read in parseMinidumpString
Summary:
It's possible to hit an unaligned memory read when reading `source_length` as
the `data` array is only aligned with 2 bytes (it's actually a UTF16 array).
This patch memcpy's `source_length` into a local variable to prevent this:
```
MinidumpTypes.cpp:49:23: runtime error: load of misaligned address
0x7f0f4792692a for type 'const uint32_t' (aka 'const unsigned int'), which
requires 4 byte alignment
```
Reviewers: dvlahovski, zturner, davide
Reviewed By: davide
Subscribers: davide, lldb-commits
Differential Revision: https://reviews.llvm.org/D42348
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp?rev=323181&r1=323180&r2=323181&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp Tue Jan 23
00:04:27 2018
@@ -44,9 +44,14 @@ llvm::Optional
lldb_private::minidump::parseMinidumpString(llvm::ArrayRef &data) {
std::string result;
- const uint32_t *source_length;
- Status error = consumeObject(data, source_length);
- if (error.Fail() || *source_length > data.size() || *source_length % 2 != 0)
+ const uint32_t *source_length_ptr;
+ Status error = consumeObject(data, source_length_ptr);
+
+ // Copy non-aligned source_length data into aligned memory.
+ uint32_t source_length;
+ std::memcpy(&source_length, source_length_ptr, sizeof(source_length));
+
+ if (error.Fail() || source_length > data.size() || source_length % 2 != 0)
return llvm::None;
auto source_start = reinterpret_cast(data.data());
@@ -54,9 +59,9 @@ lldb_private::minidump::parseMinidumpStr
// we need the length of the string in UTF-16 characters/code points (16 bits
// per char)
// that's why it's divided by 2
- const auto source_end = source_start + (*source_length) / 2;
+ const auto source_end = source_start + source_length / 2;
// resize to worst case length
- result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * (*source_length) / 2);
+ result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * source_length / 2);
auto result_start = reinterpret_cast(&result[0]);
const auto result_end = result_start + result.size();
llvm::ConvertUTF16toUTF8(&source_start, source_end, &result_start,
result_end,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42348: Prevent unaligned memory read in parseMinidumpString
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323181: Prevent unaligned memory read in parseMinidumpString
(authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D42348?vs=130798&id=131008#toc
Repository:
rL LLVM
https://reviews.llvm.org/D42348
Files:
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
===
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
@@ -44,19 +44,24 @@
lldb_private::minidump::parseMinidumpString(llvm::ArrayRef &data) {
std::string result;
- const uint32_t *source_length;
- Status error = consumeObject(data, source_length);
- if (error.Fail() || *source_length > data.size() || *source_length % 2 != 0)
+ const uint32_t *source_length_ptr;
+ Status error = consumeObject(data, source_length_ptr);
+
+ // Copy non-aligned source_length data into aligned memory.
+ uint32_t source_length;
+ std::memcpy(&source_length, source_length_ptr, sizeof(source_length));
+
+ if (error.Fail() || source_length > data.size() || source_length % 2 != 0)
return llvm::None;
auto source_start = reinterpret_cast(data.data());
// source_length is the length of the string in bytes
// we need the length of the string in UTF-16 characters/code points (16 bits
// per char)
// that's why it's divided by 2
- const auto source_end = source_start + (*source_length) / 2;
+ const auto source_end = source_start + source_length / 2;
// resize to worst case length
- result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * (*source_length) / 2);
+ result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * source_length / 2);
auto result_start = reinterpret_cast(&result[0]);
const auto result_end = result_start + result.size();
llvm::ConvertUTF16toUTF8(&source_start, source_end, &result_start,
result_end,
Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
===
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
@@ -44,19 +44,24 @@
lldb_private::minidump::parseMinidumpString(llvm::ArrayRef &data) {
std::string result;
- const uint32_t *source_length;
- Status error = consumeObject(data, source_length);
- if (error.Fail() || *source_length > data.size() || *source_length % 2 != 0)
+ const uint32_t *source_length_ptr;
+ Status error = consumeObject(data, source_length_ptr);
+
+ // Copy non-aligned source_length data into aligned memory.
+ uint32_t source_length;
+ std::memcpy(&source_length, source_length_ptr, sizeof(source_length));
+
+ if (error.Fail() || source_length > data.size() || source_length % 2 != 0)
return llvm::None;
auto source_start = reinterpret_cast(data.data());
// source_length is the length of the string in bytes
// we need the length of the string in UTF-16 characters/code points (16 bits
// per char)
// that's why it's divided by 2
- const auto source_end = source_start + (*source_length) / 2;
+ const auto source_end = source_start + source_length / 2;
// resize to worst case length
- result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * (*source_length) / 2);
+ result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * source_length / 2);
auto result_start = reinterpret_cast(&result[0]);
const auto result_end = result_start + result.size();
llvm::ConvertUTF16toUTF8(&source_start, source_end, &result_start, result_end,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
labath added a comment. A couple of thoughts come to mind: - Does this functionality really belong in the client? In case of memory reads, it's the server who patches out the breakpoint opcodes (NativeProcessProtocol::ReadMemoryWithoutTrap). I think it would make sense to do this in the same place. How does gdb handle this? - Another interesting test would be to read back the memory after writing it and make sure it returns sensible data (interaction with server-side patching). - As for implementation, wouldn't it be simpler to just disable all breakpoint locations within the range, write the memory, and then re-enable them? I don't expect performance to matter much here. https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior
On 22 January 2018 at 22:06, Jim Ingham wrote: > > >> On Jan 22, 2018, at 3:10 AM, Pavel Labath via Phabricator >> wrote: >> >> labath added subscribers: krytarowski, jingham, davide. >> labath added a comment. >> >> In https://reviews.llvm.org/D42195#982035, @owenpshaw wrote: >> >>> - Added yaml2obj dependency. It it okay to be an unconditional dependency? >> >> >> Yeah, it probably needs to be conditional (if(TARGET yaml2obj)), because >> standalone builds will not have that target. OTOH, standalone builds will >> probably also have an issue with finding yaml2obj. However, I don't think it >> should be up to you to make standalone builds work. cc'ing Kamil and Michal, >> as they are the ones who use those builds. >> >> What I am not sure about is whether we need to make any changes to >> accomodate the XCode build. Jim, Davide: What would it take to make a test >> like this work from xcode? > > Sorry, I haven't been following. These seem like standard lldb-unittest2 > type tests. What is the difficultly of getting such a test to run from Xcode? The special thing about this test is that it uses the yaml2obj binary from llvm. I'm not sure if it's really an issue (I don't have an xcode build around), but I see a potential problem that this may fail because it can't find the binary, either because it's not built, or it's not in the right location. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior
labath added a comment. Herald added a subscriber: hintonda. In https://reviews.llvm.org/D42195#984003, @owenpshaw wrote: > It looks like the yaml2obj target hasn't been defined yet when the check-lldb > target is defined, so if(TARGET yaml2obj) always returns false. Is there > another way to do the conditional dependency? I'm assuming we don't want to > reorder the includes in llvm/tools/CMakeLists.txt. Ah, I see. We can then probably gate this on IF(NOT LLDB_BUILT_STANDALONE). https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42409: Fix memory leaks in GoParser
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. It looks like all of these parsing functions would benefit from returning unique_ptr, but that's probably not something we should bother doing now that we are contemplating removing this code. https://reviews.llvm.org/D42409 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior
krytarowski added a comment. In the standalone build, we put yaml2obj into `$PREFIX/bin/`, so into the default `PATH` of the toolchain. https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r323197 - Fix memory leaks in GoParser
Author: teemperor
Date: Tue Jan 23 05:50:46 2018
New Revision: 323197
URL: http://llvm.org/viewvc/llvm-project?rev=323197&view=rev
Log:
Fix memory leaks in GoParser
Summary: The GoParser is leaking memory in the tests due to not freeing
allocated nodes when encountering some parsing errors. With this patch all
GoParser tests are passing with enabled memory sanitizers/ubsan.
Reviewers: labath, davide
Reviewed By: labath
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D42409
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
Modified: lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp?rev=323197&r1=323196&r2=323197&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp Tue Jan 23
05:50:46 2018
@@ -439,8 +439,10 @@ GoASTExpr *GoParser::CompositeLit() {
if (!type)
return r.error();
GoASTCompositeLit *lit = LiteralValue();
- if (!lit)
+ if (!lit) {
+delete type;
return r.error();
+ }
lit->SetType(type);
return lit;
}
@@ -548,6 +550,7 @@ GoASTExpr *GoParser::Arguments(GoASTExpr
GoASTExpr *GoParser::Conversion() {
Rule r("Conversion", this);
if (GoASTExpr *t = Type2()) {
+std::unique_ptr owner(t);
if (match(GoLexer::OP_LPAREN)) {
GoASTExpr *v = Expression();
if (!v)
@@ -557,6 +560,7 @@ GoASTExpr *GoParser::Conversion() {
return r.error();
GoASTCallExpr *call = new GoASTCallExpr(false);
call->SetFun(t);
+ owner.release();
call->AddArgs(v);
return call;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42409: Fix memory leaks in GoParser
This revision was automatically updated to reflect the committed changes.
Closed by commit rL323197: Fix memory leaks in GoParser (authored by teemperor,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D42409?vs=131007&id=131052#toc
Repository:
rL LLVM
https://reviews.llvm.org/D42409
Files:
lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
Index: lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
@@ -439,8 +439,10 @@
if (!type)
return r.error();
GoASTCompositeLit *lit = LiteralValue();
- if (!lit)
+ if (!lit) {
+delete type;
return r.error();
+ }
lit->SetType(type);
return lit;
}
@@ -548,6 +550,7 @@
GoASTExpr *GoParser::Conversion() {
Rule r("Conversion", this);
if (GoASTExpr *t = Type2()) {
+std::unique_ptr owner(t);
if (match(GoLexer::OP_LPAREN)) {
GoASTExpr *v = Expression();
if (!v)
@@ -557,6 +560,7 @@
return r.error();
GoASTCallExpr *call = new GoASTCallExpr(false);
call->SetFun(t);
+ owner.release();
call->AddArgs(v);
return call;
}
Index: lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Go/GoParser.cpp
@@ -439,8 +439,10 @@
if (!type)
return r.error();
GoASTCompositeLit *lit = LiteralValue();
- if (!lit)
+ if (!lit) {
+delete type;
return r.error();
+ }
lit->SetType(type);
return lit;
}
@@ -548,6 +550,7 @@
GoASTExpr *GoParser::Conversion() {
Rule r("Conversion", this);
if (GoASTExpr *t = Type2()) {
+std::unique_ptr owner(t);
if (match(GoLexer::OP_LPAREN)) {
GoASTExpr *v = Expression();
if (!v)
@@ -557,6 +560,7 @@
return r.error();
GoASTCallExpr *call = new GoASTCallExpr(false);
call->SetFun(t);
+ owner.release();
call->AddArgs(v);
return call;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
tatyana-krasnukha added a comment. > Does this functionality really belong in the client? In case of memory reads, > it's the server who patches out the breakpoint opcodes > (NativeProcessProtocol::ReadMemoryWithoutTrap). I think it would make sense > to do this in the same place. Will not work for gdb-remote mode, other servers treat this just as a block of memory. I might be wrong, but gdb inserts a breakpoint right before execution of instruction range, containing this breakpoint, and removes right after stop. What is the need to save breakpoints in general? Because when memory is overwritten, breakpoints may have no sense anymore at their locations... https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
aprantl added a comment. > Skip tests which fail when -fmodules is passed > (https://bugs.llvm.org/show_bug.cgi?id=36048). Wait.. this patch is not supposed to change the set of tests that get -fmodules passed to them. It should only add -fmodules-cache-path to tests that do pass -fmodules. Why is this necessary? https://reviews.llvm.org/D42277 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
clayborg added a comment. In https://reviews.llvm.org/D39967#984989, @tatyana-krasnukha wrote: > > Does this functionality really belong in the client? In case of memory > > reads, it's the server who patches out the breakpoint opcodes > > (NativeProcessProtocol::ReadMemoryWithoutTrap). I think it would make sense > > to do this in the same place. > > Will not work for gdb-remote mode, other servers treat this just as a block > of memory. > I might be wrong, but gdb inserts a breakpoint right before execution of > instruction range, containing this breakpoint, and removes right after stop. That is why GDB doesn't do well if you set thousands of breakpoints. It makes debugging so slow and painful that it is useless. With LLDB we have the ability to set many breakpoints and still be able to debug quite well. LLDB is being used for program guided optimizations where we set a breakpoint on every function in a shared library and as each breakpoint location gets hit, we disable it. So while the GDB method makes things easier, it is quite inefficient when it is so easy to work around by knowing how to work around software breakpoints. So I like the way LLDB does it over GDB. Debugging on remote devices (like and Apple Watch) is quite slow to begin with and every packet counts. If we even set 20 breakpoints, that can easily add over 1 second per stop and 1 second per resume due to packets being limited to 20-40 packets per second. > What is the need to save breakpoints in general? Because when memory is > overwritten, breakpoints may have no sense anymore at their locations... https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
tatyana-krasnukha added a comment. I completely agree with your point, but why isn't enough just to return an error about breakpoints in the area user wants to write? Or to disable breakpoints forcibly? https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
clayborg added a comment. In https://reviews.llvm.org/D39967#985119, @tatyana-krasnukha wrote: > I completely agree with your point, but why isn't enough just to return an > error about breakpoints in the area user wants to write? The reason I don't like this is there is no way for a user to look at the error returned and do anything sensible. The only thing they can do is scrape the error string to see if it contains something about breakpoints? I am a firm believer that the debugger needs to do what is right for the API call to work no matter what it is doing for breakpoints since the breakpoints are a side effect of using the debugger. > Or to disable breakpoints forcibly? That would be an easy fix for the ObjectFile::Load(), we could see if there are any breakpoints in the range we are trying to write, get a list of them, disable them all, write memory and re-enable. But this doesn't stop a user from trying doing something like: (lldb) script lldb.process.WriteMemory(...) And they should expect this to just work no matter where they write memory. https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
labath added a comment. In https://reviews.llvm.org/D39967#985171, @clayborg wrote: > That would be an easy fix for the ObjectFile::Load(), we could see if there > are any breakpoints in the range we are trying to write, get a list of them, > disable them all, write memory and re-enable. But this doesn't stop a user > from trying doing something like: > > (lldb) script lldb.process.WriteMemory(...) > > > And they should expect this to just work no matter where they write memory. Couldn't WriteMemory do the same thing as well? I would expect that 99% of WriteMemory calls will not intersect any breakpoints so the overhead of disabling and re-enabling should be negligible. https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
clayborg added a comment. In https://reviews.llvm.org/D39967#985181, @labath wrote: > In https://reviews.llvm.org/D39967#985171, @clayborg wrote: > > > That would be an easy fix for the ObjectFile::Load(), we could see if there > > are any breakpoints in the range we are trying to write, get a list of > > them, disable them all, write memory and re-enable. But this doesn't stop a > > user from trying doing something like: > > > > (lldb) script lldb.process.WriteMemory(...) > > > > > > And they should expect this to just work no matter where they write memory. > > > Couldn't WriteMemory do the same thing as well? I would expect that 99% of > WriteMemory calls will not intersect any breakpoints so the overhead of > disabling and re-enabling should be negligible. Indeed the fix could do this to make this easier and I agree that would be a cleaner fix. We already have all the APIs we need for this. Tatyana, let us know what you think of changing this patch to get a list of overlapping breakpoints, disable, write memory, re-enable. All from within Process::WriteMemory https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
tatyana-krasnukha added a comment. Cannot promise that I'll do it (with all tests) quickly, but I'll do. One more question: what are the cases when intersection can happen, beside user forgot to disable it manually? (Will not it be annoying for user to get breakpoints in unpredictable locations after such situation?) https://reviews.llvm.org/D39967 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r323219 - Move getBuildArtifact() from TestBase to Base and derive MiTestCaseBase from it
Author: adrian
Date: Tue Jan 23 08:43:01 2018
New Revision: 323219
URL: http://llvm.org/viewvc/llvm-project?rev=323219&view=rev
Log:
Move getBuildArtifact() from TestBase to Base and derive MiTestCaseBase from it
Thanks to Pavel Labath for pointing this out!
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=323219&r1=323218&r2=323219&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Tue Jan 23 08:43:01
2018
@@ -717,6 +717,10 @@ class Base(unittest2.TestCase):
lldb.remote_platform.Run(shell_cmd)
self.addTearDownHook(clean_working_directory)
+def getBuildArtifact(self, name="a.out"):
+"""Return absolute path to an artifact in the test's build
directory."""
+return os.path.join(os.getcwd(), name)
+
def setUp(self):
"""Fixture for unittest test case setup.
@@ -2269,10 +2273,6 @@ class TestBase(Base):
else:
self.fail("Can't build for debug info: %s" % self.debug_info)
-def getBuildArtifact(self, name="a.out"):
-"""Return absolute path to an artifact in the test's build
directory."""
-return os.path.join(os.getcwd(), name)
-
def run_platform_command(self, cmd):
platform = self.dbg.GetSelectedPlatform()
shell_command = lldb.SBPlatformShellCommand(cmd)
Modified:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py?rev=323219&r1=323218&r2=323219&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
Tue Jan 23 08:43:01 2018
@@ -8,7 +8,7 @@ from __future__ import print_function
from lldbsuite.test.lldbtest import *
-class MiTestCaseBase(TestBase):
+class MiTestCaseBase(Base):
mydir = None
myexe = None
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior
In the xcode build, yaml2obj is built as part of the llvm build stage and so
gets put in the same bin directory clang builds into. It looks like this patch
expects to find yaml2obj next to the lldb executable:
def yaml2obj_executable():
"""
Get the path to the yaml2obj executable, which can be used to create test
object files from easy to write yaml instructions.
Throws an Exception if the executable cannot be found.
"""
# Tries to find yaml2obj at the same folder as the lldb
path = os.path.join(os.path.dirname(lldbtest_config.lldbExec), "yaml2obj")
if os.path.exists(path):
return path
raise Exception("yaml2obj executable not found")
For this to work in the Xcode build, you'll need to look next to the clang
executable as well. The one downside to this is that if you redirect CC to the
system clang, then this test will fail since Xcode doesn't include yaml2obj. I
can't see if the testsuite knows about the location of the clang BUILD
directory, but that would be the correct place to look for the Xcode build.
It also seems wrong to have the locater for yaml2obj be in a gdbremote specific
test file. This seems like a general-purpose utility not at all specific to
gdb-remote testing. That should go in lldbtest.py, shouldn't it?
Jim
> On Jan 23, 2018, at 1:59 AM, Pavel Labath wrote:
>
> On 22 January 2018 at 22:06, Jim Ingham wrote:
>>
>>
>>> On Jan 22, 2018, at 3:10 AM, Pavel Labath via Phabricator
>>> wrote:
>>>
>>> labath added subscribers: krytarowski, jingham, davide.
>>> labath added a comment.
>>>
>>> In https://reviews.llvm.org/D42195#982035, @owenpshaw wrote:
>>>
- Added yaml2obj dependency. It it okay to be an unconditional dependency?
>>>
>>>
>>> Yeah, it probably needs to be conditional (if(TARGET yaml2obj)), because
>>> standalone builds will not have that target. OTOH, standalone builds will
>>> probably also have an issue with finding yaml2obj. However, I don't think
>>> it should be up to you to make standalone builds work. cc'ing Kamil and
>>> Michal, as they are the ones who use those builds.
>>>
>>> What I am not sure about is whether we need to make any changes to
>>> accomodate the XCode build. Jim, Davide: What would it take to make a test
>>> like this work from xcode?
>>
>> Sorry, I haven't been following. These seem like standard lldb-unittest2
>> type tests. What is the difficultly of getting such a test to run from
>> Xcode?
>
> The special thing about this test is that it uses the yaml2obj binary
> from llvm. I'm not sure if it's really an issue (I don't have an xcode
> build around), but I see a potential problem that this may fail
> because it can't find the binary, either because it's not built, or
> it's not in the right location.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
It seems to me better to remove breakpoint sites under any memory that you are going to overwrite. The old breakpoints aren't guaranteed to make any sense and, for instance, on x86 could do harm (they could end up in the middle of an instruction in the new TEXT.) If you want to do this regularly, you should remove the breakpoints and then ask the breakpoint resolver to re-check the new code once it has been written. If for instance you are writing down memory that you have debug info for, then it will reset the breakpoints based on the new symbols for this location. Jim > On Jan 23, 2018, at 8:42 AM, Tatyana Krasnukha via Phabricator via > lldb-commits wrote: > > tatyana-krasnukha added a comment. > > Cannot promise that I'll do it (with all tests) quickly, but I'll do. > > One more question: what are the cases when intersection can happen, beside > user forgot to disable it manually? (Will not it be annoying for user to get > breakpoints in unpredictable locations after such situation?) > > > https://reviews.llvm.org/D39967 > > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42281: WIP: compile the LLDB tests out-of-tree
aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/dotest_args.py:166 +metavar='Test build directory', +help='The root build directory for the tests') clayborg wrote: > Maybe add a default right here?: > > ``` > default=os.getcwd() > ``` The default is in dotest.py:474. Let me know if this file is the more appropriate place for it. https://reviews.llvm.org/D42281 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function
Like this idea of using breakpoint resolver very much. It's exactly how I see debugger's proper work in such cases. > -Original Message- > From: jing...@apple.com [mailto:jing...@apple.com] > Sent: Tuesday, 23 January, 2018 8:43 PM > To: reviews+d39967+public+2d921c2f326fd...@reviews.llvm.org; Tatyana > Krasnukha via Phabricator > Cc: tatyana.krasnu...@synopsys.com; clayb...@gmail.com; lldb- > comm...@lists.llvm.org > Subject: Re: [Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite > function > > It seems to me better to remove breakpoint sites under any memory that > you are going to overwrite. The old breakpoints aren't guaranteed to make > any sense and, for instance, on x86 could do harm (they could end up in the > middle of an instruction in the new TEXT.) > > If you want to do this regularly, you should remove the breakpoints and then > ask the breakpoint resolver to re-check the new code once it has been > written. If for instance you are writing down memory that you have debug > info for, then it will reset the breakpoints based on the new symbols for this > location. > > Jim > > > > On Jan 23, 2018, at 8:42 AM, Tatyana Krasnukha via Phabricator via lldb- > commits wrote: > > > > tatyana-krasnukha added a comment. > > > > Cannot promise that I'll do it (with all tests) quickly, but I'll do. > > > > One more question: what are the cases when intersection can happen, > beside user forgot to disable it manually? (Will not it be annoying for user > to > get breakpoints in unpredictable locations after such situation?) > > > > > > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__reviews.llvm.org_D39967&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r > =8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw&m=p7fMc9pHWz1TqLP > GI2BcPpRTt7ir_dS_RN3DYanOVVw&s=ddJGG8U73bqnME4GDlY-N71n- > aBo1zmXPw8KElfI19A&e= > > > > > > > > ___ > > lldb-commits mailing list > > lldb-commits@lists.llvm.org > > https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi- > 2Dbin_mailman_listinfo_lldb- > 2Dcommits&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=8NZfjV_ZLY_S7gZy > QMq8mj7tiN4vlymPiSt0Wl0jegw&m=p7fMc9pHWz1TqLPGI2BcPpRTt7ir_dS_ > RN3DYanOVVw&s=nKin51JixOP9e1x0sEjcQyEKPKwP0j_iTt2LzdkMX-I&e= ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk added a comment. In https://reviews.llvm.org/D42277#985002, @aprantl wrote: > > Skip tests which fail when -fmodules is passed > > (https://bugs.llvm.org/show_bug.cgi?id=36048). > > Wait.. this patch is not supposed to change the set of tests that get > -fmodules passed to them. It should only add -fmodules-cache-path to tests > that do pass -fmodules. Why is this necessary? It's not necessary, but we should make this change. It falls out of removing this FIXME: # FIXME: C++ modules aren't supported on all platforms. CXXFLAGS += $(subst -fmodules,, $(CFLAGS)) If we keep it around, we'd need an extra hack to strip out "-fmodules-cache-path=module-cache" as well. Without that we'd pass "-cache-path=module-cache", which clang would reject. Another benefit of making this change is that it lets us know which tests are actually failing with modules enabled, instead of hiding them. https://reviews.llvm.org/D42277 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
aprantl added a comment. Okay got it. I'll investigate the PR separately. https://reviews.llvm.org/D42277 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42434: [SymbolFilePDB] Fix null array access when parsing the type of a function without any arguments, i.e. 'int main()' and add support to test it
asmith created this revision.
asmith added reviewers: zturner, lldb-commits.
Herald added a subscriber: llvm-commits.
- Fix a null array access bug. This happens when creating the lldb type for a
function that has no argument.
- Implement SymbolFilePDB::ParseTypes method. Using `lldb-test symbols` will
show all supported types in the target.
- Create lldb types for variadic function, PDBSymbolTypePointer,
PDBSymbolTypeBuiltin
- The underlying builtin type for PDBSymbolTypeEnum is always `Int`, correct it
with the very first enumerator's encoding if any. This is more accurate when
the underlying type is not signed or another integer type.
- Fix a bug when the compiler type is not created based on PDB_BuiltinType. For
example, basic type `long` is of same width as `int` in a 32-bit target, and
the compiler type of former one will be represented by the one generated for
latter if using the default method. Introduce a static function
GetBuiltinTypeForPDBEncodingAndBitSize to correct this issue.
- Basic type `long double` and `double` have the same bit size in MSVC and
there is no information in a PDB to distinguish them. The compiler type of the
former one is represented by the latter's.
- There is no line information about typedef, enum etc in a PDB and the source
and line information for them are not shown.
- There is no information about scoped enumeration. The compiler type is
represented as an unscoped one.
Repository:
rL LLVM
https://reviews.llvm.org/D42434
Files:
lit/SymbolFile/PDB/Inputs/SimpleTypesTest.cpp
lit/SymbolFile/PDB/enums-layout.test
lit/SymbolFile/PDB/typedefs.test
source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -20,6 +20,7 @@
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Symbol/SymbolVendor.h"
#include "lldb/Symbol/TypeMap.h"
+#include "lldb/Symbol/TypeList.h"
#include "lldb/Utility/RegularExpression.h"
#include "llvm/DebugInfo/PDB/GenericError.h"
@@ -318,8 +319,33 @@
}
size_t SymbolFilePDB::ParseTypes(const lldb_private::SymbolContext &sc) {
- // TODO: Implement this
- return size_t();
+ lldbassert(sc.module_sp.get());
+ size_t num_added = 0;
+ auto results_up = m_session_up->getGlobalScope()->findAllChildren();
+ if (!results_up)
+return 0;
+ while (auto symbol_up = results_up->getNext()) {
+switch (symbol_up->getSymTag()) {
+case PDB_SymType::Enum:
+case PDB_SymType::UDT:
+case PDB_SymType::Typedef:
+ break;
+default:
+ continue;
+}
+
+auto type_uid = symbol_up->getSymIndexId();
+if (m_types.find(type_uid) != m_types.end())
+ continue;
+
+// This should cause the type to get cached and stored in the `m_types`
+// lookup.
+if (!ResolveTypeUID(symbol_up->getSymIndexId()))
+ continue;
+
+++num_added;
+ }
+ return num_added;
}
size_t
@@ -349,8 +375,11 @@
return nullptr;
lldb::TypeSP result = pdb->CreateLLDBTypeFromPDBType(*pdb_type);
- if (result.get())
+ if (result.get()) {
m_types.insert(std::make_pair(type_uid, result));
+auto type_list = GetTypeList();
+type_list->Insert(result);
+ }
return result.get();
}
@@ -649,7 +678,9 @@
return 0;
}
-lldb_private::TypeList *SymbolFilePDB::GetTypeList() { return nullptr; }
+lldb_private::TypeList *SymbolFilePDB::GetTypeList() {
+ return m_obj_file->GetModule()->GetTypeList();
+}
size_t SymbolFilePDB::GetTypes(lldb_private::SymbolContextScope *sc_scope,
uint32_t type_mask,
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -26,12 +26,12 @@
#include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionArg.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
using namespace lldb;
using namespace lldb_private;
-using namespace llvm;
using namespace llvm::pdb;
namespace {
@@ -46,7 +46,7 @@
case PDB_UdtType::Interface:
return clang::TTK_Interface;
}
- return clang::TTK_Class;
+ return -1;
}
lldb::Encoding TranslateBuiltinEncoding(PDB_BuiltinType type) {
@@ -66,6 +66,106 @@
return lldb::eEncodingInvalid;
}
}
+
+lldb::Encoding TranslateEnumEncoding(PDB_VariantType type) {
+ switch (type) {
+ case PDB_VariantType::Int8:
+ case PDB_VariantType::Int16:
+ case PDB_VariantType::Int32:
+ case PDB_VariantType::Int64:
+return lldb::eEncodingSint;
+
+ case PDB_VariantType:
[Lldb-commits] [PATCH] D42434: [SymbolFilePDB] Fix null array access when parsing the type of a function without any arguments, i.e. 'int main()' and add support to test it
asmith added a comment. https://reviews.llvm.org/D41427 was reverted because the commit was missing some of the binary files for the tests. This is the same as https://reviews.llvm.org/D41427 without the binary files. I've dropped them since there are lit tests for the same functionality now. Repository: rL LLVM https://reviews.llvm.org/D42434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42434: [SymbolFilePDB] Fix null array access when parsing the type of a function without any arguments, i.e. 'int main()' and add support to test it
majnemer added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:162-163
+return ConstString("HRESULT");
+ case PDB_BuiltinType::BCD:
+return ConstString("HRESULT");
+ case PDB_BuiltinType::None:
Copy paste bug?
Repository:
rL LLVM
https://reviews.llvm.org/D42434
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42434: [SymbolFilePDB] Fix null array access when parsing the type of a function without any arguments, i.e. 'int main()' and add support to test it
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. looks good. May as well fix the BCD typo while you're here though (either here or in a followup patch) Repository: rL LLVM https://reviews.llvm.org/D42434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42434: [SymbolFilePDB] Fix null array access when parsing the type of a function without any arguments, i.e. 'int main()' and add support to test it
asmith updated this revision to Diff 131129.
asmith added a comment.
Fix BCD typo
https://reviews.llvm.org/D42434
Files:
lit/SymbolFile/PDB/Inputs/SimpleTypesTest.cpp
lit/SymbolFile/PDB/enums-layout.test
lit/SymbolFile/PDB/typedefs.test
source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -20,6 +20,7 @@
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Symbol/SymbolVendor.h"
#include "lldb/Symbol/TypeMap.h"
+#include "lldb/Symbol/TypeList.h"
#include "lldb/Utility/RegularExpression.h"
#include "llvm/DebugInfo/PDB/GenericError.h"
@@ -318,8 +319,33 @@
}
size_t SymbolFilePDB::ParseTypes(const lldb_private::SymbolContext &sc) {
- // TODO: Implement this
- return size_t();
+ lldbassert(sc.module_sp.get());
+ size_t num_added = 0;
+ auto results_up = m_session_up->getGlobalScope()->findAllChildren();
+ if (!results_up)
+return 0;
+ while (auto symbol_up = results_up->getNext()) {
+switch (symbol_up->getSymTag()) {
+case PDB_SymType::Enum:
+case PDB_SymType::UDT:
+case PDB_SymType::Typedef:
+ break;
+default:
+ continue;
+}
+
+auto type_uid = symbol_up->getSymIndexId();
+if (m_types.find(type_uid) != m_types.end())
+ continue;
+
+// This should cause the type to get cached and stored in the `m_types`
+// lookup.
+if (!ResolveTypeUID(symbol_up->getSymIndexId()))
+ continue;
+
+++num_added;
+ }
+ return num_added;
}
size_t
@@ -349,8 +375,11 @@
return nullptr;
lldb::TypeSP result = pdb->CreateLLDBTypeFromPDBType(*pdb_type);
- if (result.get())
+ if (result.get()) {
m_types.insert(std::make_pair(type_uid, result));
+auto type_list = GetTypeList();
+type_list->Insert(result);
+ }
return result.get();
}
@@ -649,7 +678,9 @@
return 0;
}
-lldb_private::TypeList *SymbolFilePDB::GetTypeList() { return nullptr; }
+lldb_private::TypeList *SymbolFilePDB::GetTypeList() {
+ return m_obj_file->GetModule()->GetTypeList();
+}
size_t SymbolFilePDB::GetTypes(lldb_private::SymbolContextScope *sc_scope,
uint32_t type_mask,
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -26,12 +26,12 @@
#include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionArg.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
using namespace lldb;
using namespace lldb_private;
-using namespace llvm;
using namespace llvm::pdb;
namespace {
@@ -46,7 +46,7 @@
case PDB_UdtType::Interface:
return clang::TTK_Interface;
}
- return clang::TTK_Class;
+ return -1;
}
lldb::Encoding TranslateBuiltinEncoding(PDB_BuiltinType type) {
@@ -66,6 +66,106 @@
return lldb::eEncodingInvalid;
}
}
+
+lldb::Encoding TranslateEnumEncoding(PDB_VariantType type) {
+ switch (type) {
+ case PDB_VariantType::Int8:
+ case PDB_VariantType::Int16:
+ case PDB_VariantType::Int32:
+ case PDB_VariantType::Int64:
+return lldb::eEncodingSint;
+
+ case PDB_VariantType::UInt8:
+ case PDB_VariantType::UInt16:
+ case PDB_VariantType::UInt32:
+ case PDB_VariantType::UInt64:
+return lldb::eEncodingUint;
+
+ default:
+break;
+ }
+
+ return lldb::eEncodingSint;
+}
+
+CompilerType GetBuiltinTypeForPDBEncodingAndBitSize(
+ClangASTContext *clang_ast, const PDBSymbolTypeBuiltin *pdb_type,
+Encoding encoding, uint32_t width) {
+ if (!pdb_type)
+return CompilerType();
+ if (!clang_ast)
+return CompilerType();
+ auto *ast = clang_ast->getASTContext();
+ if (!ast)
+return CompilerType();
+
+ switch (pdb_type->getBuiltinType()) {
+ default: break;
+ case PDB_BuiltinType::None:
+return CompilerType();
+ case PDB_BuiltinType::Void:
+// FIXME: where is non-zero size of `void` from?
+if (width == 0)
+ return clang_ast->GetBasicType(eBasicTypeVoid);
+ case PDB_BuiltinType::Bool:
+return clang_ast->GetBasicType(eBasicTypeBool);
+ case PDB_BuiltinType::Long:
+if (width == ast->getTypeSize(ast->LongTy))
+ return CompilerType(ast, ast->LongTy);
+if (width == ast->getTypeSize(ast->LongLongTy))
+ return CompilerType(ast, ast->LongLongTy);
+break;
+ case PDB_BuiltinType::ULong:
+if (width == ast->getTypeSize(ast->UnsignedLongTy))
+ return CompilerType(ast, ast->UnsignedLongTy);
+if (width == ast->getTypeSize(ast->UnsignedLongLongTy))
+ return Compiler
[Lldb-commits] [PATCH] D42443: [SymbolFilePDB] Add support for function symbols
asmith created this revision.
asmith added reviewers: zturner, lldb-commits.
Herald added a subscriber: llvm-commits.
This is combination of following changes,
- Resolve function symbols in PDB symbol file. `lldb-test symbols` will display
information about function symbols.
- Implement SymbolFilePDB::FindFunctions methods. On lldb console, searching
function symbol by name and by regular expression are both available.
- Create lldb type for PDBSymbolFunc.
- Add test case. In the test, multiple objects are linked. Tests are added to
check if functions with same name but from different sources can be resolved
correctly.
Repository:
rL LLVM
https://reviews.llvm.org/D42443
Files:
lit/SymbolFile/PDB/Inputs/FuncSymbols.cpp
lit/SymbolFile/PDB/Inputs/FuncSymbolsTestMain.cpp
lit/SymbolFile/PDB/Inputs/SimpleTypesTest.cpp
lit/SymbolFile/PDB/func-symbols.test
source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
source/Plugins/SymbolFile/PDB/PDBASTParser.h
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -10,6 +10,7 @@
#ifndef lldb_Plugins_SymbolFile_PDB_SymbolFilePDB_h_
#define lldb_Plugins_SymbolFile_PDB_SymbolFilePDB_h_
+#include "lldb/Core/UniqueCStringMap.h"
#include "lldb/Symbol/SymbolFile.h"
#include "lldb/Utility/UserID.h"
@@ -181,6 +182,19 @@
void FindTypesByName(const std::string &name, uint32_t max_matches,
lldb_private::TypeMap &types);
+ lldb::CompUnitSP
+ GetCompileUnitContainsAddress(const lldb_private::Address &so_addr);
+
+ typedef std::vector TypeCollection;
+
+ void
+ GetTypesForPDBSymbol(const llvm::pdb::PDBSymbol *pdb_symbol,
+ uint32_t type_mask, TypeCollection &type_collection);
+
+ lldb_private::Function* ParseCompileUnitFunctionForPDBFunc(
+ const llvm::pdb::PDBSymbolFunc *pdb_func,
+ const lldb_private::SymbolContext &sc);
+
void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland *pdb_compiland,
uint32_t &index);
@@ -190,6 +204,28 @@
std::unique_ptr
GetPDBCompilandByUID(uint32_t uid);
+ lldb_private::Mangled
+ GetMangledForPDBFunc(const llvm::pdb::PDBSymbolFunc *pdb_func);
+
+ bool ResolveFunction(llvm::pdb::PDBSymbolFunc *pdb_func,
+ bool include_inlines,
+ lldb_private::SymbolContextList &sc_list);
+
+ bool ResolveFunction(uint32_t uid, bool include_inlines,
+ lldb_private::SymbolContextList &sc_list);
+
+ void CacheFunctionNames();
+
+ size_t
+ ParseFunctionBlocks(const lldb_private::SymbolContext &sc,
+ uint64_t func_file_vm_addr,
+ const llvm::pdb::PDBSymbol *pdb_symbol,
+ lldb_private::Block *parent_block,
+ bool is_top_parent);
+
+ bool DeclContextMatchesThisSymbolFile(
+ const lldb_private::CompilerDeclContext *decl_ctx);
+
llvm::DenseMap m_comp_units;
llvm::DenseMap m_types;
@@ -198,6 +234,10 @@
std::unique_ptr m_global_scope_up;
uint32_t m_cached_compile_unit_count;
std::unique_ptr m_tu_decl_ctx_up;
+
+ lldb_private::UniqueCStringMap m_func_full_names;
+ lldb_private::UniqueCStringMap m_func_base_names;
+ lldb_private::UniqueCStringMap m_func_method_names;
};
#endif // lldb_Plugins_SymbolFile_PDB_SymbolFilePDB_h_
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -30,6 +30,7 @@
#include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
#include "llvm/DebugInfo/PDB/IPDBTable.h"
#include "llvm/DebugInfo/PDB/PDBSymbol.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolBlock.h"
#include "llvm/DebugInfo/PDB/PDBSymbolCompiland.h"
#include "llvm/DebugInfo/PDB/PDBSymbolCompilandDetails.h"
#include "llvm/DebugInfo/PDB/PDBSymbolData.h"
@@ -37,6 +38,7 @@
#include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
#include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugEnd.h"
#include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugStart.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolPublicSymbol.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
#include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
@@ -261,10 +263,68 @@
return TranslateLanguage(details->getLanguage());
}
+lldb_private::Function *
+SymbolFilePDB::ParseCompileUnitFunctionForPDBFunc(
+const PDBSymbolFunc *pdb_func,
+const lldb_private::SymbolContext &sc) {
+ if (!pdb_func)
+return nullptr;
+ lldbassert(sc.comp_unit && sc.module_sp.get());
+
+ auto file_vm_addr = pdb_func->getVirtualAddress();
+ if (file_vm_addr == LLDB_INVALID_ADDRESS)
+
