[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

When I compare the backtrace from GDB and LLDB (on x86_64, I haven't tried 
aarch64 as x86_64 is much faster=easier to handle):

  frame #2: 0x00401195 sigtest2`handler(sig=6) at sigtest2.c:9:5
  #2  0x00401195 in handler (sig=6) at sigtest2.c:9
  
  frame #3: 0x7f5fd888ef00 libc.so.6`.annobin_sigaction.c + 16
  #3   => 0x7f5fd888ef00 <+0>: mov$0xf,%rax
  
  frame #4: 0x7f5fd888ee75 libc.so.6`__GI_raise at 
internal-signals.h:84:10
  #4  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 $1 
= 0x7f5fd888ee75
  
  frame #5: 0x7f5fd888ee5e libc.so.6`__GI_raise(sig=2) at raise.c:48
  Missing in GDB
  
  frame #6: 0x7f5fd8879895 libc.so.6`__GI_abort at abort.c:79:7
  #5  0x7f5fd8879895 in __GI_abort () at abort.c:79

That LLDB frame #5 is a bit bogus:

  (lldb) down
  frame #5: 0x7f5fd888ee5e libc.so.6`__GI_raise(sig=2) at raise.c:48
 45 
 46   int ret = INLINE_SYSCALL (tgkill, 3, pid, tid, sig);
 47 
  -> 48   __libc_signal_restore_set (&set);
 49 
 50   return ret;
 51 }
  (lldb) disas
  libc.so.6`__GI_raise:
  ...
  0x7f5fd888ee5b <+299>: movl   %eax, %r8d
  0x7f5fd888ee5e <+302>: movl   $0x8, %r10d
  0x7f5fd888ee64 <+308>: xorl   %edx, %edx
  0x7f5fd888ee66 <+310>: movq   %r9, %rsi
  0x7f5fd888ee69 <+313>: movl   $0x2, %edi
  0x7f5fd888ee6e <+318>: movl   $0xe, %eax
  0x7f5fd888ee73 <+323>: syscall 
  ->  0x7f5fd888ee75 <+325>: movq   0x108(%rsp), %rax
  0x7f5fd888ee7d <+333>: xorq   %fs:0x28, %rax

That `0x7f5fd888ee5e` is not even shown by `disas`, also it is no point of 
a return from any call.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63667/new/

https://reviews.llvm.org/D63667



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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This looks good to me.  I wonder if the SymbolContextScope -> Language 
calculation that you do in IsRuntimeSupportValue should really be done in 
SymbolContextScope?  If that's going to be the policy for going from 
SymbolContextScope, maybe centralize it there?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63240/new/

https://reviews.llvm.org/D63240



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

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D63240#1562527 , @jingham wrote:

> This looks good to me.  I wonder if the SymbolContextScope -> Language 
> calculation that you do in IsRuntimeSupportValue should really be done in 
> SymbolContextScope?  If that's going to be the policy for going from 
> SymbolContextScope, maybe centralize it there?


I agree, centralize this in SymbolContextScope and this should be good to go. 
Thanks for sticking with this Alex.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63240/new/

https://reviews.llvm.org/D63240



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

[Lldb-commits] [lldb] r364666 - Make sure the thread list is updated before you set the stop reason

[Jim Ingham via lldb-commits]

Author: jingham
Date: Fri Jun 28 10:57:19 2019
New Revision: 364666

URL: http://llvm.org/viewvc/llvm-project?rev=364666&view=rev
Log:
Make sure the thread list is updated before you set the stop reason
on a thread.  When talking to some older gdb-remote stubs, We were getting
a stop reason from the stop reply packet and setting it on the relevant
thread before we updated the full stop list.  That would get discarded when
the full list was updated.

Also, if you already have a thread list when you go to see if there is an
Operating System plugin, and you do indeed load a new OS plugin, you have to
re-fetch the thread list or it will only show the raw threads.

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


Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Target/Process.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py?rev=364666&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
 Fri Jun 28 10:57:19 2019
@@ -0,0 +1,139 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestRecognizeBreakpoint(GDBRemoteTestBase):
+""" This tests the case where the gdb-remote server doesn't support any
+of the thread-info packets, and just tells which thread got the stop
+signal with:
+  T05thread:01;
+There was a bug in lldb that we would set the stop reason from this 
+packet too early - before we had updated the thread list.  So when we
+later updated the thread list, we would throw away this info.  Normally
+we would be able to reconstruct it from the thread info, but not if the
+stub doesn't support it """
+ 
+def test(self):
+class MyResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.thread_info_count = 0
+self.after_cont = False
+self.current_thread = 0
+
+def cont(self):
+# Simulate process stopping due to a breakpoint:
+self.after_cont = True
+return "T05thread:01;"
+
+def vCont(self, packet):
+self.after_cont = True
+return "T05thread:01;"
+
+def haltReason(self):
+return "T02thread:01;"
+
+def threadStopInfo(self, num):
+return ""
+
+def QThreadSuffixSupported(self):
+return ""
+
+def QListThreadsInStopReply(self):
+return ""
+
+def setBreakpoint(self, packet):
+return "OK"
+
+def qfThreadInfo(self):
+return "m1"
+
+def qsThreadInfo(self):
+if (self.thread_info_count % 2) == 0:
+str = "m2"
+else:
+str = "l"
+self.thread_info_count += 1
+return str
+
+def readRegisters(self):
+if self.after_cont and self.current_thread == 1:
+return "c01e990080ff"
+else:
+return "badcfe10325476980"
+
+def readRegister(self, regno):
+return ""
+
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+return """
+   
+  i386:x86-64
+  
+
+  
+""", False
+   else:
+return None, False
+
+def selectThread(self, op, thread):
+if op != 'g':
+return ''
+
+self.current_thread = thread
+return "OK"
+
+def other (self, packet):
+if packet == "vCont?":
+return "vCont;c;C;s;S"
+return ''
+   

[Lldb-commits] [PATCH] D62887: Update the thread list before setting stop reasons with an OS plugin

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
jingham marked an inline comment as done.
Closed by commit rL364666: Make sure the thread list is updated before you set 
the stop reason (authored by jingham, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62887?vs=206980&id=207105#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62887/new/

https://reviews.llvm.org/D62887

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/trunk/source/Target/Process.cpp

Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -3019,8 +3019,16 @@
 }
   }
 
-  if (!m_os_up)
+  if (!m_os_up) {
 LoadOperatingSystemPlugin(false);
+if (m_os_up) {
+  // Somebody might have gotten threads before now, but we need to force the
+  // update after we've loaded the OperatingSystem plugin or it won't get a
+  // chance to process the threads.
+  m_thread_list.Clear();
+  UpdateThreadListIfNeeded();
+}
+  }
   // Figure out which one is the executable, and set that in our target:
   const ModuleList &target_modules = GetTarget().GetImages();
   std::lock_guard guard(target_modules.GetMutex());
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2423,6 +2423,15 @@
 
   // Scope for the lock
   {
+// Check to see if SetThreadStopInfo() filled in m_thread_ids?
+if (m_thread_ids.empty()) {
+// No, we need to fetch the thread list manually
+UpdateThreadIDList();
+}
+// We might set some stop info's so make sure the thread list is up to
+// date before we do that or we might overwrite what was computed here.
+UpdateThreadListIfNeeded();
+
 // Lock the thread stack while we access it
 std::lock_guard guard(m_last_stop_packet_mutex);
 // Get the number of stop packets on the stack
@@ -2437,13 +2446,7 @@
 // Clear the thread stop stack
 m_stop_packet_stack.clear();
   }
-
-  // Check to see if SetThreadStopInfo() filled in m_thread_ids?
-  if (m_thread_ids.empty()) {
-// No, we need to fetch the thread list manually
-UpdateThreadIDList();
-  }
-
+  
   // If we have queried for a default thread id
   if (m_initial_tid != LLDB_INVALID_THREAD_ID) {
 m_thread_list.SetSelectedThreadByID(m_initial_tid);
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system_2.py
@@ -0,0 +1,62 @@
+import lldb
+import struct
+
+
+class OperatingSystemPlugIn(object):
+"""Class that provides data for an instance of a LLDB 'OperatingSystemPython' plug-in class
+   This version stops once with threads 0x111 and 0x222, then stops a second time with threads
+   0x111 and 0x333."""
+
+def __init__(self, process):
+'''Initialization needs a valid.SBProcess object.
+
+This plug-in will get created after a live process is valid and has stopped for the first time.
+'''
+self.process = None
+self.registers = None
+self.threads = None
+self.times_called = 0
+if isinstance(process, lldb.SBProcess) and process.IsValid():
+self.process = process
+self.threads = None  # Will be an dictionary containing info for each thread
+
+def get_target(self):
+return self.process.target
+
+def get_thread_info(self):
+self.times_called += 1
+
+if self.times_called == 1:
+self.threads = [{
+'tid': 0x111,
+'name': 'one',
+'queue': 'queue1',
+'state': 'stopped',
+'stop_reason': 'none',
+'core': 1
+}, {
+'tid': 0x222,
+'name': 'two',
+'queue': 'queue2',
+'state': 'stopped',
+'stop_reason': 'none',
+'core': 0
+}]
+else:
+self.threads = [{
+'tid': 0x1

[Lldb-commits] [lldb] r364669 - [GDBRemote] Remove code that flushes GDB remote packets

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Fri Jun 28 11:14:27 2019
New Revision: 364669

URL: http://llvm.org/viewvc/llvm-project?rev=364669&view=rev
Log:
[GDBRemote] Remove code that flushes GDB remote packets

The arbitrary timeout when flushing GDB remote packets caused
non-determinism and flakiness between test runs. I suspect it is what's
causing the flakiness of the reproducer tests on GreenDragon, and want
to see if removing it causes that to go away.

This change was originally introduced in r197579 to discard a
`$T02thread:01;#4` that QEMU was sending. If anybody knows how to test
that this continues working after removing this code, I'd love to hear
it.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=364669&r1=364668&r2=364669&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Fri Jun 28 11:14:27 2019
@@ -115,13 +115,6 @@ bool GDBRemoteCommunicationClient::Hands
   // Start the read thread after we send the handshake ack since if we fail to
   // send the handshake ack, there is no reason to continue...
   if (SendAck()) {
-// Wait for any responses that might have been queued up in the remote
-// GDB server and flush them all
-StringExtractorGDBRemote response;
-PacketResult packet_result = PacketResult::Success;
-while (packet_result == PacketResult::Success)
-  packet_result = ReadPacket(response, milliseconds(10), false);
-
 // The return value from QueryNoAckModeSupported() is true if the packet
 // was sent and _any_ response (including UNIMPLEMENTED) was received), or
 // false if no response was received. This quickly tells us if we have a


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

[Lldb-commits] [PATCH] D63914: Make the expression parser work for missing weak symbols

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

A few nits, but nothing structural. Fix those and this will be good to go.




Comment at: include/lldb/Symbol/Symbol.h:257
  // doing name lookups
+  m_is_weak : 1,
   m_type : 7;

This will increase the size of Symbol since this extra bit makes it go over 16 
bits in size. We can steal a bit away from m_type since it is used to hold 
lldb::SymbolType and we only have 28 lldb::SymbolType enums. You might need to 
check that the Swift branch didn't add any. 



Comment at: include/lldb/Symbol/Symbol.h:258
+  m_is_weak : 1,
   m_type : 7;
   Mangled m_mangled; // uniqued symbol name/mangled name pair

change to:
```
m_type : 6;
```
See above comment.





Comment at: 
packages/Python/lldbsuite/test/expression_command/weak_symbols/TestWeakSymbols.py:24
+
+@decorators.skipUnlessDarwin
+def test_weak_symbol_in_expr(self):

We should get ELF support for this as well once this is in.



Comment at: source/Expression/IRInterpreter.cpp:236
 lldb_private::ConstString name(constant_func->getName());
-lldb::addr_t addr = m_execution_unit.FindSymbol(name);
-if (addr == LLDB_INVALID_ADDRESS)
+bool missing_weak;
+lldb::addr_t addr = m_execution_unit.FindSymbol(name, missing_weak);

Init this to something in case function below gets changed and doesn't init it.



Comment at: source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:436
 
+bool missing_weak;
 CFStringCreateWithBytes_addr =

init to something.



Comment at: source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:862
 
+bool missing_weak;
 static lldb_private::ConstString 
g_sel_registerName_str("sel_registerName");

init to something



Comment at: source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:1034
 
+bool missing_weak;
 static lldb_private::ConstString g_objc_getClass_str("objc_getClass");

init to something.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63914/new/

https://reviews.llvm.org/D63914



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

[Lldb-commits] [PATCH] D63914: Make the expression parser work for missing weak symbols

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 207146.
jingham added a comment.

Addressed Greg's comments


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63914/new/

https://reviews.llvm.org/D63914

Files:
  include/lldb/Expression/IRExecutionUnit.h
  include/lldb/Symbol/Symbol.h
  include/lldb/lldb-enumerations.h
  packages/Python/lldbsuite/test/expression_command/weak_symbols/Makefile
  
packages/Python/lldbsuite/test/expression_command/weak_symbols/TestWeakSymbols.py
  packages/Python/lldbsuite/test/expression_command/weak_symbols/dylib.c
  packages/Python/lldbsuite/test/expression_command/weak_symbols/dylib.h
  packages/Python/lldbsuite/test/expression_command/weak_symbols/main.c
  
packages/Python/lldbsuite/test/expression_command/weak_symbols/module.modulemap
  source/Expression/IRExecutionUnit.cpp
  source/Expression/IRInterpreter.cpp
  source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Symbol/Symbol.cpp

Index: source/Symbol/Symbol.cpp
===
--- source/Symbol/Symbol.cpp
+++ source/Symbol/Symbol.cpp
@@ -28,7 +28,8 @@
   m_is_external(false), m_size_is_sibling(false),
   m_size_is_synthesized(false), m_size_is_valid(false),
   m_demangled_is_synthesized(false), m_contains_linker_annotations(false),
-  m_type(eSymbolTypeInvalid), m_mangled(), m_addr_range(), m_flags() {}
+  m_is_weak(false), m_type(eSymbolTypeInvalid), m_mangled(), m_addr_range(),
+  m_flags() {}
 
 Symbol::Symbol(uint32_t symID, const char *name, bool name_is_mangled,
SymbolType type, bool external, bool is_debug,
@@ -41,7 +42,8 @@
   m_is_debug(is_debug), m_is_external(external), m_size_is_sibling(false),
   m_size_is_synthesized(false), m_size_is_valid(size_is_valid || size > 0),
   m_demangled_is_synthesized(false),
-  m_contains_linker_annotations(contains_linker_annotations), m_type(type),
+  m_contains_linker_annotations(contains_linker_annotations), 
+  m_is_weak(false), m_type(type),
   m_mangled(ConstString(name), name_is_mangled),
   m_addr_range(section_sp, offset, size), m_flags(flags) {}
 
@@ -56,8 +58,9 @@
   m_size_is_synthesized(false),
   m_size_is_valid(size_is_valid || range.GetByteSize() > 0),
   m_demangled_is_synthesized(false),
-  m_contains_linker_annotations(contains_linker_annotations), m_type(type),
-  m_mangled(mangled), m_addr_range(range), m_flags(flags) {}
+  m_contains_linker_annotations(contains_linker_annotations), 
+  m_is_weak(false), m_type(type), m_mangled(mangled), m_addr_range(range), 
+  m_flags(flags) {}
 
 Symbol::Symbol(const Symbol &rhs)
 : SymbolContextScope(rhs), m_uid(rhs.m_uid), m_type_data(rhs.m_type_data),
@@ -68,7 +71,7 @@
   m_size_is_valid(rhs.m_size_is_valid),
   m_demangled_is_synthesized(rhs.m_demangled_is_synthesized),
   m_contains_linker_annotations(rhs.m_contains_linker_annotations),
-  m_type(rhs.m_type), m_mangled(rhs.m_mangled),
+  m_is_weak(rhs.m_is_weak), m_type(rhs.m_type), m_mangled(rhs.m_mangled),
   m_addr_range(rhs.m_addr_range), m_flags(rhs.m_flags) {}
 
 const Symbol &Symbol::operator=(const Symbol &rhs) {
@@ -85,6 +88,7 @@
 m_size_is_valid = rhs.m_size_is_valid;
 m_demangled_is_synthesized = rhs.m_demangled_is_synthesized;
 m_contains_linker_annotations = rhs.m_contains_linker_annotations;
+m_is_weak = rhs.m_is_weak;
 m_type = rhs.m_type;
 m_mangled = rhs.m_mangled;
 m_addr_range = rhs.m_addr_range;
@@ -106,6 +110,7 @@
   m_size_is_valid = false;
   m_demangled_is_synthesized = false;
   m_contains_linker_annotations = false;
+  m_is_weak = false;
   m_type = eSymbolTypeInvalid;
   m_flags = 0;
   m_addr_range.Clear();
Index: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -4575,6 +4575,8 @@
 sym[sym_idx].GetAddressRef().SetOffset(symbol_value);
   }
   sym[sym_idx].SetFlags(nlist.n_type << 16 | nlist.n_desc);
+  if (nlist.n_desc & N_WEAK_REF)
+sym[sym_idx].SetIsWeak(true);
 
   if (symbol_byte_size > 0)
 sym[sym_idx].SetByteSize(symbol_byte_size);
Index: source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
===
--- source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -433,9 +433,11 @@
 static lldb_private::ConstString g_CFStringCreateWithBytes_str(
 "CFStringCreateWithBytes");
 
+bool missing_weak = false;
 CFStringCreateWithBytes_addr =
-m_execution_unit.FindSymbol(g_CFStringCreateWithBytes_str);
-if (CFStringCreateWithBytes_addr == LLDB_INVA

[Lldb-commits] [PATCH] D63914: Make the expression parser work for missing weak symbols

[Jim Ingham via Phabricator via lldb-commits]

jingham marked 8 inline comments as done.
jingham added inline comments.



Comment at: include/lldb/Symbol/Symbol.h:258
+  m_is_weak : 1,
   m_type : 7;
   Mangled m_mangled; // uniqued symbol name/mangled name pair

clayborg wrote:
> change to:
> ```
> m_type : 6;
> ```
> See above comment.
> 
> 
Yup, apparently I can't count...  Anyway, swift did add a few but we're still 
well under 6 bits.  I added a comment here saying where m_type gets its values 
from and in the enum saying it needs to stay under 6 bits.



Comment at: 
packages/Python/lldbsuite/test/expression_command/weak_symbols/TestWeakSymbols.py:24
+
+@decorators.skipUnlessDarwin
+def test_weak_symbol_in_expr(self):

clayborg wrote:
> We should get ELF support for this as well once this is in.
I don't know how this works in ELF, but if somebody else wants to try their 
hand at it, that would be great.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63914/new/

https://reviews.llvm.org/D63914



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

[Lldb-commits] [lldb] r364686 - Get the expression parser to handle missing weak symbols.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Fri Jun 28 14:40:05 2019
New Revision: 364686

URL: http://llvm.org/viewvc/llvm-project?rev=364686&view=rev
Log:
Get the expression parser to handle missing weak symbols.
MachO only for this patch.

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



Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/TestWeakSymbols.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/dylib.c

lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/dylib.h

lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/main.c

lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/module.modulemap
Modified:
lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
lldb/trunk/include/lldb/Symbol/Symbol.h
lldb/trunk/include/lldb/lldb-enumerations.h
lldb/trunk/source/Expression/IRExecutionUnit.cpp
lldb/trunk/source/Expression/IRInterpreter.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Symbol/Symbol.cpp

Modified: lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/IRExecutionUnit.h?rev=364686&r1=364685&r2=364686&view=diff
==
--- lldb/trunk/include/lldb/Expression/IRExecutionUnit.h (original)
+++ lldb/trunk/include/lldb/Expression/IRExecutionUnit.h Fri Jun 28 14:40:05 
2019
@@ -101,7 +101,7 @@ public:
 
   lldb::ModuleSP GetJITModule();
 
-  lldb::addr_t FindSymbol(ConstString name);
+  lldb::addr_t FindSymbol(ConstString name, bool &missing_weak);
 
   void GetStaticInitializers(std::vector &static_initializers);
 
@@ -226,7 +226,8 @@ private:
 const std::vector &C_specs);
 
   lldb::addr_t FindInSymbols(const std::vector &specs,
- const lldb_private::SymbolContext &sc);
+ const lldb_private::SymbolContext &sc,
+ bool &symbol_was_missing_weak);
 
   lldb::addr_t FindInRuntimes(const std::vector &specs,
   const lldb_private::SymbolContext &sc);
@@ -301,6 +302,13 @@ private:
   size_t Size) override {}
 
 uint64_t getSymbolAddress(const std::string &Name) override;
+
+// Find the address of the symbol Name.  If Name is a missing weak symbol
+// then missing_weak will be true.
+uint64_t GetSymbolAddressAndPresence(const std::string &Name, 
+ bool &missing_weak);
+
+llvm::JITSymbol findSymbol(const std::string &Name) override;
 
 void *getPointerToNamedFunction(const std::string &Name,
 bool AbortOnFailure = true) override;

Modified: lldb/trunk/include/lldb/Symbol/Symbol.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Symbol.h?rev=364686&r1=364685&r2=364686&view=diff
==
--- lldb/trunk/include/lldb/Symbol/Symbol.h (original)
+++ lldb/trunk/include/lldb/Symbol/Symbol.h Fri Jun 28 14:40:05 2019
@@ -165,6 +165,10 @@ public:
   bool IsTrampoline() const;
 
   bool IsIndirect() const;
+  
+  bool IsWeak() const { return m_is_weak; }
+  
+  void SetIsWeak (bool b) { m_is_weak = b; }
 
   bool GetByteSizeIsValid() const { return m_size_is_valid; }
 
@@ -250,7 +254,8 @@ protected:
   m_contains_linker_annotations : 1, // The symbol name contains linker
  // annotations, which are optional 
when
  // doing name lookups
-  m_type : 7;
+  m_is_weak : 1,
+  m_type : 6;// Values from the lldb::SymbolType enum.
   Mangled m_mangled; // uniqued symbol name/mangled name pair
   AddressRange m_addr_range; // Contains the value, or the section offset
  // address when the value is an address in a

Modified: lldb/trunk/include/lldb/lldb-enumerations.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-enumerations.h?rev=364686&r1=364685&r2=364686&view=diff
==
--- lldb/trunk/include/lldb/lldb-enumerations.h (original)
+++ lldb/trunk/include/lldb/lldb-enumerations.h Fri Jun 28 14:40:05 2019
@@ -592,6 +592,8 @@ enum CommandArgumentType {
 };
 
 // Symbol types
+// Symbol holds the SymbolType in a 6-bit field (m_type), so if you get over 
63 
+// entries you will have to resize that field.
 enum SymbolType {
   eSymbolTypeAny = 0,
   eSymbolTypeInvalid = 

[Lldb-commits] [PATCH] D63914: Make the expression parser work for missing weak symbols

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
jingham marked an inline comment as done.
Closed by commit rL364686: Get the expression parser to handle missing weak 
symbols. (authored by jingham, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63914?vs=207146&id=207147#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63914/new/

https://reviews.llvm.org/D63914

Files:
  lldb/trunk/include/lldb/Expression/IRExecutionUnit.h
  lldb/trunk/include/lldb/Symbol/Symbol.h
  lldb/trunk/include/lldb/lldb-enumerations.h
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/TestWeakSymbols.py
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/dylib.c
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/dylib.h
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/main.c
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/weak_symbols/module.modulemap
  lldb/trunk/source/Expression/IRExecutionUnit.cpp
  lldb/trunk/source/Expression/IRInterpreter.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/trunk/source/Symbol/Symbol.cpp

Index: lldb/trunk/source/Expression/IRInterpreter.cpp
===
--- lldb/trunk/source/Expression/IRInterpreter.cpp
+++ lldb/trunk/source/Expression/IRInterpreter.cpp
@@ -233,8 +233,9 @@
 case Value::FunctionVal:
   if (const Function *constant_func = dyn_cast(constant)) {
 lldb_private::ConstString name(constant_func->getName());
-lldb::addr_t addr = m_execution_unit.FindSymbol(name);
-if (addr == LLDB_INVALID_ADDRESS)
+bool missing_weak = false;
+lldb::addr_t addr = m_execution_unit.FindSymbol(name, missing_weak);
+if (addr == LLDB_INVALID_ADDRESS || missing_weak)
   return false;
 value = APInt(m_target_data.getPointerSizeInBits(), addr);
 return true;
Index: lldb/trunk/source/Expression/IRExecutionUnit.cpp
===
--- lldb/trunk/source/Expression/IRExecutionUnit.cpp
+++ lldb/trunk/source/Expression/IRExecutionUnit.cpp
@@ -774,7 +774,9 @@
 
 lldb::addr_t IRExecutionUnit::FindInSymbols(
 const std::vector &specs,
-const lldb_private::SymbolContext &sc) {
+const lldb_private::SymbolContext &sc,
+bool &symbol_was_missing_weak) {
+  symbol_was_missing_weak = false;
   Target *target = sc.target_sp.get();
 
   if (!target) {
@@ -794,11 +796,20 @@
 const lldb_private::SymbolContext &sc) -> lldb::addr_t {
   load_address = LLDB_INVALID_ADDRESS;
 
-  for (size_t si = 0, se = sc_list.GetSize(); si < se; ++si) {
-SymbolContext candidate_sc;
-
-sc_list.GetContextAtIndex(si, candidate_sc);
+  if (sc_list.GetSize() == 0)
+return false;
 
+  // missing_weak_symbol will be true only if we found only weak undefined 
+  // references to this symbol.
+  bool symbol_was_missing_weak = true;  
+  for (auto candidate_sc : sc_list.SymbolContexts()) {
+// Only symbols can be weak undefined:
+if (!candidate_sc.symbol)
+  symbol_was_missing_weak = false;
+else if (candidate_sc.symbol->GetType() != lldb::eSymbolTypeUndefined
+  || !candidate_sc.symbol->IsWeak())
+  symbol_was_missing_weak = false;
+
 const bool is_external =
 (candidate_sc.function) ||
 (candidate_sc.symbol && candidate_sc.symbol->IsExternal());
@@ -835,6 +846,13 @@
 }
   }
 
+  // You test the address of a weak symbol against NULL to see if it is
+  // present.  So we should return 0 for a missing weak symbol.
+  if (symbol_was_missing_weak) {
+load_address = 0;
+return true;
+  }
+  
   return false;
 };
 
@@ -930,31 +948,37 @@
 }
 
 lldb::addr_t
-IRExecutionUnit::FindSymbol(lldb_private::ConstString name) {
+IRExecutionUnit::FindSymbol(lldb_private::ConstString name, bool &missing_weak) {
   std::vector candidate_C_names;
   std::vector candidate_CPlusPlus_names;
 
   CollectCandidateCNames(candidate_C_names, name);
+  
+  lldb::addr_t ret = FindInSymbols(candidate_C_names, m_sym_ctx, missing_weak);
+  if (ret != LLDB_INVALID_ADDRESS)
+return ret;
+  
+  // If we find the symbol in runtimes or user defined symbols it can't be 
+  // a missing weak symbol.
+  missing_weak = false;
+  ret = FindInRuntimes(candidate_C_names, m_sym_ctx);
+  if (ret != LLDB_INVALID_ADDRESS)
+return ret;
 
-  lldb::addr_t ret = FindInSymbols(candidate_C_names, m_sym_ctx);
-  if (ret == LLDB_INVALID_A

[Lldb-commits] [lldb] r364702 - Use const auto *

[Fangrui Song via lldb-commits]

Author: maskray
Date: Fri Jun 28 17:55:13 2019
New Revision: 364702

URL: http://llvm.org/viewvc/llvm-project?rev=364702&view=rev
Log:
Use const auto *

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Symbol/Block.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp?rev=364702&r1=364701&r2=364702&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp Fri Jun 
28 17:55:13 2019
@@ -51,7 +51,7 @@ bool DWARFDebugInfoEntry::Extract(const
 
   if (m_abbr_idx) {
 lldb::offset_t offset = *offset_ptr;
-auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
+const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
 if (abbrevDecl == nullptr) {
   cu->GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
   "{0x%8.8x}: invalid abbreviation code %u, please file a bug and "
@@ -232,7 +232,7 @@ bool DWARFDebugInfoEntry::GetDIENamesAnd
   std::vector dies;
   bool set_frame_base_loclist_addr = false;
 
-  auto abbrevDecl = GetAbbreviationDeclarationPtr(cu);
+  const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
 
   SymbolFileDWARF &dwarf = cu->GetSymbolFileDWARF();
   lldb::ModuleSP module = dwarf.GetObjectFile()->GetModule();
@@ -415,7 +415,7 @@ void DWARFDebugInfoEntry::Dump(const DWA
 if (abbrCode != m_abbr_idx) {
   s.Printf("error: DWARF has been modified\n");
 } else if (abbrCode) {
-  auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
+  const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl) {
 s.PutCString(DW_TAG_value_to_name(abbrevDecl->Tag()));
 s.Printf(" [%u] %c\n", abbrCode, abbrevDecl->HasChildren() ? '*' : ' 
');
@@ -546,7 +546,7 @@ void DWARFDebugInfoEntry::DumpAttribute(
 size_t DWARFDebugInfoEntry::GetAttributes(
 const DWARFUnit *cu, DWARFAttributes &attributes,
 uint32_t curr_depth) const {
-  auto abbrevDecl = GetAbbreviationDeclarationPtr(cu);
+  const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl) {
 const DWARFDataExtractor &data = cu->GetData();
 lldb::offset_t offset = GetFirstAttributeOffset();
@@ -606,7 +606,7 @@ dw_offset_t DWARFDebugInfoEntry::GetAttr
 const DWARFUnit *cu, const dw_attr_t attr, DWARFFormValue &form_value,
 dw_offset_t *end_attr_offset_ptr,
 bool check_specification_or_abstract_origin) const {
-  if (auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu)) {
+  if (const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu)) {
 uint32_t attr_idx = abbrevDecl->FindAttributeIndex(attr);
 
 if (attr_idx != DW_INVALID_INDEX) {

Modified: lldb/trunk/source/Symbol/Block.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Block.cpp?rev=364702&r1=364701&r2=364702&view=diff
==
--- lldb/trunk/source/Symbol/Block.cpp (original)
+++ lldb/trunk/source/Symbol/Block.cpp Fri Jun 28 17:55:13 2019
@@ -214,10 +214,10 @@ Block *Block::GetInlinedParent() {
 
 Block *Block::GetContainingInlinedBlockWithCallSite(
 const Declaration &find_call_site) {
-  auto inlined_block = GetContainingInlinedBlock();
+  Block *inlined_block = GetContainingInlinedBlock();
 
   while (inlined_block) {
-auto function_info = inlined_block->GetInlinedFunctionInfo();
+const auto *function_info = inlined_block->GetInlinedFunctionInfo();
 
 if (function_info &&
 function_info->GetCallSite().FileAndLineEqual(find_call_site))


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