Skip to content
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

Debugger always enabled #197

Merged
merged 24 commits into from
Nov 28, 2024
Merged

Debugger always enabled #197

merged 24 commits into from
Nov 28, 2024

Conversation

stlintel
Copy link
Contributor

@stlintel stlintel commented Dec 28, 2023

Please re-generate configure script before use :)

Allow to have debugger and non-debugger version in same binary.

Just compile Bochs with --enable-debugger and specify -debugger option in command line - and it run Bochs internal debugger. If -debugger not supplied it will run normal optimized build.

This is first draft of the idea provided for initial testing.

When -debugger is not supplied emulation speed is affected ~1% which I think if very reasonable taking into account extra capability.

When -debugger is supplied the emulation speed is affected severely as before.

There are potential ways how to make debugger cost cheaper (not addressed here yet):

  • data watchpoints could be implemented in cheap way so they won't cost any emulation performance if not used (meaning there are no active watchpoints set).
  • similarly code breakpoints could be optimized using the same mechanisms

Also:

  • it would be nice to have GUI debugger button which will switch into debugger even if started as 'optimized'
    and if already done it would be nice to do 'optimized continue' which will switch to fast mode until some even drops you into the debugger back.

Also code is just hacky for now, will be improving the logic while doing more testing.
Especially need to test without handlers chaining compiled and when SMP is enabled.

For now I need as much feedback as possible.
What to improve ?
Bugs ?
How to make it more user friendly ?

@stlintel stlintel requested review from therealdreg and Vort December 28, 2023 21:04
@stlintel
Copy link
Contributor Author

If someone interested in having this feature - would like to have feedback and discussion !

@Vort
Copy link
Contributor

Vort commented Dec 29, 2023

First of all, I have no experience with Bochs debugger.
Also I'm used to graphical debugging UI, GDB-style looks convoluted for me.
I can try to run it anyway however.
But at this stage I can't even build it. Maybe my mistake, I don't know.
Here is what errors compiler produce:

ld.lld: error: undefined symbol: tputs
>>> referenced by libreadline.a(display.o):(rl_redisplay)
>>> referenced by libreadline.a(display.o):(rl_redisplay)
>>> referenced by libreadline.a(display.o):(rl_redisplay)
>>> referenced 19 more times

ld.lld: error: undefined symbol: tgetnum
>>> referenced by libreadline.a(terminal.o):(_rl_get_screen_size)
>>> referenced by libreadline.a(terminal.o):(_rl_get_screen_size)

ld.lld: error: undefined symbol: tgetent
>>> referenced by libreadline.a(terminal.o):(_rl_init_terminal_io)

ld.lld: error: undefined symbol: tgetstr
>>> referenced by libreadline.a(terminal.o):(_rl_init_terminal_io)

ld.lld: error: undefined symbol: tgetflag
>>> referenced by libreadline.a(terminal.o):(_rl_init_terminal_io)
>>> referenced by libreadline.a(terminal.o):(_rl_init_terminal_io)
>>> referenced by libreadline.a(terminal.o):(_rl_init_terminal_io)

ld.lld: error: undefined symbol: BC
>>> referenced by libreadline.a(terminal.o):(.refptr.BC)

ld.lld: error: undefined symbol: PC
>>> referenced by libreadline.a(terminal.o):(.refptr.PC)

ld.lld: error: undefined symbol: UP
>>> referenced by libreadline.a(terminal.o):(.refptr.UP)
g++: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:181: bochs] Error 1

@stlintel
Copy link
Contributor Author

It supposed to work also with graphic debugger in place.
Enable display_library: win32, options="gui_debug"
For compilation error you see try to :

// adds support for the GNU readline library in the debugger command
// prompt.
#define HAVE_LIBREADLINE 0
#define HAVE_READLINE_HISTORY_H 0

in config.h

@Vort
Copy link
Contributor

Vort commented Dec 29, 2023

#define HAVE_LIBREADLINE 0

This helped.

Enable display_library: win32, options="gui_debug"

Ok, it works.
Strange that default font is not monospace, good that ability to change it exists.

I have some questions about it, but I suspect they may be offtopic in this PR.
For example, how to see what bytes are located at addresses starting from ss:[ebx] now, before they are overwritten by si? Should I do some manual math for it?
bochs_debugger

Bugs ?

I see strange change.
When I click Power off, even without -debugger option I see additional line:
image

Another stange (and probably not important) thing:
Do not remember what I did, but suspicious message appeared:
(fatal() should never return, but it just did)
image

@stlintel
Copy link
Contributor Author

For example, how to see what bytes are located at addresses starting from ss:[ebx] now, before they are overwritten by si? >Should I do some manual math for it?

There is always GUI debugger command line with all its options.
You could do "x ss:ebx" and it will print you the value at ss:[ebx].
GUI debugger also has memory watch, you should be able to find it in the menus.

About the default font, the font was selected by person who originally submitted the GUI debugger code.
I bet we could change a default font if you have better suggestion :)

@stlintel
Copy link
Contributor Author

BTW, may be instead of having this .bochsrc option we should just fire debugger gui automatically if option -debugger was supplied. Or if -debugger=gui was supplied ...

@Vort
Copy link
Contributor

Vort commented Dec 29, 2023

I bet we could change a default font if you have better suggestion :)

I think answer about which font to choose mainly depends on state of support of Windows XP as host OS.
Is it completely broken or there are just no official builds for it?

Also question above can me made non-relevant in case of implementing checking for font existence.

Here is the list of popular fonts:

  • Consolas - Windows Vista+
  • Lucida Console - Windows 98+
  • Courier New - Windows 3.1+
  • Fixedsys - Windows 1+

Also Cascadia Code exists, but it is probably available only on Windows 11.

So I think it is better to select Consolas if it can be found and if not, then Lucida Console.

Here are screenshots:

Consolas:
bd_consolas

Lucida Console:
bd_lucida_console

Courier New:
bd_courier_new

Fixedsys:
bd_fixedsys

Default:
bd_default

@Vort
Copy link
Contributor

Vort commented Dec 29, 2023

You could do "x ss:ebx" and it will print you the value at ss:[ebx].

I was thinking more about how to fill this section with corresponding bytes:
image

@stlintel stlintel force-pushed the debugger-always-enabled branch from 4e58c1c to 676bfd3 Compare December 29, 2023 19:06
@stlintel stlintel force-pushed the debugger-always-enabled branch from 950c995 to 3bb37ca Compare January 12, 2024 18:26
@stlintel stlintel force-pushed the debugger-always-enabled branch from 3bb37ca to 0ffa2f1 Compare January 30, 2024 07:26
@stlintel stlintel marked this pull request as draft March 10, 2024 15:24
@stlintel stlintel force-pushed the debugger-always-enabled branch 3 times, most recently from 00ef96b to 3b79ca3 Compare March 16, 2024 08:01
@stlintel stlintel force-pushed the debugger-always-enabled branch from 3b79ca3 to 70ca40d Compare October 23, 2024 17:08
@stlintel stlintel force-pushed the debugger-always-enabled branch 2 times, most recently from b32e94f to fff9af8 Compare November 23, 2024 11:18
@stlintel stlintel force-pushed the debugger-always-enabled branch from aa215ce to 30ed864 Compare November 23, 2024 17:03
@stlintel stlintel marked this pull request as ready for review November 23, 2024 17:10
@stlintel stlintel requested a review from vruppert November 23, 2024 17:10
@Vort
Copy link
Contributor

Vort commented Nov 23, 2024

Sorry for probably stupid questions.

Title states "Debugger always enabled", but it is required to "compile Bochs with --enable-debugger".
Why enabling it if it is always enabled?

More precisely, I'm interested in how this change will affect release binaries (and binaries made with GitHub actions).
Will this feature be enabled by default or it will require building from source like before?

@stlintel
Copy link
Contributor Author

Sorry for probably stupid questions.

Title states "Debugger always enabled", but it is required to "compile Bochs with --enable-debugger". Why enabling it if it is always enabled?

More precisely, I'm interested in how this change will affect release binaries (and binaries made with GitHub actions). Will this feature be enabled by default or it will require building from source like before?

The idea is to set --enable-debugger ON by default now and compile release binaries with debugger option ALWAYS. Meaning no separate debugger enabled binary anymore

@Vort
Copy link
Contributor

Vort commented Nov 23, 2024

Will GUI debugger be also always enabled (by default)?

@stlintel
Copy link
Contributor Author

Will GUI debugger be also always enabled (by default)?

Yes, if supported by currently present GUI

@vruppert
Copy link
Contributor

I think moving this duplicated config interface code for simulation shutdown to one function with correct debugger handling is required before this code can go to master branch. I haven't noticed other issues with it yet.

Stanislav Shwartsman and others added 21 commits November 27, 2024 23:45
…zed run

unfortunatelly doesn't work with all optimizations yet
…checking for bx_dbg.debugger_active every instruction
…all with assert(! bx_dbg.debugger_active)

add if (bx_dbg.debugger_active) check in more places in the code to avoid redundant debugger methods invocation
split bx_dbg_epilog method to debugger and gdbstub
use bx_dbg.debugger_active to select proper way of printing disasm instruction
@stlintel stlintel force-pushed the debugger-always-enabled branch from fe664c1 to c8a5d3f Compare November 27, 2024 22:46
@stlintel
Copy link
Contributor Author

I think moving this duplicated config interface code for simulation shutdown to one function with correct debugger handling is required before this code can go to master branch. I haven't noticed other issues with it yet.

I made necessary changes in prev commit.
Proper code would take this code into a function :

#if BX_DEBUGGER
        if (bx_dbg.debugger_active) {
          bx_dbg_exit(1);
        }
        else
#endif
        {
          bx_atexit();
          SIM->quit_sim(1);
        }

smth like:

void bx_quit(int exit_code)
{
#if BX_DEBUGGER
        if (bx_dbg.debugger_active) {
          bx_dbg_exit(exit_code);
        }
        else
#endif
        {
          bx_atexit();
          SIM->quit_sim(exit_code);
        }
}

donno if bx_quit is the best function name

@vruppert
Copy link
Contributor

I think the new code is now ready for the master branch. If issues appear, can fix them there.

@vruppert vruppert merged commit 119af93 into master Nov 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants