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

drgn.cli: look up symbols as missing in global namespace #358

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ThinkerYzu
Copy link

Introduce a new option --auto-find to make the interactive mode load the corresponding symbol from prog if the user code access a global variable that is undefined while the name is defined by the program being debugged.

For example, you can access jiffies of the kernel directly without going through prog['jiffies'].

print(jiffies)

This line will be compiled and run. However, the auto-find mode will look into the code object and know that the code object is going to access a global variable named "jiffies". However, "jiffies" is not defined in the global namespace. The auto-find mode will check if the program being debugged defines a symbol with the same name and add it to the global namespace if there is.

Introduce a new option --auto-find to make the interactive mode load
the corresponding symbol from prog if the user code access a global
variable that is undefined while the name is defined by the program
being debugged.

For example, you can access jiffies of the kernel directly without
going through prog['jiffies'].

  print(jiffies)

This line will be compiled and run. However, the auto-find mode will
look into the code object and know that the code object is going to
access a global variable named "jiffies".  However, "jiffies" is not
defined in the global namespace.  The auto-find mode will check if the
program being debugged defines a symbol with the same name and add it
to the global namespace if there is.

Signed-off-by: Thinker Lee <thinker.li@gmail.com>
@osandov
Copy link
Owner

osandov commented Oct 16, 2023

This is really interesting, thanks. I have mixed feelings about it. It is a common complaint that drgn can be verbose, so I love that this improves on that.

One minor issue is that although this implementation is cool, it feels a little too complicated and brittle. Here's a simpler way to do it:

diff --git a/drgn/cli.py b/drgn/cli.py
index 44839ec3..c3c41569 100644
--- a/drgn/cli.py
+++ b/drgn/cli.py
@@ -302,6 +302,14 @@ def _main() -> None:
         run_interactive(prog)
 
 
+class _ProgGlobalsDict(dict):
+    def __getitem__(self, key):
+        try:
+            return dict.__getitem__(self, key)
+        except KeyError:
+            return dict.__getitem__(self, "prog")[key]
+
+
 def run_interactive(
     prog: drgn.Program,
     banner_func: Optional[Callable[[str], str]] = None,
@@ -397,7 +405,14 @@ For help, type help(drgn).
         sys.displayhook = _displayhook
 
         try:
-            code.interact(banner=banner, exitmsg="", local=init_globals)
+            code.interact(
+                banner=banner, exitmsg="",
+                # Note that the documentation for exec() states that this must
+                # be a dict and not a subclass, but this was intentionally
+                # changed in Python 3.3 (https://bugs.python.org/issue14385).
+                # The documentation has not been updated as of Python 3.12.
+                local=_ProgGlobalsDict(init_globals)
+            )
         finally:
             try:
                 readline.write_history_file(histfile)

So we definitely can do this in a reasonable way, but I want to think about whether we should do this.

First of all, it feels weird to have a command line argument for this. Most people will probably not find it. So if we want this feature, we probably want it to be the default.

But, I would definitely not want this for script mode, since that would make it impossible to do any type checking or linting or other stuff like that. This would create a pretty big difference between interactive mode and script mode, which I'm hesitant to do.

Let me ask around and see what other people think.

@ThinkerYzu
Copy link
Author

diff --git a/drgn/cli.py b/drgn/cli.py
index 44839ec3..c3c41569 100644
--- a/drgn/cli.py
+++ b/drgn/cli.py
@@ -302,6 +302,14 @@ def _main() -> None:
run_interactive(prog)

+class _ProgGlobalsDict(dict):

  • def getitem(self, key):
  •    try:
    
  •        return dict.__getitem__(self, key)
    
  •    except KeyError:
    
  •        return dict.__getitem__(self, "prog")[key]
    

This is much better.
I actually started with this approach. However, it didn't work.
I just looked back at what I did. I found that I forgot to derive it from dict.

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.

3 participants