unicorn icon indicating copy to clipboard operation
unicorn copied to clipboard

Revive qemu logs - and extend the FAQ

Open BitMaskMixer opened this issue 1 year ago • 6 comments

The qemu logs can be quite useful to understand, why the emulation does (not) work.

As an example, there is an open issue I had looked in (because I was curious) and did not find an easy way to see the qemu logging - till I understood there arent anything compiled in.

I could write that script in cpp code and debug it - but I would like to have a "quick" way to figure out whats going on with pyhton scripts too.

This PR revives the qemu_log for a "special" build - where you might want to try to find the bug or whats going on.

Example: Take the issue https://github.com/unicorn-engine/unicorn/issues/1971 and try to figure out whats going on with this PR and the enabled logs. You will see a message like:

image

It's more like to help developers to print useful log messages so we can figure out what's going on faster.

In the future, the logging should be used more and more so we gain back knowledge about things pretty easly.

BitMaskMixer avatar Jul 17 '24 10:07 BitMaskMixer

The idea is good and I planned to do this for a while. However, the logging functionality doesn't have a switch in cmake to be enabled. Meanwhile, there is a magic environment variables designed for this: UNICORN_DEBUG. We can do it like:

  • Only build logging support when NDEBUG is not defined, i.e. debug builds.
  • Only show debug output when UNICORN_DEBUG env is set.

So the logging will be only shown for developers.

wtdcode avatar Jul 17 '24 10:07 wtdcode

Cool - good to know :)


  • Only build logging support when NDEBUG is not defined, i.e. debug builds.

I am not quite sure if NDEBUG is the right approach, as we assume now that only DEBUG builds are used to get the details. The executing debug code might take another path which did not trigger the bug, so I wanted to have an independent way to enable the logs.

I wanted to add that to the CMakefiles, but - they are quite huge. For my understanding, you can not pass preprocessor defines via -DXXX directly during the call to cmake, you must add internal cmake-variables which caches the value first and add them to the desired target.

The target_compile_definitions is the way to add that to the right target I think, so I do not want to mix up the CMAKE_CXX_FLAGS and CMAKE_C_FLAGS flags directly.


  • Only show debug output when UNICORN_DEBUG env is set.

That's nice, but I prefer reading it once during instancing UC (or let it automatically be called when a log is processed). Not sure how fast the "getenv" call is, but sure, you are about to loose a lot of speed for unneeded repeated calls.

What do you think? Do we need a way to check again and again if the logging has been activated at some time during execution - which costs performance? (Quite easy to implement - see below. Remove that static keyword)

For reading it once, storing it into a static variable would be a way to go I guess.

Something like:

bool logging_enabled()
{
     static const char* env_data = getenv("UNICORN_DEBUG");
     
     return env_data != NULL;
}

Or something, which checks the pointer and try to parse the value behind the env_data. (more complex, not needed?)


Update: Ahhh I forgot - variable declaration and initialization are working differently in C. Then I need to use a "initalized flag" to fix that, should be small change in the logging_enabled method.


I see - the "getenv" approach is used at two places, which might have a huge negative impact for debug builds. I could fix that with the "logging_enabled()" method too, if the variable is static which reads out the UNICORN_DEBUG once.

If we think a little bit further, the "UNICORN_DEBUG" might have a value assigned to it (or UNICORN_DEBUG_LEVEL) which can be used to set the debug level (which is now a preprocessor constant)


Update 2: I have reworked it a little bit - and removed a lot of the macro magic. I like that version better, so please review it again :)

BitMaskMixer avatar Jul 17 '24 16:07 BitMaskMixer

@wtdcode Is there an easy way to execute the pipeline locally? Like building up the needed docker containers - and execute the whole pipeline with them?

I would not dare to ask you to trigger it again if I could do that locally - but ... here we go. Issues might be fixed, till the next compiler / tool complains ;-P

BitMaskMixer avatar Jul 18 '24 17:07 BitMaskMixer

Sorry for late.

@wtdcode Is there an easy way to execute the pipeline locally? Like building up the needed docker containers - and execute the whole pipeline with them?

I would not dare to ask you to trigger it again if I could do that locally - but ... here we go. Issues might be fixed, till the next compiler / tool complains ;-P

Personally, I used https://github.com/nektos/act to do this.

wtdcode avatar Jul 19 '24 03:07 wtdcode

Personally, I used https://github.com/nektos/act to do this.

Yes I have seen that project too - but I did not used it before. A dockerfile which wraps all dependencies would be pretty nice, however, I am not sure how the (cross) compiling for other platforms will work, as you have MSVC and Apple builds going on.

Not familiar with the github logic or terminology, but are there "shareable and free" "git-runners" available, which might execute the pipeline (workflow) for a PR automatically? The code is public anyway, so there is no issue with "leaking source code" I think.

BitMaskMixer avatar Jul 19 '24 06:07 BitMaskMixer

Personally, I used https://github.com/nektos/act to do this.

Yes I have seen that project too - but I did not used it before. A dockerfile which wraps all dependencies would be pretty nice, however, I am not sure how the (cross) compiling for other platforms will work, as you have MSVC and Apple builds going on.

Not familiar with the github logic or terminology, but are there "shareable and free" "git-runners" available, which might execute the pipeline (workflow) for a PR automatically? The code is public anyway, so there is no issue with "leaking source code" I think.

In that way, you need to fork it and enable actions in your forks.

wtdcode avatar Jul 19 '24 09:07 wtdcode

Merged, nice work!

wtdcode avatar Sep 21 '24 14:09 wtdcode