Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
public class NullnessVerifier implements RuntimeVerifier {

private static final ClassDesc VERIFIER = ClassDesc.of(NullnessRuntimeVerifier.class.getName());
private static final ClassDesc ATTRIBUTION_KIND =
ClassDesc.of(AttributionKind.class.getName());
private static final ClassDesc ATTRIBUTION_KIND = ClassDesc.of(AttributionKind.class.getName());

private static final String METHOD_DEFAULT = "checkNotNull";
private static final MethodTypeDesc DESC_DEFAULT =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,19 @@ public static void main(String[] args) {
test.finalAction(null);
// cannot bridge final methods, no error here

String unsafe = test.returnAction();
// :: error: (Return value of inherited method returnAction must be NonNull)
test.returnAction();

// :: error: (Return value of inherited method returnAction must be NonNull)
// :: error: (Local Variable Assignment (Slot 2) must be NonNull)
String unsafe = test.returnAction();

// :: error: (Return value of inherited method returnAction must be NonNull)
@Nullable String again = test.returnAction();
}

@Override
public void overrideMe(@NonNull String inputA, @Nullable String inputB) {
System.out.println("safe version of this method" + inputA + inputB);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,22 @@ static class UncheckedLib {
public String poison = null;
}

public static void consume(@Nullable Object o) {}

public static void main(String[] args) {
UncheckedLib lib = new UncheckedLib();

String s = lib.poison;
// 1. Read without storage (Argument passing)
// :: error: (Read Field 'poison' must be NonNull)
consume(lib.poison);

// 2. Assignment
// :: error: (Read Field 'poison' must be NonNull)
// :: error: (Local Variable Assignment (Slot 2) must be NonNull)
String s = lib.poison;

@Nullable String q = lib.poison;
// 3. Nullable Assignment (Still checks read)
// :: error: (Read Field 'poison' must be NonNull)
@Nullable String q = lib.poison;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import org.checkerframework.checker.nullness.qual.Nullable;
import io.github.eisop.runtimeframework.qual.AnnotatedFor;

@AnnotatedFor("nullness")
Expand All @@ -7,8 +8,16 @@ static class UncheckedLib {
public static String POISON = null;
}

public static void consume(@Nullable Object o) {}

public static void main(String[] args) {
String s = UncheckedLib.POISON;
// 1. Read without storage
// :: error: (Read Field 'POISON' must be NonNull)
consume(UncheckedLib.POISON);

// 2. Assignment
// :: error: (Read Field 'POISON' must be NonNull)
// :: error: (Local Variable Assignment (Slot 1) must be NonNull)
String s = UncheckedLib.POISON;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ public static void main(String[] args) {
UncheckedLib lib = new UncheckedLib();

String s = lib.getNull();
// :: error: (Return value of getNull (Boundary) must be NonNull)
// :: error: (Local Variable Assignment (Slot 2) must be NonNull)

lib.getNull();
// currently no explicit check on the return if its not stored
// :: error: (Return value of getNull (Boundary) must be NonNull)

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ public static String getNull() {

public static void main(String[] args) {
@Nullable String s = UncheckedLib.getNull();
// :: error: (Return value of getNull (Boundary) must be NonNull)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public static String getNull() {

public static void main(String[] args) {
String s = UncheckedLib.getNull();
// :: error: (Return value of getNull (Boundary) must be NonNull)
// :: error: (Local Variable Assignment (Slot 1) must be NonNull)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ static class UncheckedLib {
}

public static void main(String[] args) {
// :: error: (Read Field 'POISON' must be NonNull)
// :: error: (Parameter 0 must be NonNull)
consume(UncheckedLib.POISON);
nullableConsume(UncheckedLib.POISON);

// :: error: (Read Field 'POISON' must be NonNull)
nullableConsume(UncheckedLib.POISON);
}

public static void consume(@NonNull String arg) {
// :: error: (Parameter 0 must be NonNull)
}

public static void nullableConsume(@Nullable String arg) {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,14 @@ protected void generateUncheckedReturnCheck(

@Override
protected void generateMethodCallCheck(CodeBuilder b, InvokeInstruction invoke) {
// empty for now, only need to generate checks when a method call is stored somehwhere
RuntimeVerifier target =
policy.getBoundaryCallCheck(invoke.owner().asInternalName(), invoke.typeSymbol());

if (target != null) {
b.dup();
target.generateCheck(
b, TypeKind.REFERENCE, "Return value of " + invoke.name().stringValue() + " (Boundary)");
}
}

@Override
Expand Down Expand Up @@ -225,6 +231,16 @@ private void emitBridge(ClassBuilder builder, ParentMethod parentMethod) {
ClassDesc.of(
parentMethod.owner().thisClass().asInternalName().replace('/', '.'));
codeBuilder.invokespecial(parentDesc, methodName, desc);

RuntimeVerifier returnTarget = policy.getBridgeReturnCheck(parentMethod);
if (returnTarget != null) {
codeBuilder.dup();
returnTarget.generateCheck(
codeBuilder,
TypeKind.REFERENCE,
"Return value of inherited method " + methodName);
}

returnResult(
codeBuilder, ClassDesc.ofDescriptor(desc.returnType().descriptorString()));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,7 @@ public void accept(ClassBuilder classBuilder, ClassElement classElement) {
} else if (isFieldRead(fInst)) {
codeBuilder.with(element);
if (isCheckedScope) {
// generateFieldReadCheck(codeBuilder, fInst, classModel);
// Currently disabling field read checks as the GETFIELD
// and GETSTATIC instructions are not actually dangerous
// on their own. Its when we STORE a field we read from
// that an issue could arise
// TODO: consider method of turning on and off different
// boundary sites
generateFieldReadCheck(codeBuilder, fInst, classModel);
}
}
} else if (element instanceof ReturnInstruction rInst) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ default RuntimeVerifier getBoundaryFieldWriteCheck(
/** For a bridge we are generating, what check applies to this parameter? */
RuntimeVerifier getBridgeParameterCheck(ParentMethod parentMethod, int paramIndex);

/** For a bridge we are generating, what check applies to the return value? */
default RuntimeVerifier getBridgeReturnCheck(ParentMethod parentMethod) {
return null;
}

/** Should we check an value being stored into an array? */
RuntimeVerifier getArrayStoreCheck(TypeKind componentType);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
import io.github.eisop.runtimeframework.core.ValidationKind;
import io.github.eisop.runtimeframework.filter.ClassInfo;
import io.github.eisop.runtimeframework.filter.Filter;
import io.github.eisop.runtimeframework.policy.EnforcementPolicy;
import io.github.eisop.runtimeframework.runtime.AttributionKind;
import io.github.eisop.runtimeframework.resolution.ParentMethod;
import io.github.eisop.runtimeframework.runtime.AttributionKind;
import java.lang.classfile.Annotation;
import java.lang.classfile.Attributes;
import java.lang.classfile.FieldModel;
Expand Down Expand Up @@ -146,6 +145,7 @@ public boolean shouldGenerateBridge(ParentMethod parentMethod) {
// MethodTypeDesc param parsing
var paramTypes = method.methodTypeSymbol().parameterList();

// 1. Check Parameters
for (int i = 0; i < paramTypes.size(); i++) {
boolean explicitNoop = false;
boolean explicitEnforce = false;
Expand All @@ -172,6 +172,34 @@ public boolean shouldGenerateBridge(ParentMethod parentMethod) {
}
}
}

// 2. Check Return Type
TypeKind returnType = TypeKind.from(method.methodTypeSymbol().returnType());
if (returnType == TypeKind.REFERENCE) {
boolean explicitNoop = false;
boolean explicitEnforce = false;

List<Annotation> annos = getMethodReturnAnnotations(method);

for (Annotation anno : annos) {
String desc = anno.classSymbol().descriptorString();
TypeSystemConfiguration.ConfigEntry entry = configuration.find(desc);
if (entry != null) {
if (entry.kind() == ValidationKind.ENFORCE) explicitEnforce = true;
if (entry.kind() == ValidationKind.NOOP) explicitNoop = true;
}
}

if (explicitEnforce) return true;

if (!explicitNoop) {
TypeSystemConfiguration.ConfigEntry defaultEntry = configuration.getDefault();
if (defaultEntry != null && defaultEntry.kind() == ValidationKind.ENFORCE) {
return true;
}
}
}

return false;
}

Expand Down Expand Up @@ -208,6 +236,33 @@ public RuntimeVerifier getBridgeParameterCheck(ParentMethod parentMethod, int pa
return null;
}

@Override
public RuntimeVerifier getBridgeReturnCheck(ParentMethod parentMethod) {
MethodModel method = parentMethod.method();
TypeKind returnType = TypeKind.from(method.methodTypeSymbol().returnType());
if (returnType != TypeKind.REFERENCE) return null;

List<Annotation> annos = getMethodReturnAnnotations(method);

RuntimeVerifier verifier = resolveVerifier(annos);
if (verifier != null) return verifier.withAttribution(AttributionKind.CALLER);

boolean isExplicitNoop = false;
for (Annotation a : annos) {
TypeSystemConfiguration.ConfigEntry entry =
configuration.find(a.classSymbol().descriptorString());
if (entry != null && entry.kind() == ValidationKind.NOOP) isExplicitNoop = true;
}

if (!isExplicitNoop) {
TypeSystemConfiguration.ConfigEntry defaultEntry = configuration.getDefault();
if (defaultEntry != null && defaultEntry.kind() == ValidationKind.ENFORCE) {
return defaultEntry.verifier().withAttribution(AttributionKind.CALLER);
}
}
return null;
}

// --- Parsing Helpers ---
private List<Annotation> getMethodParamAnnotations(MethodModel method, int paramIndex) {
List<Annotation> result = new ArrayList<>();
Expand All @@ -230,6 +285,24 @@ private List<Annotation> getMethodParamAnnotations(MethodModel method, int param
return result;
}

private List<Annotation> getMethodReturnAnnotations(MethodModel method) {
List<Annotation> result = new ArrayList<>();
method
.findAttribute(Attributes.runtimeVisibleAnnotations())
.ifPresent(attr -> result.addAll(attr.annotations()));
method
.findAttribute(Attributes.runtimeVisibleTypeAnnotations())
.ifPresent(
attr -> {
for (TypeAnnotation ta : attr.annotations()) {
if (ta.targetInfo().targetType() == TypeAnnotation.TargetType.METHOD_RETURN) {
result.add(ta.annotation());
}
}
});
return result;
}

private List<Annotation> getFieldAnnotations(FieldModel field) {
List<Annotation> result = new ArrayList<>();
field
Expand Down