[Lldb-commits] [lldb] r364274 - DWARF: Add support for type units+split dwarf combo

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jun 24 23:59:48 2019
New Revision: 364274

URL: http://llvm.org/viewvc/llvm-project?rev=364274&view=rev
Log:
DWARF: Add support for type units+split dwarf combo

Summary:
With the last round of refactors, supporting type units in dwo files
becomes almost trivial. This patch contains a couple of small fixes,
which taken as a whole make type units work in the split dwarf scenario
(both DWARF4 and DWARF5):
- DWARFContext: make sure we actually read the debug_types.dwo section
- DWARFUnit: set string offsets base on all units in the dwo file, not
  just the main CU
- ManualDWARFIndex: index all units in the file
- SymbolFileDWARFDwo: Search for the single compile unit in the file, as
  we can no longer assume it will be the first one

The last part makes it obvious that there is still some work to be done
here, namely that we do not support dwo files with multiple compile
units. That is something that should be easier after the DIERef
refactors, but it still requires more work.

Tests are added for the type units+split dwarf + dwarf4/5 scenarios, as
well as a test that checks we behave reasonably in the presence of dwo
files with multiple CUs.

Reviewers: clayborg, JDevlieghere, aprantl

Subscribers: arphaman, lldb-commits

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

Added:
lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-multiple-cu.ll
Modified:
lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test
lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Modified: lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test?rev=364274&r1=364273&r2=364274&view=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test Mon Jun 24 23:59:48 
2019
@@ -2,15 +2,27 @@
 
 # Make sure DWARF v4 type units work.
 # RUN: %clangxx -target x86_64-pc-linux %S/Inputs/debug-types-basic.cpp \
-# RUN:   -g -gdwarf-4 -fdebug-types-section -c -o %t.o
-# RUN: ld.lld %t.o -o %t
-# RUN: %lldb %t -s %s -o exit | FileCheck %s
+# RUN:   -g -gdwarf-4 -fdebug-types-section -c -o %t4.o
+# RUN: ld.lld %t4.o -o %t4
+# RUN: %lldb %t4 -s %s -o exit | FileCheck %s
 
 # Now do the same for DWARF v5.
 # RUN: %clangxx -target x86_64-pc-linux %S/Inputs/debug-types-basic.cpp \
-# RUN:   -g -gdwarf-5 -fdebug-types-section -c -o %t.o
-# RUN: ld.lld %t.o -o %t
-# RUN: %lldb %t -s %s -o exit | FileCheck %s
+# RUN:   -g -gdwarf-5 -fdebug-types-section -c -o %t5.o
+# RUN: ld.lld %t5.o -o %t5
+# RUN: %lldb %t5 -s %s -o exit | FileCheck %s
+
+# Test type units in dwo files.
+# RUN: %clangxx -target x86_64-pc-linux %S/Inputs/debug-types-basic.cpp \
+# RUN:   -g -gdwarf-4 -fdebug-types-section -gsplit-dwarf -c -o %t4dwo.o
+# RUN: ld.lld %t4dwo.o -o %t4dwo
+# RUN: %lldb %t4dwo -s %s -o exit | FileCheck %s
+
+# And type units+dwo+dwarf5.
+# RUN: %clangxx -target x86_64-pc-linux %S/Inputs/debug-types-basic.cpp \
+# RUN:   -g -gdwarf-5 -fdebug-types-section -c -o %t5dwo.o
+# RUN: ld.lld %t5dwo.o -o %t5dwo
+# RUN: %lldb %t5dwo -s %s -o exit | FileCheck %s
 
 type lookup A
 # CHECK-LABEL: type lookup A

Modified: lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test?rev=364274&r1=364273&r2=364274&view=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test Mon Jun 24 
23:59:48 2019
@@ -2,13 +2,23 @@
 
 # Make sure DWARF v4 type units work.
 # RUN: %clangxx %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -fdebug-types-section -o %t
-# RUN: %lldb %t -s %s -o exit | FileCheck %s
+# RUN:   -g -gdwarf-4 -fdebug-types-section -o %t4
+# RUN: %lldb %t4 -s %s -o exit | FileCheck %s
 
 # Now do the same for DWARF v5.
 # RUN: %clangxx %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -gdwarf-5 -fdebug-types-section -o %t
-# RUN: %lldb %t -s %s -o exit | FileCheck %s
+# RUN:   -g -gdwarf-5 -fdebug-types-section -o %t5
+# RUN: %lldb %t5 -s %s -o exit | FileCheck %s
+
+# Test type units in dwo files.
+# RUN: %clangxx %S/Inputs/debug-types-expressions.cpp \
+# RUN:   -g -gdwarf-4 -fdebug-types-section -o %t4dwo
+# RUN: %lldb %t4dwo -s %s -o exit | FileCheck %s
+
+# And type units+dwo+dwarf5.
+# RUN: %clangxx %S/Inp

[Lldb-commits] [PATCH] D63643: DWARF: Add support for type units+split dwarf combo

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364274: DWARF: Add support for type units+split dwarf combo 
(authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63643

Files:
  lldb/trunk/lit/SymbolFile/DWARF/debug-types-basic.test
  lldb/trunk/lit/SymbolFile/DWARF/debug-types-expressions.test
  lldb/trunk/lit/SymbolFile/DWARF/split-dwarf-multiple-cu.ll
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -19,7 +19,7 @@
 
   lldb::CompUnitSP ParseCompileUnit(DWARFCompileUnit &dwarf_cu) override;
 
-  DWARFUnit *GetCompileUnit();
+  DWARFCompileUnit *GetCompileUnit();
 
   DWARFUnit *
   GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) override;
@@ -69,8 +69,11 @@
 
   SymbolFileDWARF &GetBaseSymbolFile();
 
+  DWARFCompileUnit *ComputeCompileUnit();
+
   lldb::ObjectFileSP m_obj_file_sp;
   DWARFCompileUnit &m_base_dwarf_cu;
+  DWARFCompileUnit *m_cu = nullptr;
 };
 
 #endif // SymbolFileDWARFDwo_SymbolFileDWARFDwo_h_
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -363,7 +363,10 @@
   else if (gnu_ranges_base)
 dwo_cu->SetRangesBase(*gnu_ranges_base);
 
-  SetDwoStrOffsetsBase(dwo_cu);
+  for (size_t i = 0; i < m_dwo_symbol_file->DebugInfo()->GetNumUnits(); ++i) {
+DWARFUnit *unit = m_dwo_symbol_file->DebugInfo()->GetUnitAtIndex(i);
+SetDwoStrOffsetsBase(unit);
+  }
 }
 
 DWARFDIE DWARFUnit::LookupAddress(const dw_addr_t address) {
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -104,9 +104,10 @@
 
   IndexUnitImpl(unit, cu_language, set);
 
-  SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile();
-  if (dwo_symbol_file && dwo_symbol_file->GetCompileUnit()) {
-IndexUnitImpl(*dwo_symbol_file->GetCompileUnit(), cu_language, set);
+  if (SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile()) {
+DWARFDebugInfo &dwo_info = *dwo_symbol_file->DebugInfo();
+for (size_t i = 0; i < dwo_info.GetNumUnits(); ++i)
+  IndexUnitImpl(*dwo_info.GetUnitAtIndex(i), cu_language, set);
   }
 }
 
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
+#include "llvm/Support/Casting.h"
 
 #include "DWARFCompileUnit.h"
 #include "DWARFDebugInfo.h"
@@ -54,12 +55,34 @@
   return GetBaseSymbolFile().ParseCompileUnit(m_base_dwarf_cu);
 }
 
-DWARFUnit *SymbolFileDWARFDwo::GetCompileUnit() {
-  // Only dwo files with 1 compile unit is supported
-  if (GetNumCompileUnits() == 1)
-return DebugInfo()->GetUnitAtIndex(0);
-  else
+DWARFCompileUnit *SymbolFileDWARFDwo::GetCompileUnit() {
+  if (!m_cu)
+m_cu = ComputeCompileUnit();
+  return m_cu;
+}
+
+DWARFCompileUnit *SymbolFileDWARFDwo::ComputeCompileUnit() {
+  DWARFDebugInfo *debug_info = DebugInfo();
+  if (!debug_info)
 return nullptr;
+
+  // Right now we only support dwo files with one compile unit. If we don't have
+  // type units, we can just check for the unit count.
+  if (!debug_info->ContainsTypeUnits() && debug_info->GetNumUnits() == 1)
+return llvm::cast(debug_info->GetUnitAtIndex(0));
+
+  // Otherwise, we have to run through all units, and find the compile unit that
+  // way.
+  DWARFCompileUnit *cu = nullptr;
+  for (size_t i = 0; i < debug_info->GetNumUnits(); ++i) {
+if (auto *candidate =
+llvm::dyn_cast(debug_info->GetUnitAtIndex(i))) {
+  if (cu)
+return nullptr; // More that one CU found.
+  cu = candidate;
+}
+  }
+  return cu;
 }
 
 DWARFUnit *
Index: ll

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D63540#1550858 , @clayborg wrote:

> So I am fine with symbols having zero size being in the symbol table. I would 
> be fine not changing anything in the sorting and leaving some symbols with 
> zero size, we just need to fix:
>
>   Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr);
>
>
> To ignore zero sized symbols when it finds them _if_ there is another symbol 
> that has a size for that address. Wouldn't that fix the issue here?


You are assuming here that the symbols have size zero at the time we are 
performing the lookup. If I understand correctly what is going on, the problem 
here is that the code munging the symbol table (InitAddressIndexes), will set 
these symbols to have non-zero size. This is what this patch is trying to avoid.

The reason we are fiddling with the size of the symbols is because there are 
valid instances of symbols not having a size (usually coming from hand-written 
assembly, where one just doesn't bother to add the .size directive). However, 
it certainly seems like we shouldn't be doing that if there is another symbol 
at the same address, and this symbol has the size set correctly...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [lldb] r364276 - Remove core loading timeout

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun 25 00:14:29 2019
New Revision: 364276

URL: http://llvm.org/viewvc/llvm-project?rev=364276&view=rev
Log:
Remove core loading timeout

Summary:
If target.preload-symbols is false, waiting for the process to "stop"
can take an arbitrarily long amount of time, because it will cause all
the debug info to be parsed (to compute the stop message showing the
function, its arguments, etc).

We were previously waiting for 10 seconds for the stop even to arrive,
which is a pretty long time, but it's not that hard to overcome with
huge debug info.

Since any arbitrary limit can be theoretically overcome with huge
debug_info and/or slow machine, and the stop even was sent 3 lines above
the wait, if we ever do not receive the stop even means we've got a bug
in lldb. Therefore, I remove the timeout on this wait completely.

No test because I don't know how to reproduce this without a
multi-gigabyte symbol file.

Reviewers: jingham, clayborg

Subscribers: aprantl, lldb-commits

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

Modified:
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/source/Target/Process.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=364276&r1=364275&r2=364276&view=diff
==
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Tue Jun 25 00:14:29 2019
@@ -2687,7 +2687,7 @@ Status Process::LoadCore() {
 // Wait for a stopped event since we just posted one above...
 lldb::EventSP event_sp;
 StateType state =
-WaitForProcessToStop(seconds(10), &event_sp, true, listener_sp);
+WaitForProcessToStop(llvm::None, &event_sp, true, listener_sp);
 
 if (!StateIsStoppedState(state, false)) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));


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

[Lldb-commits] [PATCH] D63730: Remove core loading timeout

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364276: Remove core loading timeout (authored by labath, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63730

Files:
  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
@@ -2687,7 +2687,7 @@
 // Wait for a stopped event since we just posted one above...
 lldb::EventSP event_sp;
 StateType state =
-WaitForProcessToStop(seconds(10), &event_sp, true, listener_sp);
+WaitForProcessToStop(llvm::None, &event_sp, true, listener_sp);
 
 if (!StateIsStoppedState(state, false)) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));


Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -2687,7 +2687,7 @@
 // Wait for a stopped event since we just posted one above...
 lldb::EventSP event_sp;
 StateType state =
-WaitForProcessToStop(seconds(10), &event_sp, true, listener_sp);
+WaitForProcessToStop(llvm::None, &event_sp, true, listener_sp);
 
 if (!StateIsStoppedState(state, false)) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63745: [CMake] Check that a certificate for lldb is present at build time.

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

In D63745#1556943 , @xiaobai wrote:

> In D63745#1556773 , @JDevlieghere 
> wrote:
>
> > On second thought, let's check that LLDB_CODESIGN_IDENTITY equals 
> > `lldb_codesign` before doing this check.
>
>
> This question isn't important but I'm kind of curious: Does it have to be 
> called lldb_codesign? Could you have an arbitrary identity and then sign with 
> that, assuming the cert exists, or does debugserver expect a cert with that 
> name exactly?


The name doesn't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63745



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

[Lldb-commits] [PATCH] D63745: [CMake] Check that a certificate for lldb is present at build time.

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

> In D63745#1556941 , @labath wrote:
>  This code should go to tools/debugserver/source/CMakeLists.txt

+1

More precisely, this is the condition where you want to do your check:
https://github.com/llvm/llvm-project/blob/287f0403e310/lldb/tools/debugserver/source/CMakeLists.txt#L131

You don't need `if(CMAKE_SYSTEM_NAME MATCHES "Darwin" AND TARGET debugserver)` 
there.
Use `${LLDB_CODESIGN_IDENTITY_USED}` instead of `lldb_codesign`.
Error message could recommend `-DLLDB_USE_SYSTEM_DEBUGSERVER=ON` and/or 
`-DLLDB_NO_DEBUGSERVER=ON`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63745



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

[Lldb-commits] [lldb] r364317 - Options: Correctly check for missing arguments

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun 25 07:02:39 2019
New Revision: 364317

URL: http://llvm.org/viewvc/llvm-project?rev=364317&view=rev
Log:
Options: Correctly check for missing arguments

Relying on the value of optind for detecting missing arguments is
unreliable because its value after a failed parse is an implementation
detail. A more correct way to achieve this is to pass ':' at the
beginning of option string, which tells getopt to return ':' for missing
arguments.

For this to work, I also had to add a nullptr at the end of the argv
vector, as some getopt implementations did not work without that. This
is also an implementation detail, as getopt should normally be called
with argc+argc "as to main function" (i.e. null-terminated).

Thanks to Michał Górny for testing this patch out on NetBSD.

Modified:
lldb/trunk/source/Interpreter/Options.cpp

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=364317&r1=364316&r2=364317&view=diff
==
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Tue Jun 25 07:02:39 2019
@@ -1336,6 +1336,9 @@ llvm::Expected Options::Parse(cons
llvm::inconvertibleErrorCode());
   }
 
+  // Leading : tells getopt to return a : for a missing option argument AND to
+  // suppress error messages.
+  sstr << ":";
   for (int i = 0; long_options[i].definition != nullptr; ++i) {
 if (long_options[i].flag == nullptr) {
   if (isprint8(long_options[i].val)) {
@@ -1357,8 +1360,7 @@ llvm::Expected Options::Parse(cons
   std::vector argv = GetArgvForParsing(args);
   // If the last option requires an argument but doesn't have one,
   // some implementations of getopt_long will still try to read it.
-  char overflow = 0;
-  argv.push_back(&overflow);
+  argv.push_back(nullptr);
   std::unique_lock lock;
   OptionParser::Prepare(lock);
   int val;
@@ -1367,7 +1369,7 @@ llvm::Expected Options::Parse(cons
 val = OptionParser::Parse(argv.size() - 1, &*argv.begin(), 
sstr.GetString(),
   long_options, &long_options_index);
 
-if ((size_t)OptionParser::GetOptionIndex() > argv.size() - 1) {
+if (val == ':') {
   error.SetErrorStringWithFormat("last option requires an argument");
   break;
 }


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

[Lldb-commits] [PATCH] D63110: Fix a crash in option parsing.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

BTW, the new version was now failing on linux for a change.

I believe r364317 ought to implement this in a way that is compatible with all 
getopt implementations that we are using...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63110



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

[Lldb-commits] [PATCH] D63745: [CMake] Check that a certificate for lldb is present at build time.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D63745#1556941 , @labath wrote:

> This code should go to `tools/debugserver/source/CMakeLists.txt` so that it 
> is next to the code which performs the actual code signing. Doing that will 
> make it easier to keep it in sync with changes to code signing, as well as 
> make it obvious that it is not in sync with them right now (there's a pretty 
> complex interaction of various cmake options (LLDB_CODESIGN_IDENTITY, 
> LLVM_CODESIGNING_IDENTITY, LLDB_USE_SYSTEM_DEBUGSERVER, etc.) which affects 
> code signing, and this code is ignoring all of those)...


New patch coming, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63745



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

[Lldb-commits] [PATCH] D63770: Options: Reduce code duplication

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: mgorny, aprantl.

While investigating breakages caused by D63110 
, I noticed we were
building the short options strings in three places. Some of them used a
leading ':' to detect missing arguments, and some didn't. This was the
indirect cause of D63110 . Here, I move the 
common code into a utility
function.

Also, unify the code which appends the sentinel value at the end of the
option vector, and make it harder for users to pass invalid argc-argv
combos to getopt (another component of D63110 
) by having the
OptionParser::Parse function take a (Mutable)ArrayRef.

This unification has uncovered that we don't handle missing arguments
while building aliases, However, it's not possible to write an effective
test for this, as right now it is not possible to return an error out of
the alias parsing code (which means we are printing the generic
"failure" message even after this patch).


https://reviews.llvm.org/D63770

Files:
  include/lldb/Host/OptionParser.h
  source/Host/common/OptionParser.cpp
  source/Interpreter/CommandAlias.cpp
  source/Interpreter/Options.cpp

Index: source/Interpreter/Options.cpp
===
--- source/Interpreter/Options.cpp
+++ source/Interpreter/Options.cpp
@@ -930,6 +930,7 @@
   result.push_back(const_cast(""));
   for (const Args::ArgEntry &entry : args)
 result.push_back(const_cast(entry.c_str()));
+  result.push_back(nullptr);
   return result;
 }
 
@@ -972,19 +973,15 @@
   return size_t(-1);
 }
 
-llvm::Expected Options::ParseAlias(const Args &args,
- OptionArgVector *option_arg_vector,
- std::string &input_line) {
-  StreamString sstr;
-  int i;
-  Option *long_options = GetLongOptions();
+static std::string BuildShortOptions(const Option *long_options) {
+  std::string storage;
+  llvm::raw_string_ostream sstr(storage);
 
-  if (long_options == nullptr) {
-return llvm::make_error("Invalid long options",
-   llvm::inconvertibleErrorCode());
-  }
+  // Leading : tells getopt to return a : for a missing option argument AND to
+  // suppress error messages.
+  sstr << ":";
 
-  for (i = 0; long_options[i].definition != nullptr; ++i) {
+  for (size_t i = 0; long_options[i].definition != nullptr; ++i) {
 if (long_options[i].flag == nullptr) {
   sstr << (char)long_options[i].val;
   switch (long_options[i].definition->option_has_arg) {
@@ -1000,6 +997,20 @@
   }
 }
   }
+  return std::move(sstr.str());
+}
+
+llvm::Expected Options::ParseAlias(const Args &args,
+ OptionArgVector *option_arg_vector,
+ std::string &input_line) {
+  Option *long_options = GetLongOptions();
+
+  if (long_options == nullptr) {
+return llvm::make_error("Invalid long options",
+   llvm::inconvertibleErrorCode());
+  }
+
+  std::string short_options = BuildShortOptions(long_options);
 
   Args args_copy = args;
   std::vector argv = GetArgvForParsing(args);
@@ -1009,8 +1020,13 @@
   int val;
   while (true) {
 int long_options_index = -1;
-val = OptionParser::Parse(argv.size(), &*argv.begin(), sstr.GetString(),
-  long_options, &long_options_index);
+val = OptionParser::Parse(argv, short_options, long_options,
+  &long_options_index);
+
+if (val == ':') {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "last option requires an argument");
+}
 
 if (val == -1)
   break;
@@ -1116,33 +1132,13 @@
 OptionElementVector Options::ParseForCompletion(const Args &args,
 uint32_t cursor_index) {
   OptionElementVector option_element_vector;
-  StreamString sstr;
   Option *long_options = GetLongOptions();
   option_element_vector.clear();
 
   if (long_options == nullptr)
 return option_element_vector;
 
-  // Leading : tells getopt to return a : for a missing option argument AND to
-  // suppress error messages.
-
-  sstr << ":";
-  for (int i = 0; long_options[i].definition != nullptr; ++i) {
-if (long_options[i].flag == nullptr) {
-  sstr << (char)long_options[i].val;
-  switch (long_options[i].definition->option_has_arg) {
-  default:
-  case OptionParser::eNoArgument:
-break;
-  case OptionParser::eRequiredArgument:
-sstr << ":";
-break;
-  case OptionParser::eOptionalArgument:
-sstr << "::";
-break;
-  }
-}
-  }
+  std::string short_options = BuildShortOptions(long_options);
 
   std::unique_lock lock;
   OptionParser::Prepare(lock);
@@ -1153,10 +1149,6 @@
 
   std

[Lldb-commits] [PATCH] D61233: Refactor ObjectFile::GetSDKVersion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor marked an inline comment as done.
teemperor added inline comments.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp:211
   "SDKs/MacOSX%u.%u.sdk",
-  xcode_contents_path.c_str(), versions[0],
-  versions[1]);
+  xcode_contents_path.c_str(), versions->at(0),
+  versions->at(1));

aprantl wrote:
> For my own education, why is this preferred over `versions[0]`? Because of 
> Optional's `operator->`?
Yeah, just preferred that style to `(*versions)[0]` :)


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

https://reviews.llvm.org/D61233



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

[Lldb-commits] [PATCH] D61233: Refactor ObjectFile::GetSDKVersion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 206451.
teemperor added a comment.

- Rewrote patch to use llvm::VersionTuple (thanks Pavel)


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

https://reviews.llvm.org/D61233

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp

Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -163,8 +163,8 @@
   std::string xcode_contents_path;
   std::string default_xcode_sdk;
   FileSpec fspec;
-  uint32_t versions[2];
-  if (objfile->GetSDKVersion(versions, 2)) {
+  llvm::VersionTuple version = objfile->GetSDKVersion();
+  if (!version.empty()) {
 fspec = HostInfo::GetShlibDir();
 if (fspec) {
   std::string path;
@@ -208,8 +208,8 @@
   StreamString sdk_path;
   sdk_path.Printf("%sDeveloper/Platforms/MacOSX.platform/Developer/"
   "SDKs/MacOSX%u.%u.sdk",
-  xcode_contents_path.c_str(), versions[0],
-  versions[1]);
+  xcode_contents_path.c_str(), version.getMajor(),
+  version.getMinor().getValue());
   fspec.SetFile(sdk_path.GetString(), FileSpec::Style::native);
   if (FileSystem::Instance().Exists(fspec))
 return ConstString(sdk_path.GetString());
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -116,7 +116,7 @@
 
   llvm::VersionTuple GetMinimumOSVersion() override;
 
-  uint32_t GetSDKVersion(uint32_t *versions, uint32_t num_versions) override;
+  llvm::VersionTuple GetSDKVersion() override;
 
   bool GetIsDynamicLinkEditor() override;
 
@@ -198,7 +198,7 @@
   std::vector m_mach_segments;
   std::vector m_mach_sections;
   llvm::Optional m_min_os_version;
-  std::vector m_sdk_versions;
+  llvm::Optional m_sdk_versions;
   typedef lldb_private::RangeVector FileRangeArray;
   lldb_private::Address m_entry_point_address;
   FileRangeArray m_thread_context_offsets;
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5847,12 +5847,10 @@
   return *m_min_os_version;
 }
 
-uint32_t ObjectFileMachO::GetSDKVersion(uint32_t *versions,
-uint32_t num_versions) {
-  if (m_sdk_versions.empty()) {
+llvm::VersionTuple ObjectFileMachO::GetSDKVersion() {
+  if (!m_sdk_versions.hasValue()) {
 lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
-bool success = false;
-for (uint32_t i = 0; !success && i < m_header.ncmds; ++i) {
+for (uint32_t i = 0; i < m_header.ncmds; ++i) {
   const lldb::offset_t load_cmd_offset = offset;
 
   version_min_command lc;
@@ -5868,10 +5866,8 @@
   const uint32_t yy = (lc.sdk >> 8) & 0xffu;
   const uint32_t zz = lc.sdk & 0xffu;
   if () {
-m_sdk_versions.push_back();
-m_sdk_versions.push_back(yy);
-m_sdk_versions.push_back(zz);
-success = true;
+m_sdk_versions = llvm::VersionTuple(, yy, zz);
+break;
   } else {
 GetModule()->ReportWarning(
 "minimum OS version load command with invalid (0) version found.");
@@ -5881,9 +5877,9 @@
   offset = load_cmd_offset + lc.cmdsize;
 }
 
-if (!success) {
+if (!m_sdk_versions.hasValue()) {
   offset = MachHeaderSizeFromMagic(m_header.magic);
-  for (uint32_t i = 0; !success && i < m_header.ncmds; ++i) {
+  for (uint32_t i = 0; i < m_header.ncmds; ++i) {
 const lldb::offset_t load_cmd_offset = offset;
 
 version_min_command lc;
@@ -5910,41 +5906,19 @@
   const uint32_t yy = (minos >> 8) & 0xffu;
   const uint32_t zz = minos & 0xffu;
   if () {
-m_sdk_versions.push_back();
-m_sdk_versions.push_back(yy);
-m_sdk_versions.push_back(zz);
-success = true;
+m_sdk_versions = llvm::VersionTuple(, yy, zz);
+break;
   }
 }
 offset = load_cmd_offset + lc.cmdsize;
   }
 }
 
-if (!success) {
-  // Push an invalid value so we don't try to find
-  // the version # again on the next call to this
-  // method.
-  m_sdk_versions.p

[Lldb-commits] [PATCH] D63745: [CMake] Check that a certificate for lldb is present at build time.

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 206475.
davide added a comment.

Address feedback from many.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63745

Files:
  lldb/tools/debugserver/source/CMakeLists.txt


Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -142,6 +142,21 @@
   message(STATUS "lldb debugserver will not be available.")
 endif()
 
+# On MacOS, debugserver needs to be codesigned when built. Check if we have
+# a certificate instead of failing in the middle of the build.
+if(build_and_sign_debugserver)
+  execute_process(
+COMMAND security find-certificate -Z -p -c ${LLDB_CODESIGN_IDENTITY_USED} 
/Library/Keychains/System.keychain
+RESULT_VARIABLE cert_return
+OUTPUT_QUIET
+ERROR_QUIET)
+
+  if (cert_return)
+message(FATAL_ERROR "Certificate for debugserver not found. Run 
scripts/macos-setup-codesign.sh or "
+"use the system debugserver passing 
-DLLDB_USE_SYSTEM_DEBUGSERVER=ON to CMake")
+  endif()
+endif()
+
 if(APPLE)
   if(IOS)
 find_library(BACKBOARD_LIBRARY BackBoardServices


Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -142,6 +142,21 @@
   message(STATUS "lldb debugserver will not be available.")
 endif()
 
+# On MacOS, debugserver needs to be codesigned when built. Check if we have
+# a certificate instead of failing in the middle of the build.
+if(build_and_sign_debugserver)
+  execute_process(
+COMMAND security find-certificate -Z -p -c ${LLDB_CODESIGN_IDENTITY_USED} /Library/Keychains/System.keychain
+RESULT_VARIABLE cert_return
+OUTPUT_QUIET
+ERROR_QUIET)
+
+  if (cert_return)
+message(FATAL_ERROR "Certificate for debugserver not found. Run scripts/macos-setup-codesign.sh or "
+"use the system debugserver passing -DLLDB_USE_SYSTEM_DEBUGSERVER=ON to CMake")
+  endif()
+endif()
+
 if(APPLE)
   if(IOS)
 find_library(BACKBOARD_LIBRARY BackBoardServices
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63745: [CMake] Check that a certificate for lldb is present at build time.

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63745



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

[Lldb-commits] [lldb] r364334 - [CMake] Check that a certificate for lldb is present at build time.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Jun 25 10:13:24 2019
New Revision: 364334

URL: http://llvm.org/viewvc/llvm-project?rev=364334&view=rev
Log:
[CMake] Check that a certificate for lldb is present at build time.

Reviewers: JDevlieghere, sgraenitz, aprantl, friss

Subscribers: mgorny, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/tools/debugserver/source/CMakeLists.txt

Modified: lldb/trunk/tools/debugserver/source/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/CMakeLists.txt?rev=364334&r1=364333&r2=364334&view=diff
==
--- lldb/trunk/tools/debugserver/source/CMakeLists.txt (original)
+++ lldb/trunk/tools/debugserver/source/CMakeLists.txt Tue Jun 25 10:13:24 2019
@@ -142,6 +142,21 @@ else()
   message(STATUS "lldb debugserver will not be available.")
 endif()
 
+# On MacOS, debugserver needs to be codesigned when built. Check if we have
+# a certificate instead of failing in the middle of the build.
+if(build_and_sign_debugserver)
+  execute_process(
+COMMAND security find-certificate -Z -p -c ${LLDB_CODESIGN_IDENTITY_USED} 
/Library/Keychains/System.keychain
+RESULT_VARIABLE cert_return
+OUTPUT_QUIET
+ERROR_QUIET)
+
+  if (cert_return)
+message(FATAL_ERROR "Certificate for debugserver not found. Run 
scripts/macos-setup-codesign.sh or "
+"use the system debugserver passing 
-DLLDB_USE_SYSTEM_DEBUGSERVER=ON to CMake")
+  endif()
+endif()
+
 if(APPLE)
   if(IOS)
 find_library(BACKBOARD_LIBRARY BackBoardServices


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

[Lldb-commits] [PATCH] D63745: [CMake] Check that a certificate for lldb is present at build time.

[Davide Italiano via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL364334: [CMake] Check that a certificate for lldb is present 
at build time. (authored by davide, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63745?vs=206475&id=206479#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63745

Files:
  lldb/trunk/tools/debugserver/source/CMakeLists.txt


Index: lldb/trunk/tools/debugserver/source/CMakeLists.txt
===
--- lldb/trunk/tools/debugserver/source/CMakeLists.txt
+++ lldb/trunk/tools/debugserver/source/CMakeLists.txt
@@ -142,6 +142,21 @@
   message(STATUS "lldb debugserver will not be available.")
 endif()
 
+# On MacOS, debugserver needs to be codesigned when built. Check if we have
+# a certificate instead of failing in the middle of the build.
+if(build_and_sign_debugserver)
+  execute_process(
+COMMAND security find-certificate -Z -p -c ${LLDB_CODESIGN_IDENTITY_USED} 
/Library/Keychains/System.keychain
+RESULT_VARIABLE cert_return
+OUTPUT_QUIET
+ERROR_QUIET)
+
+  if (cert_return)
+message(FATAL_ERROR "Certificate for debugserver not found. Run 
scripts/macos-setup-codesign.sh or "
+"use the system debugserver passing 
-DLLDB_USE_SYSTEM_DEBUGSERVER=ON to CMake")
+  endif()
+endif()
+
 if(APPLE)
   if(IOS)
 find_library(BACKBOARD_LIBRARY BackBoardServices


Index: lldb/trunk/tools/debugserver/source/CMakeLists.txt
===
--- lldb/trunk/tools/debugserver/source/CMakeLists.txt
+++ lldb/trunk/tools/debugserver/source/CMakeLists.txt
@@ -142,6 +142,21 @@
   message(STATUS "lldb debugserver will not be available.")
 endif()
 
+# On MacOS, debugserver needs to be codesigned when built. Check if we have
+# a certificate instead of failing in the middle of the build.
+if(build_and_sign_debugserver)
+  execute_process(
+COMMAND security find-certificate -Z -p -c ${LLDB_CODESIGN_IDENTITY_USED} /Library/Keychains/System.keychain
+RESULT_VARIABLE cert_return
+OUTPUT_QUIET
+ERROR_QUIET)
+
+  if (cert_return)
+message(FATAL_ERROR "Certificate for debugserver not found. Run scripts/macos-setup-codesign.sh or "
+"use the system debugserver passing -DLLDB_USE_SYSTEM_DEBUGSERVER=ON to CMake")
+  endif()
+endif()
+
 if(APPLE)
   if(IOS)
 find_library(BACKBOARD_LIBRARY BackBoardServices
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r364335 - [Python] Flush prompt before reading input

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jun 25 10:27:38 2019
New Revision: 364335

URL: http://llvm.org/viewvc/llvm-project?rev=364335&view=rev
Log:
[Python] Flush prompt before reading input

Make sure the prompt has been flushed before reading commands. Buffering
is different in Python 3, which led to the prompt not being displayed in
the Xcode console.

Modified:
lldb/trunk/source/Interpreter/embedded_interpreter.py

Modified: lldb/trunk/source/Interpreter/embedded_interpreter.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/embedded_interpreter.py?rev=364335&r1=364334&r2=364335&view=diff
==
--- lldb/trunk/source/Interpreter/embedded_interpreter.py (original)
+++ lldb/trunk/source/Interpreter/embedded_interpreter.py Tue Jun 25 10:27:38 
2019
@@ -72,6 +72,7 @@ def get_terminal_size(fd):
 
 def readfunc_stdio(prompt):
 sys.stdout.write(prompt)
+sys.stdout.flush()
 return sys.stdin.readline().rstrip()
 
 


___
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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Core/ValueObject.cpp:1699
   if (process) {
-LanguageRuntime *runtime =
-process->GetLanguageRuntime(GetObjectRuntimeLanguage());
-if (!runtime)
-  runtime = ObjCLanguageRuntime::Get(*process);
-if (runtime)
-  return runtime->IsRuntimeSupportValue(*this);
-// If there is no language runtime, trust the compiler to mark all
-// runtime support variables as artificial.
-return GetVariable() && GetVariable()->IsArtificial();
+bool marked_as_runtime_support_val = GetVariable() && 
GetVariable()->IsArtificial();
+

```
if (!process)
  return false;
if (!marked_as_runtime_support_val)
  return false;
```



Comment at: source/Core/ValueObject.cpp:1706
+for (auto *runtime : process->GetLanguageRuntimes()) {
+  if (marked_as_runtime_support_val && 
runtime->IsWhitelistedRuntimeValue(GetName()))
+return false;

then this condition becomes
```
if (runtime->IsWhitelistedRuntimeValue(GetName()))
  return false;
```



Comment at: source/Core/ValueObject.cpp:1713
+// runtime support values as artificial.
+return marked_as_runtime_support_val;
   }

`return true;`


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] D62931: [lldb-server] Add setting to force 'g' packet use

[Guilherme Andrade via Phabricator via lldb-commits]

guiandrade added a comment.

Just checking the status of this change. Are we still considering enabling 'g' 
packets unconditionally, or are we better off using the setting for safety?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D62931



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

[Lldb-commits] [PATCH] D63110: Fix a crash in option parsing.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63110



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

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

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 206505.
JosephTremoulet added a comment.
Herald added subscribers: jsji, atanasyan, jrtc27, kbarton, nemanjai.

- Fix typos
- Convey 'S' eh_frame augmentation to UnwindPlan


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63667

Files:
  lldb/include/lldb/Symbol/UnwindPlan.h
  
lldb/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/TestHandleAbort.py
  lldb/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/main.c
  lldb/source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
  lldb/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
  lldb/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
  lldb/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/SysV-hexagon/ABISysV_hexagon.cpp
  lldb/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
  lldb/source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  lldb/source/Symbol/ArmUnwindInfo.cpp
  lldb/source/Symbol/CompactUnwindInfo.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp

Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -19,6 +19,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -601,6 +602,9 @@
 }
 offset += aug_data_len;
   }
+  unwind_plan.SetUnwindPlanForSignalTrap(
+strchr(cie->augmentation, 'S') ? eLazyBoolYes : eLazyBoolNo);
+
   Address lsda_data;
   Address personality_function_ptr;
 
Index: lldb/source/Symbol/CompactUnwindInfo.cpp
===
--- lldb/source/Symbol/CompactUnwindInfo.cpp
+++ lldb/source/Symbol/CompactUnwindInfo.cpp
@@ -737,6 +737,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
@@ -1008,6 +1009,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
@@ -1304,6 +1306,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
@@ -1437,6 +1440,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
Index: lldb/source/Symbol/ArmUnwindInfo.cpp
===
--- lldb/source/Symbol/ArmUnwindInfo.cpp
+++ lldb/source/Symbol/ArmUnwindInfo.cpp
@@ -344,6 +344,7 @@
   unwind_plan.SetSourceName("ARM.exidx unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindDWARF);
 
   return true;
Index: lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
===
--- lldb/source/Plugins/UnwindAssem

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

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 206506.
xiaobai added a comment.

Address feedback


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

https://reviews.llvm.org/D63240

Files:
  include/lldb/Target/CPPLanguageRuntime.h
  include/lldb/Target/LanguageRuntime.h
  include/lldb/Target/ObjCLanguageRuntime.h
  source/Core/ValueObject.cpp
  source/Target/CPPLanguageRuntime.cpp
  source/Target/ObjCLanguageRuntime.cpp

Index: source/Target/ObjCLanguageRuntime.cpp
===
--- source/Target/ObjCLanguageRuntime.cpp
+++ source/Target/ObjCLanguageRuntime.cpp
@@ -46,19 +46,6 @@
   return name == g_self || name == g_cmd;
 }
 
-bool ObjCLanguageRuntime::IsRuntimeSupportValue(ValueObject &valobj) {
-  // All runtime support values have to be marked as artificial by the
-  // compiler. But not all artificial variables should be hidden from
-  // the user.
-  if (!valobj.GetVariable())
-return false;
-  if (!valobj.GetVariable()->IsArtificial())
-return false;
-
-  // Whitelist "self" and "_cmd".
-  return !IsWhitelistedRuntimeValue(valobj.GetName());
-}
-
 bool ObjCLanguageRuntime::AddClass(ObjCISA isa,
const ClassDescriptorSP &descriptor_sp,
const char *class_name) {
Index: source/Target/CPPLanguageRuntime.cpp
===
--- source/Target/CPPLanguageRuntime.cpp
+++ source/Target/CPPLanguageRuntime.cpp
@@ -43,20 +43,8 @@
 CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
 : LanguageRuntime(process) {}
 
-bool CPPLanguageRuntime::IsRuntimeSupportValue(ValueObject &valobj) {
-  // All runtime support values have to be marked as artificial by the
-  // compiler. But not all artificial variables should be hidden from
-  // the user.
-  if (!valobj.GetVariable())
-return false;
-  if (!valobj.GetVariable()->IsArtificial())
-return false;
-
-  // Whitelist "this" and since there is no ObjC++ runtime, any ObjC names.
-  ConstString name = valobj.GetName();
-  if (name == g_this)
-return false;
-  return !ObjCLanguageRuntime::IsWhitelistedRuntimeValue(name);
+bool CPPLanguageRuntime::IsWhitelistedRuntimeValue(ConstString name) {
+  return name == g_this;
 }
 
 bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
Index: source/Core/ValueObject.cpp
===
--- source/Core/ValueObject.cpp
+++ source/Core/ValueObject.cpp
@@ -1695,18 +1695,20 @@
 
 bool ValueObject::IsRuntimeSupportValue() {
   Process *process(GetProcessSP().get());
-  if (process) {
-LanguageRuntime *runtime =
-process->GetLanguageRuntime(GetObjectRuntimeLanguage());
-if (!runtime)
-  runtime = ObjCLanguageRuntime::Get(*process);
-if (runtime)
-  return runtime->IsRuntimeSupportValue(*this);
-// If there is no language runtime, trust the compiler to mark all
-// runtime support variables as artificial.
-return GetVariable() && GetVariable()->IsArtificial();
+  if (!process)
+return false;
+
+  // We trust the the compiler did the right thing and marked runtime support
+  // values as artificial.
+  if (!GetVariable() || !GetVariable()->IsArtificial())
+return false;
+
+  for (auto *runtime : process->GetLanguageRuntimes()) {
+if (runtime->IsWhitelistedRuntimeValue(GetName()))
+  return false;
   }
-  return false;
+
+  return true;
 }
 
 bool ValueObject::IsNilReference() {
Index: include/lldb/Target/ObjCLanguageRuntime.h
===
--- include/lldb/Target/ObjCLanguageRuntime.h
+++ include/lldb/Target/ObjCLanguageRuntime.h
@@ -301,8 +301,7 @@
 
   /// Check whether the name is "self" or "_cmd" and should show up in
   /// "frame variable".
-  static bool IsWhitelistedRuntimeValue(ConstString name);
-  bool IsRuntimeSupportValue(ValueObject &valobj) override;
+  bool IsWhitelistedRuntimeValue(ConstString name) override;
 
 protected:
   // Classes that inherit from ObjCLanguageRuntime can see and modify these
Index: include/lldb/Target/LanguageRuntime.h
===
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -152,9 +152,9 @@
   virtual lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
   bool stop_others) = 0;
 
-  /// Identify whether a value is a language implementation detaul
-  /// that should be hidden from the user interface by default.
-  virtual bool IsRuntimeSupportValue(ValueObject &valobj) { return false; }
+  /// Identify whether a name is a runtime value that should not be hidden by
+  /// from the user interface.
+  virtual bool IsWhitelistedRuntimeValue(ConstString name) { return false; }
 
   virtual void ModulesDidLoad(const ModuleList &module_list) {}
 
Index: include/lldb/Ta

[Lldb-commits] [lldb] r364344 - Add a defensive check for nullptr as in the block above.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Jun 25 12:50:12 2019
New Revision: 364344

URL: http://llvm.org/viewvc/llvm-project?rev=364344&view=rev
Log:
Add a defensive check for nullptr as in the block above.

Unfortunately I had to work backwards from a crash log,
so I don't have a good testcase at this point in time.

rdar://problem/51874647

Modified:
lldb/trunk/source/Target/CPPLanguageRuntime.cpp

Modified: lldb/trunk/source/Target/CPPLanguageRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/CPPLanguageRuntime.cpp?rev=364344&r1=364343&r2=364344&view=diff
==
--- lldb/trunk/source/Target/CPPLanguageRuntime.cpp (original)
+++ lldb/trunk/source/Target/CPPLanguageRuntime.cpp Tue Jun 25 12:50:12 2019
@@ -280,7 +280,7 @@ CPPLanguageRuntime::FindLibCppStdFunctio
   }
 
   // Case 4 or 5
-  if (!symbol->GetName().GetStringRef().startswith("vtable for")) {
+  if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for")) {
 optional_info.callable_case =
 LibCppStdFunctionCallableCase::FreeOrMemberFunction;
 optional_info.callable_address = function_address_resolved;


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

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

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet added a comment.

I've updated this with code to recognize the 'S' in the eh_frame augmentation 
and record it in a new bool member of UnwindPlan, `m_plan_is_for_signal_trap`.  
I haven't hooked up any consumers of the new bit; as you say, with the current 
code flow we don't parse the eh_frame info until after we've decided whether 
the current frame is a trap handler frame or not.

I took a look at `behaves_like_zeroth_frame`.  In my case, we're not getting to 
the point of consulting it, because before checking it, if we are processing a 
trap handler frame or if AlwaysRelyOnEHInfo returns true (both of which are 
true in my case), we use the EH Frame unwind plan if it is valid at the current 
address, and that valid at current address check is already passing.  I'm 
somewhat reluctant to fiddle with the behaves_like_zeroth_frame bit 
speculatively.  Note that with this change, it will get set for the next frame 
above __kernel_rt_sigreturn (because the current test checks if the next-lower 
frame is an eTrapHandler frame), which I think is where we want the async 
processing.

I see that we also do this "subtract 1 from the pc" thing in 
StackFrame::GetSymbolContext, and I believe this is why even with my change 
here I'm seeing the wrong function name in backtraces (e.g., in the new test, 
it shows "abort_caller -> [function that happened to precede 
__kernel_rt_sigreturn] -> handler" rather than "abort_caller -> 
kernel_rt_sigreturn -> handler").  I'm not sure how best to address that, since 
the base StackFrame and RegisterContext classes don't have this notion of 
"frame type" / "trap handler frame" that the "-LLDB" subclasses have -- seems 
like we'd need to have GetFrameInfoAtIndex have another out parameter to 
specify frame type, and UnwindLLDB's implementation could then pull it from the 
RegisterContextLLDB -- would that be a good change?  I don't actually need that 
for my current use case, the "wrong" name in the backtrace is fine...

Thanks for the notes on the asynchronous cases.  I'd certainly be interested in 
getting those to work, though I haven't sorted out from where the unwinder can 
find the above-trap frame's register values (happy for pointers here!).  I 
think I'm running into what this comment from GetFullUnwindPlanForFrame 
describes:

  // If we're in _sigtramp(), unwinding past this frame requires special
  // knowledge.  On Mac OS X this knowledge is properly encoded in the eh_frame
  // section, so prefer that if available. On other platforms we may need to
  // provide a platform-specific UnwindPlan which encodes the details of how to
  // unwind out of sigtramp.

I'd like to get the synchronous case support working as a first step regardless.


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] D63667: Support __kernel_rt_sigreturn in frame initialization

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 206516.
JosephTremoulet added a comment.

- fix copy pasta


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63667

Files:
  lldb/include/lldb/Symbol/UnwindPlan.h
  
lldb/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/TestHandleAbort.py
  lldb/packages/Python/lldbsuite/test/functionalities/signal/handle-abrt/main.c
  lldb/source/Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.cpp
  lldb/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp
  lldb/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp
  lldb/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/SysV-hexagon/ABISysV_hexagon.cpp
  lldb/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
  lldb/source/Plugins/ABI/SysV-mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/SysV-mips64/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp
  lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  lldb/source/Symbol/ArmUnwindInfo.cpp
  lldb/source/Symbol/CompactUnwindInfo.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp

Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -19,6 +19,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -601,6 +602,9 @@
 }
 offset += aug_data_len;
   }
+  unwind_plan.SetUnwindPlanForSignalTrap(
+strchr(cie->augmentation, 'S') ? eLazyBoolYes : eLazyBoolNo);
+
   Address lsda_data;
   Address personality_function_ptr;
 
Index: lldb/source/Symbol/CompactUnwindInfo.cpp
===
--- lldb/source/Symbol/CompactUnwindInfo.cpp
+++ lldb/source/Symbol/CompactUnwindInfo.cpp
@@ -737,6 +737,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
@@ -1008,6 +1009,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
@@ -1304,6 +1306,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
@@ -1437,6 +1440,7 @@
   unwind_plan.SetSourceName("compact unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindEHFrame);
 
   unwind_plan.SetLSDAAddress(function_info.lsda_address);
Index: lldb/source/Symbol/ArmUnwindInfo.cpp
===
--- lldb/source/Symbol/ArmUnwindInfo.cpp
+++ lldb/source/Symbol/ArmUnwindInfo.cpp
@@ -344,6 +344,7 @@
   unwind_plan.SetSourceName("ARM.exidx unwind info");
   unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
   unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
   unwind_plan.SetRegisterKind(eRegisterKindDWARF);
 
   return true;
Index: lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
===
--- lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
+++ lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp

[Lldb-commits] [PATCH] D63790: [dotest] Add the ability to set environment variables for the inferior.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: davide, jingham.
Herald added a project: LLDB.

This patch adds a dotest flag for setting environment variables for the 
inferior. This is different from the current `--env` flag, which sets variables 
in the debugger's environment.  This allows us to set things like 
`LD_LIBRARY_PATH` for testing.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D63790

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/packages/Python/lldbsuite/test/lldbtest_config.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest_config.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest_config.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest_config.py
@@ -20,3 +20,6 @@
 
 # path to the lldb command line executable tool
 lldbExec = None
+
+# Environment variables for the inferior
+inferior_env = None
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -835,6 +835,7 @@
 self.darwinWithFramework = False
 self.makeBuildDir()
 
+
 def setAsync(self, value):
 """ Sets async mode to True/False and ensures it is reset after the 
testcase completes."""
 old_async = self.dbg.GetAsync()
@@ -1856,6 +1857,9 @@
 # decorators.
 Base.setUp(self)
 
+if lldbtest_config.inferior_env:
+self.runCmd('env {}'.format(lldbtest_config.inferior_env))
+
 # Set the clang modules cache path used by LLDB.
 mod_cache = os.path.join(os.environ["LLDB_BUILD"], "module-cache-lldb")
 self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -210,6 +210,12 @@
 metavar='variable',
 action='append',
 help='Specify an environment variable to set to the given value before 
running the test cases e.g.: --env CXXFLAGS=-O3 --env DYLD_INSERT_LIBRARIES')
+group.add_argument(
+'--inferior-env',
+dest='set_inferior_env_vars',
+metavar='variable',
+action='append',
+help='Specify an environment variable to set to the given value for 
the inferior.')
 X('-v', 'Do verbose mode of unittest framework (print out each test case 
invocation)')
 group.add_argument(
 '--enable-crash-dialog',
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -270,6 +270,9 @@
 else:
 os.environ[parts[0]] = parts[1]
 
+if args.set_inferior_env_vars:
+lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
+
 # only print the args if being verbose (and parsable is off)
 if args.v and not args.q:
 print(sys.argv)
@@ -636,7 +639,7 @@
 def get_llvm_bin_dirs():
 """
 Returns an array of paths that may have the llvm/clang/etc binaries
-in them, relative to this current file.  
+in them, relative to this current file.
 Returns an empty array if none are found.
 """
 result = []


Index: lldb/packages/Python/lldbsuite/test/lldbtest_config.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest_config.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest_config.py
@@ -20,3 +20,6 @@
 
 # path to the lldb command line executable tool
 lldbExec = None
+
+# Environment variables for the inferior
+inferior_env = None
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -835,6 +835,7 @@
 self.darwinWithFramework = False
 self.makeBuildDir()
 
+
 def setAsync(self, value):
 """ Sets async mode to True/False and ensures it is reset after the testcase completes."""
 old_async = self.dbg.GetAsync()
@@ -1856,6 +1857,9 @@
 # decorators.
 Base.setUp(self)
 
+if lldbtest_config.inferior_env:
+self.runCmd('env {}'.format(lldbtest_config.inferior_env))
+
 # Set the clang modules cache path used by LLDB.
 mod_cache = os.path.join(os.environ["LLDB_BUILD"], "module-cache-lldb")
 self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
Index: lldb/packages/Python/lldbsuite/te

[Lldb-commits] [PATCH] D63791: [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, labath.

Fix the watchpoint/breakpoint code to search for matching thread entry
in m_threads explicitly rather than assuming that it will be present
at specified index.  The previous code segfault since it wrongly assumed
that the index will match LWP ID which was incorrect even for a single
thread (where index was 0 and LWP ID was 1).

While fixing that off-by-one error would help for this specific task,
I believe it is better to be explicit in what we are searching for.


https://reviews.llvm.org/D63791

Files:
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp


Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -238,12 +238,25 @@
 SetState(StateType::eStateStopped, true);
   } break;
   case TRAP_DBREG: {
+// Find the thread.
+NativeThreadNetBSD* thread = nullptr;
+for (const auto &t : m_threads) {
+  if (t->GetID() == info.psi_lwpid) {
+thread = static_cast(t.get());
+break;
+  }
+}
+if (!thread) {
+  LLDB_LOG(log,
+   "thread not found in m_threads, pid = {0}, LWP = {1}",
+   GetID(), info.psi_lwpid);
+  break;
+}
+
 // If a watchpoint was hit, report it
-uint32_t wp_index;
-Status error = static_cast(*m_threads[info.psi_lwpid])
-   .GetRegisterContext()
-   .GetWatchpointHitIndex(
-   wp_index, (uintptr_t)info.psi_siginfo.si_addr);
+uint32_t wp_index = LLDB_INVALID_INDEX32;
+Status error = thread->GetRegisterContext().GetWatchpointHitIndex(
+wp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for watchpoint hits, pid = "
@@ -258,11 +271,9 @@
 }
 
 // If a breakpoint was hit, report it
-uint32_t bp_index;
-error = static_cast(*m_threads[info.psi_lwpid])
-.GetRegisterContext()
-.GetHardwareBreakHitIndex(bp_index,
-   
(uintptr_t)info.psi_siginfo.si_addr);
+uint32_t bp_index = LLDB_INVALID_INDEX32;
+error = thread->GetRegisterContext().GetHardwareBreakHitIndex(
+bp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for hardware "


Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -238,12 +238,25 @@
 SetState(StateType::eStateStopped, true);
   } break;
   case TRAP_DBREG: {
+// Find the thread.
+NativeThreadNetBSD* thread = nullptr;
+for (const auto &t : m_threads) {
+  if (t->GetID() == info.psi_lwpid) {
+thread = static_cast(t.get());
+break;
+  }
+}
+if (!thread) {
+  LLDB_LOG(log,
+   "thread not found in m_threads, pid = {0}, LWP = {1}",
+   GetID(), info.psi_lwpid);
+  break;
+}
+
 // If a watchpoint was hit, report it
-uint32_t wp_index;
-Status error = static_cast(*m_threads[info.psi_lwpid])
-   .GetRegisterContext()
-   .GetWatchpointHitIndex(
-   wp_index, (uintptr_t)info.psi_siginfo.si_addr);
+uint32_t wp_index = LLDB_INVALID_INDEX32;
+Status error = thread->GetRegisterContext().GetWatchpointHitIndex(
+wp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for watchpoint hits, pid = "
@@ -258,11 +271,9 @@
 }
 
 // If a breakpoint was hit, report it
-uint32_t bp_index;
-error = static_cast(*m_threads[info.psi_lwpid])
-.GetRegisterContext()
-.GetHardwareBreakHitIndex(bp_index,
-   (uintptr_t)info.psi_siginfo.si_addr);
+uint32_t bp_index = LLDB_INVALID_INDEX32;
+error = thread->GetRegisterContext().GetHardwareBreakHitIndex(
+bp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for hardware "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, labath, joerg.

Set global enable bits (i.e. bits 1, 3, 5, 7) to enable watchpoints
on NetBSD rather than the local enable bits (0, 2, 4, 6).  The former
are necessary for watchpoints to be correctly recognized by the NetBSD
kernel.  The latter cause them to be reported as trace points.


https://reviews.llvm.org/D63792

Files:
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp


Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -691,7 +691,7 @@
 
   uint64_t control_bits = reg_value.GetAsUInt64();
 
-  is_vacant = !(control_bits & (1 << (2 * wp_index)));
+  is_vacant = !(control_bits & (2 << (2 * wp_index)));
 
   return error;
 }
@@ -730,7 +730,7 @@
 return error;
 
   // for watchpoints 0, 1, 2, or 3, respectively, set bits 1, 3, 5, or 7
-  uint64_t enable_bit = 1 << (2 * wp_index);
+  uint64_t enable_bit = 2 << (2 * wp_index);
 
   // set bits 16-17, 20-21, 24-25, or 28-29
   // with 0b01 for write, and 0b11 for read/write


Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -691,7 +691,7 @@
 
   uint64_t control_bits = reg_value.GetAsUInt64();
 
-  is_vacant = !(control_bits & (1 << (2 * wp_index)));
+  is_vacant = !(control_bits & (2 << (2 * wp_index)));
 
   return error;
 }
@@ -730,7 +730,7 @@
 return error;
 
   // for watchpoints 0, 1, 2, or 3, respectively, set bits 1, 3, 5, or 7
-  uint64_t enable_bit = 1 << (2 * wp_index);
+  uint64_t enable_bit = 2 << (2 * wp_index);
 
   // set bits 16-17, 20-21, 24-25, or 28-29
   // with 0b01 for write, and 0b11 for read/write
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r364351 - [dotest] Remove unused function

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Jun 25 14:19:44 2019
New Revision: 364351

URL: http://llvm.org/viewvc/llvm-project?rev=364351&view=rev
Log:
[dotest] Remove unused function

The function `EnvArray` has no used.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=364351&r1=364350&r2=364351&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Tue Jun 25 14:19:44 
2019
@@ -203,14 +203,6 @@ def SETTING_MSG(setting):
 return "Value of setting '%s' is correct" % setting
 
 
-def EnvArray():
-"""Returns an env variable array from the os.environ map object."""
-return list(map(lambda k,
-v: k + "=" + v,
-list(os.environ.keys()),
-list(os.environ.values(
-
-
 def line_number(filename, string_to_match):
 """Helper function to return the line number of the first matched 
string."""
 with io.open(filename, mode='r', encoding="utf-8") as f:
@@ -1877,7 +1869,7 @@ class TestBase(Base):
 # Make sure that a sanitizer LLDB's environment doesn't get passed on.
 if 'DYLD_LIBRARY_PATH' in os.environ:
 self.runCmd('settings set target.env-vars DYLD_LIBRARY_PATH=')
-
+
 if "LLDB_MAX_LAUNCH_COUNT" in os.environ:
 self.maxLaunchCount = int(os.environ["LLDB_MAX_LAUNCH_COUNT"])
 


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

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

@aadsm Is there any update on the regression fix? This patch also breaks the 
C++ module test suite, so i would prefer if we could revert it if the fix takes 
longer.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

I can't repro it right now so I prefer to revert it (I'm curious how this new 
function could influence those tests though). I actually thought this was 
sorted, sorry about that.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D63790: [dotest] Add the ability to set environment variables for the inferior.

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1861
+if lldbtest_config.inferior_env:
+self.runCmd('env {}'.format(lldbtest_config.inferior_env))
+

`env` or `settings set target.env-vars`?  On Windows, where spaces in paths are 
common, this works:

(lldb) settings set target.env-vars "MY_PATH=C:\Program Files\stuff"

but this doesn't:

(lldb) env "MY_PATH=C:\Program Files\stuff"




Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63790



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

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

Reverted here: 
https://reviews.llvm.org/rG9c10b620c0619611dfe062216459431955ac4801


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D63790: [dotest] Add the ability to set environment variables for the inferior.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 206541.

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

https://reviews.llvm.org/D63790

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/packages/Python/lldbsuite/test/lldbtest_config.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest_config.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest_config.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest_config.py
@@ -20,3 +20,6 @@
 
 # path to the lldb command line executable tool
 lldbExec = None
+
+# Environment variables for the inferior
+inferior_env = None
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -835,6 +835,7 @@
 self.darwinWithFramework = False
 self.makeBuildDir()
 
+
 def setAsync(self, value):
 """ Sets async mode to True/False and ensures it is reset after the 
testcase completes."""
 old_async = self.dbg.GetAsync()
@@ -1856,6 +1857,9 @@
 # decorators.
 Base.setUp(self)
 
+if lldbtest_config.inferior_env:
+self.runCmd('settings set target.env-vars 
{}'.format(lldbtest_config.inferior_env))
+
 # Set the clang modules cache path used by LLDB.
 mod_cache = os.path.join(os.environ["LLDB_BUILD"], "module-cache-lldb")
 self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -210,6 +210,12 @@
 metavar='variable',
 action='append',
 help='Specify an environment variable to set to the given value before 
running the test cases e.g.: --env CXXFLAGS=-O3 --env DYLD_INSERT_LIBRARIES')
+group.add_argument(
+'--inferior-env',
+dest='set_inferior_env_vars',
+metavar='variable',
+action='append',
+help='Specify an environment variable to set to the given value for 
the inferior.')
 X('-v', 'Do verbose mode of unittest framework (print out each test case 
invocation)')
 group.add_argument(
 '--enable-crash-dialog',
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -270,6 +270,9 @@
 else:
 os.environ[parts[0]] = parts[1]
 
+if args.set_inferior_env_vars:
+lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
+
 # only print the args if being verbose (and parsable is off)
 if args.v and not args.q:
 print(sys.argv)
@@ -636,7 +639,7 @@
 def get_llvm_bin_dirs():
 """
 Returns an array of paths that may have the llvm/clang/etc binaries
-in them, relative to this current file.  
+in them, relative to this current file.
 Returns an empty array if none are found.
 """
 result = []


Index: lldb/packages/Python/lldbsuite/test/lldbtest_config.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest_config.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest_config.py
@@ -20,3 +20,6 @@
 
 # path to the lldb command line executable tool
 lldbExec = None
+
+# Environment variables for the inferior
+inferior_env = None
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -835,6 +835,7 @@
 self.darwinWithFramework = False
 self.makeBuildDir()
 
+
 def setAsync(self, value):
 """ Sets async mode to True/False and ensures it is reset after the testcase completes."""
 old_async = self.dbg.GetAsync()
@@ -1856,6 +1857,9 @@
 # decorators.
 Base.setUp(self)
 
+if lldbtest_config.inferior_env:
+self.runCmd('settings set target.env-vars {}'.format(lldbtest_config.inferior_env))
+
 # Set the clang modules cache path used by LLDB.
 mod_cache = os.path.join(os.environ["LLDB_BUILD"], "module-cache-lldb")
 self.runCmd('settings set symbols.clang-modules-cache-path "%s"'
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -210,6 +210,12 @@

[Lldb-commits] [PATCH] D63790: [dotest] Add the ability to set environment variables for the inferior.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1861
+if lldbtest_config.inferior_env:
+self.runCmd('env {}'.format(lldbtest_config.inferior_env))
+

amccarth wrote:
> `env` or `settings set target.env-vars`?  On Windows, where spaces in paths 
> are common, this works:
> 
> (lldb) settings set target.env-vars "MY_PATH=C:\Program Files\stuff"
> 
> but this doesn't:
> 
> (lldb) env "MY_PATH=C:\Program Files\stuff"
> 
> 
I've updated the diff, but this sounds more like a bug in the alias?


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

https://reviews.llvm.org/D63790



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

[Lldb-commits] [PATCH] D63790: [dotest] Add the ability to set environment variables for the inferior.

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

The LGTM, but I wasn't nominated as a reviewer, and I was mostly looking at it 
from the point of Windows compatibility.


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

https://reviews.llvm.org/D63790



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

[Lldb-commits] [lldb] r364361 - Fix a typo in help text.

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Tue Jun 25 16:13:16 2019
New Revision: 364361

URL: http://llvm.org/viewvc/llvm-project?rev=364361&view=rev
Log:
Fix a typo in help text.

Modified:
lldb/trunk/source/Interpreter/CommandInterpreter.cpp

Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=364361&r1=364360&r2=364361&view=diff
==
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Tue Jun 25 16:13:16 
2019
@@ -821,7 +821,7 @@ void CommandInterpreter::LoadCommandDict
   *this, "_regexp-env",
   "Shorthand for viewing and setting environment variables.",
   "\n"
-  "_regexp-env  // Show enrivonment\n"
+  "_regexp-env  // Show environment\n"
   "_regexp-env =   // Set an environment variable",
   2, 0, false));
   if (env_regex_cmd_up) {


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

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D63540#1556989 , @labath wrote:

> In D63540#1550858 , @clayborg wrote:
>
> > So I am fine with symbols having zero size being in the symbol table. I 
> > would be fine not changing anything in the sorting and leaving some symbols 
> > with zero size, we just need to fix:
> >
> >   Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr);
> >
> >
> > To ignore zero sized symbols when it finds them _if_ there is another 
> > symbol that has a size for that address. Wouldn't that fix the issue here?
>
>
> You are assuming here that the symbols have size zero at the time we are 
> performing the lookup. If I understand correctly what is going on, the 
> problem here is that the code munging the symbol table (InitAddressIndexes), 
> will set these symbols to have non-zero size. This is what this patch is 
> trying to avoid.


I am saying to leave symbols with zero size as is _if_ there is another symbol 
that does have a valid size with the same start address.

> The reason we are fiddling with the size of the symbols is because there are 
> valid instances of symbols not having a size (usually coming from 
> hand-written assembly, where one just doesn't bother to add the .size 
> directive). However, it certainly seems like we shouldn't be doing that if 
> there is another symbol at the same address, and this symbol has the size set 
> correctly...

Exactly. The best solution in my mind is:

- leave all symbols with sizes as is
- only set a symbol size if there is no other symbol at that address that 
didn't originally have a size
- maybe leave zero sizes symbols out of the lookup map if they have no sizes 
after doing the two steps above




Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63540



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

[Lldb-commits] [PATCH] D63802: Handle nested register definition xml files from the remote serial protocol stub

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: clayborg.
Herald added a project: LLDB.

I was looking at adding support for lldb connecting to qemu's x86_64 RSP stub.  
The target definition file we get back from it has a few wrinkles that 
complicate this, I'm thinking of handling it something like this.

First, it uses nested includes.  We ask for target.xml and we get back

i386:x86-64

then we need to ask for i386-64bit.xml which gives us



  
  



and requesting those two are going to get us the GPRs and FPRs/vector regs.

The first problem is that ProcessGDBRemote::GetGDBServerRegisterInfo does have 
support for one level of includes, but it assumes that the target xml file will 
consist of only registers.

I created a new ProcessGDBRemote::GetGDBServerRegisterInfo method which calls a 
method that we can call recursively to parse the XML target definitions.  The 
one problem with THAT is that we fetch the xml file, parse it, then we find the 
root node -- normally  -- and iterate over the child elements that we 
normally deal with (architecture, osabi, xi:include/include, feature, groups) 
and for any  nodes, pass the children to ParseRegisters() which looks 
for  elements.

Now this method ("GetGDBServerRegisterInfoXMLAndProcess", whatever) needs to be 
prepared to get either the top-level  node OR a  node in a 
child include.  Which is still a bit fragile because, for instance, the 
 element is in the top level here but it's possible for the top 
level xml to just include another xml where we see an  tag, or 
whatever?  e.g.



  



{actual-content.xml}

 i386:x86-64


  
  



Maybe that's possible?

I have a test file here showing exactly what definitions were being sent.  Also 
of note is that most of the registers don't get a register grouping by our 
logic.  The floating point control registers get a group="float", and the mxcsr 
gets a group="vector", but none of the others do.  It's also interesting to see 
how the flag bits are defined plus the different vector presentations for the 
xmm registers.  We're ignoring all of that for now, obviously.

I'll look this patch over more tomorrow, but wanted to give a heads up about 
what I'm looking at on this one.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D63802

Files:
  
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNestedRegDefinitions.py
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -383,6 +383,11 @@
 
   DynamicLoader *GetDynamicLoader() override;
 
+  bool GetGDBServerRegisterInfoXMLAndProcess(ArchSpec &arch_to_use,
+ std::string xml_filename, 
+ uint32_t &cur_reg_num,
+ uint32_t ®_offset);
+
   // Query remote GDBServer for register information
   bool GetGDBServerRegisterInfo(ArchSpec &arch);
 
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4490,8 +4490,15 @@
 // Only update the register set name if we didn't get a "reg_set"
 // attribute. "set_name" will be empty if we didn't have a "reg_set"
 // attribute.
-if (!set_name && !gdb_group.empty())
-  set_name.SetCString(gdb_group.c_str());
+if (!set_name) {
+  if (!gdb_group.empty()) {
+set_name.SetCString(gdb_group.c_str());
+  } else {
+// If no register group name provided anywhere,
+// we'll create a 'general' register set
+set_name.SetCString("general");
+  }
+}
 
 reg_info.byte_offset = reg_offset;
 assert(reg_info.byte_size != 0);
@@ -4516,38 +4523,27 @@
 
 } // namespace
 
-// query the target of gdb-remote for extended target information return:
-// 'true'  on success
-//  'false' on failure
-bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
-  // Make sure LLDB has an XML parser it can use first
-  if (!XMLDocument::XMLEnabled())
-return false;
-
-  // redirect libxml2's error handler since the default prints to stdout
-
-  GDBRemoteCommunicationClient &comm = m_gdb_comm;
-
-  // check that we have extended feature read support
-  if (!comm.GetQXferFeaturesReadSupported())
-return false;
-
+bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess(ArchSpec &arch_to_use, 
+ std::string xml_filename,
+  

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

It seems to break looking up symbols from shared libraries. A simple reproducer 
is debugging a simple "Hello World" program in C on Linux (Arch Linux in my 
case, but it seems to also affect other distributions)

  #include 
  
  int main() {
printf("Hello World\n");
  }

and then trying to eval printf:

  (lldb) target create "printftest"
  Current executable set to 'printftest' (x86_64).
  (lldb) b main
  Breakpoint 1: where = printftest`main + 8 at main.cpp:4:3, address = 
0x1108
  (lldb) r
  Process 56813 launched: '/home/teemperor/llvm/test/printftest' (x86_64)
  Process 56813 stopped
  * thread #1, name = 'printftest', stop reason = breakpoint 1.1
  frame #0: 0x5108 printftest`main at main.cpp:4:3
 1#include 
 2   
 3int main() {
  -> 4  printf("Hello World\n");
 5}
  (lldb) p printf("a")
  error: Couldn't lookup symbols:
printf


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

I was debugging it yesterday but I have no real result yet. There is/was a 
problem that initially it is run with:

  intern-state ProcessGDBRemote::GetLoadedModuleList
  intern-state parsing: 
  intern-state found (link_map:0x77ffd990, base:0x77fd2000[offset], 
ld:0x77ffcdf0, name:'/lib64/ld-linux-x86-64.so.2')
  intern-state found (link_map:0x77ffe6f0, base:0x77fd1000[offset], 
ld:0x77fd1348, name:'linux-vdso.so.1')
  intern-state found 2 modules in total  

but later the module list is not refreshed with more libraries, 
`DynamicLoaderPOSIXDYLD::RendezvousBreakpointHit` just does not happen.
PASS  vs. FAIL 

But there may be also some other patch involved as one regression was fixed by 
rL363770  but that was not everything.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62503



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