[Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex
labath added inline comments.
Comment at: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:317
void DWARFUnit::ClearDIEs(bool keep_compile_unit_die) {
if (m_die_array.size() > 1) {
+llvm::sys::ScopedWriter lock(m_extractdies_mutex);
You're accessing m_die_array without holding a lock.
Repository:
rL LLVM
https://reviews.llvm.org/D40470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules
labath added inline comments.
Comment at: include/lldb/Core/Module.h:779
+ //--
+ void SetUUID(const lldb_private::UUID &uuid);
+
amccarth wrote:
> Was there no SetUUID before because the UUID was considered immutable? I
> wonder if that's worth keeping. Is the UUID always known at construction
> time?
The UUID is not always know at construction time -- sometimes it can be, if you
set it on the FileSpec you use to create the module, but you don't have to do
that.
However, it is true that the UUID was constant during the lifetime of the
Module, and this changes that, which is not ideal IMO.
It seems that in the only place you call this function, you already know the
UUID. Would it be possible to do this work in the PlaceholderModule constructor
(thereby avoiding any changes to the Module API) ?
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:118-128
+ } else if (cv_signature == CvSignature::ElfBuildId) {
+// ELF BuildID (found in Breakpad/Crashpad generated minidumps)
+//
+// This is variable-length, but usually 20 bytes
+// as the binutils ld default is a SHA-1 hash.
+// (We'll handle only 16 and 20 bytes signatures,
+// matching LLDB support for UUIDs)
Looks like the elf part of this is not tested. Do any of our linux minidumps
have a build-id we could use?
Since the parser->SB path is already tested by the windows test, this doesn't
have to be a full-blown test and a minidump parser unittest
(unittests/Process/minidump) should be sufficient.
https://reviews.llvm.org/D46292
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r331250 - Split TestGlobalVariables into two and xfail one of them for arm64 linux
Author: labath
Date: Tue May 1 03:09:53 2018
New Revision: 331250
URL: http://llvm.org/viewvc/llvm-project?rev=331250&view=rev
Log:
Split TestGlobalVariables into two and xfail one of them for arm64 linux
Displaying of global pointer variables is not working on arm64 linux
(pr37301).
I've moved this part into a separate test, so it can be xfailed
separately. I then move the "show-variables-with-process-available"
check before the "show-all-variables" command to presrve the intent of
checking that global variable caching works correctly. (I've verified
that the new arrangement still fails when I revert the fix from
r331230.)
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py?rev=331250&r1=331249&r2=331250&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
Tue May 1 03:09:53 2018
@@ -22,6 +22,22 @@ class GlobalVariablesTestCase(TestBase):
self.shlib_names = ["a"]
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
+@expectedFailureAll(oslist=["linux"], archs=["aarch64"],
bugnumber="llvm.org/pr37301")
+def test_without_process(self):
+"""Test that static initialized variables can be inspected without
+process."""
+self.build()
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+self.assertTrue(target, VALID_TARGET)
+self.expect("target variable g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['(int *)'])
+self.expect("target variable *g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['42'])
+
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
def test_c_global_variables(self):
"""Test 'frame variable --scope --no-args' which omits args and shows
scopes."""
self.build()
@@ -39,12 +55,6 @@ class GlobalVariablesTestCase(TestBase):
environment = self.registerSharedLibrariesWithTarget(
target, self.shlib_names)
-# Test that static initialized variables can be inspected without
process.
-self.expect("target variable g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=['(int *)'])
-self.expect("target variable *g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=['42'])
-
# Now launch the process, and do not stop at entry point.
process = target.LaunchSimple(
None, environment, self.get_process_working_directory())
@@ -59,6 +69,13 @@ class GlobalVariablesTestCase(TestBase):
self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
substrs=[' resolved, hit count = 1'])
+# Test that the statically initialized variable can also be
+# inspected *with* a process.
+self.expect("target variable g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['(int *)'])
+self.expect("target variable *g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['42'])
+
# Check that GLOBAL scopes are indicated for the variables.
self.runCmd("frame variable --show-types --scope --show-globals
--no-args")
self.expect(
@@ -106,7 +123,3 @@ class GlobalVariablesTestCase(TestBase):
matching=False,
substrs=["can't be resolved"])
-# Test that the statically initialized variable can also be
-# inspected *with* a process.
-self.expect("target variable *g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=['42'])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.
labath added inline comments.
Comment at:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py:42
+# Test that static initialized variables can be inspected without
process.
+self.expect("target variable g_ptr", VARIABLES_DISPLAYED_CORRECTLY,
this part wasn't correctly working for us on arm64, so I've split it off into a
separate test, so we can still have coverage from the rest of the checks here.
I think I managed to preserve the intent of your test, but you may want to have
a quick look at the result (r331250).
Comment at:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py:63-65
+self.runCmd("frame variable --show-types --scope --show-globals
--no-args")
self.expect(
"frame variable --show-types --scope --show-globals --no-args",
Why do you need to run this command twice?
Repository:
rL LLVM
https://reviews.llvm.org/D46220
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.
aprantl added inline comments.
Comment at:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py:63-65
+self.runCmd("frame variable --show-types --scope --show-globals
--no-args")
self.expect(
"frame variable --show-types --scope --show-globals --no-args",
labath wrote:
> Why do you need to run this command twice?
Sorry, that's leftover debugging code. I'll remove it!
Repository:
rL LLVM
https://reviews.llvm.org/D46220
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r331270 - Remove redundant command.
Author: adrian
Date: Tue May 1 08:38:01 2018
New Revision: 331270
URL: http://llvm.org/viewvc/llvm-project?rev=331270&view=rev
Log:
Remove redundant command.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py?rev=331270&r1=331269&r2=331270&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py
Tue May 1 08:38:01 2018
@@ -77,7 +77,6 @@ class GlobalVariablesTestCase(TestBase):
substrs=['42'])
# Check that GLOBAL scopes are indicated for the variables.
-self.runCmd("frame variable --show-types --scope --show-globals
--no-args")
self.expect(
"frame variable --show-types --scope --show-globals --no-args",
VARIABLES_DISPLAYED_CORRECTLY,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup
labath created this revision.
labath added reviewers: davide, zturner, asmith, JDevlieghere, clayborg.
Herald added subscribers: aprantl, mgorny.
Herald added a reviewer: alexshap.
lldb-test already had the ability to dump all symbol information in a
module. This is interesting, but it can be too verbose, and it also does
not use the same APIs that lldb uses to query symbol information. The
last part is interesting to me now, because I am about to add DWARF v5
debug_names support, which needs to implement these APIs.
This patch adds a set of arguments to lldb-test, which modify it's
behavior from dumping all symbols to dumping only the requested
information:
- --lookup={function,namespace,type,variable} - search for the given kind of
objects.
- --name - the name to search for.
- --regex - whether to treat the "name" as a regular expression. This is not
available for all lookup types (we do not have the required APIs for namespaces
and types).
- --context - specifies the context, which can be used to restrict the search.
This argument takes a variable name (which must be defined and be unique), and
we then use the context that this variable is defined in as the search context.
- --function-flags={auto,full,base,method,selector} - a set of flags to further
restrict the search for function symbols.
Together, these flags and their combinations cover the main SymbolFile
entry points which I will need to modify for the accelerator table
support, and so I plan to do most of the regression testing this way.
(I've also found this a useful tool for exploration of what the given
APIs are supposed to do.)
I add a couple of tests to demonstrate the usage of the usage of the
various options, and also an xfailed test which demonstrates a bug I
found while playing with this. The only requirement for these tests is
the presence of lld -- the should run on any platform which is able to
build lldb.
These tests use c++ code as input, but this isn't a requirement. It is also
possible to use IR, assembly or json to create the test module.
https://reviews.llvm.org/D46318
Files:
lit/CMakeLists.txt
lit/SymbolFile/DWARF/basic-function-lookup.cpp
lit/SymbolFile/DWARF/basic-namespace-lookup.cpp
lit/SymbolFile/DWARF/basic-type-lookup.cpp
lit/SymbolFile/DWARF/basic-variable-lookup.cpp
lit/SymbolFile/DWARF/type-in-function-lookup.cpp
lit/lit.cfg
lit/lit.site.cfg.in
tools/lldb-test/lldb-test.cpp
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -20,6 +20,9 @@
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Symbol/ClangASTImporter.h"
+#include "lldb/Symbol/SymbolVendor.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Symbol/VariableList.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/StreamString.h"
@@ -69,8 +72,65 @@
} // namespace module
namespace symbols {
-cl::list InputFilenames(cl::Positional, cl::desc(""),
- cl::OneOrMore, cl::sub(SymbolsSubcommand));
+static cl::list InputFilenames(cl::Positional,
+cl::desc(""),
+cl::OneOrMore,
+cl::sub(SymbolsSubcommand));
+enum class LookupType {
+ None,
+ Function,
+ Namespace,
+ Type,
+ Variable,
+};
+static cl::opt Lookup(
+"lookup", cl::desc("Choose lookup type:"),
+cl::values(
+clEnumValN(LookupType::None, "none",
+ "No lookup, just dump the module."),
+clEnumValN(LookupType::Function, "function", "Find functions."),
+clEnumValN(LookupType::Namespace, "namespace", "Find namespaces."),
+clEnumValN(LookupType::Type, "type", "Find types."),
+clEnumValN(LookupType::Variable, "variable", "Find global variables.")),
+cl::sub(SymbolsSubcommand));
+
+static cl::opt Name("name", cl::desc("Name to lookup."),
+ cl::sub(SymbolsSubcommand));
+static cl::opt
+Regex("regex",
+ cl::desc("Lookup using regular expressions (avaliable for variables "
+ "and functions only)."),
+ cl::sub(SymbolsSubcommand));
+static cl::opt
+Context("context",
+cl::desc("Restrict search to the context of the given variable."),
+cl::value_desc("variable"), cl::sub(SymbolsSubcommand));
+
+static cl::list FunctionNameFlags(
+"function-flags", cl::desc("Function lookup flags:"),
+cl::values(clEnumValN(eFunctionNameTypeAuto, "auto",
+ "Automatically deduce flags based on name."),
+ clEnumValN(eFunctionNameTypeFull, "full", "Full function name."),
+ clEnumValN(eFunctionNameTypeBase, "base", "Base name."),
+ clEnumValN(eFunctionNameTypeMethod, "method", "Method name."),
+
[Lldb-commits] [lldb] r331277 - [lit] Replace generator expressions in lit.site.cfg
Author: jdevlieghere
Date: Tue May 1 09:19:48 2018
New Revision: 331277
URL: http://llvm.org/viewvc/llvm-project?rev=331277&view=rev
Log:
[lit] Replace generator expressions in lit.site.cfg
The lit site configuration for the test suite can contain generator
expressions such as $ that need to be
substituted.
Modified:
lldb/trunk/test/CMakeLists.txt
Modified: lldb/trunk/test/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/CMakeLists.txt?rev=331277&r1=331276&r2=331277&view=diff
==
--- lldb/trunk/test/CMakeLists.txt (original)
+++ lldb/trunk/test/CMakeLists.txt Tue May 1 09:19:48 2018
@@ -186,9 +186,21 @@ string(REPLACE ${CMAKE_CFG_INTDIR} ${LLV
string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR
${LLVM_RUNTIME_OUTPUT_INTDIR})
string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS_STR
"${LLDB_DOTEST_ARGS_STR}")
+# FIXME: A better layering should enable us to access in lit/CMakeLists.txt
configure_lit_site_cfg(
${CMAKE_CURRENT_SOURCE_DIR}/../lit/Suite/lit.site.cfg.in
${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg)
+# We need to rename the configured lit.site.cfg so we can expand the generate
+# expressions later and configure_lit_site_cfg remembers the file name.
+file(RENAME
+ ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
+ ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg.configured)
+# We need this to expand the generator expressions.
+file(GENERATE
+ OUTPUT
+ ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
+ INPUT
+ ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg.configured)
# If we're building with an in-tree clang, then list clang as a dependency
# to run tests.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex
Good catch
> On May 1, 2018, at 1:19 AM, Pavel Labath via Phabricator
> wrote:
>
> labath added inline comments.
>
>
>
> Comment at: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:317
> void DWARFUnit::ClearDIEs(bool keep_compile_unit_die) {
> if (m_die_array.size() > 1) {
> +llvm::sys::ScopedWriter lock(m_extractdies_mutex);
>
> You're accessing m_die_array without holding a lock.
>
>
> Repository:
> rL LLVM
>
> https://reviews.llvm.org/D40470
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex
clayborg added a subscriber: jankratochvil. clayborg added a comment. Good catch Repository: rL LLVM https://reviews.llvm.org/D40470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46318: lldb-test symbols: Add ability to do name-based lookup
zturner added inline comments.
Comment at: lit/CMakeLists.txt:32
+if(TARGET lld)
+ list(APPEND LLDB_TEST_DEPS lld)
+ set(LLDB_HAVE_LLD 1)
Note that this depends on lld having had its own `CMakeLists.txt` file
processed before LLDB's. I think this happens by accident since we process
them in alphabetical order, but it's something to keep in mind.
Comment at: lit/SymbolFile/DWARF/basic-function-lookup.cpp:23
+void foo() {}
+// BASE-DAG: name = "foo()", mangled = "_Z3foov"
+// FULL-DAG: name = "foo()", mangled = "_Z3foov"
I personally find it a little confusing to have the check lines interspersed
with the source code here as we do in this file. I think it's easier to read
if all check labels are grouped together so that you get a general idea of what
the output of the tool looks like. I don't feel too strongly though, so up to
you.
https://reviews.llvm.org/D46318
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
On Wed, Apr 25, 2018 at 9:19 AM Adrian Prantl wrote: > > On Apr 25, 2018, at 9:08 AM, Zachary Turner wrote: > > But is there a reason why that is more valuable with LLDB than it is with > LLVM? In LLVM when a test fails it stops and doesn't run subsequent run > lines. So you have the same issue there. > > > That's not a good analogy. Multiple RUN lines don't necessarily mean > separate tests, they mean separate commands being executed that may depend > on each other. As Pavel pointed out before, this would be closer to how > gtests behave. > > -- adrian > The analogy I was going for is better illustrated in https://reviews.llvm.org/D46318. Here we have multiple distinct tests that are similar enough to be in the same file. lit runs them sequentially and stops as soon as any of the first one fails. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r331285 - Revert "[lit] Replace generator expressions in lit.site.cfg"
Author: jdevlieghere
Date: Tue May 1 10:08:09 2018
New Revision: 331285
URL: http://llvm.org/viewvc/llvm-project?rev=331285&view=rev
Log:
Revert "[lit] Replace generator expressions in lit.site.cfg"
Using GENERATE breaks generators that support multiple configurations,
e.g. MSVC. Reverting for now until we find a better solution.
Modified:
lldb/trunk/test/CMakeLists.txt
Modified: lldb/trunk/test/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/CMakeLists.txt?rev=331285&r1=331284&r2=331285&view=diff
==
--- lldb/trunk/test/CMakeLists.txt (original)
+++ lldb/trunk/test/CMakeLists.txt Tue May 1 10:08:09 2018
@@ -186,21 +186,9 @@ string(REPLACE ${CMAKE_CFG_INTDIR} ${LLV
string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR
${LLVM_RUNTIME_OUTPUT_INTDIR})
string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_DOTEST_ARGS_STR
"${LLDB_DOTEST_ARGS_STR}")
-# FIXME: A better layering should enable us to access in lit/CMakeLists.txt
configure_lit_site_cfg(
${CMAKE_CURRENT_SOURCE_DIR}/../lit/Suite/lit.site.cfg.in
${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg)
-# We need to rename the configured lit.site.cfg so we can expand the generate
-# expressions later and configure_lit_site_cfg remembers the file name.
-file(RENAME
- ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
- ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg.configured)
-# We need this to expand the generator expressions.
-file(GENERATE
- OUTPUT
- ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg
- INPUT
- ${CMAKE_CURRENT_BINARY_DIR}/../lit/Suite/lit.site.cfg.configured)
# If we're building with an in-tree clang, then list clang as a dependency
# to run tests.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules
lemo updated this revision to Diff 144751.
lemo marked 5 inline comments as done.
https://reviews.llvm.org/D46292
Files:
include/lldb/Core/Module.h
include/lldb/Core/ModuleSpec.h
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
source/Commands/CommandObjectTarget.cpp
source/Core/Module.cpp
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.h
source/Plugins/Process/minidump/ProcessMinidump.cpp
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -321,9 +321,10 @@
m_is_wow64 = true;
}
+const auto uuid = m_minidump_parser.GetModuleUUID(module);
const auto file_spec =
FileSpec(name.getValue(), true, GetArchitecture().GetTriple());
-ModuleSpec module_spec = file_spec;
+ModuleSpec module_spec(file_spec, uuid);
Status error;
lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
if (!module_sp || error.Fail()) {
@@ -337,6 +338,7 @@
auto placeholder_module =
std::make_shared(file_spec, GetArchitecture());
placeholder_module->CreateImageSection(module, GetTarget());
+ placeholder_module->SetUUID(uuid);
module_sp = placeholder_module;
GetTarget().GetImages().Append(module_sp);
}
Index: source/Plugins/Process/minidump/MinidumpTypes.h
===
--- source/Plugins/Process/minidump/MinidumpTypes.h
+++ source/Plugins/Process/minidump/MinidumpTypes.h
@@ -43,6 +43,21 @@
};
+enum class CvSignature : uint32_t {
+ Pdb70 = 0x53445352, // RSDS
+ ElfBuildId = 0x4270454c, // BpEL (Breakpad/Crashpad minidumps)
+};
+
+// Reference:
+// https://crashpad.chromium.org/doxygen/structcrashpad_1_1CodeViewRecordPDB70.html
+struct CvRecordPdb70 {
+ uint8_t Uuid[16];
+ llvm::support::ulittle32_t Age;
+ // char PDBFileName[];
+};
+static_assert(sizeof(CvRecordPdb70) == 20,
+ "sizeof CvRecordPdb70 is not correct!");
+
// Reference:
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms680394.aspx
enum class MinidumpStreamType : uint32_t {
Index: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -17,6 +17,7 @@
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBuffer.h"
#include "lldb/Utility/Status.h"
+#include "lldb/Utility/UUID.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
@@ -54,6 +55,8 @@
llvm::Optional GetMinidumpString(uint32_t rva);
+ UUID GetModuleUUID(const MinidumpModule* module);
+
llvm::ArrayRef GetThreads();
llvm::ArrayRef GetThreadContext(const MinidumpThread &td);
Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -96,6 +96,40 @@
return parseMinidumpString(arr_ref);
}
+UUID MinidumpParser::GetModuleUUID(const MinidumpModule *module) {
+ auto cv_record =
+ GetData().slice(module->CV_record.rva, module->CV_record.data_size);
+
+ // Read the CV record signature
+ const llvm::support::ulittle32_t *signature = nullptr;
+ Status error = consumeObject(cv_record, signature);
+ if (error.Fail())
+return UUID();
+
+ const CvSignature cv_signature =
+ static_cast(static_cast(*signature));
+
+ if (cv_signature == CvSignature::Pdb70) {
+// PDB70 record
+const CvRecordPdb70 *pdb70_uuid = nullptr;
+Status error = consumeObject(cv_record, pdb70_uuid);
+if (!error.Fail())
+ return UUID(pdb70_uuid, sizeof(*pdb70_uuid));
+ } else if (cv_signature == CvSignature::ElfBuildId) {
+// ELF BuildID (found in Breakpad/Crashpad generated minidumps)
+//
+// This is variable-length, but usually 20 bytes
+// as the binutils ld default is a SHA-1 hash.
+// (We'll handle only 16 and 20 bytes signatures,
+// matching LLDB support for UUIDs)
+//
+if (cv_record.size() == 16 || cv_record.size() == 20)
+ return UUID(cv_record.data(), cv_record.size());
+ }
+
+ return UUID();
+}
+
llvm::ArrayRef MinidumpParser::GetThreads() {
llvm::ArrayRef data = GetStream(MinidumpStreamType::ThreadList);
Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -38,6 +38,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#in
[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules
lemo added a comment.
Thanks for the feedback. I uploaded a new revision (incorporating some of the
feedback, including an ELF test case)
Comment at: include/lldb/Core/Module.h:779
+ //--
+ void SetUUID(const lldb_private::UUID &uuid);
+
labath wrote:
> amccarth wrote:
> > Was there no SetUUID before because the UUID was considered immutable? I
> > wonder if that's worth keeping. Is the UUID always known at construction
> > time?
> The UUID is not always know at construction time -- sometimes it can be, if
> you set it on the FileSpec you use to create the module, but you don't have
> to do that.
>
> However, it is true that the UUID was constant during the lifetime of the
> Module, and this changes that, which is not ideal IMO.
>
> It seems that in the only place you call this function, you already know the
> UUID. Would it be possible to do this work in the PlaceholderModule
> constructor (thereby avoiding any changes to the Module API) ?
Good point: I agree, it's a good idea to preserve the invariant that the module
UUID does not change once it's set.
I'd not add more complexity & arguments to the placeholder module constructor
though. Moreover, even if it's technically possible to reach into the protected
data members from Module I'd like to avoid doing that since Module::m_uuid is
closely tied to Module::m_mutex and Module::m_did_parse_uuid (the later now
renamed to m_did_set_uuid).
I added a check and assert to prevent Module::SetUUID from overwriting an
already set UUID. (Personally I'd would've liked to have the assert be a
fastfail in all builds but I remember that is somewhat against the LLDB
philosophy)
Comment at: source/Core/Module.cpp:341
+ m_uuid = uuid;
+ m_did_parse_uuid = true;
+}
amccarth wrote:
> I wonder if the lazy UUID lookup in Module::GetUUID should call this to avoid
> code duplication. I know it's not a lot of code, but it involves a mutex and
> an atomic flag, so it might be nice to always have this update in just one
> place.
Good point, but I'm afraid that would obscure the double-checked-locking in
Module::GetUUID() so I'd rather not change it.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:99
+UUID MinidumpParser::GetModuleUUID(const MinidumpModule *module) {
+ auto cv_record =
amccarth wrote:
> I wonder if this should return an `Expected`. The function has
> multiple ways it can fail, in which case the error info is lost and we
> silently return an empty UUID.
The goal is support the common CV records we expect to see from current
toolchains and the contract for GetModuleUUID() is to either return a valid
UUID or an empty one.
Neither is an error: the CV record is optional and we also can't guarantee to
handle every single CV record type.
https://reviews.llvm.org/D46292
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules
labath added inline comments. Comment at: include/lldb/Core/Module.h:779 + //-- + void SetUUID(const lldb_private::UUID &uuid); + lemo wrote: > labath wrote: > > amccarth wrote: > > > Was there no SetUUID before because the UUID was considered immutable? I > > > wonder if that's worth keeping. Is the UUID always known at construction > > > time? > > The UUID is not always know at construction time -- sometimes it can be, if > > you set it on the FileSpec you use to create the module, but you don't have > > to do that. > > > > However, it is true that the UUID was constant during the lifetime of the > > Module, and this changes that, which is not ideal IMO. > > > > It seems that in the only place you call this function, you already know > > the UUID. Would it be possible to do this work in the PlaceholderModule > > constructor (thereby avoiding any changes to the Module API) ? > Good point: I agree, it's a good idea to preserve the invariant that the > module UUID does not change once it's set. > > I'd not add more complexity & arguments to the placeholder module constructor > though. Moreover, even if it's technically possible to reach into the > protected data members from Module I'd like to avoid doing that since > Module::m_uuid is closely tied to Module::m_mutex and > Module::m_did_parse_uuid (the later now renamed to m_did_set_uuid). > > I added a check and assert to prevent Module::SetUUID from overwriting an > already set UUID. (Personally I'd would've liked to have the assert be a > fastfail in all builds but I remember that is somewhat against the LLDB > philosophy) > > Guarding the invariant with an assert is better than nothing, but even better is arranging stuff so that the invariant cannot be broken in the first place. And this feels like the sort of thing that should be possible to make safe by design. How about adding a special (protected?) Module constructor, which takes a UUID argument and uses it to initialize the m_uuid field. This way, it is impossible by design to change the UUID mid-flight, and you still don't have to reach into the protected Module fields from the other class (we could even make them private if we want to). https://reviews.llvm.org/D46292 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules
lemo added inline comments. Comment at: include/lldb/Core/Module.h:779 + //-- + void SetUUID(const lldb_private::UUID &uuid); + labath wrote: > lemo wrote: > > labath wrote: > > > amccarth wrote: > > > > Was there no SetUUID before because the UUID was considered immutable? > > > > I wonder if that's worth keeping. Is the UUID always known at > > > > construction time? > > > The UUID is not always know at construction time -- sometimes it can be, > > > if you set it on the FileSpec you use to create the module, but you don't > > > have to do that. > > > > > > However, it is true that the UUID was constant during the lifetime of the > > > Module, and this changes that, which is not ideal IMO. > > > > > > It seems that in the only place you call this function, you already know > > > the UUID. Would it be possible to do this work in the PlaceholderModule > > > constructor (thereby avoiding any changes to the Module API) ? > > Good point: I agree, it's a good idea to preserve the invariant that the > > module UUID does not change once it's set. > > > > I'd not add more complexity & arguments to the placeholder module > > constructor though. Moreover, even if it's technically possible to reach > > into the protected data members from Module I'd like to avoid doing that > > since Module::m_uuid is closely tied to Module::m_mutex and > > Module::m_did_parse_uuid (the later now renamed to m_did_set_uuid). > > > > I added a check and assert to prevent Module::SetUUID from overwriting an > > already set UUID. (Personally I'd would've liked to have the assert be a > > fastfail in all builds but I remember that is somewhat against the LLDB > > philosophy) > > > > > Guarding the invariant with an assert is better than nothing, but even better > is arranging stuff so that the invariant cannot be broken in the first place. > And this feels like the sort of thing that should be possible to make safe by > design. > > How about adding a special (protected?) Module constructor, which takes a > UUID argument and uses it to initialize the m_uuid field. This way, it is > impossible by design to change the UUID mid-flight, and you still don't have > to reach into the protected Module fields from the other class (we could even > make them private if we want to). Trying to make sure that the invariant can't be invalidated is a high bar considering that most Module state is protected. But I agree with the sentiment and here's a new iteration: 1. Made Module::SetUUID() protected (note that the m_uuid change is conditional, not just protected by an lldbassert) 2. PlaceholderModule constructor is where SetUUID is called from I'm hesitant to add another Module constructor: that class already has 2 public + 1 private constructors (even using a delegating constructor, the addition of a new protected constructor questionable). But if you still prefer that option better I'll make the change. https://reviews.llvm.org/D46292 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46292: Use the UUID from the minidump's CodeView Record for placeholder modules
lemo updated this revision to Diff 144790.
https://reviews.llvm.org/D46292
Files:
include/lldb/Core/Module.h
include/lldb/Core/ModuleSpec.h
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
source/Commands/CommandObjectTarget.cpp
source/Core/Module.cpp
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.h
source/Plugins/Process/minidump/ProcessMinidump.cpp
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -46,8 +46,11 @@
//--
class PlaceholderModule : public Module {
public:
- PlaceholderModule(const FileSpec &file_spec, const ArchSpec &arch) :
-Module(file_spec, arch) {}
+ PlaceholderModule(const ModuleSpec &module_spec) :
+Module(module_spec.GetFileSpec(), module_spec.GetArchitecture()) {
+if (module_spec.GetUUID().IsValid())
+ SetUUID(module_spec.GetUUID());
+ }
// Creates a synthetic module section covering the whole module image (and
// sets the section load address as well)
@@ -321,9 +324,10 @@
m_is_wow64 = true;
}
+const auto uuid = m_minidump_parser.GetModuleUUID(module);
const auto file_spec =
FileSpec(name.getValue(), true, GetArchitecture().GetTriple());
-ModuleSpec module_spec = file_spec;
+ModuleSpec module_spec(file_spec, uuid);
Status error;
lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
if (!module_sp || error.Fail()) {
@@ -335,7 +339,7 @@
// translations (ex. identifing the module for a stack frame PC) and
// modules/sections commands (ex. target modules list, ...)
auto placeholder_module =
- std::make_shared(file_spec, GetArchitecture());
+ std::make_shared(module_spec);
placeholder_module->CreateImageSection(module, GetTarget());
module_sp = placeholder_module;
GetTarget().GetImages().Append(module_sp);
Index: source/Plugins/Process/minidump/MinidumpTypes.h
===
--- source/Plugins/Process/minidump/MinidumpTypes.h
+++ source/Plugins/Process/minidump/MinidumpTypes.h
@@ -43,6 +43,21 @@
};
+enum class CvSignature : uint32_t {
+ Pdb70 = 0x53445352, // RSDS
+ ElfBuildId = 0x4270454c, // BpEL (Breakpad/Crashpad minidumps)
+};
+
+// Reference:
+// https://crashpad.chromium.org/doxygen/structcrashpad_1_1CodeViewRecordPDB70.html
+struct CvRecordPdb70 {
+ uint8_t Uuid[16];
+ llvm::support::ulittle32_t Age;
+ // char PDBFileName[];
+};
+static_assert(sizeof(CvRecordPdb70) == 20,
+ "sizeof CvRecordPdb70 is not correct!");
+
// Reference:
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms680394.aspx
enum class MinidumpStreamType : uint32_t {
Index: source/Plugins/Process/minidump/MinidumpParser.h
===
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -17,6 +17,7 @@
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBuffer.h"
#include "lldb/Utility/Status.h"
+#include "lldb/Utility/UUID.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
@@ -54,6 +55,8 @@
llvm::Optional GetMinidumpString(uint32_t rva);
+ UUID GetModuleUUID(const MinidumpModule* module);
+
llvm::ArrayRef GetThreads();
llvm::ArrayRef GetThreadContext(const MinidumpThread &td);
Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -96,6 +96,40 @@
return parseMinidumpString(arr_ref);
}
+UUID MinidumpParser::GetModuleUUID(const MinidumpModule *module) {
+ auto cv_record =
+ GetData().slice(module->CV_record.rva, module->CV_record.data_size);
+
+ // Read the CV record signature
+ const llvm::support::ulittle32_t *signature = nullptr;
+ Status error = consumeObject(cv_record, signature);
+ if (error.Fail())
+return UUID();
+
+ const CvSignature cv_signature =
+ static_cast(static_cast(*signature));
+
+ if (cv_signature == CvSignature::Pdb70) {
+// PDB70 record
+const CvRecordPdb70 *pdb70_uuid = nullptr;
+Status error = consumeObject(cv_record, pdb70_uuid);
+if (!error.Fail())
+ return UUID(pdb70_uuid, sizeof(*pdb70_uuid));
+ } else if (cv_signature == CvSignature::ElfBuildId) {
+// ELF BuildID (found in Breakpad/Crashpad generated minidumps)
+//
+// This is variable-length, b
[Lldb-commits] [PATCH] D46334: [lit] Make debugserver available to lit test
JDevlieghere created this revision.
JDevlieghere added reviewers: stella.stamenova, zturner, davide.
Herald added subscribers: jkorous, mgorny.
Because we can't expand generator expressions for the lit tests, we need to set
the path to the debugserver in the `_STR` variable so it's expanded properly
without breaking multi-target generators such as MSVC and Xcode.
Repository:
rL LLVM
https://reviews.llvm.org/D46334
Files:
test/CMakeLists.txt
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -134,11 +134,13 @@
--env ARCHIVER=${CMAKE_AR} --env OBJCOPY=${CMAKE_OBJCOPY})
endif()
-# In some cases, DEBUGSERVER_PATH is expressed as $.
This won't work in the
-# LLDB_DOTEST_ARGS_STR when using a generator that supports multiple
configurations such as Visual Studio,
-# but since debugserver is currently confined to Darwin/Apple, we can leave it
as is.
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
+ if (DEBUGSERVER_PATH STREQUAL "$")
+list(APPEND LLDB_EXECUTABLE_PATH_ARGS --server ${DEBUGSERVER_PATH})
+list(APPEND LLDB_EXECUTABLE_PATH_ARGS_STR --server
${LLVM_RUNTIME_OUTPUT_INTDIR}/debugserver${CMAKE_EXECUTABLE_SUFFIX})
+ else()
+list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
+ endif()
endif()
if(SKIP_DEBUGSERVER)
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -134,11 +134,13 @@
--env ARCHIVER=${CMAKE_AR} --env OBJCOPY=${CMAKE_OBJCOPY})
endif()
-# In some cases, DEBUGSERVER_PATH is expressed as $. This won't work in the
-# LLDB_DOTEST_ARGS_STR when using a generator that supports multiple configurations such as Visual Studio,
-# but since debugserver is currently confined to Darwin/Apple, we can leave it as is.
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
+ if (DEBUGSERVER_PATH STREQUAL "$")
+list(APPEND LLDB_EXECUTABLE_PATH_ARGS --server ${DEBUGSERVER_PATH})
+list(APPEND LLDB_EXECUTABLE_PATH_ARGS_STR --server ${LLVM_RUNTIME_OUTPUT_INTDIR}/debugserver${CMAKE_EXECUTABLE_SUFFIX})
+ else()
+list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
+ endif()
endif()
if(SKIP_DEBUGSERVER)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46334: [lit] Make debugserver available to lit test
JDevlieghere added inline comments. Comment at: test/CMakeLists.txt:129 if(LLDB_BUILD_FRAMEWORK) list(APPEND LLDB_TEST_COMMON_ARGS --framework $) endif() We'll have to do the same thing here, but the debugserver is more pressing as it's keeping GreenDragon red (http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/) Repository: rL LLVM https://reviews.llvm.org/D46334 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46334: [lit] Make debugserver available to lit test
stella.stamenova added inline comments.
Comment at: test/CMakeLists.txt:138
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
+ if (DEBUGSERVER_PATH STREQUAL "$")
+list(APPEND LLDB_EXECUTABLE_PATH_ARGS --server ${DEBUGSERVER_PATH})
I am wondering if it's possible to make the logic here simpler. Since we need
to handle each of the properties that can contain TARGET_FILE, it is starting
to get rather complicated
I think you can actually use LLDB_DOTEST_ARGS_STR for the check-lldb-single
target (since it creates a project that should be correctly substituted), so
the only other place that could create issues is the lldb-dotest script. Since
the tests are run as part of lit now, do we still need the lldb-dotest script?
Repository:
rL LLVM
https://reviews.llvm.org/D46334
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r331315 - Fix the .experimental. settings feature so that we don't return an error
Author: jmolenda
Date: Tue May 1 15:49:01 2018
New Revision: 331315
URL: http://llvm.org/viewvc/llvm-project?rev=331315&view=rev
Log:
Fix the .experimental. settings feature so that we don't return an error
if an experimental setting has been removed/is missing.
Add tests for the .experimental. settings behaviors -- that they correctly
forward through to the real setting if it has become a real setting,
that they don't generate errors when a settig has been removed.
As Pavel notes in https://reviews.llvm.org/D45348, the way I'm suppressing
errors in the setting is not completely correct - if any of the setting
path components include "experimental", a missing setting would be declared
a non-error. So
settings set target.experimental.setting-that-does-not-exist true
would not generate an error, which is correct. But as Pavel notes,
settings set setting-does-not-exist.experimental.run-stopped true
should generate an error because the unknown name occurs before the
"experimental". The amount of change to do this correctly hasn't
thrilled me, so I'm leaving this as-is for now.
Differential Revision: https://reviews.llvm.org/D45348
Modified:
lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
lldb/trunk/source/Interpreter/OptionValueProperties.cpp
Modified: lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py?rev=331315&r1=331314&r2=331315&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py Tue May
1 15:49:01 2018
@@ -524,3 +524,41 @@ class SettingsCommandTestCase(TestBase):
"target.process.extra-startup-command",
"target.process.thread.step-avoid-regexp",
"target.process.thread.trace-thread"])
+
+# settings under an ".experimental" domain should have two properties:
+# 1. If the name does not exist with "experimental" in the name path,
+# the name lookup should try to find it without "experimental". So
+# a previously-experimental setting that has been promoted to a
+# "real" setting will still be set by the original name.
+# 2. Changing a setting with .experimental., name, where the setting
+# does not exist either with ".experimental." or without, should
+# not generate an error. So if an experimental setting is removed,
+# people who may have that in their ~/.lldbinit files should not see
+# any errors.
+def test_experimental_settings(self):
+cmdinterp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+
+# Set target.arg0 to a known value, check that we can retrieve it via
+# the actual name and via .experimental.
+self.expect('settings set target.arg0 first-value')
+self.expect('settings show target.arg0', substrs=['first-value'])
+self.expect('settings show target.experimental.arg0',
substrs=['first-value'], error=False)
+
+# Set target.arg0 to a new value via a target.experimental.arg0 name,
+# verify that we can read it back via both .experimental., and not.
+self.expect('settings set target.experimental.arg0 second-value',
error=False)
+self.expect('settings show target.arg0', substrs=['second-value'])
+self.expect('settings show target.experimental.arg0',
substrs=['second-value'], error=False)
+
+# showing & setting an undefined .experimental. setting should
generate no errors.
+self.expect('settings show
target.experimental.setting-which-does-not-exist', patterns=['^\s$'],
error=False)
+self.expect('settings set
target.experimental.setting-which-does-not-exist true', error=False)
+
+# A domain component before .experimental. which does not exist should
give an error
+# But the code does not yet do that.
+# self.expect('settings set
target.setting-which-does-not-exist.experimental.arg0 true', error=True)
+
+# finally, confirm that trying to set a setting that does not exist
still fails.
+# (SHOWING a setting that does not exist does not currently yield an
error.)
+self.expect('settings set target.setting-which-does-not-exist true',
error=True)
Modified: lldb/trunk/source/Interpreter/OptionValueProperties.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueProperties.cpp?rev=331315&r1=331314&r2=331315&view=diff
==
--- lldb/trunk/source/Interpreter/OptionValueProperties.cpp (original)
+++ lldb/trunk/source/Interpreter/OptionValueProperties.cpp Tue May 1 15:49:01
2018
@@
[Lldb-commits] [PATCH] D45348: Don't return error for settings set .experimental. settings that are absent
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL331315: Fix the .experimental. settings feature so that we
don't return an error (authored by jmolenda, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45348?vs=141473&id=144807#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45348
Files:
lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
lldb/trunk/source/Interpreter/OptionValueProperties.cpp
Index: lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
+++ lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
@@ -524,3 +524,41 @@
"target.process.extra-startup-command",
"target.process.thread.step-avoid-regexp",
"target.process.thread.trace-thread"])
+
+# settings under an ".experimental" domain should have two properties:
+# 1. If the name does not exist with "experimental" in the name path,
+# the name lookup should try to find it without "experimental". So
+# a previously-experimental setting that has been promoted to a
+# "real" setting will still be set by the original name.
+# 2. Changing a setting with .experimental., name, where the setting
+# does not exist either with ".experimental." or without, should
+# not generate an error. So if an experimental setting is removed,
+# people who may have that in their ~/.lldbinit files should not see
+# any errors.
+def test_experimental_settings(self):
+cmdinterp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+
+# Set target.arg0 to a known value, check that we can retrieve it via
+# the actual name and via .experimental.
+self.expect('settings set target.arg0 first-value')
+self.expect('settings show target.arg0', substrs=['first-value'])
+self.expect('settings show target.experimental.arg0',
substrs=['first-value'], error=False)
+
+# Set target.arg0 to a new value via a target.experimental.arg0 name,
+# verify that we can read it back via both .experimental., and not.
+self.expect('settings set target.experimental.arg0 second-value',
error=False)
+self.expect('settings show target.arg0', substrs=['second-value'])
+self.expect('settings show target.experimental.arg0',
substrs=['second-value'], error=False)
+
+# showing & setting an undefined .experimental. setting should
generate no errors.
+self.expect('settings show
target.experimental.setting-which-does-not-exist', patterns=['^\s$'],
error=False)
+self.expect('settings set
target.experimental.setting-which-does-not-exist true', error=False)
+
+# A domain component before .experimental. which does not exist should
give an error
+# But the code does not yet do that.
+# self.expect('settings set
target.setting-which-does-not-exist.experimental.arg0 true', error=True)
+
+# finally, confirm that trying to set a setting that does not exist
still fails.
+# (SHOWING a setting that does not exist does not currently yield an
error.)
+self.expect('settings set target.setting-which-does-not-exist true',
error=True)
Index: lldb/trunk/source/Interpreter/OptionValueProperties.cpp
===
--- lldb/trunk/source/Interpreter/OptionValueProperties.cpp
+++ lldb/trunk/source/Interpreter/OptionValueProperties.cpp
@@ -202,12 +202,23 @@
llvm::StringRef value) {
Status error;
const bool will_modify = true;
+ llvm::SmallVector components;
+ name.split(components, '.');
+ bool name_contains_experimental = false;
+ for (const auto &part : components)
+if (Properties::IsSettingExperimental(part))
+ name_contains_experimental = true;
+
+
lldb::OptionValueSP value_sp(GetSubValue(exe_ctx, name, will_modify, error));
if (value_sp)
error = value_sp->SetValueFromString(value, op);
else {
-if (error.AsCString() == nullptr)
+// Don't set an error if the path contained .experimental. - those are
+// allowed to be missing and should silently fail.
+if (name_contains_experimental == false && error.AsCString() == nullptr) {
error.SetErrorStringWithFormat("invalid value path '%s'",
name.str().c_str());
+}
}
return error;
}
Index: lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py
+++ lldb/trunk/packages/Python/
[Lldb-commits] [PATCH] D46334: [lit] Make debugserver available to lit test
JDevlieghere added inline comments.
Comment at: test/CMakeLists.txt:138
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
+ if (DEBUGSERVER_PATH STREQUAL "$")
+list(APPEND LLDB_EXECUTABLE_PATH_ARGS --server ${DEBUGSERVER_PATH})
stella.stamenova wrote:
> I am wondering if it's possible to make the logic here simpler. Since we need
> to handle each of the properties that can contain TARGET_FILE, it is starting
> to get rather complicated
>
> I think you can actually use LLDB_DOTEST_ARGS_STR for the check-lldb-single
> target (since it creates a project that should be correctly substituted), so
> the only other place that could create issues is the lldb-dotest script.
> Since the tests are run as part of lit now, do we still need the lldb-dotest
> script?
>
I started out that route, but how would that work for `check-lldb-single` with
multiple targets? I'm not worried about `lldb-dotest`, we can do the same trick
as for `llvm-lit`(see llvm/utils/llvm-lit/CMakeLists.txt).
Repository:
rL LLVM
https://reviews.llvm.org/D46334
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46334: [lit] Make debugserver available to lit test
stella.stamenova added inline comments.
Comment at: test/CMakeLists.txt:138
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
+ if (DEBUGSERVER_PATH STREQUAL "$")
+list(APPEND LLDB_EXECUTABLE_PATH_ARGS --server ${DEBUGSERVER_PATH})
JDevlieghere wrote:
> stella.stamenova wrote:
> > I am wondering if it's possible to make the logic here simpler. Since we
> > need to handle each of the properties that can contain TARGET_FILE, it is
> > starting to get rather complicated
> >
> > I think you can actually use LLDB_DOTEST_ARGS_STR for the check-lldb-single
> > target (since it creates a project that should be correctly substituted),
> > so the only other place that could create issues is the lldb-dotest script.
> > Since the tests are run as part of lit now, do we still need the
> > lldb-dotest script?
> >
> I started out that route, but how would that work for `check-lldb-single`
> with multiple targets? I'm not worried about `lldb-dotest`, we can do the
> same trick as for `llvm-lit`(see llvm/utils/llvm-lit/CMakeLists.txt).
I cannot speak for other generators that support multiple configurations
(though I suspect it is similar), but for Visual Studio, if the path to a tool
contained CMAKE_CFG_INTDIR (which is $(Configuration)), when the project was
built, $(Configuration) would be replaced with the configuration that is
currently being used which is exactly what we want anyway.
Obviously, some additional testing will be required to verify that this is the
case always.
Repository:
rL LLVM
https://reviews.llvm.org/D46334
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r331323 - Update lldb to match clang r331244 (addition of char8_t).
Author: rsmith Date: Tue May 1 19:43:22 2018 New Revision: 331323 URL: http://llvm.org/viewvc/llvm-project?rev=331323&view=rev Log: Update lldb to match clang r331244 (addition of char8_t). Also fix misclassification of char16_t and char32_t: these are unsigned types, not signed types. Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=331323&r1=331322&r2=331323&view=diff == --- lldb/trunk/source/Symbol/ClangASTContext.cpp (original) +++ lldb/trunk/source/Symbol/ClangASTContext.cpp Tue May 1 19:43:22 2018 @@ -4914,8 +4914,6 @@ lldb::Encoding ClangASTContext::GetEncod case clang::BuiltinType::Char_S: case clang::BuiltinType::SChar: case clang::BuiltinType::WChar_S: -case clang::BuiltinType::Char16: -case clang::BuiltinType::Char32: case clang::BuiltinType::Short: case clang::BuiltinType::Int: case clang::BuiltinType::Long: @@ -4926,6 +4924,9 @@ lldb::Encoding ClangASTContext::GetEncod case clang::BuiltinType::Char_U: case clang::BuiltinType::UChar: case clang::BuiltinType::WChar_U: +case clang::BuiltinType::Char8: +case clang::BuiltinType::Char16: +case clang::BuiltinType::Char32: case clang::BuiltinType::UShort: case clang::BuiltinType::UInt: case clang::BuiltinType::ULong: ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
alur updated this revision to Diff 144834.
alur edited the summary of this revision.
alur added a comment.
Since there was some changes to the file during the last couple of days I've
updated the patches to be against HEAD again.
https://reviews.llvm.org/D45628
Files:
packages/Python/lldbsuite/test/linux/compressed-debug-info/Makefile
packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1762,13 +1762,22 @@
return eSectionTypeOther;
}
+ llvm::StringRef mapped_name;
+ if (section_name.startswith(".zdebug")) {
+// .zdebug* are compressed equivalents of .debug* sections, and should map
+// to the same corresponding type.
+mapped_name = section_name.drop_front(2);
+ } else {
+mapped_name = section_name.drop_front(1);
+ }
+
// MISSING? .gnu_debugdata - "mini debuginfo / MiniDebugInfo" section,
// http://sourceware.org/gdb/onlinedocs/gdb/MiniDebugInfo.html
// MISSING? .debug-index -
// http://src.chromium.org/viewvc/chrome/trunk/src/build/gdb-add-index?pathrev=144644
// MISSING? .debug_types - Type descriptions from DWARF 4? See
// http://gcc.gnu.org/wiki/DwarfSeparateTypeInfo
- return llvm::StringSwitch(section_name.drop_front(1))
+ return llvm::StringSwitch(mapped_name)
.Case("text", eSectionTypeCode)
.Case("data", eSectionTypeData)
.Case("bss", eSectionTypeZeroFill)
@@ -2794,6 +2803,7 @@
void ObjectFileELF::RelocateSection(lldb_private::Section *section)
{
static const llvm::StringRef debug_prefix(".debug");
+ static const llvm::StringRef zdebug_prefix(".zdebug");
// Set relocated bit so we stop getting called, regardless of whether we
// actually relocate.
@@ -2809,7 +2819,8 @@
return;
// We don't relocate non-debug sections at the moment
- if (section_name.startswith(debug_prefix))
+ if (section_name.startswith(debug_prefix) ||
+ section_name.startswith(zdebug_prefix))
return;
// Relocation section names to look for
@@ -3301,7 +3312,8 @@
return section->GetObjectFile()->ReadSectionData(section, section_offset,
dst, dst_len);
- if (!section->Test(SHF_COMPRESSED))
+ if (!llvm::object::Decompressor::isCompressedELFSection(
+ section->Get(), section->GetName().GetStringRef()))
return ObjectFile::ReadSectionData(section, section_offset, dst, dst_len);
// For compressed sections we need to read to full data to be able to
@@ -3320,7 +3332,8 @@
Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
size_t result = ObjectFile::ReadSectionData(section, section_data);
- if (result == 0 || !section->Test(SHF_COMPRESSED))
+ if (result == 0 || !llvm::object::Decompressor::isCompressedELFSection(
+ section->Get(), section->GetName().GetStringRef()))
return result;
auto Decompressor = llvm::object::Decompressor::create(
Index: packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
@@ -0,0 +1,4 @@
+int main() {
+ int z = 2;
+ return z;
+}
Index: packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
@@ -0,0 +1,46 @@
+""" Tests that compressed debug info sections are used. """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCompressedDebugInfo(TestBase):
+ mydir = TestBase.compute_mydir(__file__)
+
+ def setUp(self):
+TestBase.setUp(self)
+
+ @no_debug_info_test # Prevent the genaration of the dwarf version of this test
+ @skipUnlessPlatform(["linux"])
+ def test_compressed_debug_info(self):
+"""Tests that the 'frame variable' works with compressed debug info."""
+
+self.build()
+process = lldbutil.run_to_source_breakpoint(
+self, "main", lldb.SBFileSpec("a.c"), exe_name="compressed.out")[1]
+
+# The process should be stopped at a breakpoint, and the z variable should
+# be in the top frame.
+self.assertTrue(process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+frame = process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+self.assertTrue(frame.FindVariable("z").IsValid(), "z is not valid")
+
+ @no_debug_info_test # Prevent the genaration o
