Fix: Add super class inheritance checks in PyObjectLookupAttrNode.

Previously the specializations for `BuiltinType` and `BuiltinTypeType` only checked for inheritance via metaclasses /
excluded the possibility of inheritance via metaclasses. As classes can also inherit attributes from their
superclasses this needed be handled, too.

Author: phoeinx

graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/nodes/util/PyObjectLookupAttrTests.java

@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or
+ * data (collectively the "Software"), free of charge and under any and all
+ * copyright rights in the Software, and any and all patent rights owned or
+ * freely licensable by each licensor hereunder covering either (i) the
+ * unmodified Software as contributed to or provided by such licensor, or (ii)
+ * the Larger Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ *
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ *
+ * The above copyright notice and either this complete permission notice or at a
+ * minimum a reference to the UPL must be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package com.oracle.graal.python.nodes.util;
+
+import com.oracle.graal.python.builtins.PythonBuiltinClassType;
+import com.oracle.graal.python.builtins.objects.PNone;
+import com.oracle.graal.python.lib.PyObjectLookupAttr;
+import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.nodes.RootNode;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.oracle.graal.python.test.PythonTests;
+
+public class PyObjectLookupAttrTests {
+
+    @Before
+    public void setUp() {
+        PythonTests.enterContext();
+    }
+
+    @After
+    public void tearDown() {
+        PythonTests.closeContext();
+    }
+
+    // Regression test to ensure that super class attributes are evaluated for BuiltinClassType
+    // attribute lookup.
+    // Booleans super class PInt defines "real" which we expect to find in an attribute lookup.
+    @Test
+    public void lookupThroughMRO() {
+        Object v = new RootNode(null) {
+            @Node.Child private PyObjectLookupAttr lookupNode = PyObjectLookupAttr.create();
+
+            @Override
+            public Object execute(VirtualFrame frame) {
+                return lookupNode.execute(frame, PythonBuiltinClassType.Boolean, "real");
+            }
+        }.getCallTarget().call();
+        Assert.assertNotSame(PNone.NO_VALUE, v);
+    }
+}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PyObjectLookupAttr.java

@@ -174,15 +174,16 @@ static final Object doBuiltinModule(VirtualFrame frame, Object object, String na
         }
     }
 
-    // simple version that needs no calls and only reads from the object directly. the only
-    // difference for type.__getattribute__ over object.__getattribute__ is that it looks for a
-    // __get__ method on the value and invokes it if it is callable.
+    // If the class of an object is "type", the object must be a class and as "type" is the base
+    // metaclass, which defines only certain type slots, it can not have inherited other
+    // attributes via metaclass inheritance. For all non-type-slot attributes it therefore
+    // suffices to only check for inheritance via super classes.
     @SuppressWarnings("unused")
     @Specialization(guards = {"isTypeGetAttribute(type)", "isBuiltinTypeType(type)", "!isTypeSlot(name)"}, limit = "1")
     static final Object doBuiltinTypeType(VirtualFrame frame, Object object, String name,
                     @Cached GetClassNode getClass,
                     @Bind("getClass.execute(object)") Object type,
-                    @Cached ReadAttributeFromObjectNode readNode,
+                    @Cached LookupAttributeInMRONode.Dynamic readNode,
                     @Cached ConditionProfile valueFound,
                     @Cached("create(Get)") LookupInheritedSlotNode lookupValueGet,
                     @Cached ConditionProfile noGetMethod,
@@ -205,24 +206,24 @@ static final Object doBuiltinTypeType(VirtualFrame frame, Object object, String
         return PNone.NO_VALUE;
     }
 
-    // simple version that needs no calls and only reads from the object directly. the only
-    // difference for type.__getattribute__ over object.__getattribute__ is that it looks for a
-    // __get__ method on the value and invokes it if it is callable.
+    // simple version that only reads attributes from (super) class inheritance and the object
+    // itself. the only difference for type.__getattribute__ over object.__getattribute__
+    // is that it looks for a __get__ method on the value and invokes it if it is callable.
     @SuppressWarnings("unused")
-    @Specialization(guards = {"isTypeGetAttribute(type)", "hasNoGetAttr(type)", "name == cachedName", "isNoValue(descr)"}, limit = "1", replaces = "doBuiltinTypeType")
+    @Specialization(guards = {"isTypeGetAttribute(type)", "hasNoGetAttr(type)", "name == cachedName", "isNoValue(metaClassDescr)"}, replaces = "doBuiltinTypeType", limit = "1")
     static final Object doBuiltinType(VirtualFrame frame, Object object, String name,
                     @Cached("name") String cachedName,
                     @Cached GetClassNode getClass,
                     @Bind("getClass.execute(object)") Object type,
-                    @Cached("create(name)") LookupAttributeInMRONode lookupName,
-                    @Bind("lookupName.execute(type)") Object descr,
-                    @Cached ReadAttributeFromObjectNode readNode,
+                    @Cached("create(name)") LookupAttributeInMRONode lookupInMetaclassHierachy,
+                    @Bind("lookupInMetaclassHierachy.execute(type)") Object metaClassDescr,
+                    @Cached("create(name)") LookupAttributeInMRONode readNode,
                     @Cached ConditionProfile valueFound,
                     @Cached("create(Get)") LookupInheritedSlotNode lookupValueGet,
                     @Cached ConditionProfile noGetMethod,
                     @Cached CallTernaryMethodNode invokeValueGet,
                     @Shared("errorProfile") @Cached IsBuiltinClassProfile errorProfile) {
-        Object value = readNode.execute(object, cachedName);
+        Object value = readNode.execute(object);
         if (valueFound.profile(value != PNone.NO_VALUE)) {
             Object valueGet = lookupValueGet.execute(value);
             if (noGetMethod.profile(valueGet == PNone.NO_VALUE)) {