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

gh-90997: Show cached inline values in dis output #92360

Merged
merged 6 commits into from
May 6, 2022

Conversation

brandtbucher
Copy link
Member

This adds the actual values stored in CACHE entries to the "argrepr" field of dis output when show_caches=True.

Example:

>>> import dis
>>> 
>>> def f():
...     l = []
...     for i in range(42):
...         l.append(i)
... 
>>> f()  # Quicken...
>>> f()  # Specialize...
>>> 
>>> dis.dis(f, adaptive=True, show_caches=True)
  1           0 RESUME_QUICK                      0

  2           2 BUILD_LIST                        0
              4 STORE_FAST                        0 (l)

  3           6 LOAD_GLOBAL_BUILTIN               1 (NULL + range)
              8 CACHE                             0 (counter: 53)
             10 CACHE                             0 (index: 70)
             12 CACHE                             0 (module_keys_version: 62)
             14 CACHE                             0
             16 CACHE                             0 (builtin_keys_version: 26)
             18 LOAD_CONST                        1 (42)
             20 PRECALL_BUILTIN_CLASS             1
             22 CACHE                             0 (counter: 53)
             24 CALL_ADAPTIVE                     1
             26 CACHE                             0 (counter: 0)
             28 CACHE                             0 (func_version: 0)
             30 CACHE                             0
             32 CACHE                             0 (min_args: 0)
             34 GET_ITER
             36 FOR_ITER                         23 (to 84)
             38 STORE_FAST__LOAD_FAST             1 (i)

  4          40 LOAD_FAST                         0 (l)
             42 LOAD_METHOD_NO_DICT               1 (append)
             44 CACHE                             0 (counter: 53)
             46 CACHE                             0 (type_version: 3)
             48 CACHE                             0
             50 CACHE                             0 (dict_offset: 0)
             52 CACHE                             0 (keys_version: 0)
             54 CACHE                             0
             56 CACHE                             0 (descr: 140374364819376)
             58 CACHE                             0
             60 CACHE                             0
             62 CACHE                             0
             64 LOAD_FAST                         1 (i)
             66 PRECALL_NO_KW_LIST_APPEND         1
             68 CACHE                             0 (counter: 53)
             70 CALL_ADAPTIVE                     1
             72 CACHE                             0 (counter: 0)
             74 CACHE                             0 (func_version: 0)
             76 CACHE                             0
             78 CACHE                             0 (min_args: 0)
             80 POP_TOP
             82 JUMP_BACKWARD_QUICK              24 (to 36)

  3     >>   84 LOAD_CONST                        0 (None)
             86 RETURN_VALUE

@pablogsal, since this aids in debugging our new adaptive/specialized instructions and inline caches, I'd prefer to get this into 3.11 if possible. If you agree, please feel free to merge while I'm sleeping or whatever.

@brandtbucher brandtbucher added the 3.11 only security fixes label May 6, 2022
@brandtbucher brandtbucher mentioned this pull request May 6, 2022
26 tasks
@markshannon
Copy link
Member

Looks good, but it should have at least one test.

@markshannon
Copy link
Member

LGTM
@pablogsal could you merge this please.
It should make 3.11 more reliable, as it will be a great debugging aid

@pablogsal pablogsal merged commit 93a666b into python:main May 6, 2022
@pablogsal
Copy link
Member

This PR is failing on the buildbots: https://buildbot.python.org/all/#/builders/244/builds/2311/steps/5/logs/stdio

Please, prepare a fix ASAP or I would need to revert it :(

@pablogsal
Copy link
Member

@brandtbucher @markshannon can you take a look?

@brandtbucher
Copy link
Member Author

Looking at it now. The s390x failures suggest that this may be due to endianness. My initial hunch is that this test uncovered a bug in how we're reading and writing caches (which should be in native byte order).

@brandtbucher
Copy link
Member Author

brandtbucher commented May 6, 2022

The test seems to only be failing on big-endian architectures when the caches are actually populated with non-zero values. The symptom looks like messed-up parsing of nearby instructions (in this case, a LOAD_FAST or STORE_FAST looks like it's getting its oparg mangled by a nearby cache entry)?

@brandtbucher
Copy link
Member Author

New theory: I think dis._unpack_opargs might be treating some caches as EXTENDED_ARGs. I think this test just triggers the issue (it doesn't cause it). I'll try out a fix (I don't have a big-endian machine, though, so I guess I'll just run the buildbots on the PR).

@brandtbucher
Copy link
Member Author

#92409

@brandtbucher brandtbucher deleted the show-caches branch July 21, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants