lldb incorrectly reports multiple symbols when evaluating expressions (on Linux)

[llvmbot]

Bugzilla Link	35043
Version	unspecified
OS	Windows NT
Reporter	LLVM Bugzilla Contributor
CC	@adrian-prantl,@compnerd,@labath,@lhames

Extended Description

I think this is a bug in the expression parser/symtab handling, but I can't say for sure.

given the test in lambdas/main.cpp, after we set a breakpoint on the printf() and we try to evaluate the lambda, we get:

(lldb) target create "made" Current executable set to 'made' (x86_64). (lldb) b main.cpp:14 Breakpoint 1: where = made`main + 32 at main.cpp:14, address = 0x00000000004005f0 (lldb) r Process 32489 launched: '/home/davide/work/llvm-lldb/tools/lldb/packages/Python/lldbsuite/test/lang/cpp/lambdas/made' (x86_64) Process 32489 stopped

• thread #​1, name = 'made', stop reason = breakpoint 1.1 frame #​0: 0x00000000004005f0 made`main(argc=1, argv=0x00007fffffffe128) at main.cpp:14 11 12 int main (int argc, char const *argv[]) 13 { -> 14 printf("Stop here\n"); //% self.runCmd("expression auto $add = [](int a, int b) { return a + b; }") 15 //% self.expect("expression $add(2,3)", substrs = ['= 5']) 16 return 0; 17 } (lldb) expression auto $add = [](int a, int b) { return a + b; } error: Multiple internal symbols found for 'a' id = {0x000003cf}, range = [0x00007ffff77ec290-0x00007ffff77ec298), name="a" id = {0x0000051b}, range = [0x00007ffff77ec290-0x00007ffff77ec298), name="a" id = {0x000000fe}, range = [0x00007ffff77ec290-0x00007ffff77ec298), name="a"

error: Multiple internal symbols found for 'b' id = {0x0000051a}, range = [0x00007ffff77cfe70-0x00007ffff77cfe78), name="b" id = {0x000003ce}, range = [0x00007ffff77cfe70-0x00007ffff77cfe78), name="b" id = {0x000000fd}, range = [0x00007ffff77cfe70-0x00007ffff77cfe78), name="b"

I don't think that's quite right, as b is not really redefined, it's just a temporary binding that we use for the lambda argument. This requires a little more knowledge of the internals than I currently have, so hints on where to look would be appreciated. I can attach the ELF object, if it's helpful

[labath]

I've renamed the symbols in these tests to avoid different behavior depending on the system libraries.

Instead, I've created a self-contained test demonstrating this issue (TestConflictingSymbol.test_shadowed). This one should fail on any system as we are not depending on the system libraries to create the conflict.

[llvmbot]

You mention "and in the "good" case:", but that expression is a helper function we compile and use in all expressions. Each time a pointer is used in an expression, we run the pointer through the "_$__lldb_valid_pointer_check(...)" by instrumenting the IR for the expression. This ensures as soon as your use a bad pointer, it will crash.

Sometimes running one expression will cause another expression to be evaluated before the real expression.

[llvmbot]

I think the problem is in the textual expression that's passed to clang's ParseAST.

The textual expression in the broken case is:

define Nil (__null)

endif

ifndef nil

define nil (__null)

endif

ifndef YES

define YES ((BOOL)1)

endif

ifndef NO

define NO ((BOOL)0)

endif

typedef INT8_TYPE__ int8_t; typedef UINT8_TYPE uint8_t; typedef __INT16_TYPE int16_t; typedef UINT16_TYPE__ uint16_t; typedef INT32_TYPE int32_t; typedef __UINT32_TYPE uint32_t; typedef INT64_TYPE__ int64_t; typedef UINT64_TYPE uint64_t; typedef __INTPTR_TYPE intptr_t; typedef UINTPTR_TYPE__ uintptr_t; typedef SIZE_TYPE size_t; typedef __PTRDIFF_TYPE ptrdiff_t; typedef unsigned short unichar; extern "C" { int printf(const char * __restrict, ...); }

typedef signed char BOOL;

void $__lldb_expr(void $__lldb_arg) { ; /LLDB_BODY_START/ char c[] = "foo"; c[0]; /LLDB_BODY_END*/ }

and in the "good" case:

ifndef NULL

define NULL (__null)

endif

ifndef Nil

define Nil (__null)

endif

ifndef nil

define nil (__null)

endif

ifndef YES

define YES ((BOOL)1)

endif

ifndef NO

define NO ((BOOL)0)

endif

typedef INT8_TYPE__ int8_t; typedef UINT8_TYPE uint8_t; typedef __INT16_TYPE int16_t; typedef UINT16_TYPE__ uint16_t; typedef INT32_TYPE int32_t; typedef __UINT32_TYPE uint32_t; typedef INT64_TYPE__ int64_t; typedef UINT64_TYPE uint64_t; typedef __INTPTR_TYPE intptr_t; typedef UINTPTR_TYPE__ uintptr_t; typedef SIZE_TYPE size_t; typedef __PTRDIFF_TYPE ptrdiff_t; typedef unsigned short unichar; extern "C" { int printf(const char _restrict, ...); } extern "C" void $__lldb_valid_pointer_check (unsigned char $lldb_arg_ptr) { unsigned char $lldb_local_val = *$__lldb_arg_ptr; }

[llvmbot]

Back to the original bug, I found out this is actually failing in clang/AST

The call to ParseAST in ClangExpressionParser.cpp around here:

if (ast_transformer) { ast_transformer->Initialize(m_compiler->getASTContext()); ParseAST(m_compiler->getPreprocessor(), ast_transformer, m_compiler->getASTContext()); } else { m_code_generator->Initialize(m_compiler->getASTContext()); ParseAST(m_compiler->getPreprocessor(), m_code_generator.get(), m_compiler->getASTContext()); }

fails and:

unsigned num_errors = diag_buf->getNumErrors();

is equal to 1. I'll look into why clang is failing to parse this.

[llvmbot]

As Adrian said, it would be nice to cut over to the LLVM object file parsers at some point. I know there is a desire to rewrite the object file layer in LLVM and we have been kind of waiting for that desire to manifest or subside.

We also need to make sure we can load slightly malformed files. Many core files on MacOSX have a few things wrong with them where some sections are truncated. Right now this will cause LLVM's layer to not allow us to load the object file and it would return a null pointer when we try to parse it. This might have changed in the last 6 months or so , I could be wrong on this.

So the other side is we don't want any new asserts to kill the debugger or not allow us to load files that are slightly malformed. Another issue is mach-o file contain load commands and there was code at some point that asserted if we didn't recognize a load command in the file, so that will need to not assert as well. LLDB will be used to load future versions of mach-o files and it needs to be able to open them.

So it isn't just an easy switch over, there are many things that can go wrong. And none of these things will help with symbol coalescing.

[llvmbot]

Is that counting the COFF Parser? And MachO parser? ;-0

Yes, Plugins/SymbolFile and Plugins/ObjectFile are around 40k lines combined. Of course, they won't go away entirely but they can be slimmed down a lot.

[llvmbot]

Is that counting the COFF Parser? And MachO parser? ;-0

[llvmbot]

AFAIK that is purely historical. LLDB's object parsers I think predate LLVM's. We have a long-term goal to make LLVM's Object and DWARF parsers good enough so we can switch LLDB over eventually.

That would be nice (and it would cut out around 40-50k lines of code from LLDB :)

[adrian-prantl]

AFAIK that is purely historical. LLDB's object parsers I think predate LLVM's. We have a long-term goal to make LLVM's Object and DWARF parsers good enough so we can switch LLDB over eventually.

[llvmbot]

So it seems we have multiple symbols between the dynamic symbol table, the normal symbol table and possibly elsewhere (not sure where the 3rd symbol comes from.

ObjectFileELF should try to coalesce common symbols into a single symbol. We note that the 3 symbols we see from your info are all the same. So work will need to be done in ObjectFileELF::ParseSymbols(...) to keep track of the symbols we have added already by name + address range and don't add duplicate symbols to begin with. Duplicate symbols will adversely affect the expression parser as you have already witnessed.

So I suggest:

• figure out why we have 3 symbols. I can understand 2 symbols (one in dynsym, one in normal symbol table)
• modify ObjectFileELF::ParseSymbols() to only add 1 symbol for all symbols by keeping track of symbol name + range + type.

Thanks for your reply, Greg. Out of curiosity, why does lldb roll its own ELF parser instead of using lib/Object?

[llvmbot]

So this is an expression parser bug as well because any variable that is declared, like "int a" and "int b" in the lambda function, should not be replaced with versions from your program or from your symbol table. So I would use this bug to fix the expression parser, and then file another one where we need to not create multiple symbols for things that are the same in the ELF parser.

[llvmbot]

So it seems we have multiple symbols between the dynamic symbol table, the normal symbol table and possibly elsewhere (not sure where the 3rd symbol comes from.

ObjectFileELF should try to coalesce common symbols into a single symbol. We note that the 3 symbols we see from your info are all the same. So work will need to be done in ObjectFileELF::ParseSymbols(...) to keep track of the symbols we have added already by name + address range and don't add duplicate symbols to begin with. Duplicate symbols will adversely affect the expression parser as you have already witnessed.

So I suggest:

• figure out why we have 3 symbols. I can understand 2 symbols (one in dynsym, one in normal symbol table)
• modify ObjectFileELF::ParseSymbols() to only add 1 symbol for all symbols by keeping track of symbol name + range + type.

[llvmbot]

It seems there's a symbol name clash. If I rename the lambda arguments, the test passes. I'm not yet convinced by the solution, but at least now I understand the cause.

Here's a workaround that's very likely papering over the original problem

diff --git a/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp b/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp index 3cce3ba..4be13a6 100644 --- a/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp +++ b/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp @@ -11,7 +11,7 @@

int main (int argc, char const *argv[]) {

• printf("Stop here\n"); //% self.runCmd("expression auto $add = [](int a, int b) { return a + b; }")
• printf("Stop here\n"); //% self.runCmd("expression auto $add = [](int pata, int tino) { return pata + tino; }") //% self.expect("expression $add(2,3)", substrs = ['= 5']) return 0; }

[felipepiovezan]

A variant of this behavior can be seen in the diff of the test lldb/test/API/commands/expression/fixits/TestFixIts.py here https://reviews.llvm.org/D132940