Use "_$" prefix instead of "$" for dynamic checker function inserted by LLDB during expression evaluation

[bhushan]

Summary

There is a issue (llvm assertion) in evaluating expressions for MIPS on Linux.

(lldb) p fooptr(a,b)
lldb: /home/battarde/git/llvm/lib/MC/ELFObjectWriter.cpp:791: void {anonymous}::ELFObjectWriter::computeSymbolTable(llvm::MCAssembler&, const llvm::MCAsmLayout&, const SectionIndexMapTy&, const RevGroupMapTy&, {anonymous}::ELFObjectWriter::SectionOffsetsTy&): Assertion `Local || !Symbol.isTemporary()' failed.

This issue is caused due to the dynamic checker function’s name (hard-coded in LLDB in lldb\source\Expression\IRDynamicChecks.cpp) that start with “$” i.e “$__lldb_valid_pointer_check”.
The symbol "$" has a special meaning for MIPS i.e it is marker for temporary symbols for MIPS.

The discussion on lldb mailing list regarding this issue is at : http://lists.llvm.org/pipermail/lldb-dev/2015-October/008692.html

This patch fixes this issue by using "_$" prefix instead of "$" in dymanic checker function’s name.

-Bhushan

[clayborg]

Lets let Sean Callanan and Jim Ingham make sure this is good as they are the expression parser masters.

[bhushan]

Hi Sean/Jim

Could you please find some time to review this?

Thanks.

[dean]

Hi,
is there any update related to this patch? I don't see any activity from the last two weeks.

@Greg Clayton: you have inserted two reviewers but they don't seem to have commented on it. Can you please give a look?

Thanks

[bhushan]

Hi Greg,

Can you have look into this? This patch is important to clear expression related tests for MIPS.

[spyffe]

That looks fine to me as far as it goes, but it doesn't cover other places where $ is used in function names, e.g. the name of the expression itself, and classes it's placed in. Could you have a look at ExpressionSourceCode.cpp and see if there is anything there that needs a $ as well?

[bhushan]

In D14111#330305, @spyffe wrote:

That looks fine to me as far as it goes, but it doesn't cover other places where $ is used in function names, e.g. the name of the expression itself, and classes it's placed in. Could you have a look at ExpressionSourceCode.cpp and see if there is anything there that needs a $ as well?

The other function names in ExpressionSourceCode.cpp e.g. $__lldb_expr does not require to be prefixed with an additional underscore like _$__lldb_expr.
This is because these names get mangled by the compiler to something like _Z12$__lldb_exprPv, so they does not start with "$" and hence not marked as ‘temporary’ symbols.

The additional _ prefix is only needed for "$__lldb_valid_pointer_check" since this name does not get mangled by compiler (may be because it has C Language linkage).

[spyffe]

Thank you for that clarification. In that case, I think this patch is fine as it is.

[bhushan]

Closed by commit http://reviews.llvm.org/rL258485

source/Expression/IRDynamicChecks.cpp

 #include "llvm/IR/Module.h"
 #include "llvm/IR/Value.h"
 
 using namespace llvm;
 using namespace lldb_private;
 
 static char ID;
 
-#define VALID_POINTER_CHECK_NAME "$__lldb_valid_pointer_check"
+#define VALID_POINTER_CHECK_NAME "_$__lldb_valid_pointer_check"
 #define VALID_OBJC_OBJECT_CHECK_NAME "$__lldb_objc_object_check"
 
 static const char g_valid_pointer_check_text[] =
 "extern \"C\" void\n"
-"$__lldb_valid_pointer_check (unsigned char *$__lldb_arg_ptr)\n"
+"_$__lldb_valid_pointer_check (unsigned char *$__lldb_arg_ptr)\n"
 "{\n"
 "    unsigned char $__lldb_local_val = *$__lldb_arg_ptr;\n"
 "}";
 
 DynamicCheckerFunctions::DynamicCheckerFunctions ()
 {
 }