[Lldb-commits] [PATCH] D49739: Add new API to SBTarget class
labath added a comment. In https://reviews.llvm.org/D49739#1194037, @apolyakov wrote: > I think those options don't fit. I'm looking for behavior like this: > > ~/workspace/gsoc/build/bin/lldb-server gdbserver --pipe 1 localhost:0 > 36251 > > > Here lldb-server prints out the port number it is listening on and continues > running. > > If debugserver can choose a tcp port in case of passed `hostname:0` then > probably all we need to do is to add analogue of `--pipe` option. That's what --unix-socket and --named-pipe do, except that they write to the port number, well.. to a unix socket or a named pipe. You should be able to use that the same way as the --pipe argument, just the setup will be slightly different. However, adding support for the --pipe option should not be a problem either, given that all the pieces are already in place. Repository: rL LLVM https://reviews.llvm.org/D49739 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50525: [WIP] Move lldb-mi interpreter tests to LIT
labath added inline comments. Comment at: lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test:4 +# +# RUN: %cxx -o %t %p/inputs/main.cpp -g +# RUN: %lldbmi %t < %s | FileCheck %s aprantl wrote: > stella.stamenova wrote: > > apolyakov wrote: > > > stella.stamenova wrote: > > > > apolyakov wrote: > > > > > stella.stamenova wrote: > > > > > > One thing to consider here is that any extra parameters passed with > > > > > > -E to the test suite will not propagate to lit at the moment. > > > > > > > > > > > > I ran into this with some internal testing where we need to pass > > > > > > parameters to the compiler - all of the lldb suite tests (c++ and > > > > > > c) build correctly, but any lit tests that directly invoke the > > > > > > compiler do not because the parameters do not get propagated all > > > > > > the way. > > > > > Could you give an example of extra parameters? I didn't see them > > > > > before so I don't completely understand you. > > > > -E "--sysroot=path/to/sys/root -lc++abi -lunwind" > > > Ok, I think we won't use something like it here. Thank you. > > I think you misunderstood my concern - let's say I have a machine on which > > I run these tests for a particular architecture that depends on passing > > these arguments to the tests, so that clang (cxx) correctly complies c++ > > files. *Before* your change, these arguments would have been propagated to > > the test in the lldb suite and the code would have build correctly. *After* > > your change, the code will no longer build correctly. > > > > Essentially, by making these tests lit tests, you are removing support for > > passing these arguments to the compiler (since the lldb suite supports them > > and lit does not). It might still be worth making these lit tests for the > > sake of other benefits, but then such targets will end up having to XFAIL > > the tests. > > > > If these tests really need to become lit tests and invoke the compiler, I > > think you also need to add support for passing these arguments to the > > compiler. > > > I think the best way to solve this is by adding the platform-specific flags > to the expansion of `%cxx` in the lit configuration. Would that work here? I am wondering how much do we actually need the lldb-mi tests to run on exotic (non-host) platforms/architectures. My take on these tests is that they should test the functionality from the lldb-mi protocol, and down (only) to the SB API calls. Anything below SB is a black box, which is assumed to be working (tested via other means). In particular, these tests should not care about whether the binary is built with libc++abi or split-dwarf or something else. The default debuggable executable produced by the given compiler should be enough. In such a world, there is not much value in running the tests on other architectures, as (hopefully) all of those differences are hidden inside liblldb. The existing lldb-mi tests already do not support all the features that other dotest tests do (e.g. running the tests remotely). If getting this working it's just the matter of adding something to the expansion of %cxx, then sure, why not... But otherwise I would not spend too much time engineering a very generic solution. Comment at: lit/tools/lldb-mi/interpreter/cli-support/settings-set-target-run-after.test:13 + +# FIXME: --arg1 causes an error. +setting set target.run-args arg1 "2nd arg" third_arg fourth="4th arg" Try: ``` setting set -- target.run-args --your --args --with --dashes ``` Repository: rLLDB LLDB https://reviews.llvm.org/D50525 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r339430 - Amend "Remove unused type Either from Utility library".
Author: tkrasnukha
Date: Fri Aug 10 06:01:26 2018
New Revision: 339430
URL: http://llvm.org/viewvc/llvm-project?rev=339430&view=rev
Log:
Amend "Remove unused type Either from Utility library".
Removed:
lldb/trunk/include/lldb/Utility/Either.h
Removed: lldb/trunk/include/lldb/Utility/Either.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Either.h?rev=339429&view=auto
==
--- lldb/trunk/include/lldb/Utility/Either.h (original)
+++ lldb/trunk/include/lldb/Utility/Either.h (removed)
@@ -1,72 +0,0 @@
-//===-- Either.h ---*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef liblldb_Either_h_
-#define liblldb_Either_h_
-
-#include "llvm/ADT/Optional.h"
-
-namespace lldb_utility {
-template
-class Either {
-private:
-enum class Selected {
-One, Two
-};
-
-Selected m_selected;
-union {
-T1 m_t1;
-T2 m_t2;
-};
-
-public:
-Either(const T1& t1)
-{
-m_t1 = t1;
-m_selected = Selected::One;
-}
-
-Either(const T2& t2)
-{
-m_t2 = t2;
-m_selected = Selected::Two;
-}
-
-template ::value>::type * = nullptr >
-llvm::Optional
-GetAs()
-{
-switch (m_selected)
-{
-case Selected::One:
-return m_t1;
-default:
-return llvm::Optional();
-}
-}
-
-template ::value>::type * = nullptr >
-llvm::Optional
-GetAs()
-{
-switch (m_selected)
-{
-case Selected::Two:
-return m_t2;
-default:
-return llvm::Optional();
-}
-}
-};
-
-} // namespace lldb_utility
-
-#endif // #ifndef liblldb_Either_h_
-
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r339328 - Remove unused type Either from Utility library.
You are right, it seems I removed it only in my mind, sorry. Thank you for noting that. > -Original Message- > From: Davide Italiano > Sent: Thursday, 9 August, 2018 6:32 PM > To: tatyana.krasnu...@synopsys.com > Cc: lldb-commits > Subject: Re: [Lldb-commits] [lldb] r339328 - Remove unused type Either from > Utility library. > > On Thu, Aug 9, 2018 at 4:42 AM Tatyana Krasnukha via lldb-commits comm...@lists.llvm.org> wrote: > > > > Author: tkrasnukha > > Date: Thu Aug 9 04:42:28 2018 > > New Revision: 339328 > > > > URL: > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_ll > > vm-2Dproject-3Frev-3D339328-26view- > 3Drev&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWq > > > B0tg&r=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw&m=6p498LDu1k > uOMTB3Z > > jxwhIt- > nLt_qWPfVK472IWRb6U&s=nHSmFMtZaQ9JnQz5TSaOb_coK0Dy6GUCPe68N > yxTc > > KA&e= > > Log: > > Remove unused type Either from Utility library. > > > > I might be missing the obvious here, but it looks like `Either.h` is still > around in > my checkout after you removed it? (also, from the xcodeproj files) > > dcci@Davides-MacBook-Pro ~/w/l/l/t/lldb> ls ./include/lldb/Utility/Either.h > ./include/lldb/Utility/Either.h > > Do you plan to nuke the header from orbit completely? > > -- > Davide ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r339440 - RichManglingContext: Make m_ipd_str_len a local variable and simplify processIPDStrResult + polishing in test and Mangled
Author: stefan.graenitz
Date: Fri Aug 10 08:21:33 2018
New Revision: 339440
URL: http://llvm.org/viewvc/llvm-project?rev=339440&view=rev
Log:
RichManglingContext: Make m_ipd_str_len a local variable and simplify
processIPDStrResult + polishing in test and Mangled
Modified:
lldb/trunk/include/lldb/Core/RichManglingContext.h
lldb/trunk/source/Core/Mangled.cpp
lldb/trunk/source/Core/RichManglingContext.cpp
lldb/trunk/unittests/Core/RichManglingContextTest.cpp
Modified: lldb/trunk/include/lldb/Core/RichManglingContext.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RichManglingContext.h?rev=339440&r1=339439&r2=339440&view=diff
==
--- lldb/trunk/include/lldb/Core/RichManglingContext.h (original)
+++ lldb/trunk/include/lldb/Core/RichManglingContext.h Fri Aug 10 08:21:33 2018
@@ -25,10 +25,9 @@ namespace lldb_private {
/// providers. See Mangled::DemangleWithRichManglingInfo()
class RichManglingContext {
public:
- RichManglingContext()
- : m_provider(None), m_ipd_buf_size(2048), m_ipd_str_len(0) {
+ RichManglingContext() : m_provider(None), m_ipd_buf_size(2048) {
m_ipd_buf = static_cast(std::malloc(m_ipd_buf_size));
-m_ipd_buf[m_ipd_str_len] = '\0';
+m_ipd_buf[0] = '\0';
}
~RichManglingContext() { std::free(m_ipd_buf); }
@@ -65,6 +64,7 @@ public:
/// most recent ParseXy() operation. The next ParseXy() call invalidates it.
llvm::StringRef GetBufferRef() const {
assert(m_provider != None && "Initialize a provider first");
+assert(m_buffer.data() != nullptr && "Parse first");
return m_buffer;
}
@@ -81,7 +81,6 @@ private:
llvm::ItaniumPartialDemangler m_ipd;
char *m_ipd_buf;
size_t m_ipd_buf_size;
- size_t m_ipd_str_len;
/// Members for PluginCxxLanguage
/// Cannot forward declare inner class CPlusPlusLanguage::MethodName. The
Modified: lldb/trunk/source/Core/Mangled.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Mangled.cpp?rev=339440&r1=339439&r2=339440&view=diff
==
--- lldb/trunk/source/Core/Mangled.cpp (original)
+++ lldb/trunk/source/Core/Mangled.cpp Fri Aug 10 08:21:33 2018
@@ -261,9 +261,10 @@ static char *GetMSVCDemangledStr(const c
#endif
}
-static char *GetItaniumDemangledStr(const char *M,
-llvm::ItaniumPartialDemangler &ipd) {
+static char *GetItaniumDemangledStr(const char *M) {
char *demangled_cstr = nullptr;
+
+ llvm::ItaniumPartialDemangler ipd;
bool err = ipd.partialDemangle(M);
if (!err) {
// Default buffer and size (will realloc in case it's too small).
@@ -384,8 +385,7 @@ Mangled::GetDemangledName(lldb::Language
demangled_name = GetMSVCDemangledStr(mangled_name);
break;
case eManglingSchemeItanium: {
-llvm::ItaniumPartialDemangler ipd;
-demangled_name = GetItaniumDemangledStr(mangled_name, ipd);
+demangled_name = GetItaniumDemangledStr(mangled_name);
break;
}
case eManglingSchemeNone:
Modified: lldb/trunk/source/Core/RichManglingContext.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RichManglingContext.cpp?rev=339440&r1=339439&r2=339440&view=diff
==
--- lldb/trunk/source/Core/RichManglingContext.cpp (original)
+++ lldb/trunk/source/Core/RichManglingContext.cpp Fri Aug 10 08:21:33 2018
@@ -89,37 +89,32 @@ bool RichManglingContext::IsFunction() c
}
void RichManglingContext::processIPDStrResult(char *ipd_res, size_t res_size) {
+ // Error case: Clear the buffer.
if (LLVM_UNLIKELY(ipd_res == nullptr)) {
assert(res_size == m_ipd_buf_size &&
"Failed IPD queries keep the original size in the N parameter");
-// Error case: Clear the buffer.
-m_ipd_str_len = 0;
-m_ipd_buf[m_ipd_str_len] = '\0';
- } else {
-// IPD's res_size includes null terminator.
-size_t res_len = res_size - 1;
-assert(ipd_res[res_len] == '\0' &&
- "IPD returns null-terminated strings and we rely on that");
-
-if (LLVM_UNLIKELY(ipd_res != m_ipd_buf)) {
- // Realloc case: Take over the new buffer.
- m_ipd_buf = ipd_res; // std::realloc freed or reused the old buffer.
- m_ipd_buf_size =
- res_size; // Actual buffer may be bigger, but we can't know.
- m_ipd_str_len = res_len;
-
- Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE);
- if (log)
-log->Printf("ItaniumPartialDemangler Realloc: new buffer size %lu",
-m_ipd_buf_size);
-} else {
- // 99% case: Just remember the string length.
- m_ipd_str_len = res_len;
-}
+m_ipd_buf[0] = '\0';
+m_buffer = llvm::StringRef(m_ipd_buf, 0);
+return;
}
- m_buffer = llvm::StringRef(m_ipd_buf, m_ipd_str_len);
[Lldb-commits] [PATCH] D50525: [WIP] Move lldb-mi interpreter tests to LIT
probinson added inline comments. Comment at: lit/tools/lldb-mi/interpreter/cli-support/target-list.test:5 +# We should hardcode executable name since at the moment of running of +# lldb-mi the name must be known. +# RUN: %cxx -o a.exe %p/inputs/main.cpp -g aprantl wrote: > That's totally fine, we just need to choose a name that works on all > platforms, and `a.exe` does. Not `%t/a.exe` ? I thought CWD is sometimes not writeable (or might be the working copy, and we don't want to leave trash there). Repository: rLLDB LLDB https://reviews.llvm.org/D50525 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50536: Fix: ConstString::GetConstCStringAndSetMangledCounterPart() should update the value if the key exists already
friss added inline comments. Comment at: source/Utility/ConstString.cpp:123-126 + assert((map.find(demangled) == map.end() || strlen(map[demangled]) == 0 || + map[demangled] == mangled_ccstr) && + "The demangled string must have a unique counterpart or otherwise " + "it must be empty"); In an asserts build, this is going to d a bunch of additional lookups. Can we move the assert after the try_emplace and basically assert that entry.second == nullptr || !strcmp(mangled_ccstr, entry.second). (It's unclear to me whether a pointer comparison is enough to test string equality in this case, do we know that everything passed to this function has been uniqued?) https://reviews.llvm.org/D50536 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50536: Fix: ConstString::GetConstCStringAndSetMangledCounterPart() should update the value if the key exists already
sgraenitz updated this revision to Diff 160143.
sgraenitz added a comment.
Move assert after try_emplace
https://reviews.llvm.org/D50536
Files:
source/Utility/ConstString.cpp
unittests/Utility/ConstStringTest.cpp
Index: unittests/Utility/ConstStringTest.cpp
===
--- unittests/Utility/ConstStringTest.cpp
+++ unittests/Utility/ConstStringTest.cpp
@@ -18,40 +18,60 @@
}
TEST(ConstStringTest, MangledCounterpart) {
- ConstString foo("foo");
+ ConstString uvw("uvw");
ConstString counterpart;
- EXPECT_FALSE(foo.GetMangledCounterpart(counterpart));
+ EXPECT_FALSE(uvw.GetMangledCounterpart(counterpart));
EXPECT_EQ("", counterpart.GetStringRef());
- ConstString bar;
- bar.SetStringWithMangledCounterpart("bar", foo);
- EXPECT_EQ("bar", bar.GetStringRef());
+ ConstString xyz;
+ xyz.SetStringWithMangledCounterpart("xyz", uvw);
+ EXPECT_EQ("xyz", xyz.GetStringRef());
- EXPECT_TRUE(bar.GetMangledCounterpart(counterpart));
- EXPECT_EQ("foo", counterpart.GetStringRef());
+ EXPECT_TRUE(xyz.GetMangledCounterpart(counterpart));
+ EXPECT_EQ("uvw", counterpart.GetStringRef());
- EXPECT_TRUE(foo.GetMangledCounterpart(counterpart));
- EXPECT_EQ("bar", counterpart.GetStringRef());
+ EXPECT_TRUE(uvw.GetMangledCounterpart(counterpart));
+ EXPECT_EQ("xyz", counterpart.GetStringRef());
+}
+
+TEST(ConstStringTest, UpdateMangledCounterpart) {
+ { // Add counterpart
+ConstString some1;
+some1.SetStringWithMangledCounterpart("some", ConstString(""));
+ }
+ { // Overwrite empty string
+ConstString some2;
+some2.SetStringWithMangledCounterpart("some", ConstString("one"));
+ }
+ { // Overwrite with identical value
+ConstString some2;
+some2.SetStringWithMangledCounterpart("some", ConstString("one"));
+ }
+ { // Check counterpart is set
+ConstString counterpart;
+EXPECT_TRUE(ConstString("some").GetMangledCounterpart(counterpart));
+EXPECT_EQ("one", counterpart.GetStringRef());
+ }
}
TEST(ConstStringTest, FromMidOfBufferStringRef) {
// StringRef's into bigger buffer: no null termination
- const char *buffer = "foobarbaz";
+ const char *buffer = "abcdefghi";
llvm::StringRef foo_ref(buffer, 3);
llvm::StringRef bar_ref(buffer + 3, 3);
ConstString foo(foo_ref);
ConstString bar;
bar.SetStringWithMangledCounterpart(bar_ref, foo);
- EXPECT_EQ("bar", bar.GetStringRef());
+ EXPECT_EQ("def", bar.GetStringRef());
ConstString counterpart;
EXPECT_TRUE(bar.GetMangledCounterpart(counterpart));
- EXPECT_EQ("foo", counterpart.GetStringRef());
+ EXPECT_EQ("abc", counterpart.GetStringRef());
EXPECT_TRUE(foo.GetMangledCounterpart(counterpart));
- EXPECT_EQ("bar", counterpart.GetStringRef());
+ EXPECT_EQ("def", counterpart.GetStringRef());
}
TEST(ConstStringTest, NullAndEmptyStates) {
Index: source/Utility/ConstString.cpp
===
--- source/Utility/ConstString.cpp
+++ source/Utility/ConstString.cpp
@@ -119,11 +119,16 @@
const uint8_t h = hash(demangled);
llvm::sys::SmartScopedWriter wlock(m_string_pools[h].m_mutex);
- // Make string pool entry with the mangled counterpart already set
- StringPoolEntryType &entry =
- *m_string_pools[h]
- .m_string_map.insert(std::make_pair(demangled, mangled_ccstr))
- .first;
+ // Make or update string pool entry with the mangled counterpart
+ StringPool &map = m_string_pools[h].m_string_map;
+ StringPoolEntryType &entry = *map.try_emplace(demangled).first;
+
+ assert((entry.second == nullptr || entry.second == mangled_ccstr ||
+ strlen(entry.second) == 0) &&
+ "The demangled string must have a unique counterpart or otherwise "
+ "it must be empty");
+
+ entry.second = mangled_ccstr;
// Extract the const version of the demangled_cstr
demangled_ccstr = entry.getKeyData();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r339457 - [tests, libstdcxx] Add missing test category on the TestDataFormatterStdUniquePtr tests
Author: stella.stamenova Date: Fri Aug 10 10:52:45 2018 New Revision: 339457 URL: http://llvm.org/viewvc/llvm-project?rev=339457&view=rev Log: [tests, libstdcxx] Add missing test category on the TestDataFormatterStdUniquePtr tests Each test needs to be marked with the add_test_categories decorator individually. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py?rev=339457&r1=339456&r2=339457&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py Fri Aug 10 10:52:45 2018 @@ -66,6 +66,7 @@ class StdUniquePtrDataFormatterTestCase( @skipIfWindows # libstdcpp not ported to Windows @skipIfDarwin # doesn't compile on Darwin @skipIfwatchOS # libstdcpp not ported to watchos +@add_test_categories(["libstdcxx"]) def test_recursive_unique_ptr(self): # Tests that LLDB can handle when we have a loop in the unique_ptr # reference chain and that it correctly handles the different options ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50536: Fix: ConstString::GetConstCStringAndSetMangledCounterPart() should update the value if the key exists already
sgraenitz marked an inline comment as done.
sgraenitz added inline comments.
Comment at: source/Utility/ConstString.cpp:123-126
+ assert((map.find(demangled) == map.end() || strlen(map[demangled]) == 0
||
+ map[demangled] == mangled_ccstr) &&
+ "The demangled string must have a unique counterpart or otherwise
"
+ "it must be empty");
friss wrote:
> In an asserts build, this is going to d a bunch of additional lookups. Can we
> move the assert after the try_emplace and basically assert that entry.second
> == nullptr || !strcmp(mangled_ccstr, entry.second).
>
> (It's unclear to me whether a pointer comparison is enough to test string
> equality in this case, do we know that everything passed to this function has
> been uniqued?)
> Can we move the assert after the try_emplace
Yes, right that's better.
> and basically assert that entry.second == nullptr || !strcmp(mangled_ccstr,
> entry.second)
I think it's worth allowing "overwrite empty string". There is code that does
`my_const_string.SetCString("")` to indicate failure, maybe other code does it
to indicate e.g. later..
> It's unclear to me whether a pointer comparison is enough
I think it's safe to keep it. The `ccstr` postfix is used all over to indicate
that the code assumes a pool string.
`Pool::GetConstCStringAndSetMangledCounterPart` is only used once in
`ConstString` and the `Pool` class is defined locally in the cpp.
https://reviews.llvm.org/D50536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50525: [WIP] Move lldb-mi interpreter tests to LIT
stella.stamenova added inline comments. Comment at: lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test:4 +# +# RUN: %cxx -o %t %p/inputs/main.cpp -g +# RUN: %lldbmi %t < %s | FileCheck %s labath wrote: > aprantl wrote: > > stella.stamenova wrote: > > > apolyakov wrote: > > > > stella.stamenova wrote: > > > > > apolyakov wrote: > > > > > > stella.stamenova wrote: > > > > > > > One thing to consider here is that any extra parameters passed > > > > > > > with -E to the test suite will not propagate to lit at the moment. > > > > > > > > > > > > > > I ran into this with some internal testing where we need to pass > > > > > > > parameters to the compiler - all of the lldb suite tests (c++ and > > > > > > > c) build correctly, but any lit tests that directly invoke the > > > > > > > compiler do not because the parameters do not get propagated all > > > > > > > the way. > > > > > > Could you give an example of extra parameters? I didn't see them > > > > > > before so I don't completely understand you. > > > > > -E "--sysroot=path/to/sys/root -lc++abi -lunwind" > > > > Ok, I think we won't use something like it here. Thank you. > > > I think you misunderstood my concern - let's say I have a machine on > > > which I run these tests for a particular architecture that depends on > > > passing these arguments to the tests, so that clang (cxx) correctly > > > complies c++ files. *Before* your change, these arguments would have been > > > propagated to the test in the lldb suite and the code would have build > > > correctly. *After* your change, the code will no longer build correctly. > > > > > > Essentially, by making these tests lit tests, you are removing support > > > for passing these arguments to the compiler (since the lldb suite > > > supports them and lit does not). It might still be worth making these lit > > > tests for the sake of other benefits, but then such targets will end up > > > having to XFAIL the tests. > > > > > > If these tests really need to become lit tests and invoke the compiler, I > > > think you also need to add support for passing these arguments to the > > > compiler. > > > > > I think the best way to solve this is by adding the platform-specific flags > > to the expansion of `%cxx` in the lit configuration. Would that work here? > I am wondering how much do we actually need the lldb-mi tests to run on > exotic (non-host) platforms/architectures. My take on these tests is that > they should test the functionality from the lldb-mi protocol, and down (only) > to the SB API calls. Anything below SB is a black box, which is assumed to be > working (tested via other means). In particular, these tests should not care > about whether the binary is built with libc++abi or split-dwarf or something > else. The default debuggable executable produced by the given compiler should > be enough. > > In such a world, there is not much value in running the tests on other > architectures, as (hopefully) all of those differences are hidden inside > liblldb. The existing lldb-mi tests already do not support all the features > that other dotest tests do (e.g. running the tests remotely). If getting this > working it's just the matter of adding something to the expansion of %cxx, > then sure, why not... But otherwise I would not spend too much time > engineering a very generic solution. Re: passing the arguments in %cxx This might work, but now we have to pass arguments in two different ways and this will quickly get confusing, especially if there are other tests that need the same. In general, I think having *one* way to pass arguments to the tests is simplest from a user perspective. Re: running the tests on only some platforms As long as the tests don't need to run on all platforms, I think it's fine if they don't support passing additional arguments to the compiler. In that case, I would say the tests should be marked so that they will only run on platforms where we know they will work. Right now there's only a couple of lit LLDB tests that make use of %cxx (and most only run on Windows), but it took me a non-trivial amount of time going through the test infrastructure to figure out when and why arguments were passed (or not). If someone down the line is setting up these tests and they start failing even though they are passing arguments, they'll end up having to redo all of that investigation and there will be no record of it right now. Repository: rLLDB LLDB https://reviews.llvm.org/D50525 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r339473 - Remove copy-pasted and unrelated comment [NFC]
Author: teemperor
Date: Fri Aug 10 14:31:44 2018
New Revision: 339473
URL: http://llvm.org/viewvc/llvm-project?rev=339473&view=rev
Log:
Remove copy-pasted and unrelated comment [NFC]
That comment was copied from the
CombineConsecutiveEntriesWithEqualData() implementation below,
and doesn't actually describe what's happening in the current
function.
Modified:
lldb/trunk/include/lldb/Core/RangeMap.h
Modified: lldb/trunk/include/lldb/Core/RangeMap.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RangeMap.h?rev=339473&r1=339472&r2=339473&view=diff
==
--- lldb/trunk/include/lldb/Core/RangeMap.h (original)
+++ lldb/trunk/include/lldb/Core/RangeMap.h Fri Aug 10 14:31:44 2018
@@ -169,8 +169,6 @@ public:
#ifdef ASSERT_RANGEMAP_ARE_SORTED
bool IsSorted() const {
typename Collection::const_iterator pos, end, prev;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos !=
end;
prev = pos++) {
if (prev != end && *pos < *prev)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50587: Straight forward FastDemangle replacement in SubsPrimitiveParmItanium
sgraenitz created this revision.
sgraenitz added reviewers: erik.pilkington, friss, jingham, JDevlieghere.
Herald added subscribers: chrib, kristof.beyls.
Removing FastDemangle will greatly reduce maintenance efforts. This patch
replaces the last point of use in LLDB. Semantics should be kept intact.
Once this is agreed upon, we can:
- Remove the FastDemangle sources
- Add more features e.g. substitutions in template parameters, considering all
variations, etc.
Depends on LLVM patch https://reviews.llvm.org/D50586
https://reviews.llvm.org/D50587
Files:
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -21,6 +21,7 @@
// Other libraries and framework includes
#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/Demangle.h"
// Project includes
#include "lldb/Core/PluginManager.h"
@@ -30,7 +31,6 @@
#include "lldb/DataFormatters/FormattersHelpers.h"
#include "lldb/DataFormatters/VectorType.h"
#include "lldb/Utility/ConstString.h"
-#include "lldb/Utility/FastDemangle.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/RegularExpression.h"
@@ -278,48 +278,51 @@
static ConstString SubsPrimitiveParmItanium(llvm::StringRef mangled,
llvm::StringRef search,
llvm::StringRef replace) {
- Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
-
- const size_t max_len =
- mangled.size() + mangled.count(search) * replace.size() + 1;
+ struct SwapParms {
+llvm::StringRef mangled;
+llvm::StringRef search;
+llvm::StringRef replace;
+ptrdiff_t read_pos;
+std::back_insert_iterator writer;
+
+SwapParms(llvm::StringRef m, llvm::StringRef s, llvm::StringRef r,
+ std::back_insert_iterator w)
+: mangled(m), search(s), replace(r), read_pos(0), writer(w) {}
+
+void Substitude(llvm::StringRef tail) {
+ if (tail.startswith(search)) {
+ptrdiff_t read_len = tail.data() - mangled.data();
+auto reader = mangled.begin() + read_pos;
+
+// First write the unmatched part of the original. Then write the
+// replacement string. Finally skip the search string in the original.
+writer = std::copy(reader, reader + read_len, writer);
+writer = std::copy(replace.begin(), replace.end(), writer);
+read_pos += read_len + search.size();
+ }
+}
- // Make a temporary buffer to fix up the mangled parameter types and copy the
- // original there
- std::string output_buf;
- output_buf.reserve(max_len);
- output_buf.insert(0, mangled.str());
- ptrdiff_t replaced_offset = 0;
-
- auto swap_parms_hook = [&](const char *parsee) {
-if (!parsee || !*parsee)
- return;
-
-// Check whether we've found a substitutee
-llvm::StringRef s(parsee);
-if (s.startswith(search)) {
- // account for the case where a replacement is of a different length to
- // the original
- replaced_offset += replace.size() - search.size();
-
- ptrdiff_t replace_idx = (mangled.size() - s.size()) + replaced_offset;
- output_buf.erase(replace_idx, search.size());
- output_buf.insert(replace_idx, replace.str());
+static void Hook(void *context, const char *match) {
+ ((SwapParms *)context)->Substitude(llvm::StringRef(match));
}
};
// FastDemangle will call our hook for each instance of a primitive type,
// allowing us to perform substitution
- char *const demangled =
- FastDemangle(mangled.str().c_str(), mangled.size(), swap_parms_hook);
-
- if (log)
-log->Printf("substituted mangling for %s:{%s} %s:{%s}\n",
-mangled.str().c_str(), demangled, output_buf.c_str(),
-FastDemangle(output_buf.c_str()));
- // FastDemangle malloc'd this string.
- free(demangled);
+ std::string output_buf;
+ SwapParms context(mangled, search, replace, std::back_inserter(output_buf));
+ assert(mangled.data()[mangled.size()] == '\0' && "Expect C-String");
+ bool err = llvm::itaniumFindTypesInMangledName(mangled.data(), &context,
+ SwapParms::Hook);
+
+ if (Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE)) {
+if (err)
+ LLDB_LOG(log, "Substituted mangling {0} -> {1}", mangled, output_buf);
+else
+ LLDB_LOG(log, "Failed to substitute mangling in {0}", mangled);
+ }
- return output_buf == mangled ? ConstString() : ConstString(output_buf);
+ return ConstString(output_buf);
}
uint32_t CPlusPlusLanguage::FindAlternateFunctionManglings(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50225: Use a DenseMap for looking up functions by UID in CompileUnit::FindFunctionByUID
teemperor updated this revision to Diff 160214.
teemperor added a comment.
Herald added a subscriber: mgrang.
- Replaced m_functions and instead copy the map to a temporary sorted vector
and iterate over that.
https://reviews.llvm.org/D50225
Files:
include/lldb/Symbol/CompileUnit.h
source/Core/Module.cpp
source/Symbol/CompileUnit.cpp
Index: source/Symbol/CompileUnit.cpp
===
--- source/Symbol/CompileUnit.cpp
+++ source/Symbol/CompileUnit.cpp
@@ -22,7 +22,7 @@
lldb::LanguageType language,
lldb_private::LazyBool is_optimized)
: ModuleChild(module_sp), FileSpec(pathname, false), UserID(cu_sym_id),
- m_user_data(user_data), m_language(language), m_flags(0), m_functions(),
+ m_user_data(user_data), m_language(language), m_flags(0),
m_support_files(), m_line_table_ap(), m_variables(),
m_is_optimized(is_optimized) {
if (language != eLanguageTypeUnknown)
@@ -35,7 +35,7 @@
lldb::LanguageType language,
lldb_private::LazyBool is_optimized)
: ModuleChild(module_sp), FileSpec(fspec), UserID(cu_sym_id),
- m_user_data(user_data), m_language(language), m_flags(0), m_functions(),
+ m_user_data(user_data), m_language(language), m_flags(0),
m_support_files(), m_line_table_ap(), m_variables(),
m_is_optimized(is_optimized) {
if (language != eLanguageTypeUnknown)
@@ -66,6 +66,21 @@
<< (const FileSpec &)*this << "\", language = \"" << language << '"';
}
+void CompileUnit::ForeachFunction(llvm::function_ref lambda) {
+ std::vector sorted_functions;
+ sorted_functions.reserve(m_functions_by_uid.size());
+ for (auto &p : m_functions_by_uid)
+sorted_functions.push_back(p.second);
+ std::sort(sorted_functions.begin(), sorted_functions.end(),
+[](lldb::FunctionSP a, lldb::FunctionSP b) {
+ return a->GetID() < b->GetID();
+});
+
+ for (auto &f : sorted_functions)
+if (lambda(f))
+ return;
+}
+
//--
// Dump the current contents of this object. No functions that cause on demand
// parsing of functions, globals, statics are called, so this is a good
@@ -89,13 +104,10 @@
s->IndentLess();
}
- if (!m_functions.empty()) {
+ if (!m_functions_by_uid.empty()) {
s->IndentMore();
-std::vector::const_iterator pos;
-std::vector::const_iterator end = m_functions.end();
-for (pos = m_functions.begin(); pos != end; ++pos) {
- (*pos)->Dump(s, show_context);
-}
+ForeachFunction(
+[&s, show_context](lldb::FunctionSP f) { f->Dump(s, show_context); });
s->IndentLess();
s->EOL();
@@ -106,15 +118,7 @@
// Add a function to this compile unit
//--
void CompileUnit::AddFunction(FunctionSP &funcSP) {
- // TODO: order these by address
- m_functions.push_back(funcSP);
-}
-
-FunctionSP CompileUnit::GetFunctionAtIndex(size_t idx) {
- FunctionSP funcSP;
- if (idx < m_functions.size())
-funcSP = m_functions[idx];
- return funcSP;
+ m_functions_by_uid[funcSP->GetID()] = funcSP;
}
//--
@@ -163,18 +167,10 @@
//}
FunctionSP CompileUnit::FindFunctionByUID(lldb::user_id_t func_uid) {
- FunctionSP funcSP;
- if (!m_functions.empty()) {
-std::vector::const_iterator pos;
-std::vector::const_iterator end = m_functions.end();
-for (pos = m_functions.begin(); pos != end; ++pos) {
- if ((*pos)->GetID() == func_uid) {
-funcSP = *pos;
-break;
- }
-}
- }
- return funcSP;
+ auto it = m_functions_by_uid.find(func_uid);
+ if (it == m_functions_by_uid.end())
+return FunctionSP();
+ return it->second;
}
lldb::LanguageType CompileUnit::GetLanguage() {
Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -371,15 +371,13 @@
symbols->ParseCompileUnitFunctions(sc);
- for (size_t func_idx = 0;
- (sc.function = sc.comp_unit->GetFunctionAtIndex(func_idx).get()) !=
- nullptr;
- ++func_idx) {
+ sc.comp_unit->ForeachFunction([&sc, &symbols](lldb::FunctionSP f) {
+sc.function = f.get();
symbols->ParseFunctionBlocks(sc);
-
// Parse the variables for this function and all its blocks
symbols->ParseVariablesForContext(sc);
- }
+return false;
+ });
// Parse all types for this compile unit
sc.function = nullptr;
Index: include/lldb/Symbol/CompileUnit.h
===
--- include/lldb/Symbol/CompileUnit.h
+++ include/lldb/Symbol/CompileUnit.h
@@ -18,6 +18,8 @@
#include "lldb/Utility/UserID.h"
#inc
[Lldb-commits] [PATCH] D50225: Use a DenseMap for looking up functions by UID in CompileUnit::FindFunctionByUID
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM! https://reviews.llvm.org/D50225 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: WIP: Basic tail call frame support
vsk updated this revision to Diff 160224.
vsk added a comment.
Herald added a subscriber: mgrang.
Rebase, and update the patch to use DW_AT_call_return_pc information.
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/TODO
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail2.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail3.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail4.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail5.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/test.sh
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -27,6 +27,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/Unwind.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/SmallPtrSet.h"
//#define DEBUG_STACK_FRAMES 1
@@ -240,6 +241,134 @@
m_frames.resize(num_frames);
}
+/// Find the unique path through the call graph from \p begin (at PC offset
+/// \p call_pc) to \p end. On success this path is stored into \p path, and on
+/// failure \p path is unchanged.
+static void FindInterveningFrames(Function &begin, Function &end,
+ addr_t call_pc, std::vector &path,
+ ModuleList &images, Log *log) {
+ LLDB_LOG(log, "Finding frames between {0} and {1}, call-pc={2:x}",
+ begin.GetDisplayName(), end.GetDisplayName(), call_pc);
+
+ // Find a non-tail calling edge with the first return PC greater than the
+ // call PC.
+ auto first_level_edges = begin.GetCallEdges();
+ auto first_edge_it =
+ std::lower_bound(first_level_edges.begin(), first_level_edges.end(),
+ call_pc, [=](const CallEdge &edge, addr_t target_pc) {
+ return edge.GetReturnPCAddress() < target_pc;
+ });
+ if (first_edge_it == first_level_edges.end())
+return;
+ CallEdge &first_edge = const_cast(*first_edge_it);
+ if (first_edge.GetReturnPCAddress() == LLDB_INVALID_ADDRESS)
+return;
+
+ // The first callee may not be resolved, or there may be nothing to fill in.
+ Function *first_callee = first_edge.GetCallee(images);
+ if (!first_callee || first_callee == &end)
+return;
+
+ // Run DFS on the tail-calling edges out of the first callee to find \p end.
+ struct DFS {
+std::vector active_path = {};
+llvm::SmallPtrSet visited_nodes = {};
+bool ambiguous = false;
+Function *end;
+ModuleList &images;
+
+DFS(Function *end, ModuleList &images) : end(end), images(images) {}
+
+void search(Function *first_callee, std::vector &path) {
+ if (dfs(first_callee) && !ambiguous)
+path = std::move(active_path);
+}
+
+bool dfs(Function *callee) {
+ if (callee == end)
+return true;
+
+ // Terminate the search when tail recursion is detected.
+ if (!visited_nodes.insert(callee).second) {
+ambiguous = true;
+return false;
+ }
+
+ active_path.push_back(callee);
+ for (CallEdge &edge : callee->GetTailCallingEdges()) {
+Function *next_callee = edge.GetCallee(images);
+if (!next_callee)
+ continue;
+
+if (dfs(next_callee) && !ambiguous)
+ return true;
+
+if (ambiguous)
+ return false;
+ }
+ active_path.pop_back();
+ return false;
+}
+ };
+
+ DFS(&end, images).search(first_callee, path);
+}
+
+/// Given that \p next_frame will be appended to the frame list, synthesize
+/// tail call frames between the current end of the list and \p next_frame.
+/// If any frames are added, adjust the frame index of \p next_frame.
+void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) {
+ lldb::RegisterContextSP next_reg_ctx_sp = next_frame.GetRegisterContext();
+ if (!next_reg_ctx_sp)
+return;
+
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
+
+ assert(!m_frames.empty() && "Cannot synthesize frames in an empty stack");
+ StackFrame &prev_frame = *m_frames.back().get();
+
+ // Find the functions prev_frame and next_frame are stopped in. The function
+ // objects are needed to search the lazy call graph for intervening frames.
+ Function *prev_func =
+ prev_frame.G
