The patch include files necessary to extend the Scripting Bridge API with Java. The edits follow the existing patterns for Python and Lua.
Details
Diff Detail
- Repository
- rLLDB LLDB
Event Timeline
Apologies for the inclusion of that last file "patch" - that is the "git diff -U9999999" result, should that be useful.
You can just upload that diff file and Phabricator will display it properly. There is no need to include the raw diff as part of the patch itself (it just makes this diff 100 times larger than it needs to be) :)
Anyway, I think this seems like a reasonable thing to have. We have to figure out though how we can properly set up some Java tests for this and it would be nice if we also find a bot that could actually run the tests for us.
lldb/bindings/java/CMakeLists.txt | ||
---|---|---|
3 ↗ | (On Diff #378205) | I don't think CMake accepts this as a comment and I think we anyway don't put license headers in CMake scripts. |
lldb/source/API/CMakeLists.txt | ||
84 | I think this is some conflict with one of the SBTrace patches. |
Am obviously brand new to your process and a bit of an old dog when it comes to learning new tricks. Would you prefer I make a new submission with the -U999999 diff? Also, am more than willing to help with the Java tests if that would be useful.
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 10:46:50 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
teemperor added a comment.
In D111409#3051110 https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
Apologies for the inclusion of that last file "patch" - that is the "git diff -U9999999" result, should that be useful.
You can just upload that diff file and Phabricator will display it properly. There is no need to include the raw diff as part of the patch itself (it just makes this diff 100 times larger than it needs to be) :)
Anyway, I think this seems like a reasonable thing to have. We have to figure out though how we can properly set up some Java tests for this and it would be nice if we also find a bot that could actually run the tests for us.
Comment at: lldb/bindings/java/CMakeLists.txt:3
+ * IP: Apache License 2.0 with LLVM Exceptions
+ */
+add_custom_command(
I don't think CMake accepts this as a comment and I think we anyway don't put license headers in CMake scripts.
Comment at: lldb/source/API/CMakeLists.txt:84
SBTrace.cpp
+ SBTraceOptions.cpp
SBType.cpp
I think this is some conflict with one of the SBTrace patches.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Hi @d-millar,
Here are some ressources that will help you comply with the LLVM guidelines:
- Contributing to LLVM : https://llvm.org/docs/Contributing.html
- Code Reviews with Phabricator : https://llvm.org/docs/Phabricator.html
Do you mind updating this diff by removing the patch and running clang-format on your code ?
That would simplify code review for reviewers.
Thanks!
No problem, first time using Phabricator is always a bit confusing. You can just do a git diff -U999999 > ~/java-patch.diff, click the "Update Diff" button on the top right of this website and then select *just* this diff file that contains your changes. Phabricator will render the diff properly for you (-> it will hide all the diff context by default). There is need to attach a separate diff file or anything else (users can just download the diff you uploaded).
Regarding the tests: We would essentially just need some basic test that exercises the new API a bit so that we know this works. The test code itself will be straightforward, but we would need a nice way to (automatically) find the system JRE and then set it up to be able to run the test code.
Merde - I followed you instructions but forgot to run clang-format. Give me a minute....
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
teemperor added a comment.
No problem, first time using Phabricator is always a bit confusing. You can just do a git diff -U999999 > ~/java-patch.diff, click the "Update Diff" button on the top right of this website and then select *just* this diff file that contains your changes. Phabricator will render the diff properly for you (-> it will hide all the diff context by default). There is need to attach a separate diff file or anything else (users can just download the diff you uploaded).
Regarding the tests: We would essentially just need some basic test that exercises the new API a bit so that we know this works. The test code itself will be straightforward, but we would need a nice way to (automatically) find the system JRE and then set it up to be able to run the test code.
In D111409#3051140 https://reviews.llvm.org/D111409#3051140, @d-millar wrote:
Am obviously brand new to your process and a bit of an old dog when it comes to learning new tricks. Would you prefer I make a new submission with the -U999999 diff? Also, am more than willing to help with the Java tests if that would be useful.
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 10:46:50 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409 https://reviews.llvm.org/D111409: proposed support for Java interface to Scripting Bridgeteemperor added a comment.
In D111409#3051110 https://reviews.llvm.org/D111409#3051110 https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
Apologies for the inclusion of that last file "patch" - that is the "git diff -U9999999" result, should that be useful.
You can just upload that diff file and Phabricator will display it properly. There is no need to include the raw diff as part of the patch itself (it just makes this diff 100 times larger than it needs to be) :)
Anyway, I think this seems like a reasonable thing to have. We have to figure out though how we can properly set up some Java tests for this and it would be nice if we also find a bot that could actually run the tests for us.
Comment at: lldb/bindings/java/CMakeLists.txt:3
+ * IP: Apache License 2.0 with LLVM Exceptions
+ */+add_custom_command(
I don't think CMake accepts this as a comment and I think we anyway don't put license headers in CMake scripts.
Comment at: lldb/source/API/CMakeLists.txt:84
SBTrace.cpp+ SBTraceOptions.cpp
SBType.cpp
I think this is some conflict with one of the SBTrace patches.
Repository:
rLLDB LLDBCHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Hmmm, well, have tried running "git clang-format" with a few different invocations, but seem to always get "No modified files to format." Suggestions?
From: David Millar via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 11:33:37 AM
To: David Millar; teemperor@gmail.com
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
d-millar added a comment.
Merde - I followed you instructions but forgot to run clang-format. Give me a minute....
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409 https://reviews.llvm.org/D111409: proposed support for Java interface to Scripting Bridge
teemperor added a comment.
No problem, first time using Phabricator is always a bit confusing. You can just do a git diff -U999999 > ~/java-patch.diff, click the "Update Diff" button on the top right of this website and then select *just* this diff file that contains your changes. Phabricator will render the diff properly for you (-> it will hide all the diff context by default). There is need to attach a separate diff file or anything else (users can just download the diff you uploaded).
Regarding the tests: We would essentially just need some basic test that exercises the new API a bit so that we know this works. The test code itself will be straightforward, but we would need a nice way to (automatically) find the system JRE and then set it up to be able to run the test code.
In D111409#3051140 https://reviews.llvm.org/D111409#3051140 https://reviews.llvm.org/D111409#3051140, @d-millar wrote:
Am obviously brand new to your process and a bit of an old dog when it comes to learning new tricks. Would you prefer I make a new submission with the -U999999 diff? Also, am more than willing to help with the Java tests if that would be useful.
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 10:46:50 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409 https://reviews.llvm.org/D111409 https://reviews.llvm.org/D111409: proposed support for Java interface to Scripting Bridgeteemperor added a comment.
In D111409#3051110 https://reviews.llvm.org/D111409#3051110 https://reviews.llvm.org/D111409#3051110 https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
Apologies for the inclusion of that last file "patch" - that is the "git diff -U9999999" result, should that be useful.
You can just upload that diff file and Phabricator will display it properly. There is no need to include the raw diff as part of the patch itself (it just makes this diff 100 times larger than it needs to be) :)
Anyway, I think this seems like a reasonable thing to have. We have to figure out though how we can properly set up some Java tests for this and it would be nice if we also find a bot that could actually run the tests for us.
Comment at: lldb/bindings/java/CMakeLists.txt:3
+ * IP: Apache License 2.0 with LLVM Exceptions
+ */+add_custom_command(
I don't think CMake accepts this as a comment and I think we anyway don't put license headers in CMake scripts.
Comment at: lldb/source/API/CMakeLists.txt:84
SBTrace.cpp+ SBTraceOptions.cpp
SBType.cpp
I think this is some conflict with one of the SBTrace patches.
Repository:
rLLDB LLDBCHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
https://reviews.llvm.org/D111409
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
I was confused by the patch file, since it included the entire SBDebugger.cpp, I thought you were adding all that code.
Since clang-format mostly cares about C/Objective-C/C++ files, I guess you're fine with the formatting. However, it looks like the patch file is still here.
To remove it from the diff, make sure you delete it from your local repository, add all your changes to the git index (git add <changes>) and make a commit. Then, create your new patch (git show HEAD -U999999 > mypatch.patch) and update the differential by uploading the new patch (mypatch.patch). While doing so, you can review your changes and make sure you didn't included the patch file in the differential.
Thanks!
Another (hopefully cleaner) attempt. If the addition of SBTraceOptions in lldb/source/API/CMakelists.txt conflicts, please feel free to delete it.
I'm not sure I understand your testing strategy, in particular how it applies to the existing Lua and Python extensions. I am looking at the files in lldb/unittests/ScriptInterpreter/Lua&Python. Do you execute test from native Lua/Python environments or through C wrappers-only? You mentioned a bot - is that code in the main repository?
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
teemperor added a comment.
No problem, first time using Phabricator is always a bit confusing. You can just do a git diff -U999999 > ~/java-patch.diff, click the "Update Diff" button on the top right of this website and then select *just* this diff file that contains your changes. Phabricator will render the diff properly for you (-> it will hide all the diff context by default). There is need to attach a separate diff file or anything else (users can just download the diff you uploaded).
Regarding the tests: We would essentially just need some basic test that exercises the new API a bit so that we know this works. The test code itself will be straightforward, but we would need a nice way to (automatically) find the system JRE and then set it up to be able to run the test code.
In D111409#3051140 https://reviews.llvm.org/D111409#3051140, @d-millar wrote:
Am obviously brand new to your process and a bit of an old dog when it comes to learning new tricks. Would you prefer I make a new submission with the -U999999 diff? Also, am more than willing to help with the Java tests if that would be useful.
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 10:46:50 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409 https://reviews.llvm.org/D111409: proposed support for Java interface to Scripting Bridgeteemperor added a comment.
In D111409#3051110 https://reviews.llvm.org/D111409#3051110 https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
Apologies for the inclusion of that last file "patch" - that is the "git diff -U9999999" result, should that be useful.
You can just upload that diff file and Phabricator will display it properly. There is no need to include the raw diff as part of the patch itself (it just makes this diff 100 times larger than it needs to be) :)
Anyway, I think this seems like a reasonable thing to have. We have to figure out though how we can properly set up some Java tests for this and it would be nice if we also find a bot that could actually run the tests for us.
Comment at: lldb/bindings/java/CMakeLists.txt:3
+ * IP: Apache License 2.0 with LLVM Exceptions
+ */+add_custom_command(
I don't think CMake accepts this as a comment and I think we anyway don't put license headers in CMake scripts.
Comment at: lldb/source/API/CMakeLists.txt:84
SBTrace.cpp+ SBTraceOptions.cpp
SBType.cpp
I think this is some conflict with one of the SBTrace patches.
Repository:
rLLDB LLDBCHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Ah, OK, after more digging, I realize I have probably provided only half of what you would like for this commit. My primary intention was to expose the SB API so I could make calls from Java into it, but for it to be, in some sense, a full-fledged member of the API, it would make sense to enable Java scripting from lldb. I will attempt to fill out that side of the equation. Cheers, D
From: lldb-commits <lldb-commits-bounces@lists.llvm.org> on behalf of David Millar via lldb-commits <lldb-commits@lists.llvm.org>
Sent: Friday, October 8, 2021 1:22:07 PM
To: anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com; Raphael Isemann
Subject: Re: [Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge
I'm not sure I understand your testing strategy, in particular how it applies to the existing Lua and Python extensions. I am looking at the files in lldb/unittests/ScriptInterpreter/Lua&Python. Do you execute test from native Lua/Python environments or through C wrappers-only? You mentioned a bot - is that code in the main repository?
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 11:24:10 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
teemperor added a comment.
No problem, first time using Phabricator is always a bit confusing. You can just do a git diff -U999999 > ~/java-patch.diff, click the "Update Diff" button on the top right of this website and then select *just* this diff file that contains your changes. Phabricator will render the diff properly for you (-> it will hide all the diff context by default). There is need to attach a separate diff file or anything else (users can just download the diff you uploaded).
Regarding the tests: We would essentially just need some basic test that exercises the new API a bit so that we know this works. The test code itself will be straightforward, but we would need a nice way to (automatically) find the system JRE and then set it up to be able to run the test code.
In D111409#3051140 https://reviews.llvm.org/D111409#3051140, @d-millar wrote:
Am obviously brand new to your process and a bit of an old dog when it comes to learning new tricks. Would you prefer I make a new submission with the -U999999 diff? Also, am more than willing to help with the Java tests if that would be useful.
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 10:46:50 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409 https://reviews.llvm.org/D111409: proposed support for Java interface to Scripting Bridgeteemperor added a comment.
In D111409#3051110 https://reviews.llvm.org/D111409#3051110 https://reviews.llvm.org/D111409#3051110, @d-millar wrote:
Apologies for the inclusion of that last file "patch" - that is the "git diff -U9999999" result, should that be useful.
You can just upload that diff file and Phabricator will display it properly. There is no need to include the raw diff as part of the patch itself (it just makes this diff 100 times larger than it needs to be) :)
Anyway, I think this seems like a reasonable thing to have. We have to figure out though how we can properly set up some Java tests for this and it would be nice if we also find a bot that could actually run the tests for us.
Comment at: lldb/bindings/java/CMakeLists.txt:3
+ * IP: Apache License 2.0 with LLVM Exceptions
+ */+add_custom_command(
I don't think CMake accepts this as a comment and I think we anyway don't put license headers in CMake scripts.
Comment at: lldb/source/API/CMakeLists.txt:84
SBTrace.cpp+ SBTraceOptions.cpp
SBType.cpp
I think this is some conflict with one of the SBTrace patches.
Repository:
rLLDB LLDBCHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Support for a scripting language in lldb has three distinct pieces.
- Making the SB API's available to the scripting languages
- Adding all the callbacks so the scripting language can be used for breakpoint callbacks, data formatters, etc.
- Adding a REPL for that language to the "script" command
This patch does #1, but not #2 & #3. Do we care about whether new scripting languages will be able to provide the full "scripting language" experience? Does there need to be some plan for this before we add on the task of supporting the language? Maybe it's okay to say "a REPL's too hard, and not worth it" but we should have a plan for adding in the callback interfaces? Do we want to have somewhere you can advertise levels of support? I would be sad to spend some time using a scripting language for lldb only to find it doesn't allow me to write breakpoint callbacks, for instance.
This is language #3 so it seems like a fit time to discuss this...
Just to clarify my use case: I am one of the developers for a reverse-engineering tool called Ghidra. Part of the tool is debugging-support to allow cross-over between dynamic and static analysis. We currently support windbg/kd, the gcc MI2 interface, and direct Java debugging. I have a working prototype for lldb, but it requires users to patch and rebuild lldb, which may be a heavy lift for some. While it falls outside of my use case, I am quite willing to attempt implementing pieces 2 & 3 from Mr. Ingham's post.
From: Jim Ingham via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 2:31:13 PM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; teemperor@gmail.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
jingham added a comment.
Support for a scripting language in lldb has three distinct pieces.
- Making the SB API's available to the scripting languages
- Adding all the callbacks so the scripting language can be used for breakpoint callbacks, data formatters, etc.
- Adding a REPL for that language to the "script" command
This patch does #1, but not #2 & #3. Do we care about whether new scripting languages will be able to provide the full "scripting language" experience? Does there need to be some plan for this before we add on the task of supporting the language? Maybe it's okay to say "a REPL's too hard, and not worth it" but we should have a plan for adding in the callback interfaces? Do we want to have somewhere you can advertise levels of support? I would be sad to spend some time using a scripting language for lldb only to find it doesn't allow me to write breakpoint callbacks, for instance.
This is language #3 so it seems like a fit time to discuss this...
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
That would be great!
To be clear, I was more asking questions than stating requirements. It might be that the community feels it's sufficiently valuable to just have the API wrappers, and maintaining that in tree is fine provided someone is around to keep it up to date. IMO, the callback affordances add much of the power in having an extension language in the debugger, so it would be a shame not to take on the added complexity of another language and not have this bit working. Whether a REPL is possible is in part determined by whether the language in question is really amenable to that. It's a huge convenience for developers of scripts in the language, and it is pretty cool to be able to drop into the script interpreter and write a quick loop to find something in a bunch of objects. But it isn't the core part of the job the extension language is doing.
From: Jim Ingham via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 2:31:13 PM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; teemperor@gmail.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridgejingham added a comment.
Support for a scripting language in lldb has three distinct pieces.
- Making the SB API's available to the scripting languages
- Adding all the callbacks so the scripting language can be used for breakpoint callbacks, data formatters, etc.
- Adding a REPL for that language to the "script" command
This patch does #1, but not #2 & #3. Do we care about whether new scripting languages will be able to provide the full "scripting language" experience? Does there need to be some plan for this before we add on the task of supporting the language? Maybe it's okay to say "a REPL's too hard, and not worth it" but we should have a plan for adding in the callback interfaces? Do we want to have somewhere you can advertise levels of support? I would be sad to spend some time using a scripting language for lldb only to find it doesn't allow me to write breakpoint callbacks, for instance.
This is language #3 so it seems like a fit time to discuss this...
Repository:
rLLDB LLDBCHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Discussing it with developers here, they agreed with your opinion that we really really ought to do 2. 😊
As you just noted, 3 is trickier. Java doesn't really have a REPL. It has jshell - we're not sure that counts. In any case, am up for tackling 2. Will keep you posted - might take me a bit. In the meantime, happy to hear any further thoughts, advice, etc.
Thanks!
From: lldb-commits <lldb-commits-bounces@lists.llvm.org> on behalf of David Millar via lldb-commits <lldb-commits@lists.llvm.org>
Sent: Friday, October 8, 2021 2:44:06 PM
To: anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; teemperor@gmail.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com; Jim Ingham
Subject: Re: [Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge
Just to clarify my use case: I am one of the developers for a reverse-engineering tool called Ghidra. Part of the tool is debugging-support to allow cross-over between dynamic and static analysis. We currently support windbg/kd, the gcc MI2 interface, and direct Java debugging. I have a working prototype for lldb, but it requires users to patch and rebuild lldb, which may be a heavy lift for some. While it falls outside of my use case, I am quite willing to attempt implementing pieces 2 & 3 from Mr. Ingham's post.
From: Jim Ingham via Phabricator <reviews@reviews.llvm.org>
Sent: Friday, October 8, 2021 2:31:13 PM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; teemperor@gmail.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
jingham added a comment.
Support for a scripting language in lldb has three distinct pieces.
- Making the SB API's available to the scripting languages
- Adding all the callbacks so the scripting language can be used for breakpoint callbacks, data formatters, etc.
- Adding a REPL for that language to the "script" command
This patch does #1, but not #2 & #3. Do we care about whether new scripting languages will be able to provide the full "scripting language" experience? Does there need to be some plan for this before we add on the task of supporting the language? Maybe it's okay to say "a REPL's too hard, and not worth it" but we should have a plan for adding in the callback interfaces? Do we want to have somewhere you can advertise levels of support? I would be sad to spend some time using a scripting language for lldb only to find it doesn't allow me to write breakpoint callbacks, for instance.
This is language #3 so it seems like a fit time to discuss this...
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Sounds good. When we started adding extension points for python, we made the implementations be free functions. But then over time we realized it was often more convenient if you had a object managing the callback, that way it could more easily store state over the invocations, etc. If doing free functions is harder in Java, I think it would be fine to only implement the "class with an invocation method" version of the callback.
OK, well, took my some time but I have made an attempt to address the three areas described by Mr. Ingham:
//Support for a scripting language in lldb has three distinct pieces.
- Making the SB API's available to the scripting languages
- Adding all the callbacks so the scripting language can be used for breakpoint callbacks, data formatters, etc.
- Adding a REPL for that language to the "script" command
//
With regard to #3, I had hoped JShell would work as a viable REPL. Unfortunately, it does not appear to support System.loadLibrary, which makes it somewhat useless in the current context. Because I cannot load the JNI libraries, I would not have access to the classes in the Scripting Bridge, which sort of defeats the purpose.
As an alternative, I have implemented a quasi-REPL, which allows you to compose classes, compile, and run them. The "script" command behaves for the most part like it would for Python or Lua, but have a set of /-prefixed commands, described in the initial message, that enable the extra functionality. The JShell interface is also include via those commands - perhaps loadLibrary will be supported in the future.
Anyway, let me know what you think and what else you might need for possible inclusion of this patch.
Best,
Dave
Hi David, this looks really comprehensive. As far as the code is concerned, it has all the parts that we'd want to support another scripting language in LLDB. That leaves us with the last remaining questions:
- Testing: having a bot that actually builds & tests this configuration.
- Maintainership: having a commitment from you (or someone else) to maintain this code.
Are you willing and able to sign up for those things?
PS: I installed Java and applied your patch on top-of-tree. Things didn't built because of the recent change to the plugin manager that now expects StringRefs instead of ConstStrings. Please let me know if you've rebased the patch and I'd love to give it a try.
lldb/cmake/modules/FindJavaAndSwig.cmake | ||
---|---|---|
13 ↗ | (On Diff #386169) | This can't be REQUIRED because that will fail the CMake configuration stage when Java isn't found which is not what you want when doing autodetection (the default). Making it required is handled at a higher level for FindJavaAndSwig as a whole. |
Hi Jonas,
Apologies for the non-build - I did a fetch/rebase this morning before creating the patch. Do I need to be on a different branch re StringRef vs ConstString? (Have seen that discussion in passing but confess I did not follow it in detail.) Any pointers appreciated.
Re testing, I basically duplicated the test set from Lua and ran it using "ninja check-lldb-unit" and "ninja check-lldb-shell". I'm not really conversant with your testing infrastructure. Can you point me to something that might be suggest what I'd need to do to generate a bot? I'll give it a shot if I have a starting point.
Also am happy to maintain the code in general and have a few folks in our project that can backstop me in case I get run over by a bus or something. 😊
Best,
Dave
From: Jonas Devlieghere via Phabricator <reviews@reviews.llvm.org>
Sent: Wednesday, November 10, 2021 1:57:51 PM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; teemperor@gmail.com; medismail.bennani@gmail.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
JDevlieghere added a comment.
Hi David, this looks really comprehensive. As far as the code is concerned, it has all the parts that we'd want to support another scripting language in LLDB. That leaves us with the last remaining questions:
- Testing: having a bot that actually builds & tests this configuration.
- Maintainership: having a commitment from you (or someone else) to maintain this code.
Are you willing and able to sign up for those things?
PS: I installed Java and applied your patch on top-of-tree. Things didn't built because of the recent change to the plugin manager that now expects StringRefs instead of ConstStrings. Please let me know if you've rebased the patch and I'd love to give it a try.
Comment at: lldb/cmake/modules/FindJavaAndSwig.cmake:13
+ find_package(Java 11.0)
+ find_package(JNI REQUIRED)
+ if(JAVA_FOUND AND SWIG_FOUND)
This can't be REQUIRED because that will fail the CMake configuration stage when Java isn't found which is not what you want when doing autodetection (the default). Making it required is handled at a higher level for FindJavaAndSwig as a whole.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Apologies in advance - this may be a repeat message. Our server went down for maintenance mid-send.
So, looking at https://llvm.org/docs/HowToAddABuilder.html, I think I need some clarification on your request regarding a build bot. Are you asking for dedicated physical resources for running nightly builds? That I cannot provide. Our build systems are non-public and firewalled in a way that would make them unsuitable for your build process. (Also, they're government resources.) Do you really have a separate build machines for the Python and Lua scripting environments?
Dave
From: David Millar via Phabricator <reviews@reviews.llvm.org>
Sent: Wednesday, November 10, 2021 3:17:38 PM
To: David Millar; jonas@devlieghere.com
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
d-millar added a comment.
Hi Jonas,
Apologies for the non-build - I did a fetch/rebase this morning before creating the patch. Do I need to be on a different branch re StringRef vs ConstString? (Have seen that discussion in passing but confess I did not follow it in detail.) Any pointers appreciated.
Re testing, I basically duplicated the test set from Lua and ran it using "ninja check-lldb-unit" and "ninja check-lldb-shell". I'm not really conversant with your testing infrastructure. Can you point me to something that might be suggest what I'd need to do to generate a bot? I'll give it a shot if I have a starting point.
Also am happy to maintain the code in general and have a few folks in our project that can backstop me in case I get run over by a bus or something. 😊
Best,
Dave
From: Jonas Devlieghere via Phabricator <reviews@reviews.llvm.org>
Sent: Wednesday, November 10, 2021 1:57:51 PM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; teemperor@gmail.com; medismail.bennani@gmail.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: mgorny@gentoo.org
Subject: [PATCH] D111409 https://reviews.llvm.org/D111409: proposed support for Java interface to Scripting Bridge
JDevlieghere added a comment.
Hi David, this looks really comprehensive. As far as the code is concerned, it has all the parts that we'd want to support another scripting language in LLDB. That leaves us with the last remaining questions:
- Testing: having a bot that actually builds & tests this configuration.
- Maintainership: having a commitment from you (or someone else) to maintain this code.
Are you willing and able to sign up for those things?
PS: I installed Java and applied your patch on top-of-tree. Things didn't built because of the recent change to the plugin manager that now expects StringRefs instead of ConstStrings. Please let me know if you've rebased the patch and I'd love to give it a try.
Comment at: lldb/cmake/modules/FindJavaAndSwig.cmake:13
+ find_package(Java 11.0)
+ find_package(JNI REQUIRED)
+ if(JAVA_FOUND AND SWIG_FOUND)
This can't be REQUIRED because that will fail the CMake configuration stage when Java isn't found which is not what you want when doing autodetection (the default). Making it required is handled at a higher level for FindJavaAndSwig as a whole.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
https://reviews.llvm.org/D111409
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
Are you asking for dedicated physical resources for running nightly builds?
I don't think any of the current bots have a Java installation so I think it's either that or we get someone with a bot to setup the required Java installation.
FWIW, if no one wants to host a bot for this then I won't mind testing this in own CI, but I am not using buildbot so we'll have to see if that is acceptable for the community (I could also migrate it to buildbot, but the buildbot interface is just painful to use and I would prefer not to).
Do you really have a separate build machines for the Python and Lua scripting environments?
Python is more or less mandatory so all LLDB build bots have that. Lua is tested here: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/
On a more general note: I haven't followed this thread very closely, but I am surprised that this is now adding a Java REPL to LLDB. I think the original "Just add Java bindings" seemed like a reasonable patch but I am not so sure about this. Could we split out the changes for just adding Java bindings to SWIG (which anyway seems like a standalone patch) and then we can talk about the whole Java scripting stuff in a separate review. I don't expect the first one to be controversial so this should also speed things up a bit.
I don't have a problem with installing the necessary packages on the bot I manage, but I cannot subscribe to tracking down any failures (or flaky tests!) for this configuration (and, in my experience, any new feature like this is going to have flaky tests). Flaky tests (probably just one) are the reason that lua integration is not enabled on this bot.
FWIW, if no one wants to host a bot for this then I won't mind testing this in own CI, but I am not using buildbot so we'll have to see if that is acceptable for the community (I could also migrate it to buildbot, but the buildbot interface is just painful to use and I would prefer not to).
I would rather not proliferate test infrastructures.
I'm not sure which pain points are you referring to, but setting up a buildbot instance is a lot simpler these days than it used to be (in particular, you don't need to track down and install any outdated packages).
Do you really have a separate build machines for the Python and Lua scripting environments?
Python is more or less mandatory so all LLDB build bots have that. Lua is tested here: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/
On a more general note: I haven't followed this thread very closely, but I am surprised that this is now adding a Java REPL to LLDB. I think the original "Just add Java bindings" seemed like a reasonable patch but I am not so sure about this. Could we split out the changes for just adding Java bindings to SWIG (which anyway seems like a standalone patch) and then we can talk about the whole Java scripting stuff in a separate review. I don't expect the first one to be controversial so this should also speed things up a bit.
+1
Sure, I think it should anyway not be up to any bot owner to track down flaky tests. And that nested_sessions lua test is anyway randomly failing everywhere from what I know.
FWIW, if no one wants to host a bot for this then I won't mind testing this in own CI, but I am not using buildbot so we'll have to see if that is acceptable for the community (I could also migrate it to buildbot, but the buildbot interface is just painful to use and I would prefer not to).
I would rather not proliferate test infrastructures.
I'm not sure which pain points are you referring to, but setting up a buildbot instance is a lot simpler these days than it used to be (in particular, you don't need to track down and install any outdated packages).
It's not the setup, it's just that the lab.llvm.org interface is far less usable than Jenkins for tracking down regressions in tests (and Jenkins is already not great, so that says something). But that's just my personal preference and I agree that lab.llvm.org should be the central place for infrastructure.
Anyway, if @labath can run them on his fancy build bot then I would prefer that over having it on my bot (because I pay out of my own pocket and CPU cycles are $$$).
Am sorry I can't commit build resources, but am certainly willing to commit time to solving test problems. In a very general sense, I think our project (Ghidra) will effectively be a test platform for issues with the Java bindings to SWIG.
With regard to the set-up for testing, I have noticed differences in behavior between the "ninja check-lldb-xxx" suite-testing environment, the llvm-lit test sets, and actual lldb runs / individual tests. I believe (I should never say this out loud, but...) the current code handles these environments correctly.
The Java scripting framework requires the ability to find two things: the Java install (and within it libjvm) and the SWIG.jar built from the JNI wrappers. If either LD_LIBRARY_PATH or JAVA_HOME is set in whichever environment is in use, the code should be able to find libjvm. If CLASSPATH includes the SWIG.jar or we're running out of build, finding it should be handled. Libjvm is loaded dynamically, so the inclusion of my code "should" have no effect on environments without Java installed.
I was thinking I would also spend a couple of days isolating the swig logic in case supporting the Java interface becomes untenable for you. Am hoping I can pare it down so that our users could point-and-click at an existing lldb repo, without necessarily having to build llvm/lldb.
Best, Dave
From: Raphael Isemann via Phabricator <reviews@reviews.llvm.org>
Sent: Thursday, November 11, 2021 7:23:11 AM
To: David Millar; anoronha@apple.com; fallkrum@yahoo.com; kkleine@redhat.com; medismail.bennani@gmail.com; jonas@devlieghere.com; tedwood@quicinc.com; jmolenda@apple.com; syaghmour@apple.com; jingham@apple.com; vsk@apple.com; boris.ulasevich@gmail.com; lldb-commits@lists.llvm.org; h.imai.833@nitech.jp; bruce.mitchener@gmail.com; david.spickett@linaro.org; quic_sourabhs@quicinc.com; djordje.todorovic@syrmia.com; serhiy.redko@gmail.com; Liburd1994@outlook.com
Cc: pavel@labath.sk; mgorny@gentoo.org
Subject: [PATCH] D111409: proposed support for Java interface to Scripting Bridge
teemperor added a comment.
In D111409#3124194 https://reviews.llvm.org/D111409#3124194, @labath wrote:
In D111409#3124075 https://reviews.llvm.org/D111409#3124075, @teemperor wrote:
Are you asking for dedicated physical resources for running nightly builds?
I don't think any of the current bots have a Java installation so I think it's either that or we get someone with a bot to setup the required Java installation.
I don't have a problem with installing the necessary packages on the bot I manage, but I cannot subscribe to tracking down any failures (or flaky tests!) for this configuration (and, in my experience, any new feature like this is going to have flaky tests). Flaky tests (probably just one) are the reason that lua integration is not enabled on this bot.
Sure, I think it should anyway not be up to any bot owner to track down flaky tests. And that nested_sessions lua test is anyway randomly failing everywhere from what I know.
FWIW, if no one wants to host a bot for this then I won't mind testing this in own CI https://ci.teemperor.de, but I am not using buildbot so we'll have to see if that is acceptable for the community (I could also migrate it to buildbot, but the buildbot interface is just painful to use and I would prefer not to).
I would rather not proliferate test infrastructures.
I'm not sure which pain points are you referring to, but setting up a buildbot instance is a lot simpler these days than it used to be (in particular, you don't need to track down and install any outdated packages).
It's not the setup, it's just that the lab.llvm.org interface is far less usable than Jenkins for tracking down regressions in tests (and Jenkins is already not great, so that says something). But that's just my personal preference and I agree that lab.llvm.org should be the central place for infrastructure.
Anyway, if @labath can run them on his fancy build bot then I would prefer that over having it on my bot (because I pay out of my own pocket and CPU cycles are $$$).
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111409/new/
I think this is some conflict with one of the SBTrace patches.