Add "target.jit-args" settings

[endlessroad]

Summary

We need an interface to apply compile flags to JIT compiler.

For example, if target is built with -fno-rtti and but JIT compiler does not specify that, JIT'ed code will try to access inexistent RTTI objects. This behavior causes LLVM ExecutionEngine to crash directly.

[tfiala]

Hey Tong,

Some feedback:

• The jit-ness of this is more or less an implementation detail. Naming it something related to expression evaluation seems like a better fit. Maybe something like "evaluation-compiler-args".
• See question on custom handling that one arg in CommandObjectSettings.cpp. That smells wrong. Why is that needed?
• Can you add a test for this? If it's not tested, you need to assume it's going to be broken. I don't know if you'll end up needing a tiny piece of Android Art compiled code for this? (This might be hard given the size of the exes there - if you can isolate into something very small, it would be good to test against it like we do for the ObjectFileELF* testing).

We'll also want to see if Sean Callanan has anything to add to this since it's in the expression eval pipeline.

Thanks!

source/Commands/CommandObjectSettings.cpp

223 ↗

(On Diff #14016)

Hey Tong - why are you needing to special case this one argument here?

[endlessroad]

Address Jim's comments

• Rename jit-args to expr-parser-compiler-args.
• Add a common functionality for Args: if one argument is surrounded by quote char, don't consider it as an command option.

[jingham]

The argument parsing part of this patch doesn't work correctly. It changes all quoted values in the command input from options to arguments For instance, I have this command in my .lldbinit:

type summary add -w lldb "lldb::ModuleSP" -s "${var.__ptr_%S}"

Your patch notices that "${var.__ptr_%S}" has quotes around it, so it removes it from the option parsing. But since the -s requires an argument value, the command no longer parses correctly.

Jim

[endlessroad]

Fix option parsing.

[tfiala]

Looking at this now.

Initially I was thinking the Expr text could be expanded to Expression, but I see that Env (in EnvVar, ePropertyEnvVars, etc.) is also already similarly abbreviated, so this is not really out of line for that code. So just a minor nit but is probably fine.

Going to test it a bit.

[tfiala]

I tested this with the lldb docs for 'type summary' here: http://lldb.llvm.org/varformats.html, using the floating point type summary example:

type summary add --summary-string "Sign: ${var[31]%B} Exponent: ${var[30-23]%x} Mantissa: ${var[0-22]%u}" float

That seemed to work in the presence of this change.

The tests come back clean on Ubuntu 14.04 and MacOSX 10.9.5.

Jim, do you have any further comments on this?

[clayborg]

Looks good.

[jingham]

It's ugly to have to add quotes and then remove them, but until we write our own option parser I can't think of a better way to do it...

Jim

[tfiala]

It's ugly to have to add quotes and then remove them, but until we write

our own option parser I can't think of a better way to do it...

Ok - I'll take that as a "okay for now, let's revisit later" response.

I'll get this in.

[tfiala]

svn commit
Sending include/lldb/Expression/ClangExpressionParser.h
Sending include/lldb/Target/Target.h
Sending source/Expression/ClangExpressionParser.cpp
Sending source/Expression/ClangFunction.cpp
Sending source/Expression/ClangUserExpression.cpp
Sending source/Expression/ClangUtilityFunction.cpp
Sending source/Interpreter/Args.cpp
Sending source/Target/Target.cpp
Transmitting file data ........
Committed revision 219169.

[tfiala]

FYI Tong's internship is up, so his @Google.com email address will show up as unreachable.

It looks like this change has introduced this bug:
http://llvm.org/bugs/show_bug.cgi?id=21190

[tfiala]

I reversed this out with this change:

svn commit
Sending        include/lldb/Expression/ClangExpressionParser.h
Sending        include/lldb/Target/Target.h
Sending        source/Expression/ClangExpressionParser.cpp
Sending        source/Expression/ClangFunction.cpp
Sending        source/Expression/ClangUserExpression.cpp
Sending        source/Expression/ClangUtilityFunction.cpp
Sending        source/Interpreter/Args.cpp
Sending        source/Target/Target.cpp
Transmitting file data ........
Committed revision 219461.

This was due to this bug introduced by the change:
http://llvm.org/bugs/show_bug.cgi?id=21190

Greg had this suggestion to try as a possible way to solve this issue without needing the alternate quote handling:

Just a note for Tong: You can use single or double quotes to get around not being able to quote things:

(lldb) expression --jit-args='-foo "Bar" -baz "123"'

Not sure if that would help Tong's issue.

We'll want to get the actual compiler args back in, just without the quote handling changes.

include/lldb/Expression/ClangExpressionParser.h

     //------------------------------------------------------------------
     ClangExpressionParser (ExecutionContextScope *exe_scope,
                            ClangExpression &expr,
+                           Args &expr_parser_compiler_args,
                            bool generate_debug_info);
     
     //------------------------------------------------------------------

include/lldb/Target/Target.h

     
     void
     SetRunArguments (const Args &args);
+
+    bool
+    GetExprParserCompilerArguments (Args &args) const;
+
+    void
+    SetExprParserCompilerArguments (const Args &args);
     
     size_t
     GetEnvironmentAsArgs (Args &env) const;

source/Expression/ClangExpressionParser.cpp

 
 ClangExpressionParser::ClangExpressionParser (ExecutionContextScope *exe_scope,
                                               ClangExpression &expr,
+                                              Args &expr_parser_compiler_args,
                                               bool generate_debug_info) :
     m_expr (expr),
     m_compiler (),

 
     // 3. Set options.
 
+    clang::CompilerInvocation::CreateFromArgs(m_compiler->getInvocation(),
+        expr_parser_compiler_args.GetConstArgumentVector(),
+        expr_parser_compiler_args.GetConstArgumentVector() + expr_parser_compiler_args.GetArgumentCount(),
+        m_compiler->getDiagnostics());
+
     lldb::LanguageType language = expr.Language();
 
     switch (language)

source/Expression/ClangFunction.cpp

     if (jit_process_sp)
     {
         const bool generate_debug_info = true;
-        m_parser.reset(new ClangExpressionParser(jit_process_sp.get(), *this, generate_debug_info));
+        Args expr_parser_compiler_args;
+        jit_process_sp->GetTarget().GetExprParserCompilerArguments (expr_parser_compiler_args);
+        m_parser.reset(new ClangExpressionParser(jit_process_sp.get(), *this, expr_parser_compiler_args, generate_debug_info));
         
         num_errors = m_parser->Parse (errors);
     }

source/Expression/ClangUserExpression.cpp

     if (!exe_scope)
         exe_scope = exe_ctx.GetTargetPtr();
 
-    ClangExpressionParser parser(exe_scope, *this, generate_debug_info);
+    Args expr_parser_compiler_args;
+    target->GetExprParserCompilerArguments (expr_parser_compiler_args);
+    ClangExpressionParser parser(exe_scope, *this, expr_parser_compiler_args, generate_debug_info);
 
     unsigned num_errors = parser.Parse (error_stream);
 

source/Expression/ClangUtilityFunction.cpp

     }
     
     const bool generate_debug_info = true;
-    ClangExpressionParser parser(exe_ctx.GetBestExecutionContextScope(), *this, generate_debug_info);
+    Args expr_parser_compiler_args;
+    target->GetExprParserCompilerArguments (expr_parser_compiler_args);
+    ClangExpressionParser parser(exe_ctx.GetBestExecutionContextScope(), *this, expr_parser_compiler_args, generate_debug_info);
     
     unsigned num_errors = parser.Parse (error_stream);
     

source/Interpreter/Args.cpp

     }
     OptionParser::Prepare();
     int val;
+
+    // Before parsing arguments, insert quote char to the head of the string.
+    // So quoted arguments like "-l" won't be treated as option.
+    int argv_iter = 0;
+    for (auto args_iter = m_args.begin(); args_iter != m_args.end(); args_iter++, argv_iter++)
+    {
+        char quote_char = GetArgumentQuoteCharAtIndex(argv_iter);
+        if (quote_char != '\0')
+        {
+            *args_iter = std::string(1, quote_char) + *args_iter;
+            m_argv[argv_iter] = args_iter->c_str();
+        }
+    }
+
     while (1)
     {
         int long_options_index = -1;

             }
             else
             {
+                const char *value = OptionParser::GetOptionArgument();
+                if (value)
+                {
+                    // Remove leading quote char from option value
+                    argv_iter = 0;
+                    for (auto args_iter = m_args.begin(); args_iter != m_args.end(); args_iter++, argv_iter++)
+                    {
+                        if (*args_iter == value && GetArgumentQuoteCharAtIndex(argv_iter) != '\0')
+                        {
+                            *args_iter = args_iter->substr(1);
+                            value = args_iter->c_str();
+                            break;
+                        }
+                    }
+                }
                 error = options.SetOptionValue(long_options_index,
-                                               (def->option_has_arg == OptionParser::eNoArgument) ? nullptr : OptionParser::GetOptionArgument());
+                                               (def->option_has_arg == OptionParser::eNoArgument) ? nullptr : value);
             }
         }
         else

     // Update our ARGV now that get options has consumed all the options
     m_argv.erase(m_argv.begin(), m_argv.begin() + OptionParser::GetOptionIndex());
     UpdateArgsAfterOptionParsing ();
+
+    // Remove leading quote char from other arguments.
+    argv_iter = 0;
+    for (auto args_iter = m_args.begin(); args_iter != m_args.end(); args_iter++, argv_iter++)
+    {
+        char quote_char = GetArgumentQuoteCharAtIndex(argv_iter);
+        if (quote_char != '\0')
+        {
+            *args_iter = args_iter->substr(1);
+            m_argv[argv_iter] = args_iter->c_str();
+        }
+    }
+
     return error;
 }
 

source/Target/Target.cpp

     { "breakpoints-use-platform-avoid-list", OptionValue::eTypeBoolean   , false, true                      , NULL, NULL, "Consult the platform module avoid list when setting non-module specific breakpoints." },
     { "arg0"                               , OptionValue::eTypeString    , false, 0                         , NULL, NULL, "The first argument passed to the program in the argument array which can be different from the executable itself." },
     { "run-args"                           , OptionValue::eTypeArgs      , false, 0                         , NULL, NULL, "A list containing all the arguments to be passed to the executable when it is run. Note that this does NOT include the argv[0] which is in target.arg0." },
+    { "expr-parser-compiler-args"          , OptionValue::eTypeArgs      , false, 0                         , NULL, NULL, "A list containing all the arguments to be passed to the expression parser compiler." },
     { "env-vars"                           , OptionValue::eTypeDictionary, false, OptionValue::eTypeString  , NULL, NULL, "A list of all the environment variables to be passed to the executable's environment, and their values." },
     { "inherit-env"                        , OptionValue::eTypeBoolean   , false, true                      , NULL, NULL, "Inherit the environment from the process that is running LLDB." },
     { "input-path"                         , OptionValue::eTypeFileSpec  , false, 0                         , NULL, NULL, "The file/path to be used by the executable program for reading its standard input." },

     ePropertyBreakpointUseAvoidList,
     ePropertyArg0,
     ePropertyRunArgs,
+    ePropertyExprParserCompilerArgs,
     ePropertyEnvVars,
     ePropertyInheritEnv,
     ePropertyInputPath,

 }
 
 void
+TargetProperties::SetExprParserCompilerArguments (const Args &args)
+{
+    const uint32_t idx = ePropertyExprParserCompilerArgs;
+    m_collection_sp->SetPropertyAtIndexFromArgs (NULL, idx, args);
+}
+
+bool
+TargetProperties::GetExprParserCompilerArguments (Args &args) const
+{
+    const uint32_t idx = ePropertyExprParserCompilerArgs;
+    return m_collection_sp->GetPropertyAtIndexAsArgs (NULL, idx, args);
+}
+
+void
 TargetProperties::SetRunArguments (const Args &args)
 {
     const uint32_t idx = ePropertyRunArgs;