Page MenuHomePhabricator

Fix -break-insert not working when using absolute paths
ClosedPublic

Authored by malaperle on Jul 27 2016, 10:37 PM.

Details

Summary

When trying to parse the -break-insert arguments as a named location, the string parsing was not configured to allow directory paths. This patch adds a constructor to allow the parsing of string as directory path along with the other parameters.

This fixes https://llvm.org/bugs/show_bug.cgi?id=28709

Diff Detail

Repository
rL LLVM

Event Timeline

malaperle updated this revision to Diff 65874.Jul 27 2016, 10:37 PM
malaperle retitled this revision from to Fix -break-insert not working when using absolute paths.
malaperle updated this object.
malaperle added reviewers: clayborg, ki.stfu.
malaperle added a subscriber: Restricted Project.
malaperle edited subscribers, added: lldb-commits; removed: Restricted Project.Jul 27 2016, 10:44 PM
ki.stfu edited edge metadata.Jul 27 2016, 11:05 PM

Could you add a test case for this in packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py?

clayborg requested changes to this revision.Jul 28 2016, 10:04 AM
clayborg edited edge metadata.

Add a test and this is good to go.

This revision now requires changes to proceed.Jul 28 2016, 10:04 AM
malaperle updated this revision to Diff 66055.Jul 28 2016, 4:40 PM
malaperle edited edge metadata.

The test actually already existed but now it passed.

BTW, I had trouble running the lldb-mi tests. There seems to be a bug in dotest.py.

lldbMiExec = lldbtest_config.lldbExec + "-mi"

I used 'python dotest.py --executable /path/to/llvm/build/bin/lldb

This failed for me because lldb is a symlink to lldb-4.0.0 and lldb-4.0.0-mi doesn't exist. I'm not sure if there either a way to not make it resolve the symlink or if the script should be changed.

This failed for me because lldb is a symlink to lldb-4.0.0 and lldb-4.0.0-mi doesn't exist. I'm not sure if there either a way to not make it resolve the symlink or if the script should be changed.

By fail, I mean that the tests were getting skipped.

clayborg accepted this revision.Jul 28 2016, 5:27 PM
clayborg edited edge metadata.

There should be a way to specify the lldb-mi binary. If there isn't feel free to add one to the test suite.

This revision is now accepted and ready to land.Jul 28 2016, 5:27 PM

Thanks! Newcomer silly question: now that the change is accepted, is there any action required on my end?

ki.stfu accepted this revision.Jul 28 2016, 11:06 PM
ki.stfu edited edge metadata.

No. I'm going to land it right now! Thanks!

This revision was automatically updated to reflect the committed changes.

Would it be possible to apply this fix to the release_39 branch?