Implement @ZeroingWeak annotation which will translate annotated fiel…

…ds to "weak" in ARC, and convert to WeakReference<T> in MRC which will be translated to zeroing IOSReference in ObjC. The @ZeroingWeak annotation is designed to be safe to use, unlike existing @weak annotation which gets translated to __unsafe_unretained and can introduce crashes caused by dangling pointers. @ZeringWeak can only be used for @nullable fields, which requires users to handle nullability. PiperOrigin-RevId: 329468917
michaelyousrie · Sep 1, 2020 · 43a8a94 · 43a8a94
1 parent 47dc355
commit 43a8a94
Show file tree

Hide file tree

Showing 14 changed files with 547 additions and 8 deletions.
diff --git a/annotations/src/main/java/com/google/j2objc/annotations/ZeroingWeak.java b/annotations/src/main/java/com/google/j2objc/annotations/ZeroingWeak.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2012 Google Inc. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.j2objc.annotations;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.RetentionPolicy.CLASS;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation that indicates a variable has a weak relationship to its owner. The variable will be
+ * annotated with "weak" in ARC, and converted to WeakReference in manual reference counting.
+ *
+ * <p>Because reading from such variables may give null, this annotation must be used in combination
+ * with @Nullable annotation, and can not be used in combination with @NonNull.
+ *
+ * @author Michał Pociecha-Łoś
+ */
+@Target(FIELD)
+@Retention(CLASS)
+public @interface ZeroingWeak {}
diff --git a/cycle_finder/src/main/java/com/google/devtools/cyclefinder/GraphBuilder.java b/cycle_finder/src/main/java/com/google/devtools/cyclefinder/GraphBuilder.java
@@ -37,6 +37,7 @@
 import com.google.j2objc.annotations.RetainedWith;
 import com.google.j2objc.annotations.Weak;
 import com.google.j2objc.annotations.WeakOuter;
+import com.google.j2objc.annotations.ZeroingWeak;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -277,7 +278,7 @@ private void followFields(DeclaredType type, TypeNode node) {
             && !ElementUtil.isStatic(field)
             // Exclude self-referential fields. (likely linked DS or delegate pattern)
             && !typeUtil.isAssignable(type, fieldType)
-            && !isWeakReference(field)
+            && !isUnretainedReference(field)
             && !isRetainedWithField(field)) {
           addEdge(Edge.newFieldEdge(node, target, fieldName));
         }
@@ -306,8 +307,9 @@ private void followCaptureFields(TypeElement type, TypeNode typeNode) {
       assert ElementUtil.isAnonymous(type);
       for (VariableElement capturedVarElement : captureInfo.getLocalCaptureFields(type)) {
         TypeNode targetNode = getOrCreateNode(capturedVarElement.asType());
-        if (targetNode != null && !whitelist.containsType(targetNode)
-            && !ElementUtil.isWeakReference(capturedVarElement)) {
+        if (targetNode != null
+            && !whitelist.containsType(targetNode)
+            && !ElementUtil.isUnretainedReference(capturedVarElement)) {
           addEdge(Edge.newCaptureEdge(
               typeNode, targetNode, ElementUtil.getName(capturedVarElement)));
         }
@@ -318,6 +320,15 @@ private boolean isWeakReference(VariableElement field) {
       return ElementUtil.isWeakReference(field) || hasExternalAnnotation(field, Weak.class);
     }
 
+    private boolean isZeroingWeakReference(VariableElement field) {
+      return ElementUtil.isZeroingWeakReference(field)
+          || hasExternalAnnotation(field, ZeroingWeak.class);
+    }
+
+    private boolean isUnretainedReference(VariableElement field) {
+      return isWeakReference(field) || isZeroingWeakReference(field);
+    }
+
     private boolean isRetainedWithField(VariableElement field) {
       return ElementUtil.isRetainedWithField(field)
           || hasExternalAnnotation(field, RetainedWith.class);

diff --git a/jre_emul/Classes/J2ObjC_common.h b/jre_emul/Classes/J2ObjC_common.h
@@ -23,6 +23,7 @@
 #import "J2ObjC_types.h"
 
 @class IOSClass;
+@class JavaLangRefWeakReference;
 @protocol JavaLangIterable;
 
 #ifndef __has_feature
@@ -75,6 +76,9 @@ void JreCloneVolatileStrong(volatile_id *pVar, volatile_id *pOther);
 void JreReleaseVolatile(volatile_id *pVar);
 id JreRetainedLocalValue(id value);
 
+id JreZeroingWeakGet(id zeroingWeak);
+id JreMakeZeroingWeak(id object);
+
 id JreRetainedWithAssign(id parent, __strong id *pIvar, id value);
 id JreVolatileRetainedWithAssign(id parent, volatile_id *pIvar, id value);
 void JreRetainedWithRelease(id parent, id child);

diff --git a/jre_emul/Classes/JreZeroingWeak.m b/jre_emul/Classes/JreZeroingWeak.m
@@ -0,0 +1,26 @@
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//
+//  JreZeroingWeak.h
+//  JreEmulation
+//
+//  Created by Michał Pociecha-Łoś
+//
+
+#import "java/lang/ref/WeakReference.h"
+
+id JreZeroingWeakGet(id zeroingWeak) { return zeroingWeak ? [zeroingWeak get] : nil; }
+
+id JreMakeZeroingWeak(id object) {
+  return object ? create_JavaLangRefWeakReference_initWithId_(object) : nil;
+}
diff --git a/jre_emul/jre_sources.mk b/jre_emul/jre_sources.mk
@@ -35,6 +35,7 @@ NATIVE_JRE_SOURCES_CORE = \
   JavaThrowable.m \
   JreRetainedLocalValue.m \
   JreRetainedWith.m \
+  JreZeroingWeak.m \
   MappedByteBuffer.m \
   NSCopying+JavaCloneable.m \
   NSDataInputStream.m \

diff --git a/translator/src/main/java/com/google/devtools/j2objc/ast/ChildLink.java b/translator/src/main/java/com/google/devtools/j2objc/ast/ChildLink.java
@@ -33,6 +33,10 @@ public static <T extends TreeNode> ChildLink<T> create(Class<T> childType, TreeN
     return new ChildLink<T>(childType, parent);
   }
 
+  public Class<T> getChildType() {
+    return childType;
+  }
+
   public TreeNode getParent() {
     return parent;
   }

diff --git a/translator/src/main/java/com/google/devtools/j2objc/ast/TreeNode.java b/translator/src/main/java/com/google/devtools/j2objc/ast/TreeNode.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.j2objc.ast;
 
+import com.google.common.base.Supplier;
+
 /**
  * Base class for nodes in the J2ObjC AST.
  */
@@ -50,11 +52,22 @@ public void remove() {
     }
   }
 
+  public boolean canReplaceWith(Class<? extends TreeNode> type) {
+    return owner != null && owner.getChildType().isAssignableFrom(type);
+  }
+
   public void replaceWith(TreeNode other) {
     assert owner != null : "Can't replace a parentless node.";
     owner.setDynamic(other);
   }
 
+  public void replaceWith(Supplier<? extends TreeNode> supplier) {
+    ChildLink<? extends TreeNode> owner = this.owner;
+    assert owner != null : "Can't replace a parentless node.";
+    owner.set(null);
+    owner.setDynamic(supplier.get());
+  }
+
   public final int getStartPosition() {
     return startPosition;
   }

diff --git a/translator/src/main/java/com/google/devtools/j2objc/ast/TreeUtil.java b/translator/src/main/java/com/google/devtools/j2objc/ast/TreeUtil.java
@@ -406,4 +406,9 @@ public static List<AnnotationTypeMemberDeclaration> getAnnotationMembers(
     return Lists.newArrayList(
         Iterables.filter(node.getBodyDeclarations(), AnnotationTypeMemberDeclaration.class));
   }
+
+  public static boolean isAssignmentLeftHandSide(Expression expression) {
+    TreeNode parent = expression.getParent();
+    return parent instanceof Assignment && ((Assignment) parent).getLeftHandSide() == expression;
+  }
 }
diff --git a/translator/src/main/java/com/google/devtools/j2objc/gen/TypeDeclarationGenerator.java b/translator/src/main/java/com/google/devtools/j2objc/gen/TypeDeclarationGenerator.java
@@ -298,12 +298,20 @@ protected void printInstanceVariables() {
         lastDeclaration = declaration;
         JavadocGenerator.printDocComment(getBuilder(), declaration.getJavadoc());
         printIndent();
-        if (ElementUtil.isWeakReference(varElement) && !ElementUtil.isVolatile(varElement)) {
-          // We must add this even without -use-arc because the header may be
-          // included by a file compiled with ARC.
-          print("__unsafe_unretained ");
+        if (!ElementUtil.isVolatile(varElement)) {
+          if (ElementUtil.isWeakReference(varElement)) {
+            // We must add this even without -use-arc because the header may be
+            // included by a file compiled with ARC.
+            print("__unsafe_unretained ");
+          }
+          if (ElementUtil.isZeroingWeakReference(varElement) && options.useARC()) {
+            print("weak ");
+          }
         }
         String objcType = getDeclarationType(varElement);
+        if (ElementUtil.isZeroingWeakReference(varElement) && !options.useARC()) {
+          objcType = "JavaLangRefWeakReference *";
+        }
         needsAsterisk = objcType.endsWith("*");
         if (needsAsterisk) {
           // Strip pointer from type, as it will be added when appending fragment.

diff --git a/translator/src/main/java/com/google/devtools/j2objc/pipeline/TranslationProcessor.java b/translator/src/main/java/com/google/devtools/j2objc/pipeline/TranslationProcessor.java
@@ -59,6 +59,7 @@
 import com.google.devtools.j2objc.translate.UnsequencedExpressionRewriter;
 import com.google.devtools.j2objc.translate.VarargsRewriter;
 import com.google.devtools.j2objc.translate.VariableRenamer;
+import com.google.devtools.j2objc.translate.ZeroingWeakRewriter;
 import com.google.devtools.j2objc.types.HeaderImportCollector;
 import com.google.devtools.j2objc.types.ImplementationImportCollector;
 import com.google.devtools.j2objc.types.Import;
@@ -183,6 +184,9 @@ public static void applyMutations(
     new VariableRenamer(unit).run();
     ticker.tick("VariableRenamer");
 
+    new ZeroingWeakRewriter(unit).run();
+    ticker.tick("ZeroingWeakRewriter");
+
     // Rewrite enhanced for loops into correct C code.
     new EnhancedForRewriter(unit).run();
     ticker.tick("EnhancedForRewriter");