[lldb] Fix crash due to unicode characters and dollars in variable names.

[teemperor]

Summary

Our variable loading optimization uses some handmade lexer that currently doesn't
properly handle characters such as dollar-symbols or unicode characters. This patch
replaces our own lexer with the Clang lexer which properly handles all these corner cases.

[teemperor]

I realized after revisiting this code that this can't handle Unicode and we probably should just use Clang's Lexer to get the tokens. Still, it would be nice if we can land this in case the Lexer rewrite takes longer than expected.

[davide]

lldb/packages/Python/lldbsuite/test/expression_command/dollar-in-variable/main.c

4–5

what if you have another variable in the same scope named just foo ?
What expr $foo will print?
I would expect this to be recognized as expr $$foo.

[davide]

lldb/packages/Python/lldbsuite/test/expression_command/dollar-in-variable/main.c

4–5

Actually, I think the previous example should be fine, on a second thought. The example that I think might break is the one where you have an identifier in your source code named $R0. Is that allowed?

[teemperor]

lldb/packages/Python/lldbsuite/test/expression_command/dollar-in-variable/main.c

4–5

Is $R0 supposed to access a register? Or is that somehow a special syntax?

In any case the current code will prefer a local variable with from what I can see.

[davide]

I thought about this a little more, and we discussed offline, but these are my two comments (we should understand what's the behaviour and add tests). We can address some of them as follow-up but I would like to understand what happens here.

1. For C++, lldb names expression results using the convention $0, $1, etc... Can you have a variable named $0 in your source code? If so, what happens when you evaluate expr patatino? Will lldb be smart enough to understand that $0 is already defined and start with $1 ? Or it will override the scope?
2. If you have $pat in your example, and you type expr int $pat = 234234, does that work?

[clayborg]

Do we really expect users to actually write code where variables start with '$'? We will need to make some clear rules of the lookup order if so. I would suggest the following order

• look for real variables by exact name match first
• look for expression global variables next
• look for expression results next ($<digit>)
• look for registers

lldb/packages/Python/lldbsuite/test/expression_command/dollar-in-variable/main.c

5

The idea was that a "$" prefix wouldn't be found in normal code. In LLDB $ has the following special use cases:

• all expression results are named $<digit> just like LLDB
• if you use $ it can be followed by a register name so you can access registers in expressions ($rax, $sp, etc)
• any definitions in your expression where the variable is prefixed with '$' will become an expression global variable available to all subsequent expressions:

(lldb) expr int $my_global = 12;
(lldb) expr ++$my_global

• All function names that start with '$' are currently thought to be reserved for expression and so that any functions we create don't interact with real functions

[teemperor]

So the actual code that hit his use case was when one steps into the expression wrapper function we create in LLDB (which uses these dollar signs). So we (luckily) don't have any user code that triggers this.

I should note that I anyway discovered that the variable loading optimization that caused this regression doesn't support any unicode characters (not that we encourage that), so I'll anyway rewrite this parsing code with something that reuses the Clang lexer instead of our own ad-hoc parsing (which probably wil save us from more weird corner cases in the future).

[teemperor]

• Rewrote the patch to use the Clang lexer, dropped our ad-hoc lexer.
• Make sure we only lex the expression once, not for each local variable.
• Added tests for unicode characters and a bunch of dollar-related corner cases. The current behavior seems reasonable and what people expect in the comments, so I just keep them around.

I would prefer if we could just consider this patch as a regression fix and not some official support for dollar-variables in user code (which seems like a mess to define properly). I added some tests that at least make sure the current behavior is tested, we can decide later if that's really what we want to do (but the current behavior seems quite reasonable and similar to what people suggest).

[JDevlieghere]

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

253

Can we write this as while(!lex.LexFromRawLexer(token))?

[teemperor]

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

253

LexFromRawLexer seems to return true when the current result is the last token (but still a valid token), so I think no :(

[JDevlieghere]

LGTM

[teemperor]

Closed by 365698

lldb/packages/Python/lldbsuite/test/expression_command/dollar-in-variable/Makefile

+LEVEL = ../../make
+C_SOURCES := main.c
+include $(LEVEL)/Makefile.rules

lldb/packages/Python/lldbsuite/test/expression_command/dollar-in-variable/TestDollarInVariable.py

+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), None)
+

lldb/packages/Python/lldbsuite/test/expression_command/dollar-in-variable/main.c

+// Make sure we correctly handle $ in variable names.
+
+int main() {
+  int $foo = 42;
+  return 0; //%self.expect("expr $foo", substrs=['(int) $0 = 42'])
+}

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

case DebugMacroEntry::INDIRECT:
       break;
     default:
       // This is an unknown/invalid entry. Ignore.
       break;
     }
   }
 }
 
+/// Returns true iff the character is part of an identifier according to LLDB.
+static bool IsLLDBIdentifier(char c) {
+  // We allow all characters Clang allows by default and '$' in identifiers.
+  return clang::isIdentifierBody(static_cast<unsigned char>(c),
+                                 /*allow dollar character*/ true);
+}
+
 /// Checks if the expression body contains the given variable as a token.
 /// \param body The expression body.
 /// \param var The variable token we are looking for.
 /// \return True iff the expression body containes the variable as a token.
 static bool ExprBodyContainsVar(llvm::StringRef body, llvm::StringRef var) {
-  assert(var.find_if([](char c) { return !clang::isIdentifierBody(c); }) ==
+  assert(var.find_if([](char c) { return !IsLLDBIdentifier(c); }) ==
              llvm::StringRef::npos &&
          "variable contains non-identifier chars?");
 
   size_t start = 0;
   // Iterate over all occurences of the variable string in our expression.
   while ((start = body.find(var, start)) != llvm::StringRef::npos) {
     // We found our variable name in the expression. Check that the token
     // that contains our needle is equal to our variable and not just contains
     // the character sequence by accident.
     // Prevents situations where we for example inlcude the variable 'FOO' in an
     // expression like 'FOObar + 1'.
     bool has_characters_before =
-        start != 0 && clang::isIdentifierBody(body[start - 1]);
-    bool has_characters_after =
-        start + var.size() < body.size() &&
-        clang::isIdentifierBody(body[start + var.size()]);
+        start != 0 && IsLLDBIdentifier(body[start - 1]);
+    bool has_characters_after = start + var.size() < body.size() &&
+                                IsLLDBIdentifier(body[start + var.size()]);
 
     // Our token just contained the variable name as a substring. Continue
     // searching the rest of the expression.
     if (has_characters_before || has_characters_after) {
       ++start;
       continue;
     }
     return true;
bool ClangExpressionSourceCode::GetText(
   Target *target = exe_ctx.GetTargetPtr();
   if (target) {
     if (target->GetArchitecture().GetMachine() == llvm::Triple::aarch64) {
       target_specific_defines = "typedef bool BOOL;\n";
     }
     if (target->GetArchitecture().GetMachine() == llvm::Triple::x86_64) {
       if (lldb::PlatformSP platform_sp = target->GetPlatform()) {
         static ConstString g_platform_ios_simulator("ios-simulator");
         if (platform_sp->GetPluginName() == g_platform_ios_simulator) {
           target_specific_defines = "typedef bool BOOL;\n";
         }
       }
     }
 
     if (ClangModulesDeclVendor *decl_vendor =
             target->GetClangModulesDeclVendor()) {
       ClangPersistentVariables *persistent_vars =