Re: [Lldb-commits] FreeBSD kernel debugging fixes

2017-09-21 Thread Greg Clayton via lldb-commits
I am worried that this will adversely affect the normal DWARF that is in ELF 
files. What does typical DWARF look like when all of the file addresses in 
DWARF and the symtab are set correctly to unique virtual addresses? No 
relocations on any addresses? If all "file addresses" are set to unique offsets 
in each section, we don't need relocations. If there are no relocations on the 
DWARF, then we are probably ok.

Using .o files is tricky indeed as you are finding out. Are your modifications 
enabled by the fact that the object file is a .o file? As I mentioned above, we 
don't want any relocations applied to linked binaries. We do special things 
with .o files that we load in mach-o to ensure the segments are all happy, so 
there is definitely extra work required.

Please submit these patches via reviews.llvm.org web site. For details please 
see:

https://llvm.org/docs/Phabricator.html

Then it is easier to look at the changes because we can see the surrounding 
code. Once that happens we can start looking at the patches and offering 
inlined comments and have a discussion on any needed changes.

Greg Clayton


> On Sep 20, 2017, at 4:37 PM, Koropoff, Brian via lldb-commits 
>  wrote:
> 
> Jason,
> 
> I'm performing address to symbol resolution after setting load addresses for 
> all sections.  It correctly identifies the module
> and section where the address resides, and in many cases gives correct 
> results.  However, because it translates the
> load address to a file address before indexing into the symtab, overlapping 
> file addresses for sections in the same module
> can cause the wrong name to be returned, such as returning a symbol in the 
> bss section even though the address is
> in the text section.
> 
> An alternative way to fix this would be to split m_file_addr_to_index into 
> per-section maps, but that doesn't solve the
> problem of ResolveFileAddress being unusable, or the general expectation 
> within lldb that file addresses uniquely
> identify something within a module.
> 
> -- Brian
> 
> From: Jason Molenda [jmole...@apple.com]
> Sent: Wednesday, September 20, 2017 4:18 PM
> To: Koropoff, Brian
> Cc: lldb-commits@lists.llvm.org
> Subject: Re: [Lldb-commits] FreeBSD kernel debugging fixes
> 
> Right, we always record symbol addresses as the offset to the section that 
> contains them.  The Address class in lldb is used everywhere for this.  The 
> Target has a SectionLoadList which tells us where each Section is loaded in 
> memory -- this is how you translate an Address object to a load address.
> 
> When sections have not been given their load addresses yet, lldb will treat 
> file addresses == load addresses.  Which sounds like what you're seeing.  So 
> we are often in the situation where an address->symbol lookup results in 
> multiple symbols being matched; they are all overlapping at this point.
> 
> As soon as the sections are given load addresses in the Target, then this 
> overlapping problem is resolved.
> 
> gdb didn't have the difference between load address and file address and we 
> had to play games with shuffling things around to arbitrary addresses so they 
> don't overlap.  (and from what I can recall, changing the load address of a 
> binary in lldb meant going through the symbol table to update all the 
> addresses -- we wanted to separate the symbol table addresses from the load 
> addresses in a given target, so we came up with this system.)
> 
> 
> Are you trying to do address->symbol resolution before you know where the 
> binaries are actually loaded in the address space?  Or are you missing the 
> part that sets the load addresses for the sections in the Target?  I suspect 
> it's the latter.
> 
> 
> 
>> On Sep 20, 2017, at 4:12 PM, Koropoff, Brian  wrote:
>> 
>> Jason,
>> 
>> I'm setting the load addresses appropriately for all sections in my script.  
>> The problem is that the symbol map
>> is internally indexed by the "file address", which is the virtual address 
>> that the ELF section asks to
>> be loaded at, regardless of what the actual load address turns out to be:
>> 
>> https://github.com/llvm-mirror/lldb/blob/master/source/Symbol/Symtab.cpp#L878
>> 
>> Symbol lookup proceeds via the file address:
>> 
>> https://github.com/llvm-mirror/lldb/blob/master/source/Core/Module.cpp#L510
>> 
>> From what I can gather, the use of file addresses is to avoid needing to 
>> recompute the symtab
>> when a load address is changed.  This implementation detail means that file 
>> addresses
>> must be non-overlapping even if the load addresses are correctly set.  The 
>> generation of synthetic
>> file addresses has the added benefit of permitting offline symbolication by 
>> (module, file address) pair
>> without needing to know the load map, which appears to be an intended use 
>> case, e.g.
>> SBModule::ResolveFileAddress():
>> 
>> https://github.com/llvm-mirror/lldb/blob/master/include/lldb/API/SBModule.h#L120
>> 
>> 

Re: [Lldb-commits] FreeBSD kernel debugging fixes

2017-09-21 Thread Koropoff, Brian via lldb-commits
I've created a Phabricator review.

DWARF sections in executables/dylibs seem to specify 0 as the section address, 
and lldb depends on the abbreviation addresses in the debug_info section being 
0-relative.
My synthetic file address logic explicitly excludes non-program (e.g. symtab) 
and debug sections to avoid disturbing this arrangement.  It also doesn't touch 
non-.o files,
and neither does the relocation logic.

-- Brian


From: Greg Clayton [clayb...@gmail.com]
Sent: Thursday, September 21, 2017 8:26 AM
To: Koropoff, Brian
Cc: Jason Molenda; lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] FreeBSD kernel debugging fixes

I am worried that this will adversely affect the normal DWARF that is in ELF 
files. What does typical DWARF look like when all of the file addresses in 
DWARF and the symtab are set correctly to unique virtual addresses? No 
relocations on any addresses? If all "file addresses" are set to unique offsets 
in each section, we don't need relocations. If there are no relocations on the 
DWARF, then we are probably ok.

Using .o files is tricky indeed as you are finding out. Are your modifications 
enabled by the fact that the object file is a .o file? As I mentioned above, we 
don't want any relocations applied to linked binaries. We do special things 
with .o files that we load in mach-o to ensure the segments are all happy, so 
there is definitely extra work required.

Please submit these patches via reviews.llvm.org web site. For details please 
see:

https://llvm.org/docs/Phabricator.html

Then it is easier to look at the changes because we can see the surrounding 
code. Once that happens we can start looking at the patches and offering 
inlined comments and have a discussion on any needed changes.

Greg Clayton


> On Sep 20, 2017, at 4:37 PM, Koropoff, Brian via lldb-commits 
>  wrote:
>
> Jason,
>
> I'm performing address to symbol resolution after setting load addresses for 
> all sections.  It correctly identifies the module
> and section where the address resides, and in many cases gives correct 
> results.  However, because it translates the
> load address to a file address before indexing into the symtab, overlapping 
> file addresses for sections in the same module
> can cause the wrong name to be returned, such as returning a symbol in the 
> bss section even though the address is
> in the text section.
>
> An alternative way to fix this would be to split m_file_addr_to_index into 
> per-section maps, but that doesn't solve the
> problem of ResolveFileAddress being unusable, or the general expectation 
> within lldb that file addresses uniquely
> identify something within a module.
>
> -- Brian
> 
> From: Jason Molenda [jmole...@apple.com]
> Sent: Wednesday, September 20, 2017 4:18 PM
> To: Koropoff, Brian
> Cc: lldb-commits@lists.llvm.org
> Subject: Re: [Lldb-commits] FreeBSD kernel debugging fixes
>
> Right, we always record symbol addresses as the offset to the section that 
> contains them.  The Address class in lldb is used everywhere for this.  The 
> Target has a SectionLoadList which tells us where each Section is loaded in 
> memory -- this is how you translate an Address object to a load address.
>
> When sections have not been given their load addresses yet, lldb will treat 
> file addresses == load addresses.  Which sounds like what you're seeing.  So 
> we are often in the situation where an address->symbol lookup results in 
> multiple symbols being matched; they are all overlapping at this point.
>
> As soon as the sections are given load addresses in the Target, then this 
> overlapping problem is resolved.
>
> gdb didn't have the difference between load address and file address and we 
> had to play games with shuffling things around to arbitrary addresses so they 
> don't overlap.  (and from what I can recall, changing the load address of a 
> binary in lldb meant going through the symbol table to update all the 
> addresses -- we wanted to separate the symbol table addresses from the load 
> addresses in a given target, so we came up with this system.)
>
>
> Are you trying to do address->symbol resolution before you know where the 
> binaries are actually loaded in the address space?  Or are you missing the 
> part that sets the load addresses for the sections in the Target?  I suspect 
> it's the latter.
>
>
>
>> On Sep 20, 2017, at 4:12 PM, Koropoff, Brian  wrote:
>>
>> Jason,
>>
>> I'm setting the load addresses appropriately for all sections in my script.  
>> The problem is that the symbol map
>> is internally indexed by the "file address", which is the virtual address 
>> that the ELF section asks to
>> be loaded at, regardless of what the actual load address turns out to be:
>>
>> https://github.com/llvm-mirror/lldb/blob/master/source/Symbol/Symtab.cpp#L878
>>
>> Symbol lookup proceeds via the file address:
>>
>> https://github.com/llvm-mirror/lldb/blob/master/

[Lldb-commits] [lldb] r313904 - [LLDB] Implement interactive command interruption

2017-09-21 Thread Adrian McCarthy via lldb-commits
Author: amccarth
Date: Thu Sep 21 12:36:52 2017
New Revision: 313904

URL: http://llvm.org/viewvc/llvm-project?rev=313904&view=rev
Log:
[LLDB] Implement interactive command interruption

The core of this change is the new CommandInterpreter::m_command_state, which
models the state transitions for interactive commands, including an
"interrupted" state transition.

In general, command interruption requires cooperation from the code executing
the command, which needs to poll for interruption requests through
CommandInterpreter::WasInterrupted().

CommandInterpreter::PrintCommandOutput() implements an optionally
interruptible printing of the command output, which for large outputs was
likely the longest blocking part.  (ex. target modules dump symtab on a
complex binary could take 10+ minutes)

patch by lemo

Differential Revision: https://reviews.llvm.org/D37923

Modified:
lldb/trunk/include/lldb/API/SBCommandInterpreter.h
lldb/trunk/include/lldb/Core/IOHandler.h
lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
lldb/trunk/scripts/interface/SBCommandInterpreter.i
lldb/trunk/source/API/SBCommandInterpreter.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp

Modified: lldb/trunk/include/lldb/API/SBCommandInterpreter.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBCommandInterpreter.h?rev=313904&r1=313903&r2=313904&view=diff
==
--- lldb/trunk/include/lldb/API/SBCommandInterpreter.h (original)
+++ lldb/trunk/include/lldb/API/SBCommandInterpreter.h Thu Sep 21 12:36:52 2017
@@ -165,6 +165,8 @@ public:
int match_start_point, int max_return_elements,
lldb::SBStringList &matches);
 
+  bool WasInterrupted() const;
+
   // Catch commands before they execute by registering a callback that will
   // get called when the command gets executed. This allows GUI or command
   // line interfaces to intercept a command and stop it from happening

Modified: lldb/trunk/include/lldb/Core/IOHandler.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/IOHandler.h?rev=313904&r1=313903&r2=313904&view=diff
==
--- lldb/trunk/include/lldb/Core/IOHandler.h (original)
+++ lldb/trunk/include/lldb/Core/IOHandler.h Thu Sep 21 12:36:52 2017
@@ -195,7 +195,7 @@ public:
   enum class Completion { None, LLDBCommand, Expression };
 
   IOHandlerDelegate(Completion completion = Completion::None)
-  : m_completion(completion), m_io_handler_done(false) {}
+  : m_completion(completion) {}
 
   virtual ~IOHandlerDelegate() = default;
 
@@ -296,7 +296,6 @@ public:
 
 protected:
   Completion m_completion; // Support for common builtin completions
-  bool m_io_handler_done;
 };
 
 //--

Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h?rev=313904&r1=313903&r2=313904&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h Thu Sep 21 
12:36:52 2017
@@ -242,6 +242,8 @@ public:
  bool repeat_on_empty_command = true,
  bool no_context_switching = false);
 
+  bool WasInterrupted() const;
+
   //--
   /// Execute a list of commands in sequence.
   ///
@@ -522,6 +524,24 @@ private:
   StringList &commands_help,
   CommandObject::CommandMap &command_map);
 
+  // An interruptible wrapper around the stream output
+  void PrintCommandOutput(Stream &stream, llvm::StringRef str,
+  bool interruptible);
+
+  // A very simple state machine which models the command handling transitions
+  enum class CommandHandlingState {
+eIdle,
+eInProgress,
+eInterrupted,
+  };
+
+  std::atomic m_command_state{
+  CommandHandlingState::eIdle};
+
+  void StartHandlingCommand();
+  void FinishHandlingCommand();
+  bool InterruptCommand();
+
   Debugger &m_debugger; // The debugger session that this interpreter is
 // associated with
   ExecutionContextRef m_exe_ctx_ref; // The current execution context to use

Modified: lldb/trunk/scripts/interface/SBCommandInterpreter.i
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/interface/SBCommandInterpreter.i?rev=313904&r1=313903&r2=313904&view=diff
==
--- lldb/trunk/scripts/interface/SBCommandInterpreter.i (origi

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

2017-09-21 Thread Adrian McCarthy via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313904: [LLDB] Implement interactive command interruption 
(authored by amccarth).

Changed prior to commit:
  https://reviews.llvm.org/D37923?vs=116074&id=116247#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37923

Files:
  lldb/trunk/include/lldb/API/SBCommandInterpreter.h
  lldb/trunk/include/lldb/Core/IOHandler.h
  lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
  lldb/trunk/scripts/interface/SBCommandInterpreter.i
  lldb/trunk/source/API/SBCommandInterpreter.cpp
  lldb/trunk/source/Commands/CommandObjectTarget.cpp
  lldb/trunk/source/Core/Debugger.cpp
  lldb/trunk/source/Interpreter/CommandInterpreter.cpp

Index: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp
@@ -546,7 +546,7 @@
   char buffer[1024];
   int num_printed =
   snprintf(buffer, 1024, "%s %s", break_regexes[i][1], "-o");
-  assert(num_printed < 1024);
+  lldbassert(num_printed < 1024);
   UNUSED_IF_ASSERT_DISABLED(num_printed);
   success =
   tbreak_regex_cmd_ap->AddRegexCommand(break_regexes[i][0], buffer);
@@ -891,8 +891,8 @@
 const lldb::CommandObjectSP &cmd_sp,
 bool can_replace) {
   if (cmd_sp.get())
-assert((this == &cmd_sp->GetCommandInterpreter()) &&
-   "tried to add a CommandObject from a different interpreter");
+lldbassert((this == &cmd_sp->GetCommandInterpreter()) &&
+   "tried to add a CommandObject from a different interpreter");
 
   if (name.empty())
 return false;
@@ -913,8 +913,8 @@
 const lldb::CommandObjectSP &cmd_sp,
 bool can_replace) {
   if (cmd_sp.get())
-assert((this == &cmd_sp->GetCommandInterpreter()) &&
-   "tried to add a CommandObject from a different interpreter");
+lldbassert((this == &cmd_sp->GetCommandInterpreter()) &&
+   "tried to add a CommandObject from a different interpreter");
 
   if (!name.empty()) {
 // do not allow replacement of internal commands
@@ -1062,8 +1062,8 @@
  lldb::CommandObjectSP &command_obj_sp,
  llvm::StringRef args_string) {
   if (command_obj_sp.get())
-assert((this == &command_obj_sp->GetCommandInterpreter()) &&
-   "tried to add a CommandObject from a different interpreter");
+lldbassert((this == &command_obj_sp->GetCommandInterpreter()) &&
+   "tried to add a CommandObject from a different interpreter");
 
   std::unique_ptr command_alias_up(
   new CommandAlias(*this, command_obj_sp, args_string, alias_name));
@@ -1839,7 +1839,7 @@
   matches.Clear();
 
   // Only max_return_elements == -1 is supported at present:
-  assert(max_return_elements == -1);
+  lldbassert(max_return_elements == -1);
   bool word_complete;
   num_command_matches = HandleCompletionMatches(
   parsed_line, cursor_index, cursor_char_position, match_start_point,
@@ -2677,6 +2677,57 @@
   return total_bytes;
 }
 
+void CommandInterpreter::StartHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eInProgress);
+  lldbassert(prev_state == CommandHandlingState::eIdle);
+}
+
+void CommandInterpreter::FinishHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle);
+  lldbassert(prev_state != CommandHandlingState::eIdle);
+}
+
+bool CommandInterpreter::InterruptCommand() {
+  auto in_progress = CommandHandlingState::eInProgress;
+  return m_command_state.compare_exchange_strong(
+  in_progress, CommandHandlingState::eInterrupted);
+}
+
+bool CommandInterpreter::WasInterrupted() const {
+  return m_command_state == CommandHandlingState::eInterrupted;
+}
+
+void CommandInterpreter::PrintCommandOutput(Stream &stream, llvm::StringRef str,
+bool interruptible) {
+  if (str.empty())
+return;
+
+  if (interruptible) {
+// Split the output into lines and poll for interrupt requests
+const char *data = str.data();
+size_t size = str.size();
+while (size > 0 && !WasInterrupted()) {
+  size_t chunk_size = 0;
+  for (; chunk_size < size; ++chunk_size) {
+lldbassert(data[chunk_size] != '\0');
+if (data[chunk_size] == '\n') {
+  ++chunk_size;
+  break;
+}
+  }
+  chunk_size = stream.Write(data, chunk_size);
+  lldbassert(size >= chunk_size);
+  data += chunk_size;
+  size -= chunk_size;
+}
+if (size > 0) {
+  stream.Printf("\n... Interrupted.\n");
+}
+  } else {
+stream.PutCString(str);
+  }
+}
+
 void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_ha

[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

2017-09-21 Thread Sean Callanan via Phabricator via lldb-commits
spyffe created this revision.
spyffe added a project: LLDB.
Herald added subscribers: arichardson, aprantl, sdardis.

The IR dynamic checks are self-contained functions whose job is to

- verify that pointers referenced in an expression are valid at runtime; and
- verify that selectors sent to Objective-C objects by an expression are 
actually supported by that object.

These dynamic checks forward-declare all the functions they use and should not 
require any external debug information.  The way they ensure this is by marking 
all the names they use with a dollar sign (`$`).  The expression parser 
recognizes such symbols and perform no lookups for them.

This patch fixes three issues surrounding the use of the dollar sign:

- to fix a MIPS issue, the name of the pointer checker was changed from 
starting with `$` to starting with `_$`, but this was not properly ignored; and
- the Objective-C object checker used a temporary variable that did not start 
with `$`.
- the Objective-C object checker used an externally-defined struct (`struct 
objc_selector`) but didn't need to.

The patch also reformats the string containing the Objective-C object checker, 
which was mangled horribly when the code was transformed to a uniform width of 
80 columns.


Repository:
  rL LLVM

https://reviews.llvm.org/D38153

Files:
  source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -812,84 +812,44 @@
   int len = 0;
   if (m_has_object_getClass) {
 len = ::snprintf(check_function_code, sizeof(check_function_code),
- "extern \"C\" void *gdb_object_getClass(void *);  "
- "\n"
- "extern \"C\"  int printf(const char *format, ...);   "
- "\n"
- "extern \"C\" void"
- "\n"
- "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector)"
- "\n"
- "{"
- "\n"
- "   if ($__lldb_arg_obj == (void *)0) "
- "\n"
- "   return; // nil is ok  "
- "\n"
- "   if (!gdb_object_getClass($__lldb_arg_obj))"
- "\n"
- "   *((volatile int *)0) = 'ocgc';"
- "\n"
- "   else if ($__lldb_arg_selector != (void *)0)   "
- "\n"
- "   { "
- "\n"
- "signed char responds = (signed char) [(id) "
- "$__lldb_arg_obj   \n"
- ""
- "respondsToSelector:  \n"
- "   (struct "
- "objc_selector *) $__lldb_arg_selector];   \n"
- "   if (responds == (signed char) 0)  "
- "\n"
- "   *((volatile int *)0) = 'ocgc';"
- "\n"
- "   } "
- "\n"
- "}"
- "\n",
- name);
+ "extern \"C\" void *gdb_object_getClass(void *);\n"
+ "extern \"C\"  int printf(const char *format, ...); \n"
+ "extern \"C\" void  \n"
+ "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) {\n"
+ "  if ($__lldb_arg_obj == (void *)0)\n"
+ "return;

[Lldb-commits] [PATCH] D35356: [zorg] Enable assertions on the linux lldb bot

2017-09-21 Thread Galina via Phabricator via lldb-commits
gkistanova added a comment.

Hi Pavel,

The patch looks Ok, with a small issue to address before committing.




Comment at: zorg/buildbot/builders/LLDBBuilder.py:921
scriptExt='.sh',
+   extra_cmake_args=[],
):

Please do not use mutable default arguments.
See 
http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments
 for more details.




Comment at: zorg/buildbot/builders/LLDBBuilder.py:954
 getShellCommandStep(f, name='cmake local',
-command=[pathSep + 'cmake' + scriptExt])
+command=[pathSep + 'cmake' + scriptExt] + 
extra_cmake_args)
 

This is not a blocker for this patch. With all the same, it would be nice to 
use the CmakeCommand here instead of the ShellCommand.


https://reviews.llvm.org/D35356



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

2017-09-21 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:651-652
 return;
+  if (name_unique_cstr[0] == '_' && name_unique_cstr[1] == '$')
+return;
 

Should we use a function or macro here? This code is duped below.



Comment at: 
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:812-814
+  if (name_unique_cstr[0] == '_' && name_unique_cstr[1] == '$')
+return;
+

Should we use a function or macro here? This code is duped above



Comment at: 
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:815-831
+ "extern \"C\" void *gdb_object_getClass(void *);
\n"
+ "extern \"C\"  int printf(const char *format, ...); 
\n"
+ "extern \"C\" void  
\n"
+ "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) 
{\n"
+ "  if ($__lldb_arg_obj == (void *)0)
\n"
+ "return; // nil is ok   
\n"
+ "  if (!gdb_object_getClass($__lldb_arg_obj)) { 
\n"

Since you are modifying this string switch to R"( to make it much clearer:

```
R"(
extern "C" void *gdb_object_getClass(void *);
extern "C"  int printf(const char *format, ...);
extern "C" void
%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector)
  if ($__lldb_arg_obj == (void *)0)
return; // nil is ok
  if (!gdb_object_getClass($__lldb_arg_obj)) {
*((volatile int *)0) = 'ocgc';
  } else if ($__lldb_arg_selector != (void *)0) {
signed char $responds = (signed char)
[(id)$__lldb_arg_obj respondsToSelector:
(void *) $__lldb_arg_selector];
if ($responds == (signed char) 0)
  *((volatile int *)0) = 'ocgc';
  }
})";
```



Comment at: 
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:834-852
+ "extern \"C\" void *gdb_class_getClass(void *); 
\n"
+ "extern \"C\"  int printf(const char *format, ...); 
\n"
+ "extern \"C\"  void 
\n"
+ "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) 
{\n"
+ "  if ($__lldb_arg_obj == (void *)0)
\n"
+ "return; // nil is ok   
\n"
+ "  void **$isa_ptr = (void **)$__lldb_arg_obj;  
\n"

Use R"( as above.


Repository:
  rL LLVM

https://reviews.llvm.org/D38153



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D38153: Inhibit global lookups for symbols in the IR dynamic checks

2017-09-21 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:635-652
   const ConstString name(context.m_decl_name.getAsString().c_str());
 
   const char *name_unique_cstr = name.GetCString();
 
   static ConstString id_name("id");
   static ConstString Class_name("Class");
 

I might be missing something here, but how about:

```
std::string name = context.m_decl_name.getAsString();
if (name.empty() || name == "id" || name == "Class")
  return;
if (name.startswith("$") || name.startswith("_$"))
  return;
```

5 lines instead of ~20.


Repository:
  rL LLVM

https://reviews.llvm.org/D38153



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits