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
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://
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
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
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
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1174770, @labath wrote:
> Could you also add a test for this?
I never ran LLDB tests, not sure where they are and what they are.
Also, how would you test that? I know now my open core dump works, but I cannot
share it.
https://revi
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1176399, @labath wrote:
> In https://reviews.llvm.org/D49685#1175413, @EugeneBi wrote:
>
> > In https://reviews.llvm.org/D49685#1174770, @labath wrote:
> >
> > > Could you also add a test for this?
> >
> >
> > I never ran LLDB tests, no
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1178125, @labath wrote:
> In https://reviews.llvm.org/D49685#1177046, @EugeneBi wrote:
>
> > It is specific to shared libraries. Opening the executable and core dump
> > follows different code path.
>
>
> Which path is that? Is the pat
EugeneBi added a comment.
Hmm... I never thought I can do that :)
Anyway, with the change as it is now, LLDB would try to load executable in the
sysroot, fail that, then open it without the sysroot.
https://reviews.llvm.org/D49685
___
lldb-commits
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1178543, @lemo wrote:
> > I never *ran LLDB tests*, not sure where they are and what they are.
>
> I hope you're planning to look into this before submitting the change :)
Good idea :)
Scanning dependencies of target check-lldb
[100%
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1178553, @labath wrote:
> In https://reviews.llvm.org/D49685#1178528, @EugeneBi wrote:
>
> > Hmm... I never thought I can do that :)
> > Anyway, with the change as it is now, LLDB would try to load executable in
> > the sysroot, fail
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1179837, @labath wrote:
> In https://reviews.llvm.org/D49685#1178730, @EugeneBi wrote:
>
> > I looked at the tests - is it all in Python? Not sure I have time to learn
> > a new language... Is there anything in C++?
>
>
> We have unit
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1201351, @magardne wrote:
> @labath Eugene asked me to help add a unit test for this. I have the updated
> diff, but I can't seem to attach it to this code review -- it must be because
> I'm not the original author? I'll attach the di
EugeneBi updated this revision to Diff 161520.
EugeneBi added a comment.
Mark added the test. Please let us know if this is OK.
https://reviews.llvm.org/D49685
Files:
packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
source/Target/Platform.cpp
Index: sour
EugeneBi updated this revision to Diff 161959.
EugeneBi added a comment.
Do not use /tmp directory in the test
https://reviews.llvm.org/D49685
Files:
packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
source/Target/Platform.cpp
Index: source/Target/Platfor
EugeneBi marked an inline comment as done.
EugeneBi added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py:215
+# /home/labath/test/a.out)
+tmp_sysroot = "/tmp/lldb_i386_mock_sysroot"
+execut
EugeneBi marked an inline comment as done.
EugeneBi added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py:215
+# /home/labath/test/a.out)
+tmp_sysroot = "/tmp/lldb_i386_mock_sysroot"
+execut
EugeneBi added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108
+ if (ok) {
+if (e_phnum_hdr == 0x)
+ e_phnum = section_zero.sh_info;
davidb wrote:
> Would this make more sense to compare against a named constant ELF::PN_
EugeneBi added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108
+ if (ok) {
+if (e_phnum_hdr == 0x)
+ e_phnum = section_zero.sh_info;
EugeneBi wrote:
> davidb wrote:
> > Would this make more sense to compare against a nam
EugeneBi added inline comments.
Comment at: source/Plugins/ObjectFile/ELF/ELFHeader.cpp:108
+ if (ok) {
+if (e_phnum_hdr == 0x)
+ e_phnum = section_zero.sh_info;
EugeneBi wrote:
> EugeneBi wrote:
> > davidb wrote:
> > > Would this make more sense to
EugeneBi updated this revision to Diff 86148.
EugeneBi added a comment.
Used named constants for SHN_UNDEF and SHN_XINDEX sentinels.
Unfortunately ELF.h lacks definition of PN_XNUM, so left it as a comment.
https://reviews.llvm.org/D29095
Files:
source/Plugins/ObjectFile/ELF/ELFHeader.cpp
EugeneBi added a comment.
So, what's now? Can somebody commit it, please?
I do not see any option to do it myself.
https://reviews.llvm.org/D29095
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
25 matches
Mail list logo
