Skip to content

Commit

Permalink
Fix crash when looking up invalid attribute from type object
Browse files Browse the repository at this point in the history
Summary:
We can crash when we're looking up an invalid attribute from a type object.  In the case that was reported this was from looking up an invalid object from a function object.

What happens is we have a site where we're presumably getting some valid lookup and putting in a cache for a load method (`foo.getdoc()`).

Then we come along and pass in a function object as `foo`.  Because the object is a `type` we go into `_PyShadow_LoadMethodFromType`.  The first time we go through and raise the proper attribute error when we create a new cache entry because we do `if (((_PyShadow_InstanceAttrEntry *)entry)->value == NULL `.  But when we do it again at another call site we pick up the pre-existing cache entry which is still valid and happily try to dispatch to it via `_PyShadow_LoadMethodType`.  This assumes we have a `value` and tries to incref it and we crash.

Reviewed By: oclbdk

Differential Revision: D50386200

fbshipit-source-id: 0b8f3716dcf27d722140fd56b766f2d5790986e1
  • Loading branch information
DinoV authored and facebook-github-bot committed Oct 18, 2023
1 parent 8e269c0 commit 0672d94
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
35 changes: 35 additions & 0 deletions Lib/test/test_shadowcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from test.support.script_helper import assert_python_ok, run_python_until_end
from unittest import skipIf
from test.cinder_support import CINDERJIT_ENABLED
from types import FunctionType
import cinder
from cinder import (
cached_property,
Expand Down Expand Up @@ -3091,5 +3092,39 @@ def __init__(self):
with self.assertRaises(AttributeError):
f(C, True)

def test_load_method_function_no_attr(self):
"""Invalidating a cache and picking up a new cache from a type
needs to check that the type has the descriptor"""
class C:
def getdoc(self): return 'doc'


def f(x, z):
if x:
z.getdoc()
else:
z.getdoc()

# Setup valid cache entries, we want 2 so that we can replace
# one with a new cache, and then re-use the cache entry on
# a second call.
a = C()
for i in range(REPETITION):
f(True, a)
f(False, a)

# Force creation of the type cache for FunctionType.getdoc
try:
f(False, FunctionType)
except AttributeError:
pass

# Then attempt to re-use it in another cache
try:
f(True, FunctionType)
except AttributeError:
pass


if __name__ == "__main__":
unittest.main()
4 changes: 4 additions & 0 deletions Shadowcode/shadowcode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1823,6 +1823,10 @@ _PyShadow_LoadMethodFromType(_PyShadow_EvalState *state,
entry = _PyShadow_GetCacheForAttr(cache, name);
if (entry != NULL && _PyShadow_IsCacheValid(entry)) {
/* We have an existing valid cache, re-use it */
if (((_PyShadow_InstanceAttrEntry *)entry)->value == NULL) {
return 0;
}

Py_INCREF(entry);
goto run_entry;
}
Expand Down

0 comments on commit 0672d94

Please sign in to comment.