[llvm] 5df55bc - [FileCheck] Fix isalpha/isalnum calls

[Joel E. Denny via llvm-commits]

Author: Joel E. Denny
Date: 2020-05-14T20:24:09-04:00
New Revision: 5df55bc7a46fd5319402ac856753bc5ba5e7fb8a

URL: https://github.com/llvm/llvm-project/commit/5df55bc7a46fd5319402ac856753bc5ba5e7fb8a
DIFF: https://github.com/llvm/llvm-project/commit/5df55bc7a46fd5319402ac856753bc5ba5e7fb8a.diff

LOG: [FileCheck] Fix isalpha/isalnum calls

D79276 caused the following builder to fail:

  http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/23489

Specifically, FileCheck dumped stack in the following tests:

  LLVM :: MC/Mips/micromips-jump-pc-region.s
  LLVM :: MC/Mips/mips-jump-pc-region.s

Those tests contained characters encoded as 160 but that render (at
least for me in vim) like a single space (32).  Those characters
appeared between the `#` and `RUN:` on several lines, and D79276
caused FileCheck to process those lines differently: `RUN:` is a
comment directive.  As a result, D79276 caused FileCheck to start
calling is `isalnum` on those characters.

The problem is that FileCheck calls `isalnum` on type `char` without
casting to `unsigned char` first, so it sign-extends 160 beyond what
`unsigned char` or `EOF` can represent.  C says that has undefined
behavior.  This problem is general to FileCheck's prefix parsing and
so exists independently of D79276.

524457edbc3d fixed the above tests.  This patch changes FileCheck to
use LLVM's replacements for `ctype.h` functions, and it adds tests for
cases that are representative with or without D79276.

Reviewed By: jhenderson, thopre, efriedma

Differential Revision: https://reviews.llvm.org/D79810

Added: 
    llvm/test/FileCheck/bad-char.txt

Modified: 
    llvm/lib/Support/FileCheck.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/FileCheck.cpp b/llvm/lib/Support/FileCheck.cpp
index 9cdb14cf3b00..2797b8279cd4 100644
--- a/llvm/lib/Support/FileCheck.cpp
+++ b/llvm/lib/Support/FileCheck.cpp
@@ -151,7 +151,7 @@ Expected<std::string> StringSubstitution::getResult() const {
   return Regex::escape(*VarVal);
 }
 
-bool Pattern::isValidVarNameStart(char C) { return C == '_' || isalpha(C); }
+bool Pattern::isValidVarNameStart(char C) { return C == '_' || isAlpha(C); }
 
 Expected<Pattern::VariableProperties>
 Pattern::parseVariable(StringRef &Str, const SourceMgr &SM) {
@@ -171,7 +171,7 @@ Pattern::parseVariable(StringRef &Str, const SourceMgr &SM) {
       return ErrorDiagnostic::get(SM, Str, "invalid variable name");
 
     // Variable names are composed of alphanumeric characters and underscores.
-    if (Str[I] != '_' && !isalnum(Str[I]))
+    if (Str[I] != '_' && !isAlnum(Str[I]))
       break;
     ParsedOneChar = true;
   }
@@ -1079,7 +1079,7 @@ FileCheckDiag::FileCheckDiag(const SourceMgr &SM,
 }
 
 static bool IsPartOfWord(char c) {
-  return (isalnum(c) || c == '-' || c == '_');
+  return (isAlnum(c) || c == '-' || c == '_');
 }
 
 Check::FileCheckType &Check::FileCheckType::setCount(int C) {

diff  --git a/llvm/test/FileCheck/bad-char.txt b/llvm/test/FileCheck/bad-char.txt
new file mode 100644
index 000000000000..26129e867bd3
--- /dev/null
+++ b/llvm/test/FileCheck/bad-char.txt
@@ -0,0 +1,42 @@
+# This file contains characters that render as spaces (at least for me in vim)
+# but are encoded as 160.  Each is indicated with a "^" on the following line.
+# FileCheck used to call functions like isalnum on each without casting to
+# unsigned char first, so it sign-extended beyond what unsigned char or EOF can
+# represent.  C says that has undefined behavior, and it has caused stack dumps
+# under Windows.
+
+//------------------------------------------------
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -check-prefix=BEFORE-PREFIX %s < /dev/null 2>&1 | \
+RUN:   FileCheck -check-prefix=ERR-EMPTY-CHECK %s
+
+ BEFORE-PREFIX:
+^
+ERR-EMPTY-CHECK: error: found empty check string
+
+//------------------------------------------------
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -check-prefix=AFTER-PREFIX %s < /dev/null 2>&1 | \
+RUN:   FileCheck -check-prefix=ERR-NO-CHECK %s
+
+AFTER-PREFIX :
+            ^
+ERR-NO-CHECK: error: no check strings found
+
+//------------------------------------------------
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -check-prefix=BEFORE-VAR %s < /dev/null 2>&1 | \
+RUN:   FileCheck -check-prefix=ERR-BAD-VAR %s
+
+BEFORE-VAR: [[ VAR:]]
+              ^
+ERR-BAD-VAR: error: invalid variable name
+
+//------------------------------------------------
+RUN: %ProtectFileCheckOutput \
+RUN: not FileCheck -check-prefix=AFTER-VAR %s < /dev/null 2>&1 | \
+RUN:   FileCheck -check-prefix=ERR-BAD-STRING-VAR %s
+
+AFTER-VAR: [[VAR :]]
+                ^
+ERR-BAD-STRING-VAR: error: invalid name in string variable definition