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

2018-08-27 Thread Eugene Birukov via Phabricator via lldb-commits
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" +

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

2018-08-25 Thread Pavel Labath via Phabricator via lldb-commits
labath 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" +executable = tmp_sysroot +

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

2018-08-22 Thread Eugene Birukov via Phabricator via lldb-commits
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" +

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

2018-08-22 Thread Eugene Birukov via Phabricator via lldb-commits
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:

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

2018-08-21 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. Thank you for writing the test. This is more-or-less what I had in mind. Just make sure we don't put the test files in /tmp (e.g., because windows doesn't have it). It's unfortunate that the core file has my username embedded in it (I

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

2018-08-20 Thread Eugene Birukov via Phabricator via lldb-commits
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:

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

2018-08-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. You can always run a.out through obj2yaml and check that in. There is test suite support for using a yaml file that converts it to a binary and debugs it functionalities/gdb_remote_client/gdbclientutils.py 429:def createTarget(self, yaml_path): 431:

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

2018-08-20 Thread Eugene Birukov via Phabricator via lldb-commits
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

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

2018-08-15 Thread Mark Gardner via Phabricator via lldb-commits
magardne added a comment. @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 diff to this comment and maybe you can try to update the review.

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

2018-07-30 Thread Eugene Birukov via Phabricator via lldb-commits
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

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

2018-07-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. 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 tests in c++, but it's going to be quite hard to tickle this code path

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

2018-07-27 Thread Eugene Birukov via Phabricator via lldb-commits
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

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

2018-07-27 Thread Eugene Birukov via Phabricator via 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

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

2018-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. 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 that, then open it without the sysroot. Does that mean it is

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

2018-07-27 Thread Leonard Mosescu via lldb-commits
> > 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 :) On Fri, Jul 27, 2018 at 11:28 AM, Eugene Birukov via Phabricator < revi...@reviews.llvm.org> wrote: > EugeneBi added a comment. > > Hmm... I never

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

2018-07-27 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo added a comment. > 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 :) https://reviews.llvm.org/D49685 ___ lldb-commits mailing list

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

2018-07-27 Thread Eugene Birukov via Phabricator via lldb-commits
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

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

2018-07-27 Thread Eugene Birukov via Phabricator via lldb-commits
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

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

2018-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. 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 path different even when you don't specify an executable when opening the

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

2018-07-26 Thread Eugene Birukov via Phabricator via lldb-commits
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,

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

2018-07-26 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. 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, not sure where they are and what they are. > Also, how would you test that? I

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

2018-07-25 Thread Eugene Birukov via Phabricator via lldb-commits
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.

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

2018-07-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Could you also add a test for this? 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

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
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

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via 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.

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

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
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

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
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

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
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

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

2018-07-24 Thread Pavel Labath via lldb-commits
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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
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

2018-07-24 Thread Leonard Mosescu via Phabricator via lldb-commits
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

2018-07-24 Thread Leonard Mosescu via lldb-commits
> > 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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Pavel Labath via Phabricator via lldb-commits
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).