[Lldb-commits] [lldb] r327356 - [ExpressionParser] Fix crash when evaluating invalid expresssions.

[Frédéric Riss via lldb-commits]

> On Mar 12, 2018, at 6:40 PM, Davide Italiano via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> Author: davide
> Date: Mon Mar 12 18:40:00 2018
> New Revision: 327356
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=327356&view=rev
> Log:
> [ExpressionParser] Fix crash when evaluating invalid expresssions.
> 
> Typical example, illformed comparisons (operator== where LHS and
> RHS are not compatible). If a symbol matched `operator==` in any
> of the object files lldb inserted a generic function declaration
> in the ASTContext on which Sema operates. Maintaining the AST
> context invariants is fairly tricky and sometimes resulted in
> crashes inside clang (or assertions hit).
> 
> The real reason why this feature exists in the first place is
> that of allowing users to do something like:
> (lldb) call printf("patatino")
> 
> even if the debug informations for printf() is not available.
> Eventually, we might reconsider this feature in its
> entirety, but for now we can't remove it as it would break
> a bunch of users. Instead, try to limit it to non-C++ symbols,
> where getting the invariants right is hopefully easier.
> 
> Now you can't do in lldb anymore
> (lldb) call _Zsomethingsomething(1,2,3)
> 
> but that doesn't seem to be such a big loss.

I’m somewhat surprised by this. My understanding of the crash you were investigating is that Clang crashed because we injected a Decl looking like this: “void operator==(…)” after finding the operator== symbol somewhere. I think injecting bogus C++ Decls makes no sense and it cannot really work anyway.

On the other hand, I see no harm in injecting “_Zsomethingsomething(…)” as a C Decl. This can be useful, and people should be able to call raw symbols in their binaries. Is there no way to keep the later while preventing the creation of broken C++ decls?

Fred

> <rdar://problem/35645893>
> 
> Added:
>    lldb/trunk/lit/Expr/Inputs/basic.cpp
>    lldb/trunk/lit/Expr/TestCallCppSym.test
> Modified:
>    lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
> 
> Added: lldb/trunk/lit/Expr/Inputs/basic.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/basic.cpp?rev=327356&view=auto
> ==============================================================================
> --- lldb/trunk/lit/Expr/Inputs/basic.cpp (added)
> +++ lldb/trunk/lit/Expr/Inputs/basic.cpp Mon Mar 12 18:40:00 2018
> @@ -0,0 +1,12 @@
> +class Patatino {
> +private:
> +  long tinky;
> +
> +public:
> +  Patatino(long tinky) { this->tinky = tinky; }
> +};
> +
> +int main(void) {
> +  Patatino *a = new Patatino(26);
> +  return 0;
> +}
> 
> Added: lldb/trunk/lit/Expr/TestCallCppSym.test
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestCallCppSym.test?rev=327356&view=auto
> ==============================================================================
> --- lldb/trunk/lit/Expr/TestCallCppSym.test (added)
> +++ lldb/trunk/lit/Expr/TestCallCppSym.test Mon Mar 12 18:40:00 2018
> @@ -0,0 +1,6 @@
> +# RUN: %cxx %p/Inputs/basic.cpp -g -o %t && %lldb -b -s %s -- %t 2>&1 | FileCheck %s
> +
> +breakpoint set --file basic.cpp --line 12
> +run
> +call (int)_Znwm(23)
> +# CHECK: error: use of undeclared identifier '_Znwm'
> 
> Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp?rev=327356&r1=327355&r2=327356&view=diff
> ==============================================================================
> --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp (original)
> +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp Mon Mar 12 18:40:00 2018
> @@ -2072,6 +2072,15 @@ void ClangExpressionDeclMap::AddOneFunct
>       return;
>     }
>   } else if (symbol) {
> +    // Don't insert a generic function decl for C++ symbol names.
> +    // Creating a generic function decl is almost surely going to cause troubles
> +    // as it breaks Clang/Sema invariants and causes crashes in clang while
> +    // we're trying to evaluate the expression.
> +    // This means users can't call C++ functions by mangled name when there
> +    // are no debug info (as it happens for C symbol, e.g. printf()).
> +    if (CPlusPlusLanguage::IsCPPMangledName(
> +            symbol->GetMangled().GetMangledName().GetCString()))
> +      return;
>     fun_address = symbol->GetAddress();
>     function_decl = context.AddGenericFunDecl();
>     is_indirect_function = symbol->IsIndirect();
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits