Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

EugeneBi (Eugene Birukov)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 17 2016, 5:34 PM (371 w, 6 d)

Recent Activity

Aug 27 2018

EugeneBi added inline comments to D49685: LLDB does not respect platform sysroot when loading core on Linux.
Aug 27 2018, 10:25 AM

Aug 22 2018

EugeneBi added inline comments to D49685: LLDB does not respect platform sysroot when loading core on Linux.
Aug 22 2018, 7:57 AM
EugeneBi closed D49685: LLDB does not respect platform sysroot when loading core on Linux.
Aug 22 2018, 7:55 AM
EugeneBi updated the diff for D49685: LLDB does not respect platform sysroot when loading core on Linux.

Do not use /tmp directory in the test

Aug 22 2018, 7:55 AM

Aug 20 2018

EugeneBi updated the diff for D49685: LLDB does not respect platform sysroot when loading core on Linux.

Mark added the test. Please let us know if this is OK.

Aug 20 2018, 11:15 AM
EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

@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. Otherwise we can wait until Eugene gets back on Monday.

The diff makes the following changes:

  • Add an executable: test/testcases/functionalities/postmortem/elf-core/mock_sysroot/home/labath/test/a.out
  • This is a copy of linux-i386.out (which is why it's in mock_sysroot/home/labath...). Let me know if this is ok, or if I should try to create a new binary/core
  • Implement the test you suggested: set platform and sysroot, open the core, check threads/stacks are valid

I verified that the test fails without the changes to Platform.cpp and passes after the changes.

Aug 20 2018, 10:45 AM

Jul 30 2018

EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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 from there.

FWIW, I don't think you really need to *know* python to write a test like this. You should be able to fudge it by cargo-culting some code from existing tests and some basic python examples. I expect the test should be something like:

# copy core file an .exe into an appropriate directory tree
self.runCmd("platform select remote-linux --sysroot '%s'" % sysroot)
target = self.dbg.CreateTarget(None)
process = target.LoadCode(core)
self.assertEquals(1, target.GetNumModules())
self.assertEquals(exe, target.GetModuleAtIndex(0).GetFileSpec())
Jul 30 2018, 1:14 PM

Jul 27 2018

EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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 possible to exercise this code path (by placing the executable in the sysroot for lldb to find it)?

Jul 27 2018, 1:19 PM
EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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 :)

Jul 27 2018, 12:19 PM
EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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.

Jul 27 2018, 11:28 AM
EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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 core file (target create --core /my/file.core). I believe that in this case we should end up here https://github.com/llvm-mirror/lldb/blob/7c07b8a9314eef118f95b57b49fa099be0808eac/source/Plugins/Process/elf-core/ProcessElfCore.cpp#L255, which should eventually call Platform::GetSharedModule. Is that not the case?

Jul 27 2018, 11:23 AM

Jul 26 2018

EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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.

Good question. Is this functionality specific to shared libraries, or could it be used to locate the main exe too?

If yes, then it should be sufficient to take one of the existing core&exe files we have, copy the exe into a path matching what the core file expects, set the sysroot, and make sure the exe gets found when we open up the core file. You can look at the existing tests in test/testcases/functionalities/postmortem/elf-core for examples.

If not, then it might get trickier because we'll need new core files, and it's hard to generate small ones with shared libraries.

Jul 26 2018, 10:54 AM

Jul 25 2018

EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

Could you also add a test for this?

Jul 25 2018, 10:24 AM

Jul 24 2018

EugeneBi added inline comments to D49685: LLDB does not respect platform sysroot when loading core on Linux.
Jul 24 2018, 3:43 PM
EugeneBi updated the diff for D49685: LLDB does not respect platform sysroot when loading core on Linux.

Fix a bug with resolved_spec path.

Jul 24 2018, 3:36 PM
EugeneBi added inline comments to D49685: LLDB does not respect platform sysroot when loading core on Linux.
Jul 24 2018, 3:28 PM
EugeneBi updated the diff for D49685: LLDB does not respect platform sysroot when loading core on Linux.

Code review followup:

  • Restricted change to Platform.cpp
  • Restricted change only to remote platforms.
Jul 24 2018, 2:50 PM
EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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.

Jul 24 2018, 11:26 AM
EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.
Jul 24 2018, 11:24 AM
EugeneBi updated the diff for D49685: LLDB does not respect platform sysroot when loading core on Linux.

Rebased to recent master.
Included the whole file in diff.

Jul 24 2018, 11:21 AM
EugeneBi added a comment to D49685: LLDB does not respect platform sysroot when loading core on Linux.

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.

Jul 24 2018, 10:10 AM

Jul 23 2018

EugeneBi created D49685: LLDB does not respect platform sysroot when loading core on Linux.
Jul 23 2018, 11:23 AM

Jan 30 2017

EugeneBi added a comment to D29095: Open ELF core dumps with more than 64K sections.

So, what's now? Can somebody commit it, please?
I do not see any option to do it myself.

Jan 30 2017, 11:30 AM

Jan 27 2017

EugeneBi updated the diff for D29095: Open ELF core dumps with more than 64K sections.

Used named constants for SHN_UNDEF and SHN_XINDEX sentinels.
Unfortunately ELF.h lacks definition of PN_XNUM, so left it as a comment.

Jan 27 2017, 5:51 PM
EugeneBi added inline comments to D29095: Open ELF core dumps with more than 64K sections.
Jan 27 2017, 5:22 PM
EugeneBi added inline comments to D29095: Open ELF core dumps with more than 64K sections.
Jan 27 2017, 5:15 PM
EugeneBi added inline comments to D29095: Open ELF core dumps with more than 64K sections.
Jan 27 2017, 4:55 PM

Jan 26 2017

EugeneBi added inline comments to D29095: Open ELF core dumps with more than 64K sections.
Jan 26 2017, 10:13 AM
EugeneBi updated the diff for D29095: Open ELF core dumps with more than 64K sections.

This compiles, builds, and run.

Jan 26 2017, 10:10 AM

Jan 25 2017

EugeneBi added a comment to D29095: Open ELF core dumps with more than 64K sections.

Hmm... This fix does not work with the recent sources :(
I'll debug some more tomorrow.

Jan 25 2017, 9:01 PM
EugeneBi added a comment to D29095: Open ELF core dumps with more than 64K sections.

Sorry, messed up.
Will upload correct change soon

Jan 25 2017, 3:50 PM
EugeneBi abandoned D29095: Open ELF core dumps with more than 64K sections.
Jan 25 2017, 3:49 PM
EugeneBi updated the diff for D29095: Open ELF core dumps with more than 64K sections.

Rebased to recent master

Jan 25 2017, 3:43 PM
EugeneBi added a comment to D29095: Open ELF core dumps with more than 64K sections.
  1. Yes, this is based on release_39 branch. OK, I'll rebase it on current master.
    1. Sorry, I am not familiar with LLDB testing and unfortunately I do not have time to do it. I am testing it on the core dumps I have. If this is not acceptable, I'll keep using my private copy and wait until somebody with more resources to spend on LLDB picks it up.
    2. I thought about keeping vs. discarding values from the original header. I slightly inclined to keep them because:
Jan 25 2017, 12:26 PM

Jan 24 2017

EugeneBi added a reviewer for D29095: Open ELF core dumps with more than 64K sections: labath.
Jan 24 2017, 12:58 PM
EugeneBi created D29095: Open ELF core dumps with more than 64K sections.
Jan 24 2017, 12:56 PM

Oct 19 2016

EugeneBi accepted D25783: [Host] handle short reads and writes.
Oct 19 2016, 8:28 PM

Oct 18 2016

EugeneBi added a comment to D25712: File::Read does not handle short reads.

I took a look.

Oct 18 2016, 12:49 PM · Restricted Project
EugeneBi added a comment to D25712: File::Read does not handle short reads.

I'll take a look

Oct 18 2016, 11:01 AM · Restricted Project

Oct 17 2016

EugeneBi retitled D25712: File::Read does not handle short reads from to File::Read does not handle short reads.
Oct 17 2016, 5:56 PM · Restricted Project