[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
labath added a comment. In https://reviews.llvm.org/D41352#959051, @jasonmolenda wrote: > I guess I don't have an opinion on this one. The correct way to pass > environment variables to the inferior is through > SBLaunchInfo::SetEnvironmentEntries or in cmd line lldb, process launch -v > ENV=val. A test that assumes an environment variable set in lldb will end up > in the inferior process isn't going to work when the process is launched on a > remote target, is it? First a bit of background: We have an internal customer who uses `lldb-server ... -- ./my_app --my_app_arguments` + `process connect ...` combo for their debugging setup (mainly because this is compatible with their existing gdb setup). However, their app was failing to run because it was expecting some environment variables to be set, and lldb-server was launching it with a clean environment. In this setup, it is not possible to set the environment variables via SBLaunchInfo, as lldb (client) is not responsible for launching the inferior -- as far as the client goes this situation is most similar to an "attach". > Whether llgs or debugserver should be forwarding their environment variables > on by default - it seems fragile to me. But maybe there's a use case that > needs this behavior to be supported? I guess it's valid to test that it > actually behaves that way, at least for processes launched on the local > system. Note that I am only suggesting to pass the environment in the "pre-loaded inferior" mode (I'm not sure if it has an official name). I agree that the (regular) case where the inferior is launched under the control of the client via $A packets should remain the same, as in this case the client is able to setup the environment exactly as it wants to. However, in the pre-loaded mode we have just two options: launch with the environment inherited from the lldb-server and launch with an empty environment. Of these, I think the first one is much more useful. The test we are speaking about is also only testing that the environment passing works in the pre-loaded mode. (Right now this test cannot be run remotely, but that's a separate issue which I plan to address in the future.) > (I apologize for not keeping up with lldb-commits the past week while this > was work was being done -- I'm in the middle of a task where I've had to > concentrate on that & pause reading emails etc for a bit. ;) No worries, I am not in a rush with this. https://reviews.llvm.org/D41352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable
labath updated this revision to Diff 127500.
labath added a comment.
It took a while, but we've finally landed an llvm::WritableMemoryBuffer class.
This patch now becomes simple, as all I need to do is use that instead of the
MemoryBuffer class.
Well.. almost. The new class does not have the null-termination capability, so I
also update our buffer uses to not require null termination.
https://reviews.llvm.org/D40079
Files:
include/lldb/Interpreter/OptionValueFileSpec.h
include/lldb/Target/Target.h
include/lldb/Utility/DataBufferLLVM.h
source/Interpreter/OptionValueFileSpec.cpp
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
source/Target/Target.cpp
source/Utility/DataBufferLLVM.cpp
Index: source/Utility/DataBufferLLVM.cpp
===
--- source/Utility/DataBufferLLVM.cpp
+++ source/Utility/DataBufferLLVM.cpp
@@ -18,7 +18,8 @@
using namespace lldb_private;
-DataBufferLLVM::DataBufferLLVM(std::unique_ptr MemBuffer)
+DataBufferLLVM::DataBufferLLVM(
+std::unique_ptr MemBuffer)
: Buffer(std::move(MemBuffer)) {
assert(Buffer != nullptr &&
"Cannot construct a DataBufferLLVM with a null buffer");
@@ -28,43 +29,40 @@
std::shared_ptr
DataBufferLLVM::CreateSliceFromPath(const llvm::Twine &Path, uint64_t Size,
- uint64_t Offset, bool Private) {
+uint64_t Offset) {
// If the file resides non-locally, pass the volatile flag so that we don't
// mmap it.
- if (!Private)
-Private = !llvm::sys::fs::is_local(Path);
+ bool IsVolatile = !llvm::sys::fs::is_local(Path);
- auto Buffer = llvm::MemoryBuffer::getFileSlice(Path, Size, Offset, Private);
+ auto Buffer =
+ llvm::WritableMemoryBuffer::getFileSlice(Path, Size, Offset, IsVolatile);
if (!Buffer)
return nullptr;
return std::shared_ptr(
new DataBufferLLVM(std::move(*Buffer)));
}
std::shared_ptr
-DataBufferLLVM::CreateFromPath(const llvm::Twine &Path, bool NullTerminate, bool Private) {
+DataBufferLLVM::CreateFromPath(const llvm::Twine &Path) {
// If the file resides non-locally, pass the volatile flag so that we don't
// mmap it.
- if (!Private)
-Private = !llvm::sys::fs::is_local(Path);
+ bool IsVolatile = !llvm::sys::fs::is_local(Path);
- auto Buffer = llvm::MemoryBuffer::getFile(Path, -1, NullTerminate, Private);
+ auto Buffer = llvm::WritableMemoryBuffer::getFile(Path, -1, IsVolatile);
if (!Buffer)
return nullptr;
return std::shared_ptr(
new DataBufferLLVM(std::move(*Buffer)));
}
uint8_t *DataBufferLLVM::GetBytes() {
- return const_cast(GetBuffer());
+ return reinterpret_cast(Buffer->getBufferStart());
}
-const uint8_t *DataBufferLLVM::GetBytes() const { return GetBuffer(); }
+const uint8_t *DataBufferLLVM::GetBytes() const {
+ return reinterpret_cast(Buffer->getBufferStart());
+}
lldb::offset_t DataBufferLLVM::GetByteSize() const {
return Buffer->getBufferSize();
}
-
-const uint8_t *DataBufferLLVM::GetBuffer() const {
- return reinterpret_cast(Buffer->getBufferStart());
-}
Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2313,7 +2313,7 @@
result_valobj_sp = persistent_var_sp->GetValueObject();
execution_results = eExpressionCompleted;
} else {
-const char *prefix = GetExpressionPrefixContentsAsCString();
+llvm::StringRef prefix = GetExpressionPrefixContents();
Status error;
execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix,
result_valobj_sp, error,
@@ -4033,18 +4033,19 @@
return LanguageType();
}
-const char *TargetProperties::GetExpressionPrefixContentsAsCString() {
+llvm::StringRef TargetProperties::GetExpressionPrefixContents() {
const uint32_t idx = ePropertyExprPrefix;
OptionValueFileSpec *file =
m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
idx);
if (file) {
-const bool null_terminate = true;
-DataBufferSP data_sp(file->GetFileContents(null_terminate));
+DataBufferSP data_sp(file->GetFileContents());
if (data_sp)
- return (const char *)data_sp->GetBytes();
+ return llvm::StringRef(
+ reinterpret_cast(data_sp->GetBytes()),
+ data_sp->GetByteSize());
}
- return nullptr;
+ return "";
}
bool TargetProperties::GetBreakpointsConsultPlatformAvoidList() {
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1173,7 +1173,7 @@
xcode_dir_pa
[Lldb-commits] [PATCH] D41359: Add Utility/Environment class for handling... environments
clayborg added a comment. Thanks for the explanation. Good to go. https://reviews.llvm.org/D41359 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
clayborg added a comment. So the main question is what do we expect to happen by default. I kind of agree that if we launch the inferior directly when launching I would expect the environment to be passed along. Jason: since we always just launch debugserver for Xcode in the mode that doesn't expect environment variables to be passed along, I think changing the default behavior would be a good idea and it would only affect internal Apple customers. What do you think? We might need to add a "--no-forward-env" option in case anyone doesn't want this behavior just in case if we do switch the default. https://reviews.llvm.org/D41352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.
clayborg added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:394
+ if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
+FindTypesByRegex(RegularExpression(name_str), max_matches, types);
else
You should make the RegularExpression object first and then check for errors.
If there are errors, the type lookup should happen by normal name. This is
again why I don't like us sniffing for a regular expression. If there is an
error, like when trying to lookup "char*" as mentioned by Aaron, would you
expect to see an error message saying "char*" isn't a valid regex? Then the
user thinks that the type lookup always takes a regular expression which is not
the case, just something the PDB plugin added for some reason. I do believe
part of this patch should be adding the
lldb_private::SymbolFile::FindTypesByRegex(...) to the base class and fixing
this issue by removing the regex sniffing code since it is fragile at best (how
are we doing to lookup a template type without triggering the regex issues?).
Repository:
rL LLVM
https://reviews.llvm.org/D41086
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40079: Make sure DataBufferLLVM contents are writable
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
See inlined comment. Quick fix and this will be good to go.
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:409-410
if (!data_sp) {
-data_sp =
-DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length,
file_offset, true);
+data_sp = DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length,
+ file_offset);
if (!data_sp)
We should put the DataBufferLLVM::CreateSliceFromPath() call into a new static
function in ObjectFile:
```
static DataBufferSP ObjectFile::MapFileData(FileSpec file, uint64_t Size,
uint64_t Offset);
```
Then when we need to make fixes, we change just one place instead of all of
them. We should add this new function and switch all ObjectFile subclasses over
to using it.
https://reviews.llvm.org/D40079
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Thanks for the background Pavel. I'm fine with changing the default behavior. I don't see this having any impact on users of debugserver, and if it does, we can add a flag like Greg suggests here. https://reviews.llvm.org/D41352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r321095 - Temporarily XFAIL test/functionalities/exec while investiagting bot breakage.
Author: adrian Date: Tue Dec 19 10:21:28 2017 New Revision: 321095 URL: http://llvm.org/viewvc/llvm-project?rev=321095&view=rev Log: Temporarily XFAIL test/functionalities/exec while investiagting bot breakage. When building with cmake on green gragon or on ci.swift.org, this test fails. rdar://problem/36134350 Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py?rev=321095&r1=321094&r2=321095&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py Tue Dec 19 10:21:28 2017 @@ -29,12 +29,14 @@ class ExecTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) @skipUnlessDarwin +@expectedFailureAll(oslist=['macosx'], bugnumber="rdar://36134350") # when building with cmake on green gragon or on ci.swift.org, this test fails. @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532") @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], bugnumber="rdar://problem/34559552") # this exec test has problems on ios systems def test_hitting_exec (self): self.do_test(False) @skipUnlessDarwin +@expectedFailureAll(oslist=['macosx'], bugnumber="rdar://36134350") # when building with cmake on green gragon or on ci.swift.org, this test fails. @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532") @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], bugnumber="rdar://problem/34559552") # this exec test has problems on ios systems def test_skipping_exec (self): ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
clayborg added a comment.
The existing code for the "--forward-env" does this:
case 'F':
// Pass the current environment down to the process that gets launched
{
char **host_env = *_NSGetEnviron();
char *env_entry;
size_t i;
for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
remote->Context().PushEnvironment(env_entry);
}
break;
Seems like your fix doesn't do the remote->Context().PushEnvironment(...). Not
sure if this is needed or not.
https://reviews.llvm.org/D41352
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So we can also specify extra environment variable using the "--env" option with debugserver and your current fix will break that. https://reviews.llvm.org/D41352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
clayborg added a comment. Environment vars might be set by using --env, so those need to go into "inferior_envp" first. If we are launching, we will copy only the host environment vars that haven't been already set manually (they don't already exist in the inferior_envp). https://reviews.llvm.org/D41352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)
clayborg added a comment. This is exposing a bug where if we have options like: % debugserver --env USER=hello --forward-env -- /bin/ls -lAF We would set USER to hello, then "--forward-env" would clobber the setting... https://reviews.llvm.org/D41352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r321120 - Fix a couple of warnings (NFC)
Author: adrian
Date: Tue Dec 19 14:54:37 2017
New Revision: 321120
URL: http://llvm.org/viewvc/llvm-project?rev=321120&view=rev
Log:
Fix a couple of warnings (NFC)
Modified:
lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp
lldb/trunk/tools/lldb-mi/MIUtilString.cpp
Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp?rev=321120&r1=321119&r2=321120&view=diff
==
--- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp Tue
Dec 19 14:54:37 2017
@@ -98,7 +98,7 @@ bool CommunicationKDP::SendRequestAndGet
#ifdef LLDB_CONFIGURATION_DEBUG
// NOTE: this only works for packets that are in native endian byte order
assert(request_packet.GetSize() ==
- *((uint16_t *)(request_packet.GetData() + 2)));
+ *((const uint16_t *)(request_packet.GetData() + 2)));
#endif
lldb::offset_t offset = 1;
const uint32_t num_retries = 3;
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp?rev=321120&r1=321119&r2=321120&view=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp Tue
Dec 19 14:54:37 2017
@@ -60,6 +60,7 @@ StateType GDBRemoteClientBase::SendConti
continue;
if (steady_clock::now() >= m_interrupt_time + kInterruptTimeout)
return eStateInvalid;
+ break;
}
case PacketResult::Success:
break;
Modified: lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp?rev=321120&r1=321119&r2=321120&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp Tue Dec 19
14:54:37 2017
@@ -18,6 +18,7 @@
#include "lldb/API/SBTarget.h"
#include "lldb/API/SBThread.h"
#include "lldb/API/SBUnixSignals.h"
+#include "llvm/Support/Compiler.h"
#ifdef _WIN32
#include // For the ::_access()
#else
@@ -899,6 +900,7 @@ bool CMICmnLLDBDebuggerHandleEvents::Han
bOk = HandleProcessEventStateStopped(vEvent, bShouldBrk);
if (bShouldBrk)
break;
+break;
case lldb::eStateCrashed:
case lldb::eStateSuspended:
pEventType = "eStateSuspended";
Modified: lldb/trunk/tools/lldb-mi/MIUtilString.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilString.cpp?rev=321120&r1=321119&r2=321120&view=diff
==
--- lldb/trunk/tools/lldb-mi/MIUtilString.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MIUtilString.cpp Tue Dec 19 14:54:37 2017
@@ -8,6 +8,7 @@
//===--===//
// Third party headers
+#include "llvm/Support/Compiler.h"
#include
#include // for PRIx8
#include// for ULONG_MAX
@@ -890,7 +891,7 @@ CMIUtilString CMIUtilString::ConvertToPr
case '"':
if (bEscapeQuotes)
return "\\\"";
- // fall thru
+LLVM_FALLTHROUGH;
default:
if (::isprint(vChar))
return Format("%c", vChar);
@@ -924,7 +925,7 @@ CMIUtilString::ConvertCharValueToPrintab
case '"':
if (bEscapeQuotes)
return "\\\"";
- // fall thru
+LLVM_FALLTHROUGH;
default:
if (::isprint(vChar))
return Format("%c", vChar);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r321123 - Replace an accidentally added "break" with an LLVM_FALLTHROUGH.
Author: adrian Date: Tue Dec 19 15:16:38 2017 New Revision: 321123 URL: http://llvm.org/viewvc/llvm-project?rev=321123&view=rev Log: Replace an accidentally added "break" with an LLVM_FALLTHROUGH. Thanks to Greg Clayton for catchting this! Modified: lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp Modified: lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp?rev=321123&r1=321122&r2=321123&view=diff == --- lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp (original) +++ lldb/trunk/tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp Tue Dec 19 15:16:38 2017 @@ -900,7 +900,7 @@ bool CMICmnLLDBDebuggerHandleEvents::Han bOk = HandleProcessEventStateStopped(vEvent, bShouldBrk); if (bShouldBrk) break; -break; +LLVM_FALLTHROUGH; case lldb::eStateCrashed: case lldb::eStateSuspended: pEventType = "eStateSuspended"; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41427: [lldb] Fix crash when parsing the type of a function without any arguments
asmith created this revision.
asmith added reviewers: zturner, lldb-commits.
For `int main()`, the number of arguments is zero. Trying to access the element
of a null array causes trouble.
Repository:
rL LLVM
https://reviews.llvm.org/D41427
Files:
source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -141,7 +141,7 @@
} else if (auto func_sig = llvm::dyn_cast(&type)) {
auto arg_enum = func_sig->getArguments();
uint32_t num_args = arg_enum->getChildCount();
-std::vector arg_list(num_args);
+std::vector arg_list;
while (auto arg = arg_enum->getNext()) {
lldb_private::Type *arg_type =
m_ast.GetSymbolFile()->ResolveTypeUID(arg->getSymIndexId());
@@ -152,6 +152,8 @@
CompilerType arg_ast_type = arg_type->GetFullCompilerType();
arg_list.push_back(arg_ast_type);
}
+lldbassert(arg_list.size() <= num_args);
+
auto pdb_return_type = func_sig->getReturnType();
lldb_private::Type *return_type =
m_ast.GetSymbolFile()->ResolveTypeUID(pdb_return_type->getSymIndexId());
@@ -166,7 +168,7 @@
if (func_sig->isVolatileType())
type_quals |= clang::Qualifiers::Volatile;
CompilerType func_sig_ast_type = m_ast.CreateFunctionType(
-return_ast_type, &arg_list[0], num_args, false, type_quals);
+return_ast_type, arg_list.data(), arg_list.size(), false, type_quals);
return std::make_shared(
func_sig->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(), 0,
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -141,7 +141,7 @@
} else if (auto func_sig = llvm::dyn_cast(&type)) {
auto arg_enum = func_sig->getArguments();
uint32_t num_args = arg_enum->getChildCount();
-std::vector arg_list(num_args);
+std::vector arg_list;
while (auto arg = arg_enum->getNext()) {
lldb_private::Type *arg_type =
m_ast.GetSymbolFile()->ResolveTypeUID(arg->getSymIndexId());
@@ -152,6 +152,8 @@
CompilerType arg_ast_type = arg_type->GetFullCompilerType();
arg_list.push_back(arg_ast_type);
}
+lldbassert(arg_list.size() <= num_args);
+
auto pdb_return_type = func_sig->getReturnType();
lldb_private::Type *return_type =
m_ast.GetSymbolFile()->ResolveTypeUID(pdb_return_type->getSymIndexId());
@@ -166,7 +168,7 @@
if (func_sig->isVolatileType())
type_quals |= clang::Qualifiers::Volatile;
CompilerType func_sig_ast_type = m_ast.CreateFunctionType(
-return_ast_type, &arg_list[0], num_args, false, type_quals);
+return_ast_type, arg_list.data(), arg_list.size(), false, type_quals);
return std::make_shared(
func_sig->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(), 0,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41428: [lldb] This commit adds support to cache a PDB's global scope and fixes a bug in getting the source file name for a compiland
asmith created this revision.
asmith added reviewers: zturner, lldb-commits.
This commit is a combination of following changes:
(1) Cache PDB's global scope(executable) in SymbolFilePDB
(2) Change naming of `cu` to `compiland` which is PDB specific
(3) Change ParseCompileUnitForSymIndex to ParseCompileUnitForUID.
Prefer using a common name `UID` instead of PDB's `System Index`
Adding one more argument `index` to this method, which is used to
specify the index of the compile unit in a cached compile unit array
(4) Add GetPDBCompilandByUID method to simply code
(5) Fix a bug in getting the source file name for a PDB compiland.
For some reason, PDBSymbolCompiland::getSourceFileName() could
return an empty name, so if that is true, we have to walk through all
source files of this compiland and determine the right source file
used to generate this compiland based on language indicated.
Also the previous implementation intended to call
PDBSession::findOneSourceFile method to get its name for the
compiland. This is not accurate since the `one source file` found
could be a header other than source file.
Repository:
rL LLVM
https://reviews.llvm.org/D41428
Files:
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
@@ -16,6 +16,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/DebugInfo/PDB/IPDBSession.h"
#include "llvm/DebugInfo/PDB/PDB.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
class SymbolFilePDB : public lldb_private::SymbolFile {
public:
@@ -163,13 +164,14 @@
const llvm::pdb::IPDBSession &GetPDBSession() const;
private:
- lldb::CompUnitSP ParseCompileUnitForSymIndex(uint32_t id);
+ lldb::CompUnitSP
+ ParseCompileUnitForUID(uint32_t id, uint32_t index = UINT32_MAX);
bool ParseCompileUnitLineTable(const lldb_private::SymbolContext &sc,
uint32_t match_line);
void BuildSupportFileIdToSupportFileIndexMap(
- const llvm::pdb::PDBSymbolCompiland &cu,
+ const llvm::pdb::PDBSymbolCompiland &pdb_compiland,
llvm::DenseMap &index_map) const;
void FindTypesByRegex(const std::string ®ex, uint32_t max_matches,
@@ -178,11 +180,21 @@
void FindTypesByName(const std::string &name, uint32_t max_matches,
lldb_private::TypeMap &types);
+ void GetCompileUnitIndex(const llvm::pdb::PDBSymbolCompiland *pdb_compiland,
+ uint32_t &index);
+
+ std::string GetSourceFileNameForPDBCompiland(
+ const llvm::pdb::PDBSymbolCompiland *pdb_compiland);
+
+ std::unique_ptr
+ GetPDBCompilandByUID(uint32_t uid);
+
llvm::DenseMap m_comp_units;
llvm::DenseMap m_types;
std::vector m_builtin_types;
std::unique_ptr m_session_up;
+ std::unique_ptr m_global_scope_up;
uint32_t m_cached_compile_unit_count;
std::unique_ptr m_tu_decl_ctx_up;
};
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -18,6 +18,7 @@
#include "lldb/Symbol/LineTable.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/SymbolVendor.h"
#include "lldb/Symbol/TypeMap.h"
#include "llvm/DebugInfo/PDB/GenericError.h"
@@ -39,6 +40,7 @@
#include
+using namespace lldb;
using namespace lldb_private;
using namespace llvm::pdb;
@@ -88,7 +90,8 @@
}
SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file)
-: SymbolFile(object_file), m_cached_compile_unit_count(0) {}
+: SymbolFile(object_file), m_session_up(), m_global_scope_up(),
+ m_cached_compile_unit_count(0), m_tu_decl_ctx_up() {}
SymbolFilePDB::~SymbolFilePDB() {}
@@ -108,41 +111,95 @@
void SymbolFilePDB::InitializeObject() {
lldb::addr_t obj_load_address = m_obj_file->GetFileOffset();
+ lldbassert(obj_load_address &&
+ obj_load_address != LLDB_INVALID_ADDRESS);
m_session_up->setLoadAddress(obj_load_address);
+ if (!m_global_scope_up)
+m_global_scope_up = m_session_up->getGlobalScope();
+ lldbassert(m_global_scope_up.get());
TypeSystem *type_system =
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
ClangASTContext *clang_type_system =
llvm::dyn_cast_or_null(type_system);
+ lldbassert(clang_type_system);
m_tu_decl_ctx_up = llvm::make_unique(
type_system, clang_type_system->GetTranslationUnitDecl());
}
uint32_t SymbolFilePDB::GetNumCompileUnits() {
if (m_cached_compile_unit_count == 0) {
-auto global = m_session_up->getGlobalScope();
-auto compilands = global->findAllChildren();
+
[Lldb-commits] [PATCH] D41092: Enable more abilities in SymbolFilePDB
asmith updated this revision to Diff 127658.
asmith added a comment.
If a `Symbols` table is present then lldb can retrieve symbols for the types
listed in PDBSym_Type. In other words, if we want symbolic information for a
function then checking for the `Symbols` table is sufficient.
Repository:
rL LLVM
https://reviews.llvm.org/D41092
Files:
source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -154,8 +154,7 @@
EXPECT_NE(nullptr, symfile);
EXPECT_EQ(symfile->GetPluginName(), SymbolFilePDB::GetPluginNameStatic());
- uint32_t expected_abilities =
- SymbolFile::CompileUnits | SymbolFile::LineTables;
+ uint32_t expected_abilities = SymbolFile::kAllAbilities;
EXPECT_EQ(expected_abilities, symfile->CalculateAbilities());
}
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -21,12 +21,15 @@
#include "lldb/Symbol/TypeMap.h"
#include "llvm/DebugInfo/PDB/GenericError.h"
+#include "llvm/DebugInfo/PDB/IPDBDataStream.h"
#include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
#include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
#include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
+#include "llvm/DebugInfo/PDB/IPDBTable.h"
#include "llvm/DebugInfo/PDB/PDBSymbol.h"
#include "llvm/DebugInfo/PDB/PDBSymbolCompiland.h"
#include "llvm/DebugInfo/PDB/PDBSymbolCompilandDetails.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolData.h"
#include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
#include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
#include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugEnd.h"
@@ -93,6 +96,10 @@
SymbolFilePDB::~SymbolFilePDB() {}
uint32_t SymbolFilePDB::CalculateAbilities() {
+ uint32_t abilities = 0;
+ if (!m_obj_file)
+return 0;
+
if (!m_session_up) {
// Lazily load and match the PDB file, but only do this once.
std::string exePath = m_obj_file->GetFileSpec().GetPath();
@@ -100,10 +107,46 @@
m_session_up);
if (error) {
llvm::consumeError(std::move(error));
- return 0;
+ auto module_sp = m_obj_file->GetModule();
+ if (!module_sp)
+return 0;
+ // See if any symbol file is specified through `--symfile` option.
+ FileSpec symfile = module_sp->GetSymbolFileFileSpec();
+ if (!symfile)
+return 0;
+ error = loadDataForPDB(PDB_ReaderType::DIA,
+ llvm::StringRef(symfile.GetPath()),
+ m_session_up);
+ if (error) {
+llvm::consumeError(std::move(error));
+return 0;
+ }
+}
+ }
+ if (!m_session_up.get())
+return 0;
+
+ auto enum_tables_up = m_session_up->getEnumTables();
+ if (!enum_tables_up)
+return 0;
+ while (auto table_up = enum_tables_up->getNext()) {
+if (table_up->getItemCount() == 0)
+ continue;
+auto type = table_up->getTableType();
+switch (type) {
+case PDB_TableType::Symbols:
+ // This table represents a store of symbols with types listed in
+ // PDBSym_Type
+ abilities |= (CompileUnits | Functions | Blocks |
+GlobalVariables | LocalVariables | VariableTypes);
+ break;
+case PDB_TableType::LineNumbers:
+ abilities |= LineTables;
+ break;
+default: break;
}
}
- return CompileUnits | LineTables;
+ return abilities;
}
void SymbolFilePDB::InitializeObject() {
Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -154,8 +154,7 @@
EXPECT_NE(nullptr, symfile);
EXPECT_EQ(symfile->GetPluginName(), SymbolFilePDB::GetPluginNameStatic());
- uint32_t expected_abilities =
- SymbolFile::CompileUnits | SymbolFile::LineTables;
+ uint32_t expected_abilities = SymbolFile::kAllAbilities;
EXPECT_EQ(expected_abilities, symfile->CalculateAbilities());
}
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -21,12 +21,15 @@
#include "lldb/Symbol/TypeMap.h"
#include "llvm/DebugInfo/PDB/GenericError.h"
+#include "llvm/DebugInfo/PDB/IPDBDataStream.h"
#include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
#include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
#include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
+#include "llvm/DebugInfo/PDB/IPDBTable.h"
#include "llvm/DebugInfo/PDB/PDBSymbol.h"
#include "llvm/DebugInfo/PDB/PDBSymbol
