Skip to content

Commit 6aa7d35

Browse files
mcimadamoremcimadamore
committedFeb 23, 2021
8262198: Overhaul bitfield parsing logic
Reviewed-by: sundar, jvernee
1 parent 6c47c8d commit 6aa7d35

File tree

10 files changed

+54
-120
lines changed

10 files changed

+54
-120
lines changed
 

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/ConstantBuilder.java

+24-21
Original file line numberDiff line numberDiff line change
@@ -251,21 +251,21 @@ private Constant emitLayoutField(String javaName, MemoryLayout layout) {
251251
incrAlign();
252252
indent();
253253
append(memberMods() + "MemoryLayout " + fieldName + " = ");
254-
emitLayoutString(layout);
254+
emitLayoutString(layout, false);
255255
append(";\n");
256256
decrAlign();
257257
return new Constant(className(), javaName, Constant.Kind.LAYOUT);
258258
}
259259

260-
private void emitLayoutString(MemoryLayout l) {
260+
private void emitLayoutString(MemoryLayout l, boolean inBitfield) {
261261
if (l instanceof ValueLayout val) {
262-
append(typeToLayoutName(val));
262+
append(typeToLayoutName(val, inBitfield));
263263
} else if (l instanceof SequenceLayout seq) {
264264
append("MemoryLayout.ofSequence(");
265265
if (seq.elementCount().isPresent()) {
266266
append(seq.elementCount().getAsLong() + ", ");
267267
}
268-
emitLayoutString(seq.elementLayout());
268+
emitLayoutString(seq.elementLayout(), false);
269269
append(")");
270270
} else if (l instanceof GroupLayout group) {
271271
if (group.isStruct()) {
@@ -275,10 +275,11 @@ private void emitLayoutString(MemoryLayout l) {
275275
}
276276
incrAlign();
277277
String delim = "";
278+
boolean isBitfield = LayoutUtils.isBitfields(group);
278279
for (MemoryLayout e : group.memberLayouts()) {
279280
append(delim);
280281
indent();
281-
emitLayoutString(e);
282+
emitLayoutString(e, isBitfield);
282283
delim = ",\n";
283284
}
284285
append("\n");
@@ -305,7 +306,7 @@ private Constant emitFunctionDescField(String javaName, FunctionDescriptor desc)
305306
append(" = ");
306307
if (desc.returnLayout().isPresent()) {
307308
append("FunctionDescriptor.of(");
308-
emitLayoutString(desc.returnLayout().get());
309+
emitLayoutString(desc.returnLayout().get(), false);
309310
if (!noArgs) {
310311
append(",");
311312
}
@@ -319,7 +320,7 @@ private Constant emitFunctionDescField(String javaName, FunctionDescriptor desc)
319320
for (MemoryLayout e : desc.argumentLayouts()) {
320321
append(delim);
321322
indent();
322-
emitLayoutString(e);
323+
emitLayoutString(e, false);
323324
delim = ",\n";
324325
}
325326
append("\n");
@@ -359,23 +360,25 @@ private Constant emitConstantAddress(String javaName, Object value) {
359360
return new Constant(className(), javaName, Constant.Kind.ADDRESS);
360361
}
361362

362-
private static String typeToLayoutName(ValueLayout vl) {
363+
private static String typeToLayoutName(ValueLayout vl, boolean inBitfields) {
363364
if (UnsupportedLayouts.isUnsupported(vl)) {
364365
return "MemoryLayout.ofPaddingBits(" + vl.bitSize() + ")";
366+
} else if (inBitfields) {
367+
return "MemoryLayout.ofValueBits(" + vl.bitSize() + ", ByteOrder.nativeOrder())";
368+
} else {
369+
CLinker.TypeKind kind = (CLinker.TypeKind) vl.attribute(CLinker.TypeKind.ATTR_NAME).orElseThrow(
370+
() -> new IllegalStateException("Unexpected value layout: could not determine ABI class"));
371+
return switch (kind) {
372+
case CHAR -> "C_CHAR";
373+
case SHORT -> "C_SHORT";
374+
case INT -> "C_INT";
375+
case LONG -> "C_LONG";
376+
case LONG_LONG -> "C_LONG_LONG";
377+
case FLOAT -> "C_FLOAT";
378+
case DOUBLE -> "C_DOUBLE";
379+
case POINTER -> "C_POINTER";
380+
};
365381
}
366-
367-
CLinker.TypeKind kind = (CLinker.TypeKind)vl.attribute(CLinker.TypeKind.ATTR_NAME).orElseThrow(
368-
() -> new IllegalStateException("Unexpected value layout: could not determine ABI class"));
369-
return switch (kind) {
370-
case CHAR -> "C_CHAR";
371-
case SHORT -> "C_SHORT";
372-
case INT -> "C_INT";
373-
case LONG -> "C_LONG";
374-
case LONG_LONG -> "C_LONG_LONG";
375-
case FLOAT -> "C_FLOAT";
376-
case DOUBLE -> "C_DOUBLE";
377-
case POINTER -> "C_POINTER";
378-
};
379382
}
380383

381384
private Constant emitSegmentField(String javaName, String nativeName, MemoryLayout layout) {

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/JavaSourceBuilder.java

+1
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ protected void emitPackagePrefix() {
228228
protected void emitImportSection() {
229229
append("import java.lang.invoke.MethodHandle;\n");
230230
append("import java.lang.invoke.VarHandle;\n");
231+
append("import java.nio.ByteOrder;\n");
231232
append("import java.util.Objects;\n");
232233
append("import jdk.incubator.foreign.*;\n");
233234
append("import jdk.incubator.foreign.MemoryLayout.PathElement;\n");

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/LayoutUtils.java

+11
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
package jdk.internal.jextract.impl;
2828

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

4546
public static final String JEXTRACT_ANONYMOUS = "jextract/anonymous";
47+
public static final String JEXTRACT_BITFIELDS = "jextract/bitfields";
4648

4749
private LayoutUtils() {}
4850

@@ -216,4 +218,13 @@ public static Primitive.Kind valueLayoutForSize(long size) {
216218
default -> throw new IllegalStateException("Cannot infer container layout");
217219
};
218220
}
221+
222+
static boolean isBitfields(GroupLayout layout) {
223+
return layout.attribute(JEXTRACT_BITFIELDS).isPresent();
224+
}
225+
226+
@SuppressWarnings("unchecked")
227+
static <Z extends MemoryLayout> Z setBitfields(Z layout) {
228+
return (Z) layout.withAttribute(JEXTRACT_BITFIELDS, true);
229+
}
219230
}

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/Parser.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public Declaration.Scoped parse(Path path, Collection<String> args) {
8585
} else {
8686
Declaration decl = treeMaker.createTree(c);
8787
if (decl != null) {
88-
decls.add(treeMaker.createTree(c));
88+
decls.add(decl);
8989
}
9090
}
9191
} else if (isMacro(c) && src.path() != null) {

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/RecordLayoutComputer.java

+2-9
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@
2626

2727
package jdk.internal.jextract.impl;
2828

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

140-
ValueLayout bitfield(ValueLayout container, List<MemoryLayout> sublayouts) {
141-
return Utils.addContents(container, MemoryLayout.ofStruct(sublayouts.toArray(new MemoryLayout[0])));
142-
}
143-
144-
ValueLayout bitfield(long containerSize, List<MemoryLayout> sublayouts) {
145-
return bitfield((ValueLayout)LayoutUtils.valueLayoutForSize(containerSize)
146-
.layout().orElseThrow(() -> new IllegalStateException("Unsupported size: " + containerSize)), sublayouts);
138+
MemoryLayout bitfield(List<MemoryLayout> sublayouts) {
139+
return LayoutUtils.setBitfields(MemoryLayout.ofStruct(sublayouts.toArray(new MemoryLayout[0])));
147140
}
148141

149142
long offsetOf(Type parent, Cursor c) {

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/StructLayoutComputer.java

+1-58
Original file line numberDiff line numberDiff line change
@@ -131,65 +131,8 @@ MemoryLayout finishLayout() {
131131
// process bitfields if any and clear bitfield layouts
132132
private void handleBitfields() {
133133
if (bitfieldLayouts != null) {
134-
fieldLayouts.addAll(convertBitfields(bitfieldLayouts));
134+
fieldLayouts.add(bitfield(bitfieldLayouts));
135135
bitfieldLayouts = null;
136136
}
137137
}
138-
139-
private List<MemoryLayout> convertBitfields(List<MemoryLayout> layouts) {
140-
long offset = 0L;
141-
List<MemoryLayout> newFields = new ArrayList<>();
142-
List<MemoryLayout> pendingFields = new ArrayList<>();
143-
for (MemoryLayout l : layouts) {
144-
offset += l.bitSize();
145-
if (offset > MAX_STORAGE_SIZE) {
146-
throw new IllegalStateException("Crossing storage unit boundaries");
147-
}
148-
pendingFields.add(l);
149-
long storageSize = storageSize(offset);
150-
if (!pendingFields.isEmpty() && storageSize != -1) {
151-
//emit new
152-
newFields.add(bitfield(storageSize, pendingFields));
153-
pendingFields.clear();
154-
offset = 0L;
155-
}
156-
}
157-
if (!pendingFields.isEmpty()) {
158-
long storageSize = nextStorageSize(offset);
159-
//emit new
160-
newFields.add(bitfield(storageSize, pendingFields));
161-
pendingFields.clear();
162-
}
163-
return newFields;
164-
}
165-
166-
static int[] STORAGE_SIZES = { 64, 32, 16, 8 };
167-
static int[] ALIGN_SIZES = { 8, 16, 32, 64 };
168-
static int MAX_STORAGE_SIZE = 64;
169-
170-
private long storageSize(long size) {
171-
// offset should be < MAX_STORAGE_SIZE
172-
for (int s : STORAGE_SIZES) {
173-
if (size == s) {
174-
return s;
175-
}
176-
}
177-
return -1;
178-
}
179-
180-
private long nextStorageSize(long size) {
181-
// offset should be < MAX_STORAGE_SIZE
182-
for (int s : ALIGN_SIZES) {
183-
long alignedSize = alignUp(size, s);
184-
long storageSize = storageSize(alignedSize);
185-
if (storageSize != -1) {
186-
return storageSize;
187-
}
188-
}
189-
return -1;
190-
}
191-
192-
private static long alignUp(long n, long alignment) {
193-
return (n + alignment - 1) & -alignment;
194-
}
195138
}

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/TreeMaker.java

+3-5
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import java.util.List;
3535
import java.util.Map;
3636
import java.util.Objects;
37-
import java.util.Optional;
3837
import java.util.Set;
3938
import java.util.stream.Collectors;
4039

@@ -299,10 +298,9 @@ private List<Declaration> collectBitfields(MemoryLayout layout, List<Declaration
299298
.collect(Collectors.toSet());
300299
List<Declaration> newDecls = new ArrayList<>();
301300
for (MemoryLayout e : ((GroupLayout)layout).memberLayouts()) {
302-
Optional<GroupLayout> contents = Utils.getContents(e);
303-
if (contents.isPresent()) {
301+
if (e instanceof GroupLayout contents && LayoutUtils.isBitfields(contents)) {
304302
List<Declaration.Variable> bfDecls = new ArrayList<>();
305-
outer: for (MemoryLayout bitfield : contents.get().memberLayouts()) {
303+
outer: for (MemoryLayout bitfield : contents.memberLayouts()) {
306304
if (bitfield.name().isPresent() && !nestedBitfieldNames.contains(bitfield.name().get())) {
307305
Iterator<Declaration> declIt = declarations.iterator();
308306
while (declIt.hasNext()) {
@@ -317,7 +315,7 @@ private List<Declaration> collectBitfields(MemoryLayout layout, List<Declaration
317315
}
318316
}
319317
if (!bfDecls.isEmpty()) {
320-
newDecls.add(Declaration.bitfields(bfDecls.get(0).pos(), "", contents.get(), bfDecls.toArray(new Declaration.Variable[0])));
318+
newDecls.add(Declaration.bitfields(bfDecls.get(0).pos(), "", contents, bfDecls.toArray(new Declaration.Variable[0])));
321319
}
322320
}
323321
}

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/UnionLayoutComputer.java

+10-4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import jdk.internal.clang.Type;
3434
import jdk.internal.clang.TypeKind;
3535

36+
import java.util.ArrayList;
3637
import java.util.List;
3738

3839
/**
@@ -66,8 +67,7 @@ void startBitfield() {
6667
@Override
6768
MemoryLayout fieldLayout(Cursor c) {
6869
if (c.isBitField()) {
69-
MemoryLayout layout = LayoutUtils.getLayout(c.type());
70-
return bitfield((ValueLayout) layout, List.of(super.fieldLayout(c)));
70+
return bitfield(List.of(super.fieldLayout(c)));
7171
} else {
7272
return super.fieldLayout(c);
7373
}
@@ -77,16 +77,22 @@ MemoryLayout fieldLayout(Cursor c) {
7777
long fieldSize(Cursor c) {
7878
if (c.type().kind() == TypeKind.IncompleteArray) {
7979
return 0;
80+
} else if (c.isBitField()) {
81+
return c.getBitFieldWidth();
82+
} else {
83+
return c.type().size() * 8;
8084
}
81-
return c.type().size() * 8;
8285
}
8386

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

9298
MemoryLayout[] fields = fieldLayouts.toArray(new MemoryLayout[0]);

‎src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/Utils.java

-15
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,22 @@
2626

2727
package jdk.internal.jextract.impl;
2828

29-
import jdk.incubator.foreign.GroupLayout;
30-
import jdk.incubator.foreign.MemoryLayout;
3129
import jdk.internal.clang.Cursor;
3230
import jdk.internal.clang.CursorKind;
3331
import jdk.internal.clang.SourceLocation;
3432
import jdk.internal.clang.Type;
35-
import jdk.internal.clang.TypeKind;
3633

3734
import javax.lang.model.SourceVersion;
3835
import javax.tools.JavaFileObject;
3936
import javax.tools.SimpleJavaFileObject;
4037
import java.io.IOException;
41-
import java.lang.reflect.Method;
4238
import java.net.URI;
4339
import java.nio.file.Files;
4440
import java.nio.file.Path;
4541
import java.util.ArrayList;
4642
import java.util.Arrays;
47-
import java.util.HashMap;
4843
import java.util.List;
4944
import java.util.Optional;
50-
import java.util.stream.Collectors;
5145
import java.util.stream.Stream;
5246

5347
/**
@@ -322,13 +316,4 @@ static String quote(char ch) {
322316
private static boolean isPrintableAscii(char ch) {
323317
return ch >= ' ' && ch <= '~';
324318
}
325-
326-
static Optional<GroupLayout> getContents(MemoryLayout layout) {
327-
return layout.attribute("contents").map(GroupLayout.class::cast);
328-
}
329-
330-
@SuppressWarnings("unchecked")
331-
static <Z extends MemoryLayout> Z addContents(Z layout, GroupLayout contents) {
332-
return (Z) layout.withAttribute("contents", contents);
333-
}
334319
}

‎test/jdk/tools/jextract/BadBitfieldTest.java

+1-7
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,13 @@
2323

2424
/*
2525
* @test
26-
* @requires os.family != "windows"
2726
* @modules jdk.incubator.jextract
2827
* @library /test/lib
2928
* @build BadBitfieldTest
3029
* @run testng/othervm -Dforeign.restricted=permit BadBitfieldTest
3130
*/
3231

3332
/*
34-
* Not running on Windows since MSVC will not cross storage unit boundaries.
35-
* Resulting struct on SysV is 16 bytes, but 24 on MSx64
36-
*
3733
* MSVC: (/d1reportSingleClassLayoutFoo)
3834
* class Foo size(24):
3935
* +---
@@ -65,8 +61,6 @@ public class BadBitfieldTest extends JextractToolRunner {
6561
@Test
6662
public void testBadBitfield() {
6763
run("-d", getOutputFilePath("badBitfieldsGen").toString(),
68-
getInputFilePath("badBitfields.h").toString())
69-
.checkFailure()
70-
.checkContainsOutput("Crossing storage unit boundaries");
64+
getInputFilePath("badBitfields.h").toString()).checkSuccess();
7165
}
7266
}

0 commit comments

Comments
 (0)
Please sign in to comment.