-
Notifications
You must be signed in to change notification settings - Fork 569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
capture and handle logging messages from Binary Ninja backend #1413
Comments
@xusheng6 for awareness of tracking issue |
Noted with thanks!
This one should be pretty easy. My colleague has already written the code
for the Python binding of the log listener and I just need to steal it.
Though I will have to get back to it at a later moment because we are
working on a new BN release.
Best,
Xusheng
…On Wed, Mar 29, 2023 at 9:36 PM Willi Ballenthin ***@***.***> wrote:
@xusheng6 <https://github.com/xusheng6> for awareness of tracking issue
—
Reply to this email directly, view it on GitHub
<#1413 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AWRACE5SPDRVPVOGWJHLXNDW6Q3GTANCNFSM6AAAAAAWL7V73Y>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I just realized that I should better fix the issue on our end before the new release. So that when we release it, we can update capa to utilize it. Otherwise, since capa CI uses the stable build of BN, we will have to wait longer. |
I notice a BN bug while dealing with this: Vector35/binaryninja-api#4205 Update: I have pushed a fix for it. Once we release the new stable, we should see far less warn messages |
I tried to understand why the BN logs are even printed at all. I checked our code and did some experiment, I figured that if we do not do anything, logs would not be printed to stdout/stderr by default. We must call |
Oh nvm, I figured out. The output is dumped to stderr by default: https://github.com/Vector35/binaryninja-api/blob/94391b2730dcc2e6cfe9f26d13569b15c6d1a0ad/python/__init__.py#L168. So we would not be able to see it if we run something directly, but it can be captured by things like pytest. |
So we have decided to not merge the branch that fixes this issue Vector35/binaryninja-api#4156, primarily because of a performance consideration. It could become a footgun and slow down the analysis speed if the user uses in a wrong way. This means we would not be able to fix this issue in a perfect way. However, I propose we solve it like this:
|
Not requiring the Python LogListener means this does not need to be completed before the next BN stable release. All the infra mentioned above is already existing |
as first described and triaged in the thread here: #1343 (comment)
Binary Ninja emits logging messages to stdout/stderr directly, which doesn't give Python a chance to handle/format/suppress messages. When they add support for registering a logging listener, we should use it to improve the user and tester experience of capa.
The text was updated successfully, but these errors were encountered: