[GR-24417] Generate clean query code.

PullRequest: graal/6959
XUZHOUWANG · Aug 26, 2020 · 768d001 · 768d001
2 parents 7230652 + 885446d
commit 768d001
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 100 deletions.
diff --git a/.../src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/headers/PosixDirectives.java b/.../src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/headers/PosixDirectives.java
@@ -43,7 +43,7 @@ public class PosixDirectives implements CContext.Directives {
                     "<pthread.h>",
                     "<pwd.h>",
                     "<signal.h>",
-                    "<sys/errno.h>",
+                    "<errno.h>",
                     "<sys/mman.h>",
                     "<sys/resource.h>",
                     "<sys/stat.h>",

diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java
@@ -455,6 +455,9 @@ public static Predicate<String> makeFilter(String[] definedFilters) {
     @Option(help = "Provide custom C compiler option used for query code compilation.", type = OptionType.User)//
     public static final HostedOptionKey<String[]> CCompilerOption = new HostedOptionKey<>(new String[0]);
 
+    @Option(help = "Use strict checks when performing query code compilation.", type = OptionType.User)//
+    public static final HostedOptionKey<Boolean> StrictQueryCodeCompilation = new HostedOptionKey<>(true);
+
     @APIOption(name = "native-image-info")//
     @Option(help = "Show native-toolchain information and image-build settings", type = User)//
     public static final HostedOptionKey<Boolean> DumpTargetInfo = new HostedOptionKey<>(false);

diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/CAnnotationProcessor.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/CAnnotationProcessor.java
@@ -35,7 +35,9 @@
 import java.util.List;
 
 import org.graalvm.nativeimage.ImageSingletons;
+import org.graalvm.nativeimage.Platform;
 
+import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateUtil;
 import com.oracle.svm.core.c.libc.LibCBase;
 import com.oracle.svm.core.util.InterruptImageBuilding;
@@ -48,7 +50,6 @@
 import com.oracle.svm.hosted.c.query.QueryResultParser;
 import com.oracle.svm.hosted.c.query.RawStructureLayoutPlanner;
 import com.oracle.svm.hosted.c.query.SizeAndSignednessVerifier;
-import org.graalvm.nativeimage.Platform;
 
 /**
  * Processes native library information for one C Library header file (one { NativeCodeContext }).
@@ -96,7 +97,7 @@ public NativeCodeInfo process(CAnnotationProcessorCache cache) {
              * Generate C source file (the "Query") that will produce the information needed (e.g.,
              * size of struct/union and offsets to their fields, value of enum/macros etc.).
              */
-            writer = new QueryCodeWriter(queryCodeDirectory);
+            writer = new QueryCodeWriter(compilerInvoker, queryCodeDirectory);
             Path queryFile = writer.write(codeInfo);
             if (nativeLibs.getErrors().size() > 0) {
                 return codeInfo;
@@ -160,7 +161,7 @@ private Path compileQueryCode(Path queryFile) {
         if (Platform.includedIn(Platform.LINUX.class)) {
             options.addAll(LibCBase.singleton().getAdditionalQueryCodeCompilerOptions());
         }
-        compilerInvoker.compileAndParseError(options, queryFile, binary, this::reportCompilerError, nativeLibs.debug);
+        compilerInvoker.compileAndParseError(SubstrateOptions.StrictQueryCodeCompilation.getValue(), options, queryFile, binary, this::reportCompilerError, nativeLibs.debug);
         return binary;
     }
 

diff --git a/...atevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/codegen/CCompilerInvoker.java b/...atevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/codegen/CCompilerInvoker.java
@@ -39,7 +39,6 @@
 import java.util.Scanner;
 import java.util.function.Consumer;
 
-import com.oracle.svm.core.c.libc.LibCBase;
 import org.graalvm.compiler.debug.DebugContext;
 import org.graalvm.compiler.serviceprovider.JavaVersionUtil;
 import org.graalvm.nativeimage.ImageSingletons;
@@ -48,6 +47,7 @@
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateTargetDescription;
 import com.oracle.svm.core.SubstrateUtil;
+import com.oracle.svm.core.c.libc.LibCBase;
 import com.oracle.svm.core.option.SubstrateOptionsParser;
 import com.oracle.svm.core.util.InterruptImageBuilding;
 import com.oracle.svm.core.util.UserError;
@@ -193,6 +193,17 @@ protected void verify() {
             }
         }
 
+        @Override
+        protected List<String> compileStrictOptions() {
+            /*
+             * On Windows `/Wall` corresponds to `-Wall -Wextra`. Therefore we use /W4 instead.
+             * Options `/wd4244` and `/wd4245` are needed because our query code makes use of
+             * implicit unsigned/signed conversions to detect signedness of types. `/wd4800`,
+             * `/wd4804` are needed to silence warnings when querying bool types. `/wd4214` is
+             * needed to make older versions of cl.exe accept bitfields larger than int-size.
+             */
+            return Arrays.asList("/WX", "/W4", "/wd4244", "/wd4245", "/wd4800", "/wd4804", "/wd4214");
+        }
     }
 
     private static class LinuxCCompilerInvoker extends CCompilerInvoker {
@@ -393,7 +404,8 @@ public interface CompilerErrorHandler {
     }
 
     @SuppressWarnings("try")
-    public void compileAndParseError(List<String> options, Path source, Path target, CompilerErrorHandler handler, DebugContext debug) {
+    public void compileAndParseError(boolean strict, List<String> compileOptions, Path source, Path target, CompilerErrorHandler handler, DebugContext debug) {
+        List<String> options = strict ? createStrictOptions(compileOptions) : compileOptions;
         ProcessBuilder pb = new ProcessBuilder()
                         .command(createCompilerCommand(options, target.normalize(), source.normalize()))
                         .directory(tempDirectory.toFile());
@@ -435,6 +447,16 @@ public void compileAndParseError(List<String> options, Path source, Path target,
         }
     }
 
+    private List<String> createStrictOptions(List<String> compileOptions) {
+        ArrayList<String> strictCompileOptions = new ArrayList<>(compileStrictOptions());
+        strictCompileOptions.addAll(compileOptions);
+        return strictCompileOptions;
+    }
+
+    protected List<String> compileStrictOptions() {
+        return Arrays.asList("-Wall", "-Werror");
+    }
+
     protected boolean detectError(String line) {
         return line.contains(": error:") || line.contains(": fatal error:");
     }

diff --git a/...tevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/codegen/CSourceCodeWriter.java b/...tevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/c/codegen/CSourceCodeWriter.java
@@ -26,16 +26,18 @@
 
 import static com.oracle.svm.core.util.VMError.shouldNotReachHere;
 import static com.oracle.svm.hosted.NativeImageOptions.CStandards.C11;
+import static com.oracle.svm.hosted.NativeImageOptions.CStandards.C99;
 
 import java.io.BufferedWriter;
 import java.io.IOException;
 import java.nio.channels.ClosedByInterruptException;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
@@ -66,7 +68,6 @@ public class CSourceCodeWriter {
 
     private static final String INDENT4 = "    ";
     public static final String C_SOURCE_FILE_EXTENSION = ".c";
-    public static final String CXX_SOURCE_FILE_EXTENSION = ".cpp";
 
     private final List<String> lines;
     private final StringBuilder currentLine;
@@ -80,6 +81,21 @@ public CSourceCodeWriter(Path tempDirectory) {
         this.currentLine = new StringBuilder(100);
     }
 
+    public void writeCStandardHeaders() {
+        if (NativeImageOptions.getCStandard().compatibleWith(C99)) {
+            if (!Platform.includedIn(Platform.WINDOWS.class)) {
+                /*
+                 * No stdbool.h in Windows SDK 7.1. If we add native-compiler version detection this
+                 * should only be omitted if cl.exe version is < 19.*.
+                 */
+                includeFiles(Collections.singletonList("<stdbool.h>"));
+            }
+        }
+        if (NativeImageOptions.getCStandard().compatibleWith(C11)) {
+            includeFiles(Collections.singletonList("<stdint.h>"));
+        }
+    }
+
     public int currentLineNumber() {
         return lines.size() + 1;
     }
@@ -94,7 +110,7 @@ public String getLine(int lineNumber) {
 
     public void includeFiles(List<String> headerFiles) {
         for (String headerFile : headerFiles) {
-            String headerFileName = null;
+            String headerFileName;
             if (headerFile.startsWith("<") && headerFile.endsWith(">")) {
                 headerFileName = headerFile.substring(1, headerFile.length() - 1);
                 Path headerFilePath = Paths.get(headerFileName);
@@ -157,21 +173,10 @@ public CSourceCodeWriter append(String str) {
     }
 
     public Path writeFile(String fileName) {
-        return writeFile(fileName, true);
-    }
-
-    public Path writeFile(String fileName, boolean ensureCorrectExtension) {
         assert currentLine.length() == 0 : "last line not finished";
 
-        String fixedFileName = fileName;
-        String srcFileExtension = Platform.includedIn(Platform.WINDOWS.class) ? CXX_SOURCE_FILE_EXTENSION : C_SOURCE_FILE_EXTENSION;
-        if (!fileName.endsWith(srcFileExtension) && ensureCorrectExtension) {
-            fixedFileName = fileName.concat(srcFileExtension);
-        }
-
-        Path outputFile = tempDirectory.resolve(fixedFileName);
-
-        try (BufferedWriter writer = Files.newBufferedWriter(outputFile, Charset.forName("UTF-8"))) {
+        Path outputFile = tempDirectory.resolve(fileName);
+        try (BufferedWriter writer = Files.newBufferedWriter(outputFile, StandardCharsets.UTF_8)) {
             for (String line : lines) {
                 writer.write(line);
                 writer.write("\n");
@@ -254,7 +259,6 @@ private static String toCIntegerType(ResolvedJavaType type, boolean isUnsigned)
             case Byte:
                 return prefix + (c11Compatible ? "int8_t" : "char");
             case Char:
-                return prefix + (c11Compatible ? "int16_t" : "short");
             case Short:
                 return prefix + (c11Compatible ? "int16_t" : "short");
             case Int:
@@ -275,6 +279,8 @@ private static boolean isFunctionPointer(MetaAccessProvider metaAccess, Resolved
      * Appends definition of "flags" like macro.
      */
     public void appendMacroDefinition(String preDefine) {
+        appendln("#ifndef " + preDefine);
         appendln("#define " + preDefine);
+        appendln("#endif");
     }
 }