[commit: ghc] master: Fix uninformative hp2ps error when the cmdline contains double quotes (0136906)

git at git.haskell.org git at git.haskell.org
Tue Dec 11 23:21:40 UTC 2018


Repository : ssh://git@git.haskell.org/ghc

On branch  : master
Link       : http://ghc.haskell.org/trac/ghc/changeset/0136906c9e69b02cd1ffe2704fa5d737d8c4cfaf/ghc

>---------------------------------------------------------------

commit 0136906c9e69b02cd1ffe2704fa5d737d8c4cfaf
Author: Zejun Wu <watashi at fb.com>
Date:   Tue Dec 11 13:18:03 2018 -0500

    Fix uninformative hp2ps error when the cmdline contains double quotes
    
    Reapply D5346 with fix incompatible shell quoting in tests. It seems
    like `$'string'` is not recognized under all test environments, so let's
    avoid it in tests.
    
    Test Plan:
    ```
    hp2ps: "T15904".hp, line 2: integer must follow identifier
    ```
    
    use new ghc and hp2ps to profile a simple program.
    
    Reviewers: simonmar, bgamari, erikd, tdammers
    
    Reviewed By: bgamari
    
    Subscribers: tdammers, carter, rwbarton
    
    GHC Trac Issues: #15904
    
    Differential Revision: https://phabricator.haskell.org/D5388


>---------------------------------------------------------------

0136906c9e69b02cd1ffe2704fa5d737d8c4cfaf
 rts/ProfHeap.c                      | 30 ++++++++++++++++++++++--------
 testsuite/tests/hp2ps/Makefile      |  9 +++++++++
 testsuite/tests/hp2ps/T15904.hs     |  8 ++++++++
 testsuite/tests/hp2ps/T15904.stdout |  6 ++++++
 testsuite/tests/hp2ps/all.T         |  1 +
 utils/hp2ps/HpFile.c                | 37 +++++++++++++++++--------------------
 6 files changed, 63 insertions(+), 28 deletions(-)

diff --git a/rts/ProfHeap.c b/rts/ProfHeap.c
index de3d2b6..517702f 100644
--- a/rts/ProfHeap.c
+++ b/rts/ProfHeap.c
@@ -361,6 +361,18 @@ void endProfiling( void )
 #endif /* !PROFILING */
 
 static void
+printEscapedString(const char* string)
+{
+    for (const char* p = string; *p != '\0'; ++p) {
+        if (*p == '\"') {
+            // Escape every " as ""
+            fputc('"', hp_file);
+        }
+        fputc(*p, hp_file);
+    }
+}
+
+static void
 printSample(bool beginSample, StgDouble sampleValue)
 {
     fprintf(hp_file, "%s %f\n",
@@ -428,16 +440,18 @@ initHeapProfiling(void)
     initEra( &censuses[era] );
 
     /* initProfilingLogFile(); */
-    fprintf(hp_file, "JOB \"%s", prog_name);
+    fprintf(hp_file, "JOB \"");
+    printEscapedString(prog_name);
 
 #if defined(PROFILING)
-    {
-        int count;
-        for(count = 1; count < prog_argc; count++)
-            fprintf(hp_file, " %s", prog_argv[count]);
-        fprintf(hp_file, " +RTS");
-        for(count = 0; count < rts_argc; count++)
-            fprintf(hp_file, " %s", rts_argv[count]);
+    for (int i = 1; i < prog_argc; ++i) {
+        fputc(' ', hp_file);
+        printEscapedString(prog_argv[i]);
+    }
+    fprintf(hp_file, " +RTS");
+    for (int i = 0; i < rts_argc; ++i) {
+        fputc(' ', hp_file);
+        printEscapedString(rts_argv[i]);
     }
 #endif /* PROFILING */
 
diff --git a/testsuite/tests/hp2ps/Makefile b/testsuite/tests/hp2ps/Makefile
new file mode 100644
index 0000000..ec7deb5
--- /dev/null
+++ b/testsuite/tests/hp2ps/Makefile
@@ -0,0 +1,9 @@
+TOP=../..
+include $(TOP)/mk/boilerplate.mk
+include $(TOP)/mk/test.mk
+
+.PHONY: T15904
+T15904:
+	"$(TEST_HC)" $(TEST_HC_OPTS) -rtsopts -main-is "$@" "$@.hs" -o "\"$@\""
+	"./\"$@\"" '{"e": 2.72, "pi": 3.14}' "\\" "" '"' +RTS -h
+	"$(HP2PS_ABS)" "\"$@\".hp"
diff --git a/testsuite/tests/hp2ps/T15904.hs b/testsuite/tests/hp2ps/T15904.hs
new file mode 100644
index 0000000..7c009ff
--- /dev/null
+++ b/testsuite/tests/hp2ps/T15904.hs
@@ -0,0 +1,8 @@
+module T15904 (main) where
+
+import System.Environment
+
+main :: IO ()
+main = do
+  args <- getArgs
+  mapM_ putStrLn args
diff --git a/testsuite/tests/hp2ps/T15904.stdout b/testsuite/tests/hp2ps/T15904.stdout
new file mode 100644
index 0000000..e77005b
--- /dev/null
+++ b/testsuite/tests/hp2ps/T15904.stdout
@@ -0,0 +1,6 @@
+[1 of 1] Compiling T15904           ( T15904.hs, T15904.o )
+Linking "T15904" ...
+{"e": 2.72, "pi": 3.14}
+\
+
+"
diff --git a/testsuite/tests/hp2ps/all.T b/testsuite/tests/hp2ps/all.T
new file mode 100644
index 0000000..bebeb56
--- /dev/null
+++ b/testsuite/tests/hp2ps/all.T
@@ -0,0 +1 @@
+test('T15904', [], run_command, ['$MAKE -s --no-print-directory T15904'])
diff --git a/utils/hp2ps/HpFile.c b/utils/hp2ps/HpFile.c
index a64a74a..bc172e5 100644
--- a/utils/hp2ps/HpFile.c
+++ b/utils/hp2ps/HpFile.c
@@ -398,45 +398,42 @@ GetIdent(FILE *infp)
 
 
 /*
- *      Read a sequence of characters that make up a string and
- *      assign the result to "thestring".
+ * Read a sequence of characters that make up a string and assign the result to
+ * "thestring". A string is surrounded by double quotes, with a double quote
+ * itself escaped as two contiguous double quotes.
  */
 
 void
 GetString(FILE *infp)
 {
-    unsigned int i;
-    char *stringbuffer;
-    size_t stringbuffersize;
-
     ASSERT(ch == '\"');
 
-    stringbuffersize = 5000;
-    stringbuffer = xmalloc(stringbuffersize);
+    size_t stringbuffersize = 5000;
+    char *stringbuffer = xmalloc(stringbuffersize);
 
     ch = getc(infp);    /* skip the '\"' that begins the string */
 
-    i = 0;
-    while (ch != '\"') {
+    for (size_t i = 0; ; ++i) {
         if (ch == EOF) {
-                Error("%s, line %d: EOF when expecting \"", hpfile, linenum, ch);
+            Error("%s, line %d: EOF when expecting \"", hpfile, linenum, ch);
         }
-        else if (i == stringbuffersize - 1) {
-            stringbuffersize = 2 * stringbuffersize;
+        if (i == stringbuffersize) {
+            stringbuffersize *= 2;
             stringbuffer = xrealloc(stringbuffer, stringbuffersize);
         }
-        stringbuffer[ i++ ] = ch;
+        if (ch == '\"') {
+            ch = getc(infp);
+            if (ch != '\"') {
+                stringbuffer[i] = '\0';
+                break;
+            }
+        }
+        stringbuffer[i] = ch;
         ch = getc(infp);
     }
 
-    stringbuffer[i] = '\0';
     thestring = copystring(stringbuffer);
-
     free(stringbuffer);
-
-    ASSERT(ch == '\"');
-
-    ch = getc(infp);      /* skip the '\"' that terminates the string */
 }
 
 boolish



More information about the ghc-commits mailing list