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).
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;
+
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.
=
Author: labath
Date: Tue Jul 24 03:49:14 2018
New Revision: 337819
URL: http://llvm.org/viewvc/llvm-project?rev=337819&view=rev
Log:
Reimplement EventDataBytes::Dump to avoid DumpDataExtractor
This is the only external non-trivial dependency of the Event classes.
Added:
lldb/trunk/unittests/
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
unittests/Cor
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);
---
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 ori
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 Xcod
Author: labath
Date: Tue Jul 24 08:48:13 2018
New Revision: 337832
URL: http://llvm.org/viewvc/llvm-project?rev=337832&view=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
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/tru
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 com
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
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
module
>
> 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 s
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
___
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, and
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 conce
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 buffered
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
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 description)
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
__
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
unit
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
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 st
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?)
https://reviews
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. If
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: source/Target/Platform.c
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
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.
https://
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 https://rev
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
sourc
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
http://lists.llvm.org/cgi-bin/mailman/list
Author: teemperor
Date: Tue Jul 24 14:09:17 2018
New Revision: 337855
URL: http://llvm.org/viewvc/llvm-project?rev=337855&view=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
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&id=157128#toc
Rep
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
Inde
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
http://lists.
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
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/ o
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.
https://reviews.llv
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();
---
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
===
--- source/Targ
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.
Now
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 mini
Author: jmolenda
Date: Tue Jul 24 16:19:56 2018
New Revision: 337865
URL: http://llvm.org/viewvc/llvm-project?rev=337865&view=rev
Log:
Add DumpRegisterValue.cpp.
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/view
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
include/lldb/Core/H
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
==
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
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.
Chang
Author: teemperor
Date: Tue Jul 24 16:52:39 2018
New Revision: 337873
URL: http://llvm.org/viewvc/llvm-project?rev=337873&view=rev
Log:
Add unit tests for VMRange
Subscribers: clayborg, labath, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D49415
Added:
lldb/trunk/uni
49 matches
Mail list logo
