Create ColorConfig class and use it in nearpc

[gsingh93]

The old way of defining theme color parameters was verbose, as you needed to first add a Parameter and then add a function for each parameter that applies the correct color function to the argument. The code between all these functions was almost identical. Given the verbosity of defining the color parameters, it made sense to keep them in a separate module and import them where we need them.

This PR adds a ColorConfig class. Any command that wants to add color parameters can create an instance of it locally and call the add_color_param method for each parameter. We no longer need to pass in long names like nearpc-branch-marker-color, we can just pass in branch-marker and the rest is added for you. The Parameter that's created is stored in this config class. The command can then call any of the color functions using the ColorConfig object, i.e. c.branch_marker(...), and the __getattr__ method is able to handle all of these functions without you needing to define them. Because of this, we're also able to delete the entire file that originally defined all those functions. The benefit here is that it's easy to see what the configuration options are for a command, as now they're in the command source file itself.

I suspect performance might be brought up, so to specifically address it, this should be pretty similar to what we have now, and I haven't noticed any performance issues while testing. Maybe there's a better approach that can speed things up a few milliseconds, but it will probably sacrifice the other benefits here.

This PR only converts the nearpc.py color file. Future PRs will convert the rest.

[codecov-commenter]

Codecov Report

Merging #1317 (91e1155) into dev (5b56071) will decrease coverage by 0.02%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##              dev    #1317      +/-   ##
==========================================
- Coverage   55.03%   55.01%   -0.03%     
==========================================
  Files         156      155       -1     
  Lines       19408    19401       -7     
  Branches     1808     1811       +3     
==========================================
- Hits        10681    10673       -8     
  Misses       8278     8278              
- Partials      449      450       +1     

Impacted Files	Coverage Δ
pwndbg/color/__init__.py	81.30% <88.23%> (+0.87%)	⬆️
pwndbg/commands/nearpc.py	88.70% <88.88%> (+0.18%)	⬆️
pwndbg/arguments.py	70.90% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[disconnect3d]

Does it make sense to call c.add_color_param after the initialization of a ColorConfig? Maybe the creation should look like this so we do not allow to add more parameters afterwards?:

c = ColorConfig("nearpc", [
    ["symbol", "normal", "color for nearpc command (symbol)"],
    ["address", "normal", "color for nearpc command (address)"],
    ...
])

Also maybe we should make a factory function out of this since compared to the old behavior, where the caller had to 1) load a global imported attribute and 2) call it, here, we will: 1) load the global ColorConfig instance, 2) get its attribute, 3) call it, which then 4) makes some dict lookups and finally calls the generateColorFunction.

Also since we are touching this code, it is generally not great that generateColorFunction performs so much logic that should be cached. I mean, eventually any color function should really just do:

def color_sth(txt):
    return PARAM1 + PARAM2 + PARAM3 + txt + NORMAL

(where PARAM1, PARAM2, PARAM3 were set into some kind of command-sth-color = PARAM1,PARAM2,PARAM3)

Currently a color function does call generateColorFunctionInner in a loop for each theme parameter value and will also call a terminateWith(...) which will strip some redundant ansii escape codes, heh.

Maybe what we should do is to change ColorConfig to some factory function that will:

1. Dynamically create a class with a (static?) method for each color parameter
2. Add triggers for each color parameter and if it is set to a new value, it would recreate the color prefix field
3. Probably save the created parameters as class fields, so if anybody wants to access them through the Python code, they can
4. Maybe use __slots__ because why not; but whatever :P
5. Return the dynamically created class

Here is an example code how it could look like. Ofc we would have to compute the proper color escape code prefix in the trigger function (that we will plug with GDB to be executed when the theme color parameter is changed):

Also, if we are even crazier, we could avoid the creation and access into a "theme parameters dictionary" by dynamically creating the ColorConfig class by rendering its code as a string and using eval(..) to execute it (fun-fact: this is what NamedTuple code does!). We would have to make sure the changed parameters are properly validated against invalid data so that one cannot create an invalid or malicious (lol) code in that case :P.

I know this sounds and is a bit of a premature optimization, since those things are called "only" few thousands of times , but small calls like this tends to add up. And if we look into how the performance of current solution is, its kinda stupid that for 10 executions of the context command, we ended up loosing few % of time on coloring:

FWIW, I have also tried checking the performance of this PR and it was fairly similar. However, I probably didn't make a proper benchmark even though I also checked it against the nearpc command instead of context. By proper benchmark I mean one that would trigger the execution of specific coloring functions for which we changed their calls here (I profiled it against an empty .c file so it didn't have much addresses/symbols/etc in there to be colored via nearpc).

Btw here is the profiling code I used. I just modified what we had in ./profiling/:

• profile.sh

#!/bin/bash

if ! (($#)); then
    cat <<-_EOF_
		$0: [profile-script]

		Example: $0 context.py
_EOF_
    exit 1
fi

module=$(basename "${1/.py/}")
basedir=$(dirname "$0")

# Quick and dirty script to profile pwndbg using cProfile.
make -C "${basedir}" test >/dev/null


#    -ex "set dereference-limit 20" \
#    -ex "set memoize off" \

gdb --quiet --nx \
    "${basedir}/test" \
    -ex "source ${basedir}/../gdbinit.py" \
    -ex "set context-stack-lines 50" \
    -ex "set dereference-limit 20" \
    -ex "b main" \
    -ex "r" \
    -ex "python
import cProfile;
import profiling.${module} as profile;
profile.warmup();
cProfile.run('profile.run()', '${basedir}/stats')" \
    -ex "quit"

python3 -c "
import pstats
p = pstats.Stats('${basedir}/stats')
p.strip_dirs().sort_stats('tottime').print_stats(20)
"

if command -v pyprof2calltree >/dev/null 2>&1 && command -v kcachegrind >/dev/null 2>&1; then
    pyprof2calltree -k -i "${basedir}/stats"
fi

# vim: ts=4 sw=4 noet

And context.py:

import pwndbg.commands.context


def warmup():
    pwndbg.commands.context.context()


def run():
    for i in range(10):
        pwndbg.commands.context.context()

And executed as ./profile.sh context.py

[disconnect3d]

Since I went a bit offtopic in my previous comment: I generally like this PR apart from minor things:

• we should probably not have a way to add_color_param after ColorConfig is initialized
• the raise AttributeError should probably have a meaningful error string

But generally, we should further refactor/change this and coloring relevant code as I described in the comment above.

We can either do this in this PR, or merge this one and make another one with the changes I mentioned.