Skip to content
Permalink
Browse files
8262198: Overhaul bitfield parsing logic
Reviewed-by: sundar, jvernee
  • Loading branch information
mcimadamore committed Feb 23, 2021
1 parent 6c47c8d commit 6aa7d355b1e002724d4f4c88a9ef0eb817d77548
@@ -251,21 +251,21 @@ private Constant emitLayoutField(String javaName, MemoryLayout layout) {
incrAlign();
indent();
append(memberMods() + "MemoryLayout " + fieldName + " = ");
emitLayoutString(layout);
emitLayoutString(layout, false);
append(";\n");
decrAlign();
return new Constant(className(), javaName, Constant.Kind.LAYOUT);
}

private void emitLayoutString(MemoryLayout l) {
private void emitLayoutString(MemoryLayout l, boolean inBitfield) {
if (l instanceof ValueLayout val) {
append(typeToLayoutName(val));
append(typeToLayoutName(val, inBitfield));
} else if (l instanceof SequenceLayout seq) {
append("MemoryLayout.ofSequence(");
if (seq.elementCount().isPresent()) {
append(seq.elementCount().getAsLong() + ", ");
}
emitLayoutString(seq.elementLayout());
emitLayoutString(seq.elementLayout(), false);
append(")");
} else if (l instanceof GroupLayout group) {
if (group.isStruct()) {
@@ -275,10 +275,11 @@ private void emitLayoutString(MemoryLayout l) {
}
incrAlign();
String delim = "";
boolean isBitfield = LayoutUtils.isBitfields(group);
for (MemoryLayout e : group.memberLayouts()) {
append(delim);
indent();
emitLayoutString(e);
emitLayoutString(e, isBitfield);
delim = ",\n";
}
append("\n");
@@ -305,7 +306,7 @@ private Constant emitFunctionDescField(String javaName, FunctionDescriptor desc)
append(" = ");
if (desc.returnLayout().isPresent()) {
append("FunctionDescriptor.of(");
emitLayoutString(desc.returnLayout().get());
emitLayoutString(desc.returnLayout().get(), false);
if (!noArgs) {
append(",");
}
@@ -319,7 +320,7 @@ private Constant emitFunctionDescField(String javaName, FunctionDescriptor desc)
for (MemoryLayout e : desc.argumentLayouts()) {
append(delim);
indent();
emitLayoutString(e);
emitLayoutString(e, false);
delim = ",\n";
}
append("\n");
@@ -359,23 +360,25 @@ private Constant emitConstantAddress(String javaName, Object value) {
return new Constant(className(), javaName, Constant.Kind.ADDRESS);
}

private static String typeToLayoutName(ValueLayout vl) {
private static String typeToLayoutName(ValueLayout vl, boolean inBitfields) {
if (UnsupportedLayouts.isUnsupported(vl)) {
return "MemoryLayout.ofPaddingBits(" + vl.bitSize() + ")";
} else if (inBitfields) {
return "MemoryLayout.ofValueBits(" + vl.bitSize() + ", ByteOrder.nativeOrder())";
} else {
CLinker.TypeKind kind = (CLinker.TypeKind) vl.attribute(CLinker.TypeKind.ATTR_NAME).orElseThrow(
() -> new IllegalStateException("Unexpected value layout: could not determine ABI class"));
return switch (kind) {
case CHAR -> "C_CHAR";
case SHORT -> "C_SHORT";
case INT -> "C_INT";
case LONG -> "C_LONG";
case LONG_LONG -> "C_LONG_LONG";
case FLOAT -> "C_FLOAT";
case DOUBLE -> "C_DOUBLE";
case POINTER -> "C_POINTER";
};
}

CLinker.TypeKind kind = (CLinker.TypeKind)vl.attribute(CLinker.TypeKind.ATTR_NAME).orElseThrow(
() -> new IllegalStateException("Unexpected value layout: could not determine ABI class"));
return switch (kind) {
case CHAR -> "C_CHAR";
case SHORT -> "C_SHORT";
case INT -> "C_INT";
case LONG -> "C_LONG";
case LONG_LONG -> "C_LONG_LONG";
case FLOAT -> "C_FLOAT";
case DOUBLE -> "C_DOUBLE";
case POINTER -> "C_POINTER";
};
}

private Constant emitSegmentField(String javaName, String nativeName, MemoryLayout layout) {
@@ -228,6 +228,7 @@ protected void emitPackagePrefix() {
protected void emitImportSection() {
append("import java.lang.invoke.MethodHandle;\n");
append("import java.lang.invoke.VarHandle;\n");
append("import java.nio.ByteOrder;\n");
append("import java.util.Objects;\n");
append("import jdk.incubator.foreign.*;\n");
append("import jdk.incubator.foreign.MemoryLayout.PathElement;\n");
@@ -27,6 +27,7 @@
package jdk.internal.jextract.impl;

import jdk.incubator.foreign.FunctionDescriptor;
import jdk.incubator.foreign.GroupLayout;
import jdk.incubator.foreign.MemoryLayout;
import jdk.incubator.jextract.Type.Primitive;
import jdk.internal.clang.Cursor;
@@ -43,6 +44,7 @@
public final class LayoutUtils {

public static final String JEXTRACT_ANONYMOUS = "jextract/anonymous";
public static final String JEXTRACT_BITFIELDS = "jextract/bitfields";

private LayoutUtils() {}

@@ -216,4 +218,13 @@ public static Primitive.Kind valueLayoutForSize(long size) {
default -> throw new IllegalStateException("Cannot infer container layout");
};
}

static boolean isBitfields(GroupLayout layout) {
return layout.attribute(JEXTRACT_BITFIELDS).isPresent();
}

@SuppressWarnings("unchecked")
static <Z extends MemoryLayout> Z setBitfields(Z layout) {
return (Z) layout.withAttribute(JEXTRACT_BITFIELDS, true);
}
}
@@ -85,7 +85,7 @@ public Declaration.Scoped parse(Path path, Collection<String> args) {
} else {
Declaration decl = treeMaker.createTree(c);
if (decl != null) {
decls.add(treeMaker.createTree(c));
decls.add(decl);
}
}
} else if (isMacro(c) && src.path() != null) {
@@ -26,9 +26,7 @@

package jdk.internal.jextract.impl;

import jdk.incubator.foreign.GroupLayout;
import jdk.incubator.foreign.MemoryLayout;
import jdk.incubator.foreign.ValueLayout;
import jdk.internal.clang.Cursor;
import jdk.internal.clang.CursorKind;
import jdk.internal.clang.Type;
@@ -137,13 +135,8 @@ long fieldSize(Cursor c) {
return c.isBitField() ? c.getBitFieldWidth() : c.type().size() * 8;
}

ValueLayout bitfield(ValueLayout container, List<MemoryLayout> sublayouts) {
return Utils.addContents(container, MemoryLayout.ofStruct(sublayouts.toArray(new MemoryLayout[0])));
}

ValueLayout bitfield(long containerSize, List<MemoryLayout> sublayouts) {
return bitfield((ValueLayout)LayoutUtils.valueLayoutForSize(containerSize)
.layout().orElseThrow(() -> new IllegalStateException("Unsupported size: " + containerSize)), sublayouts);
MemoryLayout bitfield(List<MemoryLayout> sublayouts) {
return LayoutUtils.setBitfields(MemoryLayout.ofStruct(sublayouts.toArray(new MemoryLayout[0])));
}

long offsetOf(Type parent, Cursor c) {
@@ -131,65 +131,8 @@ MemoryLayout finishLayout() {
// process bitfields if any and clear bitfield layouts
private void handleBitfields() {
if (bitfieldLayouts != null) {
fieldLayouts.addAll(convertBitfields(bitfieldLayouts));
fieldLayouts.add(bitfield(bitfieldLayouts));
bitfieldLayouts = null;
}
}

private List<MemoryLayout> convertBitfields(List<MemoryLayout> layouts) {
long offset = 0L;
List<MemoryLayout> newFields = new ArrayList<>();
List<MemoryLayout> pendingFields = new ArrayList<>();
for (MemoryLayout l : layouts) {
offset += l.bitSize();
if (offset > MAX_STORAGE_SIZE) {
throw new IllegalStateException("Crossing storage unit boundaries");
}
pendingFields.add(l);
long storageSize = storageSize(offset);
if (!pendingFields.isEmpty() && storageSize != -1) {
//emit new
newFields.add(bitfield(storageSize, pendingFields));
pendingFields.clear();
offset = 0L;
}
}
if (!pendingFields.isEmpty()) {
long storageSize = nextStorageSize(offset);
//emit new
newFields.add(bitfield(storageSize, pendingFields));
pendingFields.clear();
}
return newFields;
}

static int[] STORAGE_SIZES = { 64, 32, 16, 8 };
static int[] ALIGN_SIZES = { 8, 16, 32, 64 };
static int MAX_STORAGE_SIZE = 64;

private long storageSize(long size) {
// offset should be < MAX_STORAGE_SIZE
for (int s : STORAGE_SIZES) {
if (size == s) {
return s;
}
}
return -1;
}

private long nextStorageSize(long size) {
// offset should be < MAX_STORAGE_SIZE
for (int s : ALIGN_SIZES) {
long alignedSize = alignUp(size, s);
long storageSize = storageSize(alignedSize);
if (storageSize != -1) {
return storageSize;
}
}
return -1;
}

private static long alignUp(long n, long alignment) {
return (n + alignment - 1) & -alignment;
}
}
@@ -34,7 +34,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

@@ -299,10 +298,9 @@ private List<Declaration> collectBitfields(MemoryLayout layout, List<Declaration
.collect(Collectors.toSet());
List<Declaration> newDecls = new ArrayList<>();
for (MemoryLayout e : ((GroupLayout)layout).memberLayouts()) {
Optional<GroupLayout> contents = Utils.getContents(e);
if (contents.isPresent()) {
if (e instanceof GroupLayout contents && LayoutUtils.isBitfields(contents)) {
List<Declaration.Variable> bfDecls = new ArrayList<>();
outer: for (MemoryLayout bitfield : contents.get().memberLayouts()) {
outer: for (MemoryLayout bitfield : contents.memberLayouts()) {
if (bitfield.name().isPresent() && !nestedBitfieldNames.contains(bitfield.name().get())) {
Iterator<Declaration> declIt = declarations.iterator();
while (declIt.hasNext()) {
@@ -317,7 +315,7 @@ private List<Declaration> collectBitfields(MemoryLayout layout, List<Declaration
}
}
if (!bfDecls.isEmpty()) {
newDecls.add(Declaration.bitfields(bfDecls.get(0).pos(), "", contents.get(), bfDecls.toArray(new Declaration.Variable[0])));
newDecls.add(Declaration.bitfields(bfDecls.get(0).pos(), "", contents, bfDecls.toArray(new Declaration.Variable[0])));
}
}
}
@@ -33,6 +33,7 @@
import jdk.internal.clang.Type;
import jdk.internal.clang.TypeKind;

import java.util.ArrayList;
import java.util.List;

/**
@@ -66,8 +67,7 @@ void startBitfield() {
@Override
MemoryLayout fieldLayout(Cursor c) {
if (c.isBitField()) {
MemoryLayout layout = LayoutUtils.getLayout(c.type());
return bitfield((ValueLayout) layout, List.of(super.fieldLayout(c)));
return bitfield(List.of(super.fieldLayout(c)));
} else {
return super.fieldLayout(c);
}
@@ -77,16 +77,22 @@ MemoryLayout fieldLayout(Cursor c) {
long fieldSize(Cursor c) {
if (c.type().kind() == TypeKind.IncompleteArray) {
return 0;
} else if (c.isBitField()) {
return c.getBitFieldWidth();
} else {
return c.type().size() * 8;
}
return c.type().size() * 8;
}

@Override
MemoryLayout finishLayout() {
// size mismatch indicates anonymous bitfield used for padding
// size mismatch indicates use of bitfields in union
long expectedSize = type.size() * 8;
if (actualSize < expectedSize) {
// emit an extra padding of expected size to make sure union layout size is computed correctly
addFieldLayout(MemoryLayout.ofPaddingBits(expectedSize));
} else if (actualSize > expectedSize) {
throw new AssertionError("Invalid union size - expected: " + expectedSize + "; found: " + actualSize);
}

MemoryLayout[] fields = fieldLayouts.toArray(new MemoryLayout[0]);
@@ -26,28 +26,22 @@

package jdk.internal.jextract.impl;

import jdk.incubator.foreign.GroupLayout;
import jdk.incubator.foreign.MemoryLayout;
import jdk.internal.clang.Cursor;
import jdk.internal.clang.CursorKind;
import jdk.internal.clang.SourceLocation;
import jdk.internal.clang.Type;
import jdk.internal.clang.TypeKind;

import javax.lang.model.SourceVersion;
import javax.tools.JavaFileObject;
import javax.tools.SimpleJavaFileObject;
import java.io.IOException;
import java.lang.reflect.Method;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
@@ -322,13 +316,4 @@ static String quote(char ch) {
private static boolean isPrintableAscii(char ch) {
return ch >= ' ' && ch <= '~';
}

static Optional<GroupLayout> getContents(MemoryLayout layout) {
return layout.attribute("contents").map(GroupLayout.class::cast);
}

@SuppressWarnings("unchecked")
static <Z extends MemoryLayout> Z addContents(Z layout, GroupLayout contents) {
return (Z) layout.withAttribute("contents", contents);
}
}
@@ -23,17 +23,13 @@

/*
* @test
* @requires os.family != "windows"
* @modules jdk.incubator.jextract
* @library /test/lib
* @build BadBitfieldTest
* @run testng/othervm -Dforeign.restricted=permit BadBitfieldTest
*/

/*
* Not running on Windows since MSVC will not cross storage unit boundaries.
* Resulting struct on SysV is 16 bytes, but 24 on MSx64
*
* MSVC: (/d1reportSingleClassLayoutFoo)
* class Foo size(24):
* +---
@@ -65,8 +61,6 @@ public class BadBitfieldTest extends JextractToolRunner {
@Test
public void testBadBitfield() {
run("-d", getOutputFilePath("badBitfieldsGen").toString(),
getInputFilePath("badBitfields.h").toString())
.checkFailure()
.checkContainsOutput("Crossing storage unit boundaries");
getInputFilePath("badBitfields.h").toString()).checkSuccess();
}
}

0 comments on commit 6aa7d35

Please sign in to comment.