[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature
labath added a comment. I don't really like that we are adding a public shared library for every tiny intel feature. Could we at least merge this "plugin" with the existing "intel-mpx plugin" to create one "intel support" library? Also, adding an external dependency probably deserves a discussion on lldb-dev. Comment at: tools/CMakeLists.txt:8 add_subdirectory(lldb-mi) +option(LLDB_BUILD_INTEL_PT "Enable Building of Intel(R) Processor Trace Tool" OFF) +if (LLDB_BUILD_INTEL_PT) clayborg wrote: > Can we default this to enabled? We probably can't, as this code depends on a third party library. In any case, this option should go to LLDBConfig.cmake Comment at: tools/intel-pt/CMakeLists.txt:42 + +add_library(lldbIntelPT SHARED + PTDecoder.cpp any reason you're not using add_lldb_library here? Comment at: tools/intel-pt/CMakeLists.txt:53 + +if (NOT LLDB_DISABLE_PYTHON) + target_link_libraries(lldbIntelPT PRIVATE All of this needs to go away. I think you only needed it because you are plucking NativeProcessLinux internals, so fixing that should fix this too. https://reviews.llvm.org/D33035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature
abhishek.aggarwal added inline comments.
Comment at: tools/CMakeLists.txt:8
add_subdirectory(lldb-mi)
+option(LLDB_BUILD_INTEL_PT "Enable Building of Intel(R) Processor Trace Tool"
OFF)
+if (LLDB_BUILD_INTEL_PT)
clayborg wrote:
> Can we default this to enabled?
The tool has a dependency on a decoder library. People who are not interested
in building this tool will have to explicitly set this flag OFF to avoid build
failures caused by absence of this decoder library on their system. I wanted to
avoid that. But if you think enabling it by default is a better idea then I
will change it.
Comment at: tools/intel-pt/Decoder.cpp:18
+// Other libraries and framework includes
+#include "../../source/Plugins/Process/Linux/NativeProcessLinux.h"
+#include "lldb/API/SBModule.h"
clayborg wrote:
> This kind of reach in to internal plug-in sources shouldn't happen as it
> violates the boundaries. Remove and see comment below.
I am removing this include from here.
Comment at: tools/intel-pt/Decoder.cpp:27
+ std::string &result) {
+ uint32_t error_code = sberror.GetError();
+ switch (error_code) {
clayborg wrote:
> We really shouldn't be interpreting integer error codes here. The string in
> the "sberror" should be what you use. Modify the code that creates these
> errors in the first place to also put a string values you have here into the
> lldb_private::Error to begin with and remove this function.
Removing this function.
Comment at: tools/intel-pt/Decoder.cpp:177
+ sberror.Clear();
+ CheckDebuggerID(sbprocess, sberror);
+ if (!sberror.Success())
clayborg wrote:
> Why do we care which debugger this belongs to?
Please see my reply to your comment in Decoder.h file.
Comment at: tools/intel-pt/Decoder.cpp:200
+
+ const char *custom_trace_params = s.GetData();
+ std::string trace_params(custom_trace_params);
clayborg wrote:
> We meed to add API to SBStructuredData to expose some of the stuff from
> lldb_private::StructuredData so you don't end up text scraping here. Just do
> what you need for now but I would suggest at the very least:
>
> ```
> class SBStructuredData {
> enum Type {
> eTypeArray,
> eTypeDictionary,
> eTypeString,
> eTypeInteger,
> eTypeBoolean
> }
> // Get the type of data in this structured data.
> Type GetType();
> // Get a dictionary for a key value given a key from the top level of this
> structured data if type is eTypeDictionary
> SBStructuredData GetValueForKey(const char *key);
> // Get the value of this object as a string if type is eTypeString
> bool GetStringValue(char *s, size_t max_len);
> // Get an integer of this object as an integer if type is eTypeInteger
> bool GetIntegerValue(uint64_t &value);
> ...
> };
> ```
> See cleanup code below if we do this.
I am on it.
Comment at: tools/intel-pt/Decoder.cpp:538-547
+ const char *custom_trace_params = s.GetData();
+ if (!custom_trace_params || (s.GetSize() == 0)) {
+sberror.SetErrorStringWithFormat("lldb couldn't provide cpuinfo");
+return;
+ }
+
+ long int family = 0, model = 0, stepping = 0, vendor = 0;
clayborg wrote:
> No text scraping. Please use SBStructureData methods that we will need to add.
Working on it.
Comment at: tools/intel-pt/Decoder.cpp:602-603
+
+void Decoder::Parse(const std::string &trace_params, const std::string &key,
+lldb::SBError &sberror, std::string &result) {
+ std::string key_value_pair_separator(",");
clayborg wrote:
> Remove this function, add real API to SBStructuredData
Working on it.
Comment at: tools/intel-pt/Decoder.cpp:1046-1047
+// SBDebugger with which this tool instance is associated.
+void Decoder::CheckDebuggerID(lldb::SBProcess &sbprocess,
+ lldb::SBError &sberror) {
+ if (!sbprocess.IsValid()) {
clayborg wrote:
> Remove this function?
Please see reply to your comment in Decoder.h file.
Comment at: tools/intel-pt/Decoder.h:86
+//--
+class Info : public lldb::SBTraceOptions {
+public:
clayborg wrote:
> Should this be named better? IntelTraceOptions? Also does it need to inherit
> from lldb::SBTraceOptions? Would it make more sense to have this be a "has a"
> where SBTraceOptions is a member variable?
1. I am changing its name to "TraceOptions".
2. Inheriting it from SBTraceOptions makes life easier in the sense of reusing
the APIs without re-defining them here. This is a backend class. Exposing any
new API in PTInfo class (in case SBTraceOptions class undergo any
change/addition in future) will require change only in PTDecoder.h file and
corresponding implementat
[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types
labath added a comment. As this is an objective C feature, wouldn't a better place for it be in testcases/lang/objc ? Comment at: packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/Makefile:10 + +include $(LEVEL)/Makefile.rules This looks like a copy-paste error, as you have everything twice. Repository: rL LLVM https://reviews.llvm.org/D33077 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32899: [RuntimeDyld] Fix debug section relocation (pr20457)
labath updated this revision to Diff 98967.
labath added a comment.
Herald added a subscriber: krytarowski.
Add llvm-rtdyld test case
https://reviews.llvm.org/D32899
Files:
lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
test/ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_debug_frame.s
Index: test/ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_debug_frame.s
===
--- /dev/null
+++ test/ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_debug_frame.s
@@ -0,0 +1,20 @@
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj -o
%T/ELF_x86-64_debug_frame.o %s
+# RUN: llvm-rtdyld -triple=x86_64-pc-linux -verify -check=%s
%T/ELF_x86-64_debug_frame.o
+
+.text
+.file "debug_frame_test.c"
+.align 16, 0x90
+.type foo,@function
+foo:
+.cfi_startproc
+retq
+.Ltmp0:
+.size foo, .Ltmp0-foo
+.cfi_endproc
+.cfi_sections .debug_frame
+
+# Check that .debug_frame is mapped to 0.
+# rtdyld-check: section_addr(ELF_x86-64_debug_frame.o, .debug_frame) = 0
+
+# Check that The relocated FDE's CIE offset also points to zero.
+# rtdyld-check: *{4}(section_addr(ELF_x86-64_debug_frame.o, .debug_frame) +
0x1C) = 0
Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
===
--- lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
+++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
@@ -705,7 +705,7 @@
unsigned Alignment = (unsigned)Alignment64 & 0xL;
unsigned PaddingSize = 0;
unsigned StubBufSize = 0;
- bool IsRequired = isRequiredForExecution(Section) || ProcessAllSections;
+ bool IsRequired = isRequiredForExecution(Section);
bool IsVirtual = Section.isVirtual();
bool IsZeroInit = isZeroInit(Section);
bool IsReadOnly = isReadOnlyData(Section);
@@ -745,8 +745,8 @@
Alignment = std::max(Alignment, getStubAlignment());
// Some sections, such as debug info, don't need to be loaded for execution.
- // Leave those where they are.
- if (IsRequired) {
+ // Process those only if explicitly requested.
+ if (IsRequired || ProcessAllSections) {
Allocate = DataSize + PaddingSize + StubBufSize;
if (!Allocate)
Allocate = 1;
@@ -790,6 +790,10 @@
Sections.push_back(
SectionEntry(Name, Addr, DataSize, Allocate, (uintptr_t)pData));
+ // Debug info sections are linked as if their load address was zero
+ if (!IsRequired)
+Sections.back().setLoadAddress(0);
+
if (Checker)
Checker->registerSection(Obj.getFileName(), SectionID);
Index: test/ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_debug_frame.s
===
--- /dev/null
+++ test/ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_debug_frame.s
@@ -0,0 +1,20 @@
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj -o %T/ELF_x86-64_debug_frame.o %s
+# RUN: llvm-rtdyld -triple=x86_64-pc-linux -verify -check=%s %T/ELF_x86-64_debug_frame.o
+
+.text
+.file "debug_frame_test.c"
+.align 16, 0x90
+.type foo,@function
+foo:
+.cfi_startproc
+retq
+.Ltmp0:
+.size foo, .Ltmp0-foo
+.cfi_endproc
+.cfi_sections .debug_frame
+
+# Check that .debug_frame is mapped to 0.
+# rtdyld-check: section_addr(ELF_x86-64_debug_frame.o, .debug_frame) = 0
+
+# Check that The relocated FDE's CIE offset also points to zero.
+# rtdyld-check: *{4}(section_addr(ELF_x86-64_debug_frame.o, .debug_frame) + 0x1C) = 0
Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
===
--- lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
+++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
@@ -705,7 +705,7 @@
unsigned Alignment = (unsigned)Alignment64 & 0xL;
unsigned PaddingSize = 0;
unsigned StubBufSize = 0;
- bool IsRequired = isRequiredForExecution(Section) || ProcessAllSections;
+ bool IsRequired = isRequiredForExecution(Section);
bool IsVirtual = Section.isVirtual();
bool IsZeroInit = isZeroInit(Section);
bool IsReadOnly = isReadOnlyData(Section);
@@ -745,8 +745,8 @@
Alignment = std::max(Alignment, getStubAlignment());
// Some sections, such as debug info, don't need to be loaded for execution.
- // Leave those where they are.
- if (IsRequired) {
+ // Process those only if explicitly requested.
+ if (IsRequired || ProcessAllSections) {
Allocate = DataSize + PaddingSize + StubBufSize;
if (!Allocate)
Allocate = 1;
@@ -790,6 +790,10 @@
Sections.push_back(
SectionEntry(Name, Addr, DataSize, Allocate, (uintptr_t)pData));
+ // Debug info sections are linked as if their load address was zero
+ if (!IsRequired)
+Sections.back().setLoadAddress(0);
+
if (Checker)
Checker->registerSection(Obj.getFileName(), SectionID);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
[Lldb-commits] [PATCH] D32899: [RuntimeDyld] Fix debug section relocation (pr20457)
labath added a comment. Thanks for the help, this definitely looks much better. :) Since I already have it written, would you still be interested in the ProcessAllSections MCJIT test by any chance? I noticed that you none of the MCJIT tests set ProcessAllSections=true, or use Modules with debug info, so I think it has some value for smoke testing parts these parts of the pipeline (and it is nowhere near as ugly as the other create-an-.o-file-by hand test :) ). https://reviews.llvm.org/D32899 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32295: [RFC] Fix crash in expression evaluation
labath abandoned this revision. labath added a comment. Fix is in https://reviews.llvm.org/D32899. https://reviews.llvm.org/D32295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
labath added a comment.
In https://reviews.llvm.org/D32585#752115, @ravitheja wrote:
> In https://reviews.llvm.org/D32585#740632, @labath wrote:
>
> > I quite like that you have added just the packet plumbing code without an
> > concrete implementation. However, that is still a significant amount of
> > parsing code that should be accompanied by a test. The test suite for the
> > client side of the protocol is ready (TestGdbRemoteCommunicationClient), so
> > I'd like to see at least that.
>
>
> @labath I was considering writing Unit tests for the remote packets but I
> thought then I have to write the mock implementation for the trace operations
> as well, which might end up being a bigger piece of code than the actual
> packet parsing code.
I don't think that is the case. If you look at the tests in
TestGdbRemoteCommunicationClient.cpp, all of them are quite simple. E.g. a test
for TestGdbRemoteCommunicationClient could be something like:
TraceOptions opt;
opt.setType(...);
opt.setBufferSize(...);
opt.setMetaBufferSize(); // with an appropriate TraceOptions constructor,
this could be a one-liner
std::future result = std::async(std::launch::async, [&] {
return client.StartTrace(opt, error);
});
HandlePacket(server,
"JTrace:start:type:dead:buffersize:beef:metabuffersize:baad:threadid:f00d:jparams:...",
"47");
ASSERT_EQ(47, result.get());
ASSERT_TRUE(error.Success());
This depends on the packet code being in GdbRemoteCommunicationClient and not
ProcessGdbRemote as it is now, but moving it there is a good idea anyway --
ProcessGdbRemote should do higher level stuff, packet parsing should be left in
the GDBRCClient.
The server side code unfortunately cannot be tested this way, but I believe I
will get around to enabling that soon as well.
> After this patch, I will upload the actual server side code doing the trace
> stuff for linux, that part of code has unit tests for some core functions.
The problem with that test vector is that the test will only run on linux, and
only on processors that support the feature, which means there's a very high
chance it will end up being dead code (my guesstimate is our bot does not have
the required hardware feature). What is also quite difficult to test with that
approach is the failure cases (what happens if the server sends a malformed
packed or the response misses some data, etc.).
Comment at: include/lldb/Utility/StringExtractor.h:114
+ bool Consume_front(const llvm::StringRef &str);
+
The name looks wrong. Either use `CamelCase` or `snake_case`, don't try to mix
the two. (I'd propose camel case).
https://reviews.llvm.org/D32585
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
labath added a comment.
Is this feature really darwin specific? Isn't the `__private_extern__` thingy
equivalent to `__attribute__(visibility("hidden")))`, which is supported by gcc
and clang alike?
Comment at: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile:1
+LEVEL = ../../../make
+
For a portable way to write a Makefile with multiple shared libraries, please
look at `testcases/lang/cpp/namespace_definitions`. The solution is not very
elegant, but at least it avoid hardcoding `.dylib` everywhere.
https://reviews.llvm.org/D33083
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.
labath added a comment.
A bunch more pedantic comments from me.
Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:26
+ for (int i = 0; i < thread_count; i++) {
+threads.push_back(std::thread([&delay]{while(delay);}));
+ }
Could you add a (e.g. 1 second) sleep into the loop, to avoid the threads
hogging the cpu.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:10
+
+#include
+#include
Do you still need these includes?
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:55
+.Case("big", support::big)
+.Default(support::native);
+
This should probably be a parsing error instead. Having the server tell us "my
endianness is `native`" is not really helpful :).
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:92
+ StructuredData::Array* array = json->GetAsArray();
+ if (!array) {
+return make_error("JThreadsInfo: JSON array");
If you're not too attached to curly braces, you can save some space here by
dropping them. They have some value when the statement is complex, but for
one-line bodies like this they just add clutter.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:197
+ unsigned int register_id = 0;
+ while (true) {
+std::stringstream ss;
There's no guarantee lldb-server will send the register numbers in sequence. In
the current implementation it expedites general purpose registers (which also
happen to be the lowest numbered registers on x86 at least, but I am not sure
if that is the case on all architectures). You would be better of looping
through the key-value pairs and checking whether the key is a number (that's
what the real client does).
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:199
+std::stringstream ss;
+ss << std::hex << std::setw(2) << std::setfill('0') << register_id;
+std::string hex_id = ss.str();
llvm::formatv or something similar
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:10
+
+#include
+#include
System includes should go last. (If you delete the empty lines between the
include blocks, clang-format will take care of the order for you).
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:20
+
+namespace CommunicationTests {
+ class ThreadInfo;
our namespaces tend to be `snake_cased` for some reason. I'm not entirely happy
with the "communication" in the name, as they test much more than just
communication. I guess I'd call it `llgs_test`, even though it's not entirely
correct (but it's shorter!), but I don't feel about that strongly.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:25
+
+class ParsingError : public llvm::ErrorInfo {
+public:
If we're only going to be using this type for printing errors to the user
(which seems to be the case now), we might as well use llvm::StringError
(possibly with a custom factory function which would insert the "argument was
invalid" stuff and such for brevity). What do you think?
I am asking that because I am not sure if all errors we encounter can be
described as "parsing errors", and I wanted to avoid defining a bunch of
additional error types, if we don't need it. If you see a use for that, then
fine.
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:50
+
+protected:
+ ProcessInfo();
Using protected seems to hint that this class can be inherited from, which is
generally a bad idea without providing a virtual destructor. (I'm not sure why
would anyone want to inherit from this)
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:54
+private:
+ pid_t pid;
+ pid_t parent_pid;
This type will not exist on windows. We can use lldb::pid_t instead. There is
no lldb::uid_t and gid_t equivalent though (lldb seems to use simply uint32_t
for these).
Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:33
+namespace CommunicationTests {
+const char* LOCALHOST = "127.0.0.1";
+
`static const char g_local_host[] = ...; ` (static because the variable is
file-local, [] avoids an extra indirection and makes the variable really const,
the variable name is debatable, but it definitely shouldn't be all caps).
Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:38
+: test_name(test_name), test_case_name(test_case_name), pc_register(UINT_MAX) {
+ HostInfo::Initialize();
+}
`HostInfo::Initialize` belongs to the SetUpTestCase (or possibly SetUp) of the
fixture for these tests (that's how all other of our tests handle this).
Comment at: u
[Lldb-commits] [PATCH] D33167: Get rid of some uses of StringConvert and reduce some indentation
labath added a comment.
I think this is a step in the right direction. Besides reducing boilerplate,
this will also help us ensure correctness, as we get a constant trickle of bug
reports for commands which forgot to set the result status.
The name is not ideal, but it's probably the best we can get. (The ideal
solution for me would be to get rid of the duality in the DoExecute function --
e.g. remove the bool return and let the execution state be fully signalled by
the result object -- but that's way off from what you were originally trying to
do).
That said, I'm not sure I should be the person approving this. :)
Comment at: lldb/source/Commands/CommandObjectPlatform.cpp:611
+if (!platform_sp->CloseFile(fd, error))
+ return result.AppendError(eReturnStatusFailed, error.AsCString());
+
I believe this should be `"{0}", error` to avoid hitting problems in case the
error contains '{'. (Plus it's 5 chars shorter :) )
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1181
+"Failed to send signal {0}: {1}\n", signo,
+ST.AsCString());
+
AsCString unnecessary
https://reviews.llvm.org/D33167
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32823: Remove an expensive lock from Timer
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303058: Remove an expensive lock from Timer (authored by
labath).
Changed prior to commit:
https://reviews.llvm.org/D32823?vs=97973&id=98994#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32823
Files:
lldb/trunk/include/lldb/Core/Timer.h
lldb/trunk/source/API/SystemInitializerFull.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Core/Disassembler.cpp
lldb/trunk/source/Core/Mangled.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/Timer.cpp
lldb/trunk/source/Host/common/Symbols.cpp
lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
lldb/trunk/source/Symbol/ObjectFile.cpp
lldb/trunk/source/Symbol/Symtab.cpp
lldb/trunk/source/Target/Target.cpp
lldb/trunk/source/Target/TargetList.cpp
lldb/trunk/unittests/Core/TimerTest.cpp
Index: lldb/trunk/include/lldb/Core/Timer.h
===
--- lldb/trunk/include/lldb/Core/Timer.h
+++ lldb/trunk/include/lldb/Core/Timer.h
@@ -37,10 +37,23 @@
class Timer {
public:
+ class Category {
+ public:
+explicit Category(const char *category_name);
+
+ private:
+friend class Timer;
+const char *m_name;
+std::atomic m_nanos;
+std::atomic m_next;
+
+DISALLOW_COPY_AND_ASSIGN(Category);
+ };
+
//--
/// Default constructor.
//--
- Timer(const char *category, const char *format, ...)
+ Timer(Category &category, const char *format, ...)
__attribute__((format(printf, 3, 4)));
//--
@@ -62,7 +75,7 @@
using TimePoint = std::chrono::steady_clock::time_point;
void ChildDuration(TimePoint::duration dur) { m_child_duration += dur; }
- const char *m_category;
+ Category &m_category;
TimePoint m_total_start;
TimePoint::duration m_child_duration{0};
Index: lldb/trunk/unittests/Core/TimerTest.cpp
===
--- lldb/trunk/unittests/Core/TimerTest.cpp
+++ lldb/trunk/unittests/Core/TimerTest.cpp
@@ -18,7 +18,8 @@
TEST(TimerTest, CategoryTimes) {
Timer::ResetCategoryTimes();
{
-Timer t("CAT1", "");
+static Timer::Category tcat("CAT1");
+Timer t(tcat, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
StreamString ss;
@@ -32,25 +33,31 @@
TEST(TimerTest, CategoryTimesNested) {
Timer::ResetCategoryTimes();
{
-Timer t1("CAT1", "");
+static Timer::Category tcat1("CAT1");
+Timer t1(tcat1, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
-Timer t2("CAT1", "");
+// Explicitly testing the same category as above.
+Timer t2(tcat1, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
StreamString ss;
Timer::DumpCategoryTimes(&ss);
double seconds;
+ // It should only appear once.
+ ASSERT_EQ(ss.GetString().count("CAT1"), 1U);
ASSERT_EQ(1, sscanf(ss.GetData(), "%lf sec for CAT1", &seconds));
EXPECT_LT(0.002, seconds);
EXPECT_GT(0.2, seconds);
}
TEST(TimerTest, CategoryTimes2) {
Timer::ResetCategoryTimes();
{
-Timer t1("CAT1", "");
+static Timer::Category tcat1("CAT1");
+Timer t1(tcat1, "");
std::this_thread::sleep_for(std::chrono::milliseconds(100));
-Timer t2("CAT2", "");
+static Timer::Category tcat2("CAT2");
+Timer t2(tcat2, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
StreamString ss;
Index: lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
===
--- lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
+++ lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
@@ -301,8 +301,9 @@
[Lldb-commits] [lldb] r303058 - Remove an expensive lock from Timer
Author: labath
Date: Mon May 15 08:02:37 2017
New Revision: 303058
URL: http://llvm.org/viewvc/llvm-project?rev=303058&view=rev
Log:
Remove an expensive lock from Timer
The Timer destructor would grab a global mutex in order to update
execution time. Add a class to define a category once, statically; the
class adds itself to an atomic singly linked list, and thus subsequent
updates only need to use an atomic rather than grab a lock and perform a
hashtable lookup.
Differential Revision: https://reviews.llvm.org/D32823
Patch by Scott Smith .
Modified:
lldb/trunk/include/lldb/Core/Timer.h
lldb/trunk/source/API/SystemInitializerFull.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Core/Disassembler.cpp
lldb/trunk/source/Core/Mangled.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/Timer.cpp
lldb/trunk/source/Host/common/Symbols.cpp
lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/trunk/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp
lldb/trunk/source/Symbol/ObjectFile.cpp
lldb/trunk/source/Symbol/Symtab.cpp
lldb/trunk/source/Target/Target.cpp
lldb/trunk/source/Target/TargetList.cpp
lldb/trunk/unittests/Core/TimerTest.cpp
Modified: lldb/trunk/include/lldb/Core/Timer.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Timer.h?rev=303058&r1=303057&r2=303058&view=diff
==
--- lldb/trunk/include/lldb/Core/Timer.h (original)
+++ lldb/trunk/include/lldb/Core/Timer.h Mon May 15 08:02:37 2017
@@ -37,10 +37,23 @@ namespace lldb_private {
class Timer {
public:
+ class Category {
+ public:
+explicit Category(const char *category_name);
+
+ private:
+friend class Timer;
+const char *m_name;
+std::atomic m_nanos;
+std::atomic m_next;
+
+DISALLOW_COPY_AND_ASSIGN(Category);
+ };
+
//--
/// Default constructor.
//--
- Timer(const char *category, const char *format, ...)
+ Timer(Category &category, const char *format, ...)
__attribute__((format(printf, 3, 4)));
//--
@@ -62,7 +75,7 @@ protected:
using TimePoint = std::chrono::steady_clock::time_point;
void ChildDuration(TimePoint::duration dur) { m_child_duration += dur; }
- const char *m_category;
+ Category &m_category;
TimePoint m_total_start;
TimePoint::duration m_child_duration{0};
Modified: lldb/trunk/source/API/SystemInitializerFull.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SystemInitializerFull.cpp?rev=303058&r1=303057&r2=303058&view=diff
==
--- lldb/trunk/source/API/SystemInitializerFull.cpp (original)
+++ lldb/trunk/source/API/SystemInitializerFull.cpp Mon May 15 08:02:37 2017
@@ -400,7 +400,8 @@ void SystemInitializerFull::InitializeSW
}
void SystemInitializerFull::Terminate() {
- Timer scoped_timer(LLVM_PRETTY_FUNCTION, LLVM_PRETTY_FUNCTION);
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
Debugger::SettingsTerminate();
Modified: lldb/trunk/source/Commands/CommandObjectTarget.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectTarget.cpp?rev=303058&r1=303057&r2=303058&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectTarget.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectTarget.cpp Mon May 15 08:02:37 2017
@@ -269,8 +269,8 @@ protected:
}
const char *file_path = command.GetArgumentAtIndex(0);
- Timer scoped_timer(LLVM_PRETTY_FUNCTION, "(lldb) target create '%s'",
- file_path);
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scope
[Lldb-commits] [lldb] r303061 - Fix darwin build for r303058
Author: labath Date: Mon May 15 08:41:38 2017 New Revision: 303061 URL: http://llvm.org/viewvc/llvm-project?rev=303061&view=rev Log: Fix darwin build for r303058 Modified: lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp Modified: lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp?rev=303061&r1=303060&r2=303061&view=diff == --- lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp (original) +++ lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp Mon May 15 08:41:38 2017 @@ -113,7 +113,8 @@ SymbolVendorMacOSX::CreateInstance(const if (obj_name != obj_file_macho) return NULL; - Timer scoped_timer(LLVM_PRETTY_FUNCTION, + static Timer::Category func_cat(LLVM_PRETTY_FUNCTION); + Timer scoped_timer(func_cat, "SymbolVendorMacOSX::CreateInstance (module = %s)", module_sp->GetFileSpec().GetPath().c_str()); SymbolVendorMacOSX *symbol_vendor = new SymbolVendorMacOSX(module_sp); @@ -122,8 +123,10 @@ SymbolVendorMacOSX::CreateInstance(const path[0] = '\0'; // Try and locate the dSYM file on Mac OS X +static Timer::Category func_cat2( +"SymbolVendorMacOSX::CreateInstance() locate dSYM"); Timer scoped_timer2( -"SymbolVendorMacOSX::CreateInstance () locate dSYM", +func_cat2, "SymbolVendorMacOSX::CreateInstance (module = %s) locate dSYM", module_sp->GetFileSpec().GetPath().c_str()); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D33167: Get rid of some uses of StringConvert and reduce some indentation
Indeed, ideally there is no bool return value and we just write return
make_error("foo")
On Mon, May 15, 2017 at 5:58 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath added a comment.
>
> I think this is a step in the right direction. Besides reducing
> boilerplate, this will also help us ensure correctness, as we get a
> constant trickle of bug reports for commands which forgot to set the result
> status.
>
> The name is not ideal, but it's probably the best we can get. (The ideal
> solution for me would be to get rid of the duality in the DoExecute
> function -- e.g. remove the bool return and let the execution state be
> fully signalled by the result object -- but that's way off from what you
> were originally trying to do).
>
> That said, I'm not sure I should be the person approving this. :)
>
>
>
>
> Comment at: lldb/source/Commands/CommandObjectPlatform.cpp:611
> +if (!platform_sp->CloseFile(fd, error))
> + return result.AppendError(eReturnStatusFailed, error.AsCString());
> +
>
> I believe this should be `"{0}", error` to avoid hitting problems in case
> the error contains '{'. (Plus it's 5 chars shorter :) )
>
>
>
> Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1181
> +"Failed to send signal {0}: {1}\n", signo,
> +ST.AsCString());
> +
>
> AsCString unnecessary
>
>
> https://reviews.llvm.org/D33167
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types
clayborg added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/Makefile:10 + +include $(LEVEL)/Makefile.rules labath wrote: > This looks like a copy-paste error, as you have everything twice. good catch Repository: rL LLVM https://reviews.llvm.org/D33077 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r303076 - Disable a test in TestReturnValue on arm64 linux
Author: labath
Date: Mon May 15 11:25:28 2017
New Revision: 303076
URL: http://llvm.org/viewvc/llvm-project?rev=303076&view=rev
Log:
Disable a test in TestReturnValue on arm64 linux
as described in pr33042, we cannot reliably retrieve the return value on
arm64 in cases it is returned via x8 pointer. I tried to do this as
surgically as possible and disabled it only on targets I know to be
affected, as the code is still useful, even though it can only work on
best-effort basis.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py?rev=303076&r1=303075&r2=303076&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
Mon May 15 11:25:28 2017
@@ -18,6 +18,10 @@ class ReturnValueTestCase(TestBase):
mydir = TestBase.compute_mydir(__file__)
+def affected_by_pr33042(self):
+return ("clang" in self.getCompiler() and self.getArchitecture() ==
+"aarch64" and self.getPlatform() == "linux")
+
@expectedFailureAll(oslist=["freebsd"], archs=["i386"])
@expectedFailureAll(oslist=["macosx"], archs=["i386"],
bugnumber="")
@expectedFailureAll(
@@ -148,7 +152,8 @@ class ReturnValueTestCase(TestBase):
self.return_and_test_struct_value("return_two_int")
self.return_and_test_struct_value("return_three_int")
self.return_and_test_struct_value("return_four_int")
-self.return_and_test_struct_value("return_five_int")
+if not self.affected_by_pr33042():
+self.return_and_test_struct_value("return_five_int")
self.return_and_test_struct_value("return_two_double")
self.return_and_test_struct_value("return_one_double_two_float")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33167: Get rid of some uses of StringConvert and reduce some indentation
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
This looks fine. Like Kamil, I think it would help to document your new
interfaces.
Going away from StringConvert, you are switching from an API that gives a value
on fail to one that doesn't change the input value on error. You mostly handle
the failures right away, so in that case it's fine to just declare them. But
anywhere it's not immediately clear you are directly returning, you should
probably initialize the variables to the fail value.
Comment at: lldb/include/lldb/Interpreter/CommandReturnObject.h:132-154
+ template
+ bool AppendError(lldb::ReturnStatus Status, const char *Format,
+ Args &&... args) {
+SetStatus(Status);
+AppendErrorWithFormatv(Format, std::forward(args)...);
+return Succeeded();
+ }
Can you add a comment saying what these functions return? It's clear now that
the code is in the header file, but that doesn't describe a contract just a
current state, and anyway, will keep it clear if the code gets moved to the
implementation file.
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1178-1181
+if (auto ST = process->Signal(signo))
+ return result.AppendError(eReturnStatusFailed,
+"Failed to send signal {0}: {1}\n", signo,
+ST.AsCString());
"ST" is not how we name variables in lldb. Besides the capitalization, which
is wrong, we used to call all Error variables "error" which gave its use a bit
of consistency. You didn't change the variable names when you renamed them to
"error" which is probably why calling this one "error" gave you pause. But we
should maintain consistency for now, and that will make it easier to go rename
them to whatever is appropriate when we get around to that.
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:2687-2688
+bool success = false;
+addr_t load_addr;
+if (!llvm::strton(load_addr_cstr, load_addr)) {
+ result.AppendError(eReturnStatusFailed,
I don't think it does any harm here. But your switching from an API that sets
a fail value to one that doesn't, so you're adding the possibility of
uninitialized variable access. Probably good as you are making this transition
to initialize to the fail value.
https://reviews.llvm.org/D33167
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types
spyffe updated this revision to Diff 99052.
spyffe marked an inline comment as done.
spyffe added a comment.
Updated the Makefile to fix a problem caught by Pavel Labath.
Also relocated the new test to lang/objc.
https://reviews.llvm.org/D33077
Files:
packages/Python/lldbsuite/test/lang/objc/ptr_refs/Makefile
packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
source/Symbol/ClangASTContext.cpp
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -4493,7 +4493,7 @@
case clang::Type::ObjCObjectPointer: {
const clang::ObjCObjectPointerType *objc_class_type =
- qual_type->getAsObjCInterfacePointerType();
+ qual_type->getAs();
const clang::ObjCInterfaceType *objc_interface_type =
objc_class_type->getInterfaceType();
if (objc_interface_type &&
@@ -4602,7 +4602,7 @@
case clang::Type::ObjCObjectPointer: {
const clang::ObjCObjectPointerType *objc_class_type =
- qual_type->getAsObjCInterfacePointerType();
+ qual_type->getAs();
const clang::ObjCInterfaceType *objc_interface_type =
objc_class_type->getInterfaceType();
if (objc_interface_type &&
@@ -5671,7 +5671,7 @@
case clang::Type::ObjCObjectPointer: {
const clang::ObjCObjectPointerType *objc_class_type =
-qual_type->getAsObjCInterfacePointerType();
+qual_type->getAs();
const clang::ObjCInterfaceType *objc_interface_type =
objc_class_type->getInterfaceType();
if (objc_interface_type &&
@@ -5819,7 +5819,7 @@
case clang::Type::ObjCObjectPointer: {
const clang::ObjCObjectPointerType *objc_class_type =
-qual_type->getAsObjCInterfacePointerType();
+qual_type->getAs();
const clang::ObjCInterfaceType *objc_interface_type =
objc_class_type->getInterfaceType();
if (objc_interface_type &&
Index: packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
===
--- packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
+++ packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
@@ -0,0 +1,39 @@
+//===-- main.c --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#import
+
+@interface MyClass : NSObject {
+};
+-(void)test;
+@end
+
+@implementation MyClass
+-(void)test {
+printf("%p\n", self); // break here
+}
+@end
+
+@interface MyOwner : NSObject {
+ @public id ownedThing; // should be id, to test
+};
+@end
+
+@implementation MyOwner
+@end
+
+int main (int argc, char const *argv[]) {
+@autoreleasepool {
+MyOwner *owner = [[MyOwner alloc] init];
+owner->ownedThing = [[MyClass alloc] init];
+[(MyClass*)owner->ownedThing test];
+}
+return 0;
+}
+
Index: packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
===
--- packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
+++ packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
@@ -0,0 +1,50 @@
+"""
+Test the ptr_refs tool on Darwin with Objective-C
+"""
+
+from __future__ import print_function
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPtrRefsObjC(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_ptr_refs(self):
+"""Test the ptr_refs tool on Darwin with Objective-C"""
+self.build()
+exe_name = 'a.out'
+exe = os.path.join(os.getcwd(), exe_name)
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+main_file_spec = lldb.SBFileSpec('main.m')
+breakpoint = target.BreakpointCreateBySourceRegex(
+'break', main_file_spec)
+self.assertTrue(breakpoint and
+breakpoint.GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Frame #0 should be on self.line1 and the break condition should hold.
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+
+frame = thread.GetFrameAtIndex(0)
+
+
[Lldb-commits] [lldb] r303110 - [TypeSystem] Fix inspection of Objective-C object types
Author: spyffe
Date: Mon May 15 14:55:20 2017
New Revision: 303110
URL: http://llvm.org/viewvc/llvm-project?rev=303110&view=rev
Log:
[TypeSystem] Fix inspection of Objective-C object types
ptr_refs exposed a problem in ClangASTContext's implementation: it
uses an accessor to downcast a QualType to an
ObjCObjectPointerType, but the accessor is not fully general.
getAs() is the safer way to go.
I've added a test case that uses ptr_refs in a way that would
crash before the fix.
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
Modified:
lldb/trunk/source/Symbol/ClangASTContext.cpp
Added: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/Makefile
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/Makefile?rev=303110&view=auto
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/Makefile
(added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/Makefile Mon
May 15 14:55:20 2017
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+OBJC_SOURCES := main.m
+
+include $(LEVEL)/Makefile.rules
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py?rev=303110&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/TestPtrRefsObjC.py
Mon May 15 14:55:20 2017
@@ -0,0 +1,50 @@
+"""
+Test the ptr_refs tool on Darwin with Objective-C
+"""
+
+from __future__ import print_function
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPtrRefsObjC(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+def test_ptr_refs(self):
+"""Test the ptr_refs tool on Darwin with Objective-C"""
+self.build()
+exe_name = 'a.out'
+exe = os.path.join(os.getcwd(), exe_name)
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+main_file_spec = lldb.SBFileSpec('main.m')
+breakpoint = target.BreakpointCreateBySourceRegex(
+'break', main_file_spec)
+self.assertTrue(breakpoint and
+breakpoint.GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Frame #0 should be on self.line1 and the break condition should hold.
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+
+frame = thread.GetFrameAtIndex(0)
+
+self.dbg.HandleCommand("script import lldb.macosx.heap")
+self.expect("ptr_refs self", substrs=["malloc", "stack"])
+
Added: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m?rev=303110&view=auto
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/ptr_refs/main.m Mon May
15 14:55:20 2017
@@ -0,0 +1,39 @@
+//===-- main.c --*- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#import
+
+@interface MyClass : NSObject {
+};
+-(void)test;
+@end
+
+@implementation MyClass
+-(void)test {
+printf("%p\n", self); // break here
+}
+@end
+
+@interface MyOwner : NSObject {
+ @public id ownedThing; // should be id, to test
+};
+@end
+
+@implementation MyOwner
+@end
+
+int main (int argc, char const *argv[]) {
+@autoreleasepool {
+MyOwner *owner = [[MyOwner alloc] init];
+owner->ownedThing = [[MyClass alloc] init];
+[(MyClass*)owner->ownedThing test];
+}
+return 0;
+}
+
Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL:
http://llvm.org/view
[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types
spyffe closed this revision. spyffe added a comment. Committed r303110. https://reviews.llvm.org/D33077 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally
spyffe updated this revision to Diff 99084.
spyffe added a comment.
Two changes after Greg and Pavel's suggestions:
- Moved the symbol lookup function into the global context; and
- Rewrote the test case's build system to be more generic.
https://reviews.llvm.org/D33083
Files:
include/lldb/Symbol/SymbolContext.h
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two.mk
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
source/Symbol/SymbolContext.cpp
Index: source/Symbol/SymbolContext.cpp
===
--- source/Symbol/SymbolContext.cpp
+++ source/Symbol/SymbolContext.cpp
@@ -799,6 +799,163 @@
return true;
}
+const Symbol *
+SymbolContext::FindBestGlobalDataSymbol(const ConstString &name, Status &error) {
+ error.Clear();
+
+ if (!target_sp) {
+return nullptr;
+ }
+
+ Target &target = *target_sp;
+ Module *module = module_sp.get();
+
+ auto ProcessMatches = [this, &name, &target, module]
+ (SymbolContextList &sc_list, Status &error) -> const Symbol* {
+llvm::SmallVector external_symbols;
+llvm::SmallVector internal_symbols;
+const uint32_t matches = sc_list.GetSize();
+for (uint32_t i = 0; i < matches; ++i) {
+ SymbolContext sym_ctx;
+ sc_list.GetContextAtIndex(i, sym_ctx);
+ if (sym_ctx.symbol) {
+const Symbol *symbol = sym_ctx.symbol;
+const Address sym_address = symbol->GetAddress();
+
+if (sym_address.IsValid()) {
+ switch (symbol->GetType()) {
+case eSymbolTypeData:
+case eSymbolTypeRuntime:
+case eSymbolTypeAbsolute:
+case eSymbolTypeObjCClass:
+case eSymbolTypeObjCMetaClass:
+case eSymbolTypeObjCIVar:
+ if (symbol->GetDemangledNameIsSynthesized()) {
+// If the demangled name was synthesized, then don't use it
+// for expressions. Only let the symbol match if the mangled
+// named matches for these symbols.
+if (symbol->GetMangled().GetMangledName() != name)
+ break;
+ }
+ if (symbol->IsExternal()) {
+external_symbols.push_back(symbol);
+ } else {
+internal_symbols.push_back(symbol);
+ }
+ break;
+case eSymbolTypeReExported: {
+ ConstString reexport_name = symbol->GetReExportedSymbolName();
+ if (reexport_name) {
+ModuleSP reexport_module_sp;
+ModuleSpec reexport_module_spec;
+reexport_module_spec.GetPlatformFileSpec() =
+symbol->GetReExportedSymbolSharedLibrary();
+if (reexport_module_spec.GetPlatformFileSpec()) {
+ reexport_module_sp =
+ target.GetImages().FindFirstModule(reexport_module_spec);
+ if (!reexport_module_sp) {
+reexport_module_spec.GetPlatformFileSpec()
+.GetDirectory()
+.Clear();
+reexport_module_sp =
+target.GetImages().FindFirstModule(reexport_module_spec);
+ }
+}
+// Don't allow us to try and resolve a re-exported symbol if it is
+// the same as the current symbol
+if (name == symbol->GetReExportedSymbolName() &&
+module == reexport_module_sp.get())
+ return nullptr;
+
+return FindBestGlobalDataSymbol(
+symbol->GetReExportedSymbolName(), error);
+ }
+} break;
+
+case eSymbolTypeCode: // We already lookup functions elsewhere
+case eSymbolTypeVariable:
+case eSymbolTypeLocal:
+case eSymbolTypeParam:
+case eSymbolTypeTrampoline:
+case eSymbolTypeInvalid:
+case eSymbolTypeException:
+case eSymbolTypeSourceFile:
+case eSymbolTypeHeaderFile:
+case eSymbolTypeObjectFile:
+case eSymbolTyp
