[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL337873: Add unit tests for VMRange (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits.

[Lldb-commits] [lldb] r337873 - Add unit tests for VMRange

Author: teemperor Date: Tue Jul 24 16:52:39 2018 New Revision: 337873 URL: http://llvm.org/viewvc/llvm-project?rev=337873=rev Log: Add unit tests for VMRange Subscribers: clayborg, labath, mgorny, lldb-commits Differential Revision: https://reviews.llvm.org/D49415 Added:

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

teemperor added a comment. Merging this in as Davide suggested. https://reviews.llvm.org/D49415 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

teemperor updated this revision to Diff 157172. teemperor added a comment. - Applied Greg's requested changes (thank you very much). https://reviews.llvm.org/D49415 Files: unittests/Utility/CMakeLists.txt unittests/Utility/VMRangeTest.cpp Index: unittests/Utility/VMRangeTest.cpp

[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support

teemperor updated this revision to Diff 157169. teemperor added a reviewer: davide. teemperor added a comment. - Removed some leftover code from the refactoring. - Correctly SetUp/TearDown the test case. https://reviews.llvm.org/D49334 Files: include/lldb/Core/Debugger.h

[Lldb-commits] [lldb] r337865 - Add DumpRegisterValue.cpp.

Author: jmolenda Date: Tue Jul 24 16:19:56 2018 New Revision: 337865 URL: http://llvm.org/viewvc/llvm-project?rev=337865=rev Log: Add DumpRegisterValue.cpp. Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj URL:

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

lemo added inline comments. Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:216 + if (csd_version.find("Linux") != std::string::npos) +triple.setOS(llvm::Triple::OSType::Linux); + break; clayborg wrote: > I have run into some

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi added inline comments. Comment at: source/Target/Platform.cpp:252 +if (error.Success() && module_sp) + module_sp->SetPlatformFileSpec(spec.GetFileSpec()); +return error; EugeneBi wrote: > A bug here. must be resolved_spec if it succeeds.

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi updated this revision to Diff 157146. EugeneBi added a comment. Fix a bug with resolved_spec path. https://reviews.llvm.org/D49685 Files: source/Target/Platform.cpp Index: source/Target/Platform.cpp === ---

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg added a comment. I will make update the patch with many of the suggested inline comments. Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:150 + if (m_arch.IsValid()) +return m_arch; const MinidumpSystemInfo *system_info = GetSystemInfo();

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi added inline comments. Comment at: source/Target/Platform.cpp:252 +if (error.Success() && module_sp) + module_sp->SetPlatformFileSpec(spec.GetFileSpec()); +return error; A bug here. must be resolved_spec if it succeeds.

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

lemo added a comment. Looks really good, a few comments inline. This may not big a big deal, but the part about FP (and Apple vs. non-Apple) is confusing: the FP is a pretty weak convention, and in some ABIs is not actually "fixed" (ex. FP can be either https://reviews.llvm.org/source/openmp/

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. This will help, but not fix us loading incorrect versions of the shared library. I wonder if there is anything in the core file that could help use get the build ID/UUID of each binary. I

[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support

teemperor planned changes to this revision. teemperor added a comment. Actually, we also have to tear down the plugins, so let me fix that first. https://reviews.llvm.org/D49334 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi updated this revision to Diff 157135. EugeneBi marked 6 inline comments as done. EugeneBi added a comment. Code review followup: - Restricted change to Platform.cpp - Restricted change only to remote platforms. https://reviews.llvm.org/D49685 Files: source/Target/Platform.cpp

[Lldb-commits] [PATCH] D49755: Remove unused History class

This revision was automatically updated to reflect the committed changes. Closed by commit rL337855: Remove unused History class (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49755?vs=157075=157128#toc

[Lldb-commits] [lldb] r337855 - Remove unused History class

Author: teemperor Date: Tue Jul 24 14:09:17 2018 New Revision: 337855 URL: http://llvm.org/viewvc/llvm-project?rev=337855=rev Log: Remove unused History class Summary: This class doesn't seem to be used anywhere, so we might as well remove the code. Reviewers: labath Reviewed By: labath

[Lldb-commits] [PATCH] D49755: Remove unused History class

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Yes, please. https://reviews.llvm.org/D49755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D49755: Remove unused History class

teemperor created this revision. Herald added a subscriber: mgorny. This class doesn't seem to be used anywhere, so we might as well remove the code. https://reviews.llvm.org/D49755 Files: include/lldb/Utility/History.h lldb.xcodeproj/project.pbxproj source/Utility/CMakeLists.txt

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg created this revision. clayborg added reviewers: labath, zturner, markmentovai. Herald added a reviewer: javed.absar. Herald added subscribers: chrib, kristof.beyls. In this patch I add support for ARM and ARM64 break pad files. There are two flavors of ARM: Apple where FP is

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi added a comment. > You would just move: > > auto resolved_module_spec(module_spec); > if (sysroot != nullptr) > resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot); > > > into the code in Platform.cpp and do it all there. Ah, I see. Will do, thanks.

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi added a comment. I need to convert char* to StringRef yet. https://reviews.llvm.org/D49685 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi updated this revision to Diff 157088. EugeneBi added a comment. Rebased to recent master. Included the whole file in diff. https://reviews.llvm.org/D49685 Files: include/lldb/Core/ModuleList.h source/Core/ModuleList.cpp source/Target/Platform.cpp Index:

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg added a comment. In https://reviews.llvm.org/D49685#1173720, @EugeneBi wrote: > In https://reviews.llvm.org/D49685#1173640, @clayborg wrote: > > > We should never be loading the wrong shared libraries. The module spec we > > fill out must contain the UUID of the file are looking for.

[Lldb-commits] [PATCH] D49713: Replace StreamTee's recursive_mutex with a normal mutex.

teemperor added a reviewer: clayborg. teemperor added a comment. Adding Greg because he seem to be the one who added the lock. I think it's also worth investigating why this lock is there in the first place (as other Stream implementations are not thread safe, but this one is?)

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

EugeneBi added a comment. In https://reviews.llvm.org/D49685#1173640, @clayborg wrote: > We should never be loading the wrong shared libraries. The module spec we > fill out must contain the UUID of the file are looking for. If it doesn't we > have no chance of every really loading the right

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

sgraenitz marked an inline comment as done. sgraenitz added inline comments. Comment at: unittests/Core/CMakeLists.txt:4 DataExtractorTest.cpp + MangledTest.cpp ListenerTest.cpp teemperor wrote: > This should be after ListenerTest.cpp to keep this file in

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

sgraenitz updated this revision to Diff 157062. sgraenitz added a comment. Keep alphabetical order of files in CMakeLists.txt https://reviews.llvm.org/D49612 Files: cmake/modules/LLDBConfig.cmake lldb.xcodeproj/project.pbxproj source/Core/Mangled.cpp unittests/Core/CMakeLists.txt

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

teemperor added inline comments. Comment at: unittests/Core/CMakeLists.txt:4 DataExtractorTest.cpp + MangledTest.cpp ListenerTest.cpp This should be after ListenerTest.cpp to keep this file in alphabetical order. https://reviews.llvm.org/D49612

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

sgraenitz updated this revision to Diff 157061. sgraenitz added a comment. Change EXPECTED_EQ to EXPECTED_STREQ, because ConstString::operator==() compares identity of pointers, not equality of strings. While "they must come from the same pool in order to be equal" (as stated in the

[Lldb-commits] [PATCH] D49740: Move RegisterValue, Scalar, State from Core to Utility

teemperor added a comment. Did you test that with lldb's C++ modules? If not I can test it locally. https://reviews.llvm.org/D49740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support

teemperor updated this revision to Diff 157054. teemperor added a comment. [Updated patch to address Pavel's comments, thanks!] @zturner So I looked into the Windows support: Windows requires us to directly flush/signal/write/flush to the console output stream. However lldb's output is

Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

On Tue, 24 Jul 2018 at 17:17, Leonard Mosescu wrote: >> >> The problem is that shared libraries differ on these machines and >> LLDB either fails to load some libraries or loads wrong ones. > > > Not finding the modules is not surprising but the latter (loading the wrong > modules) is a bit

[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

labath added a comment. I am sure that particular test is worth trying to implement in lit. That script of yours is full of operating system specifics, and it's going to be super flaky. I'd suggest keeping this as a python test, as there you can abstract the platform specifics much easier,

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg added a comment. We should never be loading the wrong shared libraries. The module spec we fill out must contain the UUID of the file are looking for. If it doesn't we have no chance of every really loading the right stuff. Repository: rL LLVM https://reviews.llvm.org/D49685

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

lemo added subscribers: clayborg, EugeneBi, labath, lemo. lemo added a comment. > The problem is that shared libraries differ on these machines and > LLDB either fails to load some libraries *or loads wrong ones*. Not finding the modules is not surprising but the latter (loading the wrong

Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

> > The problem is that shared libraries differ on these machines and > LLDB either fails to load some libraries *or loads wrong ones*. > Not finding the modules is not surprising but the latter (loading the wrong modules) is a bit concerning. Do you know why the module build-id is not used when

[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

clayborg added inline comments. Comment at: include/lldb/API/SBTarget.h:275-276 + void AppendImageSearchPath(const char *from, const char *to); + + void AppendImageSearchPath(const char *from, const char *to, Remove this first one? Other functions were first

[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

apolyakov created this revision. apolyakov added reviewers: aprantl, clayborg, labath. Herald added a subscriber: ki.stfu. In this patch I suggest a way to deal with the problem started in https://reviews.llvm.org/D47302 and use it to re-implement MI target-select command. You are welcome to

[Lldb-commits] [PATCH] D48351: Move dumping code out of RegisterValue class

This revision was automatically updated to reflect the committed changes. Closed by commit rL337832: Move dumping code out of RegisterValue class (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48351 Files:

[Lldb-commits] [lldb] r337832 - Move dumping code out of RegisterValue class

Author: labath Date: Tue Jul 24 08:48:13 2018 New Revision: 337832 URL: http://llvm.org/viewvc/llvm-project?rev=337832=rev Log: Move dumping code out of RegisterValue class Summary: The dump function was the only part of this class which depended on high-level functionality. This was due to the

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I think doing this in the module list is not the right place. Why? Some platforms might have multiple sysroot to check. iOS for instance has a directory for each device that

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

ramana-nvr added a comment. Sorry, I am not helpful to you in providing a unit test case for this patch. I am still learning about test suite framework and/or trying to get the lldb test suite up and running. Regarding how I got to this, it is not that I have run into some issue due to the

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

sgraenitz marked 2 inline comments as done. sgraenitz added inline comments. Comment at: source/Core/Mangled.cpp:310 +#elif defined(LLDB_USE_LLVM_DEMANGLER) +llvm::ItaniumPartialDemangler IPD; +bool demangle_err = IPD.partialDemangle(mangled_name);

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

sgraenitz updated this revision to Diff 157001. sgraenitz added a comment. Minor improvements on naming and comments https://reviews.llvm.org/D49612 Files: cmake/modules/LLDBConfig.cmake lldb.xcodeproj/project.pbxproj source/Core/Mangled.cpp unittests/Core/CMakeLists.txt

[Lldb-commits] [lldb] r337819 - Reimplement EventDataBytes::Dump to avoid DumpDataExtractor

Author: labath Date: Tue Jul 24 03:49:14 2018 New Revision: 337819 URL: http://llvm.org/viewvc/llvm-project?rev=337819=rev Log: Reimplement EventDataBytes::Dump to avoid DumpDataExtractor This is the only external non-trivial dependency of the Event classes. Added:

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

JDevlieghere added inline comments. Comment at: source/Core/Mangled.cpp:30 #include "llvm/ADT/StringRef.h"// for StringRef +#include "llvm/Demangle/Demangle.h" While you're here I'd remove these redundant comments so this block looks more consistent.

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

labath added a comment. The patch makes sense to me. It's nice to get a performance boost, while reducing the number of demanglers at the same time. Comment at: source/Core/Mangled.cpp:310 +#elif defined(LLDB_USE_LLVM_DEMANGLER) +llvm::ItaniumPartialDemangler IPD; +

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

labath added a comment. Which version is this patch based on? The line numbers don't seem to match what I see on master. Could you rebase the patch to master, and upload the patch with a full diff (e.g. via `git diff -U`, see https://llvm.org/docs/Phabricator.html#id3).