[MC] - Don't assert when non-english characters are used.

[grimar]

Summary

I found that llvm-mc does not like non-english characters even in comments,
which it tries to tokenize.

Problem happens because of functions like isdigit(), isalnum() which takes
int argument and expects it is not negative.
But at the same time MCParser uses char* to store input buffer poiner, char has signed value,
so it is possible to pass negative value to one of functions from above and
that triggers an assert.
Testcase for demonstration is provided.

To fix the issue helper functions were introduced in StringExtras.h.

[davide]

Indeed isdigit should have its argument converted to uchar proor to the call but, is the result you get from mc correct? http://en.cppreference.com/w/cpp/string/byte/isdigit

If so, can you please check the output in your test?

[grimar]

Yes, output is correct. Updated testcase as requested.

[rnk]

cppreference suggests writing our own wrapper to avoid this issue: http://en.cppreference.com/w/cpp/string/byte/isdigit

I'd suggest adding llvm::isDigit and llvm::isHexDigit in StringExtras.h next to our other hex digit conversion routines, so we don't have to repeat this surprising cast everywhere.

[davide]

In D38461#885923, @rnk wrote:

cppreference suggests writing our own wrapper to avoid this issue: http://en.cppreference.com/w/cpp/string/byte/isdigit

I'd suggest adding llvm::isDigit and llvm::isHexDigit in StringExtras.h next to our other hex digit conversion routines, so we don't have to repeat this surprising cast everywhere.

Yes, that sounds the best solution actually.

[grimar]

• Added helpers to StringExtras as was suggested.

[ruiu]

LGTM

llvm/trunk/include/llvm/ADT/StringExtras.h

 /// If \p C is not a valid hex digit, -1U is returned.
 static inline unsigned hexDigitValue(char C) {
   if (C >= '0' && C <= '9') return C-'0';
   if (C >= 'a' && C <= 'f') return C-'a'+10U;
   if (C >= 'A' && C <= 'F') return C-'A'+10U;
   return -1U;
 }
 
+/// Checks if character \p C is one of the 10 decimal digits.
+static inline bool isDigit(char C) { return C >= '0' && C <= '9'; }
+
+/// Checks if character \p C is a hexadecimal numeric character.
+static inline bool isHexDigit(char C) { return hexDigitValue(C) != -1U; }
+
+/// Checks if character \p C is a valid letter as classified by "C" locale.
+static inline bool isAlpha(char C) {
+  return ('a' <= C && C <= 'z') || ('A' <= C && C <= 'Z');
+}
+
+/// Checks whether character \p C is either a decimal digit or an uppercase or
+/// lowercase letter as classified by "C" locale.
+static inline bool isAlnum(char C) { return isAlpha(C) || isDigit(C); }
+
 static inline std::string utohexstr(uint64_t X, bool LowerCase = false) {
   char Buffer[17];
   char *BufPtr = std::end(Buffer);
 
   if (X == 0) *--BufPtr = '0';
 
   while (X) {
     unsigned char Mod = static_cast<unsigned char>(X) & 15;

llvm/trunk/lib/MC/MCParser/AsmLexer.cpp

 //===- AsmLexer.cpp - Lexer for Assembly Files ----------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This class implements the lexer for assembly files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/MC/MCParser/AsmLexer.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include <cassert>
 #include <cctype>

 
 /// LexFloatLiteral: [0-9]*[.][0-9]*([eE][+-]?[0-9]*)?
 ///
 /// The leading integral digit sequence and dot should have already been
 /// consumed, some or all of the fractional digit sequence *can* have been
 /// consumed.
 AsmToken AsmLexer::LexFloatLiteral() {
   // Skip the fractional digit sequence.
-  while (isdigit(*CurPtr))
+  while (isDigit(*CurPtr))
     ++CurPtr;
 
   // Check for exponent; we intentionally accept a slighlty wider set of
   // literals here and rely on the upstream client to reject invalid ones (e.g.,
   // "1e+").
   if (*CurPtr == 'e' || *CurPtr == 'E') {
     ++CurPtr;
     if (*CurPtr == '-' || *CurPtr == '+')
       ++CurPtr;
-    while (isdigit(*CurPtr))
+    while (isDigit(*CurPtr))
       ++CurPtr;
   }
 
   return AsmToken(AsmToken::Real,
                   StringRef(TokStart, CurPtr - TokStart));
 }
 
 /// LexHexFloatLiteral matches essentially (.[0-9a-fA-F]*)?[pP][+-]?[0-9a-fA-F]+
 /// while making sure there are enough actual digits around for the constant to
 /// be valid.
 ///
 /// The leading "0x[0-9a-fA-F]*" (i.e. integer part) has already been consumed
 /// before we get here.
 AsmToken AsmLexer::LexHexFloatLiteral(bool NoIntDigits) {
   assert((*CurPtr == 'p' || *CurPtr == 'P' || *CurPtr == '.') &&
          "unexpected parse state in floating hex");
   bool NoFracDigits = true;
 
   // Skip the fractional part if there is one
   if (*CurPtr == '.') {
     ++CurPtr;
 
     const char *FracStart = CurPtr;
-    while (isxdigit(*CurPtr))
+    while (isHexDigit(*CurPtr))
       ++CurPtr;
 
     NoFracDigits = CurPtr == FracStart;
   }
 
   if (NoIntDigits && NoFracDigits)
     return ReturnError(TokStart, "invalid hexadecimal floating-point constant: "
                                  "expected at least one significand digit");
 
   // Make sure we do have some kind of proper exponent part
   if (*CurPtr != 'p' && *CurPtr != 'P')
     return ReturnError(TokStart, "invalid hexadecimal floating-point constant: "
                                  "expected exponent part 'p'");
   ++CurPtr;
 
   if (*CurPtr == '+' || *CurPtr == '-')
     ++CurPtr;
 
   // N.b. exponent digits are *not* hex
   const char *ExpStart = CurPtr;
-  while (isdigit(*CurPtr))
+  while (isDigit(*CurPtr))
     ++CurPtr;
 
   if (CurPtr == ExpStart)
     return ReturnError(TokStart, "invalid hexadecimal floating-point constant: "
                                  "expected at least one exponent digit");
 
   return AsmToken(AsmToken::Real, StringRef(TokStart, CurPtr - TokStart));
 }
 
 /// LexIdentifier: [a-zA-Z_.][a-zA-Z0-9_$.@?]*
 static bool IsIdentifierChar(char c, bool AllowAt) {
-  return isalnum(c) || c == '_' || c == '$' || c == '.' ||
+  return isAlnum(c) || c == '_' || c == '$' || c == '.' ||
          (c == '@' && AllowAt) || c == '?';
 }
 
 AsmToken AsmLexer::LexIdentifier() {
   // Check for floating point literals.
-  if (CurPtr[-1] == '.' && isdigit(*CurPtr)) {
+  if (CurPtr[-1] == '.' && isDigit(*CurPtr)) {
     // Disambiguate a .1243foo identifier from a floating literal.
-    while (isdigit(*CurPtr))
+    while (isDigit(*CurPtr))
       ++CurPtr;
     if (*CurPtr == 'e' || *CurPtr == 'E' ||
         !IsIdentifierChar(*CurPtr, AllowAtInIdentifier))
       return LexFloatLiteral();
   }
 
   while (IsIdentifierChar(*CurPtr, AllowAtInIdentifier))
     ++CurPtr;

 }
 
 // Look ahead to search for first non-hex digit, if it's [hH], then we treat the
 // integer as a hexadecimal, possibly with leading zeroes.
 static unsigned doLookAhead(const char *&CurPtr, unsigned DefaultRadix) {
   const char *FirstHex = nullptr;
   const char *LookAhead = CurPtr;
   while (true) {
-    if (isdigit(*LookAhead)) {
+    if (isDigit(*LookAhead)) {
       ++LookAhead;
-    } else if (isxdigit(*LookAhead)) {
+    } else if (isHexDigit(*LookAhead)) {
       if (!FirstHex)
         FirstHex = LookAhead;
       ++LookAhead;
     } else {
       break;
     }
   }
   bool isHex = *LookAhead == 'h' || *LookAhead == 'H';

 ///   Decimal integer: [1-9][0-9]*
 AsmToken AsmLexer::LexDigit() {
   // MASM-flavor binary integer: [01]+[bB]
   // MASM-flavor hexadecimal integer: [0-9][0-9a-fA-F]*[hH]
   if (IsParsingMSInlineAsm && isdigit(CurPtr[-1])) {
     const char *FirstNonBinary = (CurPtr[-1] != '0' && CurPtr[-1] != '1') ?
                                    CurPtr - 1 : nullptr;
     const char *OldCurPtr = CurPtr;
-    while (isxdigit(*CurPtr)) {
+    while (isHexDigit(*CurPtr)) {
       if (*CurPtr != '0' && *CurPtr != '1' && !FirstNonBinary)
         FirstNonBinary = CurPtr;
       ++CurPtr;
     }
 
     unsigned Radix = 0;
     if (*CurPtr == 'h' || *CurPtr == 'H') {
       // hexadecimal number
if (CurPtr[-1] != '0' || CurPtr[0] == '.') {
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(Result, Value);
   }
 
   if (!IsParsingMSInlineAsm && ((*CurPtr == 'b') || (*CurPtr == 'B'))) {
     ++CurPtr;
     // See if we actually have "0b" as part of something like "jmp 0b\n"
-    if (!isdigit(CurPtr[0])) {
+    if (!isDigit(CurPtr[0])) {
       --CurPtr;
       StringRef Result(TokStart, CurPtr - TokStart);
       return AsmToken(AsmToken::Integer, Result, 0);
     }
     const char *NumStart = CurPtr;
     while (CurPtr[0] == '0' || CurPtr[0] == '1')
       ++CurPtr;
 
if (!IsParsingMSInlineAsm && ((*CurPtr == 'b') || (*CurPtr == 'B'))) {
     SkipIgnoredIntegerSuffix(CurPtr);
 
     return intToken(Result, Value);
   }
 
   if ((*CurPtr == 'x') || (*CurPtr == 'X')) {
     ++CurPtr;
     const char *NumStart = CurPtr;
-    while (isxdigit(CurPtr[0]))
+    while (isHexDigit(CurPtr[0]))
       ++CurPtr;
 
     // "0x.0p0" is valid, and "0x0p0" (but not "0xp0" for example, which will be
     // diagnosed by LexHexFloatLiteral).
     if (CurPtr[0] == '.' || CurPtr[0] == 'p' || CurPtr[0] == 'P')
       return LexHexFloatLiteral(NumStart == CurPtr);
 
     // Otherwise requires at least one hex digit.

llvm/trunk/test/MC/AsmParser/non-english-characters.s

+# RUN: llvm-mc -triple i386-linux-gnu -filetype=obj -o %t %s
+# RUN: llvm-readobj %t | FileCheck %s
+# CHECK: Format: ELF32-i386
+
+# 0bﾑ
+# 0xﾑ
+# .ﾑ4
+# .Xﾑ
+# .1ﾑ
+# .1eﾑ
+# 0x.ﾑ
+# 0x0pﾑ
+.intel_syntax
+# 1ﾑ