[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D75929#1924373 , @ikudrin wrote:

> @dblaikie, @labath, as far as I can understand, the patch complies with your 
> vision. The main difference is that the enum is still intended for internal 
> use only, but it probably should not go to the public part before the 
> proposed values are accepted. Anyway, even while the proposal of the combined 
> index is not approved, I believe that the patch is useful per se because it 
> allows reading standard index sections and can later be easily extended for 
> combined indexes. The patch does not restrict that movement. Please, correct 
> me if I misunderstand anything.


Sorry about the delay. It took a while to get this to the top of my stack.

Yes, this "complies with my/our vission", but looking at the `UnknownColumnIds` 
field, I am starting to have second thoughts about that vision. :( Being able 
to represent "unknown" columns and at the same time mapping all columns to a 
internal representation makes things a bit awkward.

The reason this wasn't a problem for location lists is that you cannot "skip 
over" an unknown DW_LLE entry -- it terminates the parse right away.

However, that is not the case for debug_?u_index, where you can easily ignore 
unknown columns. In that light, I am wondering if it wouldn't be better after 
all to stick to the on-disk column numbers internally (but maybe still keep the 
"unified" v5 enum in the public interface). @dblaikie, what do you make of that?

(btw, is there a test case for the "unknown column" code path?)


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

https://reviews.llvm.org/D75929



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


[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

2020-03-18 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.

Side note: This has become complicated enough it might be worth separating 
these patches now rather than later - changes to dumping should be separate 
from changes to llvm-dwp, for instance.

In D75929#1926834 , @labath wrote:

> In D75929#1924373 , @ikudrin wrote:
>
> > @dblaikie, @labath, as far as I can understand, the patch complies with 
> > your vision. The main difference is that the enum is still intended for 
> > internal use only, but it probably should not go to the public part before 
> > the proposed values are accepted. Anyway, even while the proposal of the 
> > combined index is not approved, I believe that the patch is useful per se 
> > because it allows reading standard index sections and can later be easily 
> > extended for combined indexes. The patch does not restrict that movement. 
> > Please, correct me if I misunderstand anything.
>
>
> Sorry about the delay. It took a while to get this to the top of my stack.
>
> Yes, this "complies with my/our vission", but looking at the 
> `UnknownColumnIds` field, I am starting to have second thoughts about that 
> vision. :( Being able to represent "unknown" columns and at the same time 
> mapping all columns to a internal representation makes things a bit awkward.
>
> The reason this wasn't a problem for location lists is that you cannot "skip 
> over" an unknown DW_LLE entry -- it terminates the parse right away.
>
> However, that is not the case for debug_?u_index, where you can easily ignore 
> unknown columns. In that light, I am wondering if it wouldn't be better after 
> all to stick to the on-disk column numbers internally (but maybe still keep 
> the "unified" v5 enum in the public interface). @dblaikie, what do you make 
> of that?
>
> (btw, is there a test case for the "unknown column" code path?)


I'm undecided - yeah, it is awkward having an extra data structure to store the 
original parsed values & then remapping them back, etc. Alternatively we could 
use a single 64 bit value - and store unknown values shifted up into the top 32 
bits (really I sort of wish they'd just used only 1 byte for these values - 
seems unreasonable that we'd need 256 sections... clearly we don't need them 
now)? & shift them back if the values too high & report it as unknown. Sort of 
doing something /like/ if the DWARF spec actually had an range reserved for 
implementation extensions, which will hopefully be added in the future.

But I wouldn't be completely opposed to the data structure keeping things in 
its parsed form & printing out based on the version of the index, etc. While 
having an API for querying based on the v5-with-extensions identifiers, though 
that seems a bit awkward.




Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:47-48
+// For pre-standard ones, which correspond to sections being deprecated in
+// DWARFv5, the values are chosen more or less arbitrary and a tag "_EXT_"
+// is added to the names.
+//

Probably not arbitrarily - in the sense that this is an extension that 
consumers/producers will need to agree to - so maybe saying that ("non-standard 
extension"/"proposed for standardization" or something to that effect) and/or 
linking to the dwarf-discuss thread to support why these values were chosen & 
they can't be changed arbitrarily.



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp:96-100
   Version = IndexData.getU32(OffsetPtr);
+  if (Version != 2) {
+*OffsetPtr = BeginOffset;
+Version = IndexData.getU16(OffsetPtr);
+if (Version != 5)

What endianness is this encoded with? If it's little endian, then a 2 byte 
field with 2 bytes of zero padding should be the same as reading a 4 bytes, or 
the other way around, right? So perhaps we could just always read it as 2 bytes 
then 2 bytes of padding rather than having to the version/reset/reread dance 
here?



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp:131
 
+  // Fix InfoColumnKind: in DWARFv5, type units also lay in .debug_info.dwo.
+  if (Header.Version == 5)

jhenderson wrote:
> also lay -> are
Should we be fixing it up here - or should we avoid setting it incorrectly in 
the first place?


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

https://reviews.llvm.org/D75929



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


[Lldb-commits] [PATCH] D76351: [lldb-vscode] Don't use SBLaunchInfo in request_attach

2020-03-18 Thread Anton Kolesov via Phabricator via lldb-commits
anton.kolesov created this revision.
anton.kolesov added reviewers: labath, clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

If LLDB attaches to an already running target, then structure SBAttachInfo is
used instead of SBLaunchInfo. lldb-vscode function request_attach sets some
values to g_vsc.launch_info, however this field is then not passed anywhere, so
this action has no effect. This commit removes invocation of
SBLaunchInfo::SetDetachOnError, which has no equivalent in SBAttachInfo.

File package.json doesn't describe detachOnError property for "attach" request
type, therefore it is not needed to update it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76351

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -543,9 +543,6 @@
 return;
   }
 
-  const bool detatchOnError = GetBoolean(arguments, "detachOnError", false);
-  g_vsc.launch_info.SetDetachOnError(detatchOnError);
-
   // Run any pre run LLDB commands the user specified in the launch.json
   g_vsc.RunPreRunCommands();
 


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -543,9 +543,6 @@
 return;
   }
 
-  const bool detatchOnError = GetBoolean(arguments, "detachOnError", false);
-  g_vsc.launch_info.SetDetachOnError(detatchOnError);
-
   // Run any pre run LLDB commands the user specified in the launch.json
   g_vsc.RunPreRunCommands();
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

2020-03-18 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

In D73206#1871895 , @labath wrote:

> Yep. It may be interesting to try this out on regular dwo first. Right now, 
> dwo dodges this problem by storing a CompileUnit pointer on both skeleton and 
> split units. If we can make this work without the split unit back pointer, 
> then we can come move closer to how things work in llvm, and also pave the 
> way for the dwz stuff.


Just clarifying if you would therefore like to remove these two items:

  SymbolFileDWARF &SymbolFileDWARFDwo::GetBaseSymbolFile() { return 
m_base_symbol_file; }
  SymbolFileDWARF &SymbolFileDWARFDwo::m_base_symbol_file;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73206



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


[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

looks good to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76188



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


[Lldb-commits] [lldb] db31e2e - [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

2020-03-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2020-03-18T14:15:58+01:00
New Revision: db31e2e1e6cacb31f3fe5a9b0e9e52cd626b5ff2

URL: 
https://github.com/llvm/llvm-project/commit/db31e2e1e6cacb31f3fe5a9b0e9e52cd626b5ff2
DIFF: 
https://github.com/llvm/llvm-project/commit/db31e2e1e6cacb31f3fe5a9b0e9e52cd626b5ff2.diff

LOG: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

This patch changes the way the StackFrame Recognizers match a certain
frame.

Until now, recognizers could be registered with a function
name but also an alternate symbol.
This change is motivated by a test failure for the Assert frame
recognizer on Linux. Depending the version of the libc, the abort
function (triggered by an assertion), could have more than two
signatures (i.e. `raise`, `__GI_raise` and `gsignal`).

Instead of only checking the default symbol name and the alternate one,
lldb will iterate over a list of symbols to match against.

rdar://60386577

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/packages/Python/lldbsuite/test/lldbutil.py
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/Options.td

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Target/AssertFrameRecognizer.cpp
lldb/source/Target/StackFrameRecognizer.cpp
lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
lldb/test/API/commands/frame/recognizer/main.m
lldb/unittests/Target/StackFrameRecognizerTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/StackFrameRecognizer.h 
b/lldb/include/lldb/Target/StackFrameRecognizer.h
index 3e4d6abe7aeb..6be506391deb 100644
--- a/lldb/include/lldb/Target/StackFrameRecognizer.h
+++ b/lldb/include/lldb/Target/StackFrameRecognizer.h
@@ -101,8 +101,8 @@ class ScriptedStackFrameRecognizer : public 
StackFrameRecognizer {
 class StackFrameRecognizerManager {
 public:
   static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
-ConstString module, ConstString symbol,
-ConstString alternate_symbol,
+ConstString module,
+llvm::ArrayRef symbols,
 bool first_instruction_only = true);
 
   static void AddRecognizer(lldb::StackFrameRecognizerSP recognizer,
@@ -110,11 +110,11 @@ class StackFrameRecognizerManager {
 lldb::RegularExpressionSP symbol,
 bool first_instruction_only = true);
 
-  static void ForEach(
-  std::function const
-  &callback);
+  static void
+  ForEach(std::function symbols,
+ bool regexp)> const &callback);
 
   static bool RemoveRecognizerWithID(uint32_t recognizer_id);
 

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index d603091677f3..ef0df90c416e 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -797,7 +797,7 @@ def run_to_breakpoint_do_run(test, target, bkpt, 
launch_info = None,
 test.assertEqual(num_threads, 1, "Expected 1 thread to stop at 
breakpoint, %d did."%(num_threads))
 else:
 test.assertGreater(num_threads, 0, "No threads stopped at breakpoint")
-
+
 thread = threads[0]
 return (target, process, thread, bkpt)
 

diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp 
b/lldb/source/Commands/CommandObjectFrame.cpp
index baf64e5c884d..4fef9f4847e0 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -746,7 +746,7 @@ class CommandObjectFrameRecognizerAdd : public 
CommandObjectParsed {
 m_module = std::string(option_arg);
 break;
   case 'n':
-m_function = std::string(option_arg);
+m_symbols.push_back(std::string(option_arg));
 break;
   case 'x':
 m_regex = true;
@@ -760,7 +760,7 @@ class CommandObjectFrameRecognizerAdd : public 
CommandObjectParsed {
 
 void OptionParsingStarting(ExecutionContext *execution_context) override {
   m_module = "";
-  m_function = "";
+  m_symbols.clear();
   m_class_name = "";
   m_regex = false;
 }
@@ -772,7 +772,7 @@ class CommandObjectFrameRecognizerAdd : public 
CommandObjectParsed {
 // Instance variables to hold the values for command options.
 std::string m_class_name;
 std::string m_module;
-std::string m_function;
+std::vector m_symbols;
 bool m_regex;
   };
 
@@ -854,9 +854,18 @@ bool CommandObjectFrameRecognizerAdd::DoExecute(Args 
&command,
 return false;
   }
 
-  if (m_options.m_function.empty()) {
-result.AppendErrorWithFormat("%s needs a function name

[Lldb-commits] [PATCH] D76316: [lldb] [testsuite] Enable forgotten -gsplit-dwarf for 2 testfiles

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Thanks for catching that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76316



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


[Lldb-commits] [PATCH] D75626: Add support for owning module information to TypeSystemClang.

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:146
DWARFDIE *decl_ctx_die);
+  lldb_private::ModuleID GetOwningModuleID(const DWARFDIE &die);
 

I am kinda lost at this point, so I'm not sure which patch introduces this, 
but... this name does not seem very ideal, as it sounds like it is related to 
`lldb_private::Module`, while it does not really have that much in common with 
it.

And then there's `ClangModulesDeclVendor::ModuleID` -- I have no idea what's 
the relationship of this to that..


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

https://reviews.llvm.org/D75626



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


[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

2020-03-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdb31e2e1e6ca: [lldb/Target] Support more than 2 symbols in 
StackFrameRecognizer (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76188

Files:
  lldb/include/lldb/Target/StackFrameRecognizer.h
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/Options.td
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
  lldb/test/API/commands/frame/recognizer/main.m
  lldb/unittests/Target/StackFrameRecognizerTest.cpp

Index: lldb/unittests/Target/StackFrameRecognizerTest.cpp
===
--- lldb/unittests/Target/StackFrameRecognizerTest.cpp
+++ lldb/unittests/Target/StackFrameRecognizerTest.cpp
@@ -76,8 +76,7 @@
   bool any_printed = false;
   StackFrameRecognizerManager::ForEach(
   [&any_printed](uint32_t recognizer_id, std::string name,
- std::string function, std::string symbol,
- std::string alternate_symbol,
+ std::string function, llvm::ArrayRef symbols,
  bool regexp) { any_printed = true; });
 
   EXPECT_TRUE(any_printed);
Index: lldb/test/API/commands/frame/recognizer/main.m
===
--- lldb/test/API/commands/frame/recognizer/main.m
+++ lldb/test/API/commands/frame/recognizer/main.m
@@ -5,10 +5,7 @@
 printf("%d %d\n", a, b);
 }
 
-void bar(int *ptr)
-{
-	printf("%d\n", *ptr);
-}
+void bar(int *ptr) { printf("%d\n", *ptr); }
 
 int main (int argc, const char * argv[])
 {
Index: lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
===
--- lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -18,9 +18,7 @@
 @skipUnlessDarwin
 def test_frame_recognizer_1(self):
 self.build()
-
-target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
-self.assertTrue(target, VALID_TARGET)
+exe = self.getBuildArtifact("a.out")
 
 # Clear internal & plugins recognizers that get initialized at launch
 self.runCmd("frame recognizer clear")
@@ -33,21 +31,21 @@
 self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo")
 
 self.expect("frame recognizer list",
-substrs=['0: recognizer.MyFrameRecognizer, module a.out, function foo'])
+substrs=['0: recognizer.MyFrameRecognizer, module a.out, symbol foo'])
 
 self.runCmd("frame recognizer add -l recognizer.MyOtherFrameRecognizer -s a.out -n bar -x")
 
 self.expect(
 "frame recognizer list",
 substrs=[
-'1: recognizer.MyOtherFrameRecognizer, module a.out, function bar (regexp)',
-'0: recognizer.MyFrameRecognizer, module a.out, function foo'
+'1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)',
+'0: recognizer.MyFrameRecognizer, module a.out, symbol foo'
 ])
 
 self.runCmd("frame recognizer delete 0")
 
 self.expect("frame recognizer list",
-substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, function bar (regexp)'])
+substrs=['1: recognizer.MyOtherFrameRecognizer, module a.out, symbol bar (regexp)'])
 
 self.runCmd("frame recognizer clear")
 
@@ -56,19 +54,10 @@
 
 self.runCmd("frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo")
 
-lldbutil.run_break_set_by_symbol(self, "foo")
-self.runCmd("r")
-
-self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-substrs=['stopped', 'stop reason = breakpoint'])
-
-process = target.GetProcess()
-thread = process.GetSelectedThread()
+target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "foo",
+ exe_name = exe)
 frame = thread.GetSelectedFrame()
 
-self.assertEqual(frame.GetSymbol().GetName(), "foo")
-self.assertFalse(frame.GetLineEntry().IsValid())
-
 self.expect("frame variable",
 substrs=['(int) a = 42', '(int) b = 56'])
 
@@ -110,8 +99,9 @@
 
 # FIXME: The following doesn't work yet, but should be fixed.
 """
-lldbutil.run_break_set_by_symbol(self, "bar")
-self.runCmd("c")
+target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "bar",

[Lldb-commits] [PATCH] D76351: [lldb-vscode] Don't use SBLaunchInfo in request_attach

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

sounds good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76351



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


[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D73206#1928710 , @jankratochvil 
wrote:

> In D73206#1871895 , @labath wrote:
>
> > Yep. It may be interesting to try this out on regular dwo first. Right now, 
> > dwo dodges this problem by storing a CompileUnit pointer on both skeleton 
> > and split units. If we can make this work without the split unit back 
> > pointer, then we can come move closer to how things work in llvm, and also 
> > pave the way for the dwz stuff.
>
>
> Just clarifying if you would therefore like to remove these two items:
>
>   SymbolFileDWARF &SymbolFileDWARFDwo::GetBaseSymbolFile() { return 
> m_base_symbol_file; }
>   SymbolFileDWARF &SymbolFileDWARFDwo::m_base_symbol_file;
>   


Eventually I'd like to delete the entire `SymbolFileDWARFDwo` class. Right now, 
it serves almost no useful purpose -- nearly all of its methods just redirect 
to the "base" symbol file in interesting (and confusing) ways. But I'm not sure 
that removing `GetBaseSymbolFile` is the right way to start that, as without it 
you cannot implement the redirection.

The way I was considering doing this was first ensuring that all other objects 
which store a SymbolFileDWARF pointer are updated to point to the "base" 
object. But there are probably even other things that need to be done before 
that, such as moving the lower-level dwarf stuff from SymbolFileDWARF to 
DWARFContext. Then the "base" DWARFContext could be directly responsible for 
creating "dwo" DWARFContexts (just like it is in llvm) and SymbolFileDWARFDwo 
could be removed. I am not sure how much work all of that is though...

Where are you going with that question? If it's stopping you from doing 
anything, I can try to look into that a bit closer...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73206



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


[Lldb-commits] [lldb] 3481062 - [lldb] [testsuite] Enable forgotten -gsplit-dwarf for 2 testfiles

2020-03-18 Thread Jan Kratochvil via lldb-commits

Author: Jan Kratochvil
Date: 2020-03-18T15:49:24+01:00
New Revision: 3481062bc688e21eac175aae0142adcaae361c1c

URL: 
https://github.com/llvm/llvm-project/commit/3481062bc688e21eac175aae0142adcaae361c1c
DIFF: 
https://github.com/llvm/llvm-project/commit/3481062bc688e21eac175aae0142adcaae361c1c.diff

LOG: [lldb] [testsuite] Enable forgotten -gsplit-dwarf for 2 testfiles

D63643 added these testfiles but some of the %t4dwo and %t5dwo builds
are the same as corresponding %t4 and %t5 builds. Fortunately the
testcases do PASS.

After just adding -gsplit-dwarf these both skeleton files:
  tools/lldb/test/SymbolFile/DWARF/Output/debug-types-expressions.test.tmp4dwo
  tools/lldb/test/SymbolFile/DWARF/Output/debug-types-expressions.test.tmp5dwo

were referencing to this one non-skeleton file:
  tools/lldb/test/SymbolFile/DWARF/debug-types-expressions.dwo

Surprisingly it does not affect the other test debug-types-basic.test
probably because it compiles to .o and then links it. While
debug-types-expressions.test compiles directly to an executable.

So fixed that while keeping the direct executable compilation.

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

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test 
b/lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
index 24a6a651d5f9..947b43ba6b2f 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
+++ b/lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
@@ -20,7 +20,7 @@
 
 # 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:   -g -gdwarf-5 -fdebug-types-section -gsplit-dwarf -c -o %t5dwo.o
 # RUN: ld.lld %t5dwo.o -o %t5dwo
 # RUN: %lldb %t5dwo -s %s -o exit | FileCheck %s
 

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test 
b/lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
index 5964eea40adf..0442cd8faa3b 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
+++ b/lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
@@ -12,12 +12,16 @@
 
 # Test type units in dwo files.
 # RUN: %clangxx_host %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -gdwarf-4 -fdebug-types-section -o %t4dwo
+# RUN:   -g -gdwarf-4 -fdebug-types-section -gsplit-dwarf -o %t4dwo \
+# RUN:   -Xclang -split-dwarf-output -Xclang %t4dwo.dwo \
+# RUN:   -Xclang -split-dwarf-file   -Xclang %t4dwo.dwo
 # RUN: %lldb %t4dwo -s %s -o exit | FileCheck %s
 
 # And type units+dwo+dwarf5.
 # RUN: %clangxx_host %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -gdwarf-5 -fdebug-types-section -o %t5dwo
+# RUN:   -g -gdwarf-5 -fdebug-types-section -gsplit-dwarf -o %t5dwo \
+# RUN:   -Xclang -split-dwarf-output -Xclang %t5dwo.dwo \
+# RUN:   -Xclang -split-dwarf-file   -Xclang %t5dwo.dwo
 # RUN: %lldb %t5dwo -s %s -o exit | FileCheck %s
 
 breakpoint set -n foo



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


[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks. Could you also add the other kind of test (the one inline asm) I 
mentioned. In an ideal world we'd have a test case for every boundary 
condition, but we're pretty far from that right now. Even so, one test case 
like that would be nice.

I am imagining the inferior doing something like:

  movabsq $0xdead, %rax
  pushq %rax ; fake pc
  leaq 0x1000(%rsp), %rbp ; larger than kMaxFrameSize
  pushq %rbp
  movq %rsp, %rbp
  pushq $1 ; fake frame contents
  pushq $2
  pushq $3
  
  incq %rax
  push %rax; second fake pc
  pushq %rbp
  movq %rsp, %rbp
  pushq $4 ; fake frame contents
  pushq $5
  pushq $6
  int3

and then the test would assert that the result contains the entirety of the 
first fake frame, the bp+pc of the second fake frame, and then would stop due 
to hitting the kMaxFrameSize boundary.




Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:373
+# Read memory chunks from jThreadsInfo.
+memory_chunks = self.gather_threads_info_memory()
+# Check the chunks are correct.

Also assert that you have at least one chunk here?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:501-504
+static void AddMemoryChunk(json::Array &stack_memory_chunks, addr_t address,
+   std::vector &bytes) {
+  if (bytes.empty())
+return;

I think it would be cleaner if this returned the json::Object as a return value 
(much like `GetStackMemoryAsJSON` returns a json::Array).



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:534
+  } else {
+return stack_memory_chunks;
+  }

You will need to "handle" the error inside the Expected object here. The object 
asserts (at runtime) that you do that at the right time. 
(http://llvm.org/docs/ProgrammersManual.html#recoverable-errors).

That said, if you're not going to do anything with the error, then maybe the 
GetRegisterValue doesn't really need to return an Expected (maybe 
Optional)



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:548
+   static_cast(fp_value - sp_value));
+  std::vector buf(byte_count, 0);
+

we usually use arrays of *u*int8_t to represent uninterpreted data.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:578
+lldb_private::DataExtractor extractor(fp_ra_buf.data(), fp_and_ra_size,
+  endian::InlHostByteOrder(),
+  address_size);

It is probably going to be the same, but you might as well use 
`process.GetArchitecture().GetByteOrder()`, since you have it handy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



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


[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

2020-03-18 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

In D73206#1928869 , @labath wrote:

> But I'm not sure that removing `GetBaseSymbolFile` is the right way to start 
> that, as without it you cannot implement the redirection.


Current code is:

  SymbolFileDWARF::DIEToTypePtr &SymbolFileDWARFDwo::GetDIEToType() { return 
GetBaseSymbolFile().GetDIEToType(); }
  typedef llvm::DenseMap 
DIEToTypePtr;

There will need to be the following change for DWZ anyway - as the same 
`DWARFDebugInfoEntry *` can lead to different `Types` depending on its Main CU:

  typedef llvm::DenseMap, 
lldb_private::Type *> DIEToTypePtr;

Where the `pair.first` would be `DWARFUnit *main_cu` for DWZ. And so then the 
`SymbolFileDWARFDwo` redirection can be done from that `main_cu` pointer (with 
some interface refactorization).

> Where are you going with that question? If it's stopping you from doing 
> anything, I can try to look into that a bit closer...

As I have currently some difficulties unifying the DWZ MainCU tracking with DWO 
MainCU tracking. DWZ deals only with `DW_TAG_compile_unit` and not with 
`DW_TAG_type_unit` as DWZ can unify+reference types more effectively using 
`DW_FORM_ref*`. While DWO deals also with `DWARFTypeUnit`. Each DWZ 
`DWARFCompileUnit` has corresponding `CompileUnit` but `DWARFTypeUnit` does not 
have any. So in `SymbolFileDWARF::GetTypeForDIE` for DWO it will use the second 
code path:

  SymbolContextScope *scope;
  if (auto *dwarf_cu = llvm::dyn_cast(die.GetCU()))
scope = GetCompUnitForDWARFCompUnit(*dwarf_cu);
  else
scope = GetObjectFile()->GetModule().get();

Therefore this code of mine no longer works as `comp_unit==nullptr` for 
`DWARFTypeUnit`s:

  DWARFUnit *main_unit = dwarf->GetDWARFCompileUnit(sc.comp_unit);

So either I would need to add new tracking `DWARFUnit *main_unit` even to 
functions which already have `SymbolContext &sc` parameter. Or more probably I 
need to somehow track the skeleton file some other way (looking it up from 
`sc.module_sp` by its hash?).
Without DWO support it is easier as I no longer have to work on the DWO MainCU 
tracking which is a task of its own. Also I could use the type 
`DWARFCompileUnit *main_unit` which saves some downcastings.

So I am asking if the DWO support together with DWZ brings some real 
improvements as it complicates the situation for the DWZ support.

Just for illustration: 
https://people.redhat.com/jkratoch/lldb-main_unit-DWARFUnit.patch  or the 
non-DWO start of simplification: 
https://people.redhat.com/jkratoch/lldb-main_unit-DWARFCompileUnit.patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73206



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


[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

2020-03-18 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk updated this revision to Diff 251086.
kwk added a comment.
Herald added a reviewer: jdoerfert.

- Added debuginfod2.py
- after running: autopep8 --in-place --aggressive --aggressive debuginfod2.py
- exponential backoff implemented
- Added http.py with doctests
- autopep8 --in-place  --aggressive http.py
- change import
- Removed simulated startup time
- fixup
- Changed wording
- Using os.path.abspath on directory before using it
- Fixups
- Fixups
- Fixups
- hide port and hostname from ServeDirectoryWithHTTP
- lit test working for debuginfod and source list


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,121 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//--
+// Test Purpose:
+// =
+// Test that we can display the source of functions using debuginfod when the
+// original source file is no longer present.
+// 
+// The debuginfod client requires a buildid in the binary, so we compile one in.
+// We can create the directory structure on disc that the client expects on a
+// webserver that serves source files. Then we fire up a python based http
+// server in the root of that mock directory, set the DEBUGINFOD_URLS
+// environment variable and let LLDB do the rest. 
+//
+// Go here to find more about debuginfod:
+// https://sourceware.org/elfutils/Debuginfod.html
+//--
+
+// We copy this file because we want to compile and later move it away
+// ===
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+// We cd into the directory before compiling to get DW_AT_comp_dir pickup
+// %t.buildroot as well so it will be replaced by /my/new/path.
+// ===
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+// We move the original source file to a directory that looks like a debuginfod
+// URL part.
+// 
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+// Adjust where debuginfod stores cache files:
+// ===
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+// Start HTTP file server on port picked by OS and wait until it is ready
+// The server will be closed on exit of the test.
+// ==
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'echo "SERVER LOG:"; cat %t.server.log; kill $PID;' EXIT INT
+

[Lldb-commits] [PATCH] D76316: [lldb] [testsuite] Enable forgotten -gsplit-dwarf for 2 testfiles

2020-03-18 Thread Jan Kratochvil via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3481062bc688: [lldb] [testsuite] Enable forgotten 
-gsplit-dwarf for 2 testfiles (authored by jankratochvil).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76316

Files:
  lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
  lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test


Index: lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
===
--- lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
+++ lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
@@ -12,12 +12,16 @@
 
 # Test type units in dwo files.
 # RUN: %clangxx_host %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -gdwarf-4 -fdebug-types-section -o %t4dwo
+# RUN:   -g -gdwarf-4 -fdebug-types-section -gsplit-dwarf -o %t4dwo \
+# RUN:   -Xclang -split-dwarf-output -Xclang %t4dwo.dwo \
+# RUN:   -Xclang -split-dwarf-file   -Xclang %t4dwo.dwo
 # RUN: %lldb %t4dwo -s %s -o exit | FileCheck %s
 
 # And type units+dwo+dwarf5.
 # RUN: %clangxx_host %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -gdwarf-5 -fdebug-types-section -o %t5dwo
+# RUN:   -g -gdwarf-5 -fdebug-types-section -gsplit-dwarf -o %t5dwo \
+# RUN:   -Xclang -split-dwarf-output -Xclang %t5dwo.dwo \
+# RUN:   -Xclang -split-dwarf-file   -Xclang %t5dwo.dwo
 # RUN: %lldb %t5dwo -s %s -o exit | FileCheck %s
 
 breakpoint set -n foo
Index: lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
===
--- lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
+++ lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
@@ -20,7 +20,7 @@
 
 # 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:   -g -gdwarf-5 -fdebug-types-section -gsplit-dwarf -c -o %t5dwo.o
 # RUN: ld.lld %t5dwo.o -o %t5dwo
 # RUN: %lldb %t5dwo -s %s -o exit | FileCheck %s
 


Index: lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
===
--- lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
+++ lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
@@ -12,12 +12,16 @@
 
 # Test type units in dwo files.
 # RUN: %clangxx_host %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -gdwarf-4 -fdebug-types-section -o %t4dwo
+# RUN:   -g -gdwarf-4 -fdebug-types-section -gsplit-dwarf -o %t4dwo \
+# RUN:   -Xclang -split-dwarf-output -Xclang %t4dwo.dwo \
+# RUN:   -Xclang -split-dwarf-file   -Xclang %t4dwo.dwo
 # RUN: %lldb %t4dwo -s %s -o exit | FileCheck %s
 
 # And type units+dwo+dwarf5.
 # RUN: %clangxx_host %S/Inputs/debug-types-expressions.cpp \
-# RUN:   -g -gdwarf-5 -fdebug-types-section -o %t5dwo
+# RUN:   -g -gdwarf-5 -fdebug-types-section -gsplit-dwarf -o %t5dwo \
+# RUN:   -Xclang -split-dwarf-output -Xclang %t5dwo.dwo \
+# RUN:   -Xclang -split-dwarf-file   -Xclang %t5dwo.dwo
 # RUN: %lldb %t5dwo -s %s -o exit | FileCheck %s
 
 breakpoint set -n foo
Index: lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
===
--- lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
+++ lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
@@ -20,7 +20,7 @@
 
 # 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:   -g -gdwarf-5 -fdebug-types-section -gsplit-dwarf -c -o %t5dwo.o
 # RUN: ld.lld %t5dwo.o -o %t5dwo
 # RUN: %lldb %t5dwo -s %s -o exit | FileCheck %s
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This looks pretty good overall. I have a bunch of comments, but nothing major.

Could you also please rebase the other patch (D74636 
) on top of this. I think that a very common 
use case for this will be taking the platform environment, tweaking it, and 
then using it to launch a process (i.e. what you are about to do), so it would 
be good to make sure that flow is sufficiently straight-forward.




Comment at: lldb/include/lldb/API/SBEnvironment.h:38
+  /// \return
+  /// The value of the environment variable or null if not present.
+  lldb::SBStringList GetEntries();

This doesn't sounds right.



Comment at: lldb/include/lldb/API/SBEnvironment.h:41-43
+  /// Adds an environment variable to this object. The input must be a string
+  /// with the format
+  ///   VARIABLE=value

Maybe explicitly mention that this overwrites any previous value with that name?



Comment at: lldb/include/lldb/API/SBEnvironment.h:46
+  /// \return
+  void PutEntry(const char *nameAndValue);
+

we use the `name_and_value` style elsewhere



Comment at: lldb/include/lldb/API/SBTarget.h:97
 
+  /// Return the environment variables or the target.
+  ///

s/or/of



Comment at: lldb/include/lldb/API/SBTarget.h:100
+  /// \return
+  /// An lldb::SBEnvironment object with the taget's environment variables.
+

s/taget/target/

Also the concept of a "target's environment variables" is somewhat fuzzy. I 
take it this is the environment that would be used by default to launch a new 
process (aka the value of target.env-vars setting)?

And maybe also mention that this is a *copy* of that environment (so any 
changes made to it don't propagate back anywhere)



Comment at: lldb/source/API/SBEnvironment.cpp:52
+ name);
+  return 
LLDB_RECORD_RESULT(ConstString(m_opaque_up->lookup(name)).AsCString());
+}

Can you check what will this return for non-existing variables and for 
variables which are set to an empty string `FOO=`. I have a feeling this will 
return the same thing, but I would expect to get a nullptr in the first case, 
and `""` in the second.



Comment at: lldb/source/API/SBEnvironment.cpp:57
+  LLDB_RECORD_METHOD_NO_ARGS(SBStringList, SBEnvironment, GetEntries);
+  auto entries = SBStringList();
+  for (const auto &KV : *m_opaque_up) {

This is using `auto` just to be fancy. That is not consistent with [[ 
http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
 | llvm style ]].



Comment at: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py:31-32
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+env = self.dbg.GetSelectedTarget().GetEnvironment()

I don't think you actually need to build an inferior for this. `target = 
dbg.CreateTarget("")` should be sufficient, I believe.



Comment at: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py:45
+env.PutEntry("FOO=bar")
+env.PutEntry("BAR=foo")
+

As per the other comment, please also test a case like `env.PutEntry("BAZ=")`



Comment at: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py:54
+for entry in env.GetEntries():
+self.assertTrue(entry in ["FOO=bar", "BAR=foo"])

self.assertIn(needle, haystack)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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


Re: [Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-18 Thread Pavel Labath via lldb-commits
[Splitting this off to not derail the review]

On 14/03/2020 00:45, Greg Clayton via Phabricator wrote:
> labath wrote:
>> This will return a dangling pointer as Envp is a temporary object. It's 
>> intended to be used to pass the environment object to execve-like function. 
>> The current "state of the art" is to ConstStringify all temporary strings 
>> that go out of the SB api. (Though I think we should start using 
>> std::string).
>>
>> Also, constructing the Envp object just to fetch a single string out of it 
>> is pretty expensive. You can fetch the desired result from the Environment 
>> object directly.
>>
>> Putting it all together, this kind of an API would be implemented via 
>> something like:
>> ```
>>   if (idx >= m_opaque_up->size())
>> return nullptr;
>>   return ConstString(Environment::compose(*std::next(m_opaque_up.begin(), 
>> idx)).AsCString();
>> ```
>>
>> ... but I am not sure this is really the right api. More on that in a 
>> separate comment.
> Yes, any C strings returned from the SB API should use 
> ConstString(cstr).AsCString() as the ConstString puts the string into a 
> string pool and there is no need to clients to destruct the string or take 
> ownership.
> 
> No STL in the SB API please, so no std::string. std::string isn't stable on 
> all platforms and competing C++ runtimes on different platforms can cause 
> problems. If we need string ownership, we can introduce SBString as a class.

What do you exactly mean by "not stable"? All major C++ standard
libraries I have knowledge of (libstdc++, libc++, msvc stl) take abi
stability very seriously (more seriously than we do for SB api).

I can see how returning std::strings would be a problem if one builds
liblldb with one c++ library (say libstdc++) and then tries to build the
dependant program with libc++ (are those the competing runtimes you
mentioned?), but.. is that something we have signed up to support?

The main reason I am even thinking about this is because swig already
knows how to map common stl types onto native types of other languages.
This is particularly important if we are going to have multiple
scripting languages, as we wouldn't need to redo the mapping for each
script language and each stl-like class we have (e.g. all SBXXXLists)

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


[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

2020-03-18 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk added a comment.

@labath I've updated my patch and would love to hear your opinion on it. So far 
I've only written the python `ServeDirectoryWithHTTP()` function with proper 
doctest and documentation but since you mentioned the `0` port thingy I've 
tried that on the command line when using `python -m http.server 0` and it 
works smoothly. That's why I've included the `llvm-lit` test I was working on. 
Maybe `lldb/test/Shell/SymbolFile/DWARF/source-list.cpp` is the wrong file for 
this, but we can move it around if you like it so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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


[Lldb-commits] [PATCH] D76168: CPlusPlusNameParser does not handles templated operator< properly

2020-03-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D76168#1925176 , @shafik wrote:

> Long-term I would like to modify clang to stop doing that for LLDB, but LLDB 
> will still have to support older compilers for a while. So I think this fix 
> is still needed.


So is this some alternative to D75761  (where 
we'd use CPlusPlusNameParser to decode DW_AT_names of templates)? If so, I 
think that is an interesting direction, but beware that that class is kind of 
meant for processing the demangler output. The contents of DW_AT_name looks a 
bit like a demangled name, but in reality there are some deviations from that 
format (which is why I did not recommend this direction initially -- but I am 
not against it either).

Also it looks like llvm and gnu demanglers disagree on the exact formatting of 
demangled operator names:

  $ c++filt _ZlsI1AEvT_S1_
  void operator<< (A, A)
  $ llvm-cxxfilt _ZlsI1AEvT_S1_
  void operator<<(A, A)

I think the gnu format is superior (and unambiguous) so we could change llvm to 
match that -- and this change probably won't require any kind of compatibility 
hacks.


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

https://reviews.llvm.org/D76168



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


[Lldb-commits] [PATCH] D75626: Add support for owning module information to TypeSystemClang.

2020-03-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl abandoned this revision.
aprantl marked an inline comment as done.
aprantl added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:996
   m_clang_ast_context->GetUniqueNamespaceDeclaration(
-  g_lldb_local_vars_namespace_cstr, nullptr);
+  g_lldb_local_vars_namespace_cstr, nullptr, 0);
   if (!namespace_decl)

shafik wrote:
> We are passing around `0` everywhere and it is not obvious what it means at 
> all.
> 
> Can we name this somehow or make it a type?
This is just an intermediate step and gets replaced in 
https://reviews.llvm.org/D75488



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:146
DWARFDIE *decl_ctx_die);
+  lldb_private::ModuleID GetOwningModuleID(const DWARFDIE &die);
 

labath wrote:
> I am kinda lost at this point, so I'm not sure which patch introduces this, 
> but... this name does not seem very ideal, as it sounds like it is related to 
> `lldb_private::Module`, while it does not really have that much in common 
> with it.
> 
> And then there's `ClangModulesDeclVendor::ModuleID` -- I have no idea what's 
> the relationship of this to that..
I agree. I'm going to clean this up some more and I will also merge this review 
into https://reviews.llvm.org/D75488 because it doesn't actually make reviewing 
easier to see these API changes without their use.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:73
+  unsigned GetOwningModuleID() { return Flags(m_payload).Clear(ObjCClassBit); }
+  void SetOwningModuleID(unsigned id) {
+assert(id < ObjCClassBit);

shafik wrote:
> Why not use `uint32_t` like we did above? If we are going to assume `32 bits` 
> we should just use the fixed width type.
That is consistent with the type Clang uses for module IDs.


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

https://reviews.llvm.org/D75626



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


[Lldb-commits] [PATCH] D75626: Add support for owning module information to TypeSystemClang.

2020-03-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

> And then there's ClangModulesDeclVendor::ModuleID -- I have no idea what's 
> the relationship of this to that..

`ClangModulesDeclVendor::ModuleID` is a typedef to `clang::Module *` so it is a 
bit of a misnomer, since clang calls the unsigned module numbers module ids.


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

https://reviews.llvm.org/D75626



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


[Lldb-commits] [PATCH] D75626: Add support for owning module information to TypeSystemClang.

2020-03-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

I'm merging this back into https://reviews.llvm.org/D75488 for easier reviewing.


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

https://reviews.llvm.org/D75626



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


[Lldb-commits] [PATCH] D75488: Preserve the owning module information from DWARF in the synthesized AST

2020-03-18 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 251116.
aprantl added a comment.

Rolled https://reviews.llvm.org/D75626 back into this patch, addressed more 
review feedback.


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

https://reviews.llvm.org/D75488

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Symbol/TypeSystem.cpp
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap
  lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg
  lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
  lldb/unittests/Symbol/TestTypeSystemClang.cpp
  lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h

Index: lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h
===
--- lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h
+++ lldb/unittests/TestingSupport/Symbol/ClangTestUtils.h
@@ -28,8 +28,8 @@
 
 inline CompilerType createRecord(TypeSystemClang &ast, llvm::StringRef name) {
   return ast.CreateRecordType(ast.getASTContext().getTranslationUnitDecl(),
-  lldb::AccessType::eAccessPublic, name, 0,
-  lldb::LanguageType::eLanguageTypeC);
+  OptionalClangModuleID(), lldb::AccessType::eAccessPublic, name,
+  0, lldb::LanguageType::eLanguageTypeC);
 }
 
 /// Create a record with the given name and a field with the given type
Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/Declaration.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
 #include "gtest/gtest.h"
 
@@ -257,9 +258,10 @@
   CompilerType basic_compiler_type = ast.GetBasicType(basic_type);
   EXPECT_TRUE(basic_compiler_type.IsValid());
 
-  CompilerType enum_type =
-  ast.CreateEnumerationType("my_enum", ast.GetTranslationUnitDecl(),
-Declaration(), basic_compiler_type, scoped);
+  CompilerType enum_type = ast.CreateEnumerationType(
+  "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(), Declaration(),
+  basic_compiler_type, scoped);
+
   CompilerType t = ast.GetEnumerationIntegerType(enum_type);
   // Check that the type we put in at the start is found again.
   EXPECT_EQ(basic_compiler_type.GetTypeName(), t.GetTypeName());
@@ -267,14 +269,38 @@
   }
 }
 
+TEST_F(TestTypeSystemClang, TestOwningModule) {
+  TypeSystemClang ast("module_ast", HostInfo::GetTargetTriple());
+  CompilerType basic_compiler_type = ast.GetBasicType(BasicType::eBasicTypeInt);
+  CompilerType enum_type = ast.CreateEnumerationType(
+  "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(100), Declaration(),
+  basic_compiler_type, false);
+  auto *ed = TypeSystemClang::GetAsEnumDecl(enum_type);
+  EXPECT_FALSE(!ed);
+  EXPECT_EQ(ed->getOwningModuleID(), 100u);
+
+  CompilerType record_type = ast.CreateRecordType(
+  nullptr, OptionalClangModuleID(200), lldb::eAccessPublic, "FooRecord",
+  clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr);
+  auto *rd = TypeSystemClang::GetAsRecordDecl(record_type);
+  EXPECT_FALSE(!rd);
+  EXPECT_EQ(rd->getOwningModuleID(), 200u);
+
+  CompilerType class_type = ast.CreateObjCClass(
+  "objc_class", ast.GetTranslationUnitDecl(), OptionalClangModuleID(300), false, false);
+  auto *cd = TypeSystemClang::GetAsObjCInterfaceDecl(class_type);
+  EXPECT_FALSE(!cd);
+  EXPECT_EQ(cd->getOwningModuleID(), 300u);
+}
+
 TEST_F(TestTypeSystemClang, TestIsClangType) {
   clang::ASTContext &context = m_ast->getASTContext();
   lldb::opaque_compiler_type_t b

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251143.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Utility/Environment.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py

Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
===
--- /dev/null
+++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -0,0 +1,67 @@
+"""Test the SBEnvironment APIs."""
+
+
+
+from math import fabs
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class SBEnvironmentAPICase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+
+@add_test_categories(['pyapi'])
+def test_platform_environment(self):
+env = self.dbg.GetSelectedPlatform().GetEnvironment()
+# We assume at least PATH is set
+self.assertNotEqual(env.GetEntry("PATH"), None)
+
+
+@add_test_categories(['pyapi'])
+def test_target_environment(self):
+env = self.dbg.GetSelectedTarget().GetEnvironment()
+# There is no target, so env should be empty
+self.assertEqual(len(env.GetEntries()), 0)
+self.assertEqual(env.GetEntry("PATH"), None)
+
+target = self.dbg.CreateTarget("")
+env = target.GetEnvironment()
+path = env.GetEntry("PATH")
+# Now there's a target, so at least PATH should exist
+self.assertNotEqual(path, None)
+
+# Make sure we are getting a copy by modifying the env we just got
+env.PutEntry("PATH=#" + path)
+self.assertEqual(target.GetEnvironment().GetEntry("PATH"), path)
+
+@add_test_categories(['pyapi'])
+def test_creating_and_modifying_environment(self):
+env = lldb.SBEnvironment()
+
+self.assertEqual(env.GetEntry("FOO"), None)
+self.assertEqual(env.GetEntry("BAR"), None)
+
+# We also test empty values
+env.PutEntry("FOO=")
+env.PutEntry("BAR=foo")
+
+self.assertEqual(env.GetEntry("FOO"), "")
+self.assertEqual(env.GetEntry("BAR"), "foo")
+
+entries = env.GetEntries()
+self.assertEqual(len(entries), 2)
+
+for entry in env.GetEntries():
+self.assertIn(entry, ["FOO=", "BAR=foo"])
+
+# Make sure modifications work
+env.PutEntry("FOO=bar")
+self.assertEqual(env.GetEntry("FOO"), "bar")
+
+for entry in env.GetEntries():
+self.assertIn(entry, ["FOO=bar", "BAR=foo"])
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -13,6 +13,7 @@
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBExpressionOptions.h"
 #include "lldb/API/SBFileSpec.h"
@@ -2388,6 +2389,17 @@
 m_opaque_sp->SetProcessLaunchInfo(launch_info.ref());
 }
 
+SBEnvironment SBTarget::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBTarget, GetEnvironment);
+  TargetSP target_sp(GetSP());
+
+  if (target_sp) {
+return LLDB_RECORD_RESULT(SBEnvironment(target_sp->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -2643,6 +2655,7 @@
   LLDB_REGISTER_METHOD(lldb::SBInstructionList, SBTarget,
GetInstructionsWithFlavor,
(lldb::addr_t, const char *, const void *, size_t));
+  LLDB_REGISTER_METHOD(lldb::SBEnvironment, SBTarget, GetEnvironment, ());
 }
 
 }
Index: lldb/source/API/SBPlatform.cpp
===
--- lldb/source/API/SBPlatform.cpp
+++ lldb/source/API/SBPlatform.cpp
@@ -8,9 +8,11 @@
 
 #include "lldb/API/SBPlatform.h"
 #include "SBReproducerPrivate.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBLaunchInfo.h"
+#include "lldb/API/SBPlatform.h"
 #include "lldb/API/SBUnixSignals.h"
 #include "lldb/Host/File.h"
 #include "lldb/Target/Platform.h"
@@ -649,6 +651,17 @@
   return LLDB_RECORD_RESULT(SBUnixSignals());
 }
 
+S

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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


[Lldb-commits] [PATCH] D76341: [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces

2020-03-18 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76341



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


[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

We need to determine if the objects we return are copies (from SBPlatform and 
SBTarget), or if they are objects that will modify the environment for the 
platform and target respectively. If we are returning copies, we need setters 
on both the platform and target. If they are references, then they will just 
update the current environment. I think I would rather have the ability to 
modify them using the returned object, but I am not set on this if everyone 
else thinks otherwise.

I added many comments in the SBEnvironment header.




Comment at: lldb/include/lldb/API/SBEnvironment.h:26
+
+  size_t GetNumEntries();
+

would GetNumValues() be better? Also, if we have this, we need some accessors 
that work by index:

```
const char *GetNameAtIndex(size_t);
const char *GetValueAtIndex(size_t);
```

Again, I would rather people not have to split up the name and value themselves 
if we can avoid it.



Comment at: lldb/include/lldb/API/SBEnvironment.h:32
+  /// The value of the enviroment variable or null if not present.
+  const char *GetEntry(const char *name);
+

Would get Get(...) be better? Mirror the getenv() call?



Comment at: lldb/include/lldb/API/SBEnvironment.h:39
+  /// The value of the environment variable or null if not present.
+  lldb::SBStringList GetEntries();
+

If we can get a list of values and edit the SBStringList, do we want a setter?:

```
void SetEntries(SBStringList);
```



Comment at: lldb/include/lldb/API/SBEnvironment.h:46
+  /// \return
+  void PutEntry(const char *nameAndValue);
+

labath wrote:
> we use the `name_and_value` style elsewhere
Change to:

```
bool Set(const char *name, const char *value, bool overwrite);
```

The bool indicates if it was set. I would rather people not have to make string 
values that contain "name=value" just to set an env var. This mirrors the 
setenv() API in libc






Comment at: lldb/include/lldb/API/SBEnvironment.h:47
+  void PutEntry(const char *nameAndValue);
+
+protected:

We need a Unset accessor:

```
bool Unset(const char *name);
```

The returned bool can indicate if the value was removed.



Comment at: lldb/include/lldb/API/SBTarget.h:97
 
+  /// Return the environment variables or the target.
+  ///

labath wrote:
> s/or/of
Maybe better as:

```
/// Return an object that contains the environment that will be used when 
/// launching processes.
```

Then we need to explain if this is a copy, or a value that can be modified. It 
is can be modified, we should add

```
/// The SBEnvironment object can be used to add or remove environment 
/// variables prior to launching a process.
```

If it is a copy, mention using the SetEnvironment accessor I mention below




Comment at: lldb/include/lldb/API/SBTarget.h:100
+  /// \return
+  /// An lldb::SBEnvironment object with the taget's environment variables.
+

labath wrote:
> s/taget/target/
> 
> Also the concept of a "target's environment variables" is somewhat fuzzy. I 
> take it this is the environment that would be used by default to launch a new 
> process (aka the value of target.env-vars setting)?
> 
> And maybe also mention that this is a *copy* of that environment (so any 
> changes made to it don't propagate back anywhere)
If this is a copy of the environment, then there should be a set accessor:

```
void SBTarget::SetEnvironment(SBEnvironment);
```

I would be ok with either way (return a copy and have setting, or return the 
actual object so it can be manipulated). What is the best approach?



Comment at: lldb/source/API/SBEnvironment.cpp:52
+ name);
+  return 
LLDB_RECORD_RESULT(ConstString(m_opaque_up->lookup(name)).AsCString());
+}

labath wrote:
> Can you check what will this return for non-existing variables and for 
> variables which are set to an empty string `FOO=`. I have a feeling this will 
> return the same thing, but I would expect to get a nullptr in the first case, 
> and `""` in the second.
Yes, we need a test for env vars with that are set with no value and it should 
return "" (non NULL).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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


[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251150.
wallace added a comment.

nit 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314

Files:
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2821,7 +2821,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,21 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program, stopOnEntry=True)
+self.vscode.request_disconnect(terminateDebuggee=True) 
+time.sleep(1)
+# The underlying lldb-vscode process must have already finished even 
though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the
@@ -329,7 +345,7 @@
 # Get output from the console. This should contain both the
 # "exitCommands" that were run after the second breakpoint was hit
 output = self.get_console(timeout=1.0)
-self.verify_commands('exitCommands', output, exitCommands)
+self.verify_commands('exitCommands', output, exitCommands)q
 
 @skipIfWindows
 @skipIfRemote
@@ -394,3 +410,4 @@
 # "exitCommands" that were run after the second breakpoint was hit
 output = self.get_console(timeout=1.0)
 self.verify_commands('exitCommands', output, exitCommands)
+
\ No newline at end of file


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2821,7 +2821,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,21 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program, stopOnEntry=True)
+self.vscode.request_disconnect(terminateDebuggee=True) 
+time.sleep(1)
+# The underlying lldb-vscode process must have already finished even though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the
@@ -329,7 +345,7 @@
 # Get output from the console. This should contain both the
 # "exitCommands" that were run after the second breakpoint was hit
 output = self.get_console(timeout=1.0)
-self.verify_commands('exitCommands', output, exitCommands)
+self.verify_commands('exitCommands', output, exitCommands)q
 
 @skipIfWindows
 @skipIfRemote
@@ -394,3 +410,4 @@
 # "exitCommands" that were run after the second breakpoint was hit
 output = self.get_console(timeout=1.0)
 self.verify_commands('exitCommands', output, exitCommands)
+
\ No newline at end of file
__

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251149.
wallace added a comment.

add a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314

Files:
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2821,7 +2821,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,21 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program, stopOnEntry=True)
+self.vscode.request_disconnect(terminateDebuggee=True) 
+time.sleep(1)
+# The underlying lldb-vscode process must have already finished even 
though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the
@@ -393,4 +409,4 @@
 # Get output from the console. This should contain both the
 # "exitCommands" that were run after the second breakpoint was hit
 output = self.get_console(timeout=1.0)
-self.verify_commands('exitCommands', output, exitCommands)
+self.verify_commands('exitCommands', output, exitCommands)
\ No newline at end of file


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2821,7 +2821,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,21 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program, stopOnEntry=True)
+self.vscode.request_disconnect(terminateDebuggee=True) 
+time.sleep(1)
+# The underlying lldb-vscode process must have already finished even though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the
@@ -393,4 +409,4 @@
 # Get output from the console. This should contain both the
 # "exitCommands" that were run after the second breakpoint was hit
 output = self.get_console(timeout=1.0)
-self.verify_commands('exitCommands', output, exitCommands)
+self.verify_commands('exitCommands', output, exitCommands)
\ No newline at end of file
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] b40ee7f - [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces

2020-03-18 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-03-18T13:18:02-07:00
New Revision: b40ee7ff1b16982b39582eee04ca82cac5f3d154

URL: 
https://github.com/llvm/llvm-project/commit/b40ee7ff1b16982b39582eee04ca82cac5f3d154
DIFF: 
https://github.com/llvm/llvm-project/commit/b40ee7ff1b16982b39582eee04ca82cac5f3d154.diff

LOG: [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces

Summary:
The memory history plugin for Asan creates a HistoryThread with the
recorded PC values provided by the Asan runtime. In other cases,
thoses PCs are gathered by LLDB directly.

The PCs returned by the Asan runtime are the PCs of the calls in the
backtrace, not the return addresses you would normally get when
unwinding the stack (look for a call to GetPreviousIntructionPc in
AsanGetStack).

When the above addresses are passed to the unwinder, it will subtract
1 from each address of the non zero frames because it treats them as
return addresses. This can lead to the final report referencing the
wrong line.

This patch fixes this issue by threading a flag through HistoryThread
and HistoryUnwinder that tells them to treat every frame like the
first one. The Asan MemoryHistory plugin can then use this flag.

This fixes running TestMemoryHistory on arm64 devices, although it's
hard to guarantee that the test will continue to exhibit the boundary
condition that triggers this bug.

Reviewers: jasonmolenda, kubamracek

Subscribers: kristof.beyls, danielkiss, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
lldb/source/Plugins/Process/Utility/HistoryThread.cpp
lldb/source/Plugins/Process/Utility/HistoryThread.h
lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
lldb/source/Plugins/Process/Utility/HistoryUnwind.h

Removed: 




diff  --git a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp 
b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
index 4b9da8f76fd2..333113a0b17a 100644
--- a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
+++ b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
@@ -138,7 +138,12 @@ static void CreateHistoryThreadFromValueObject(ProcessSP 
process_sp,
 pcs.push_back(pc);
   }
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, pcs);
+  // The ASAN runtime already massages the return addresses into call
+  // addresses, we don't want LLDB's unwinder to try to locate the previous
+  // instruction again as this might lead to us reporting a 
diff erent line.
+  bool pcs_are_call_addresses = true;
+  HistoryThread *history_thread =
+  new HistoryThread(*process_sp, tid, pcs, pcs_are_call_addresses);
   ThreadSP new_thread_sp(history_thread);
   std::ostringstream thread_name_with_number;
   thread_name_with_number << thread_name << " Thread " << tid;

diff  --git a/lldb/source/Plugins/Process/Utility/HistoryThread.cpp 
b/lldb/source/Plugins/Process/Utility/HistoryThread.cpp
index 815883d9e2f6..0649cd2f07de 100644
--- a/lldb/source/Plugins/Process/Utility/HistoryThread.cpp
+++ b/lldb/source/Plugins/Process/Utility/HistoryThread.cpp
@@ -25,12 +25,13 @@ using namespace lldb_private;
 //  Constructor
 
 HistoryThread::HistoryThread(lldb_private::Process &process, lldb::tid_t tid,
- std::vector pcs)
+ std::vector pcs,
+ bool pcs_are_call_addresses)
 : Thread(process, tid, true), m_framelist_mutex(), m_framelist(),
   m_pcs(pcs), m_extended_unwind_token(LLDB_INVALID_ADDRESS), 
m_queue_name(),
   m_thread_name(), m_originating_unique_thread_id(tid),
   m_queue_id(LLDB_INVALID_QUEUE_ID) {
-  m_unwinder_up.reset(new HistoryUnwind(*this, pcs));
+  m_unwinder_up.reset(new HistoryUnwind(*this, pcs, pcs_are_call_addresses));
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
   LLDB_LOGF(log, "%p HistoryThread::HistoryThread", static_cast(this));
 }

diff  --git a/lldb/source/Plugins/Process/Utility/HistoryThread.h 
b/lldb/source/Plugins/Process/Utility/HistoryThread.h
index 434cf6af7197..a66e0f2d4207 100644
--- a/lldb/source/Plugins/Process/Utility/HistoryThread.h
+++ b/lldb/source/Plugins/Process/Utility/HistoryThread.h
@@ -33,7 +33,8 @@ namespace lldb_private {
 class HistoryThread : public lldb_private::Thread {
 public:
   HistoryThread(lldb_private::Process &process, lldb::tid_t tid,
-std::vector pcs);
+std::vector pcs,
+bool pcs_are_call_addresses = false);
 
   ~HistoryThread() override;
 

diff  --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp 
b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
index 93fcde72bf99..9b9522955de9 100644
--- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
+++ b/lldb/source/Plugins/Process/Utility/HistoryU

[Lldb-commits] [PATCH] D76168: CPlusPlusNameParser does not handles templated operator< properly

2020-03-18 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

In D76168#1929211 , @labath wrote:

> In D76168#1925176 , @shafik wrote:
>
> > Long-term I would like to modify clang to stop doing that for LLDB, but 
> > LLDB will still have to support older compilers for a while. So I think 
> > this fix is still needed.
>
>
> So is this some alternative to D75761  
> (where we'd use CPlusPlusNameParser to decode DW_AT_names of templates)? If 
> so, I think that is an interesting direction, but beware that that class is 
> kind of meant for processing the demangler output. The contents of DW_AT_name 
> looks a bit like a demangled name, but in reality there are some deviations 
> from that format (which is why I did not recommend this direction initially 
> -- but I am not against it either).
>
> Also it looks like llvm and gnu demanglers disagree on the exact formatting 
> of demangled operator names:
>
>   $ c++filt _ZlsI1AEvT_S1_
>   void operator<< (A, A)
>   $ llvm-cxxfilt _ZlsI1AEvT_S1_
>   void operator<<(A, A)
>
>
> I think the gnu format is superior (and unambiguous) so we could change llvm 
> to match that -- and this change probably won't require any kind of 
> compatibility hacks.


This is not an alternative, this is a complement to that fix. So even with 
D75761  we still fail in expressions and 
setting breakpoints for symbols of that type. So both fixes are needed.

I personally would like to stop emitting template parameters for `DW_AT_names` 
but I know this will break some stuff and I have to see if it is workable or 
not, I think it is.

Lastly, we will still need to support older compilers regardless.


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

https://reviews.llvm.org/D76168



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


[Lldb-commits] [PATCH] D76341: [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb40ee7ff1b16: [lldb/MemoryHistoryAsan] Fix address 
resolution for recorded backtraces (authored by friss).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76341

Files:
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Plugins/Process/Utility/HistoryThread.cpp
  lldb/source/Plugins/Process/Utility/HistoryThread.h
  lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
  lldb/source/Plugins/Process/Utility/HistoryUnwind.h

Index: lldb/source/Plugins/Process/Utility/HistoryUnwind.h
===
--- lldb/source/Plugins/Process/Utility/HistoryUnwind.h
+++ lldb/source/Plugins/Process/Utility/HistoryUnwind.h
@@ -18,7 +18,8 @@
 
 class HistoryUnwind : public lldb_private::Unwind {
 public:
-  HistoryUnwind(Thread &thread, std::vector pcs);
+  HistoryUnwind(Thread &thread, std::vector pcs,
+bool pcs_are_call_addresses = false);
 
   ~HistoryUnwind() override;
 
@@ -35,6 +36,9 @@
 
 private:
   std::vector m_pcs;
+  /// This boolean indicates that the PCs in the non-0 frames are call
+  /// addresses and not return addresses.
+  bool m_pcs_are_call_addresses;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
===
--- lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
+++ lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp
@@ -23,8 +23,10 @@
 
 // Constructor
 
-HistoryUnwind::HistoryUnwind(Thread &thread, std::vector pcs)
-: Unwind(thread), m_pcs(pcs) {}
+HistoryUnwind::HistoryUnwind(Thread &thread, std::vector pcs,
+ bool pcs_are_call_addresses)
+: Unwind(thread), m_pcs(pcs),
+  m_pcs_are_call_addresses(pcs_are_call_addresses) {}
 
 // Destructor
 
@@ -59,7 +61,10 @@
   if (frame_idx < m_pcs.size()) {
 cfa = frame_idx;
 pc = m_pcs[frame_idx];
-behaves_like_zeroth_frame = (frame_idx == 0);
+if (m_pcs_are_call_addresses)
+  behaves_like_zeroth_frame = true;
+else
+  behaves_like_zeroth_frame = (frame_idx == 0);
 return true;
   }
   return false;
Index: lldb/source/Plugins/Process/Utility/HistoryThread.h
===
--- lldb/source/Plugins/Process/Utility/HistoryThread.h
+++ lldb/source/Plugins/Process/Utility/HistoryThread.h
@@ -33,7 +33,8 @@
 class HistoryThread : public lldb_private::Thread {
 public:
   HistoryThread(lldb_private::Process &process, lldb::tid_t tid,
-std::vector pcs);
+std::vector pcs,
+bool pcs_are_call_addresses = false);
 
   ~HistoryThread() override;
 
Index: lldb/source/Plugins/Process/Utility/HistoryThread.cpp
===
--- lldb/source/Plugins/Process/Utility/HistoryThread.cpp
+++ lldb/source/Plugins/Process/Utility/HistoryThread.cpp
@@ -25,12 +25,13 @@
 //  Constructor
 
 HistoryThread::HistoryThread(lldb_private::Process &process, lldb::tid_t tid,
- std::vector pcs)
+ std::vector pcs,
+ bool pcs_are_call_addresses)
 : Thread(process, tid, true), m_framelist_mutex(), m_framelist(),
   m_pcs(pcs), m_extended_unwind_token(LLDB_INVALID_ADDRESS), m_queue_name(),
   m_thread_name(), m_originating_unique_thread_id(tid),
   m_queue_id(LLDB_INVALID_QUEUE_ID) {
-  m_unwinder_up.reset(new HistoryUnwind(*this, pcs));
+  m_unwinder_up.reset(new HistoryUnwind(*this, pcs, pcs_are_call_addresses));
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
   LLDB_LOGF(log, "%p HistoryThread::HistoryThread", static_cast(this));
 }
Index: lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
===
--- lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
+++ lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
@@ -138,7 +138,12 @@
 pcs.push_back(pc);
   }
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, pcs);
+  // The ASAN runtime already massages the return addresses into call
+  // addresses, we don't want LLDB's unwinder to try to locate the previous
+  // instruction again as this might lead to us reporting a different line.
+  bool pcs_are_call_addresses = true;
+  HistoryThread *history_thread =
+  new HistoryThread(*process_sp, tid, pcs, pcs_are_call_addresses);
   ThreadSP new_thread_sp(history_thread);
   std::ostringstream thread_name_with_number;
   thread_name_with_number << thread_name << " Thread " << tid;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinf

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

2020-03-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py:46-48
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program, stopOnEntry=True)
+self.vscode.request_disconnect(terminateDebuggee=True) 

I don't think we need to run a program. Just send the initialize packet and 
then the terminate packet, then sleep?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314



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


[Lldb-commits] [lldb] 85bd436 - [Host] Remove some code that's not needed anymore.

2020-03-18 Thread Davide Italiano via lldb-commits

Author: Davide Italiano
Date: 2020-03-18T14:44:53-07:00
New Revision: 85bd4369610fe60397455c8e0914a09288285e84

URL: 
https://github.com/llvm/llvm-project/commit/85bd4369610fe60397455c8e0914a09288285e84
DIFF: 
https://github.com/llvm/llvm-project/commit/85bd4369610fe60397455c8e0914a09288285e84.diff

LOG: [Host] Remove some code that's not needed anymore.

Discussed offline with Jason.

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/Host.mm

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/Host.mm 
b/lldb/source/Host/macosx/objcxx/Host.mm
index 2475338a37fd..eba3060f8ec6 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -1088,43 +1088,6 @@ static Status LaunchProcessPosixSpawn(const char 
*exe_path,
 return error;
   }
 
-// posix_spawnattr_setbinpref_np appears to be an Apple extension per:
-// http://www.unix.com/man-page/OSX/3/posix_spawnattr_setbinpref_np/
-#if !defined(__arm__)
-
-  // Don't set the binpref if a shell was provided.  After all, that's only
-  // going to affect what version of the shell
-  // is launched, not what fork of the binary is launched.  We insert "arch
-  // --arch  as part of the shell invocation
-  // to do that job on OSX.
-
-  if (launch_info.GetShell() == FileSpec()) {
-// We don't need to do this for ARM, and we really shouldn't now that we
-// have multiple CPU subtypes and no posix_spawnattr call that allows us
-// to set which CPU subtype to launch...
-const ArchSpec &arch_spec = launch_info.GetArchitecture();
-cpu_type_t cpu = arch_spec.GetMachOCPUType();
-cpu_type_t sub = arch_spec.GetMachOCPUSubType();
-if (cpu != 0 && cpu != static_cast(UINT32_MAX) &&
-cpu != static_cast(LLDB_INVALID_CPUTYPE) &&
-!(cpu == 0x0107 && sub == 8)) // If haswell is specified, don't try
-  // to set the CPU type or we will 
fail
-{
-  size_t ocount = 0;
-  error.SetError(::posix_spawnattr_setbinpref_np(&attr, 1, &cpu, &ocount),
- eErrorTypePOSIX);
-  if (error.Fail())
-LLDB_LOG(log,
- "error: {0}, ::posix_spawnattr_setbinpref_np ( &attr, 1, "
- "cpu_type = {1:x}, count => {2} )",
- error, cpu, ocount);
-
-  if (error.Fail() || ocount != 1)
-return error;
-}
-  }
-#endif // !defined(__arm__)
-
   const char *tmp_argv[2];
   char *const *argv = const_cast(
   launch_info.GetArguments().GetConstArgumentVector());



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


[Lldb-commits] [lldb] 5ffb30f - [lldb/PlatformDarwin] Expose current toolchain and CL tools directory

2020-03-18 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-03-18T15:08:24-07:00
New Revision: 5ffb30fd6c7faff314b01ef0dc75f2683ca85cdf

URL: 
https://github.com/llvm/llvm-project/commit/5ffb30fd6c7faff314b01ef0dc75f2683ca85cdf
DIFF: 
https://github.com/llvm/llvm-project/commit/5ffb30fd6c7faff314b01ef0dc75f2683ca85cdf.diff

LOG: [lldb/PlatformDarwin] Expose current toolchain and CL tools directory

Expose two methods to find the current toolchain and the current command
line tools directory. These are used by Swift to find the resource
directory.

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/unittests/Platform/PlatformDarwinTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index aa1f8994ecb6..46dd3774e5a9 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1829,6 +1829,21 @@ lldb_private::Status 
PlatformDarwin::FindBundleBinaryInExecSearchPaths(
   return Status();
 }
 
+std::string PlatformDarwin::FindComponentInPath(llvm::StringRef path,
+llvm::StringRef component) {
+  auto begin = llvm::sys::path::begin(path);
+  auto end = llvm::sys::path::end(path);
+  for (auto it = begin; it != end; ++it) {
+if (it->contains(component)) {
+  llvm::SmallString<128> buffer;
+  llvm::sys::path::append(buffer, begin, ++it,
+  llvm::sys::path::Style::posix);
+  return buffer.str().str();
+}
+  }
+  return {};
+}
+
 std::string
 PlatformDarwin::FindXcodeContentsDirectoryInPath(llvm::StringRef path) {
   auto begin = llvm::sys::path::begin(path);
@@ -1959,3 +1974,15 @@ FileSpec PlatformDarwin::GetXcodeContentsDirectory() {
   });
   return g_xcode_contents_path;
 }
+
+FileSpec PlatformDarwin::GetCurrentToolchainDirectory() {
+  if (FileSpec fspec = HostInfo::GetShlibDir())
+return FileSpec(FindComponentInPath(fspec.GetPath(), ".xctoolchain"));
+  return {};
+}
+
+FileSpec PlatformDarwin::GetCurrentCommandLineToolsDirectory() {
+  if (FileSpec fspec = HostInfo::GetShlibDir())
+return FileSpec(FindComponentInPath(fspec.GetPath(), "CommandLineTools"));
+  return {};
+}

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index d385712db8e6..6d51edbc9294 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -100,6 +100,13 @@ class PlatformDarwin : public PlatformPOSIX {
   static lldb_private::FileSpec GetXcodeSDK(SDKType type);
   static lldb_private::FileSpec GetXcodeContentsDirectory();
 
+  /// Return the toolchain directroy the current LLDB instance is located in.
+  static lldb_private::FileSpec GetCurrentToolchainDirectory();
+
+  /// Return the command line tools directory the current LLDB instance is
+  /// located in.
+  static lldb_private::FileSpec GetCurrentCommandLineToolsDirectory();
+
 protected:
   struct CrashInfoAnnotations {
 uint64_t version;  // unsigned long
@@ -172,6 +179,8 @@ class PlatformDarwin : public PlatformPOSIX {
   const lldb_private::FileSpecList *module_search_paths_ptr,
   lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
 
+  static std::string FindComponentInPath(llvm::StringRef path,
+ llvm::StringRef component);
   static std::string FindXcodeContentsDirectoryInPath(llvm::StringRef path);
 
   std::string m_developer_directory;

diff  --git a/lldb/unittests/Platform/PlatformDarwinTest.cpp 
b/lldb/unittests/Platform/PlatformDarwinTest.cpp
index 06287c63227b..20916f3cd125 100644
--- a/lldb/unittests/Platform/PlatformDarwinTest.cpp
+++ b/lldb/unittests/Platform/PlatformDarwinTest.cpp
@@ -19,6 +19,7 @@ using namespace lldb_private;
 
 struct PlatformDarwinTester : public PlatformDarwin {
 public:
+  using PlatformDarwin::FindComponentInPath;
   using PlatformDarwin::FindXcodeContentsDirectoryInPath;
   static bool SDKSupportsModules(SDKType desired_type,
  const lldb_private::FileSpec &sdk_path) {
@@ -132,3 +133,20 @@ TEST(PlatformDarwinTest, GetSDKNameForType) {
   EXPECT_EQ(
   "", PlatformDarwin::GetSDKNameForType(PlatformDarwin::SDKType::unknown));
 }
+
+TEST(PlatformDarwinTest, FindComponentInPath) {
+  EXPECT_EQ("/path/to/foo",
+PlatformDarwinTester::FindComponentInPath("/path/to/foo/", "foo"));
+
+  EXPECT_EQ("/path/to/foo",
+PlatformDarwinTester::FindComponentInPath("/path/to/foo", "foo"));
+
+  EXPECT_EQ("/path/to/foobar", PlatformDarwinTester::FindComponentInPath(
+   "/path/to/foobar", "foo"));
+
+  EXPECT_EQ("/path/to/foobar", Platfo

[Lldb-commits] [lldb] 1497066 - [lldb/Test] Add unittest for FileSpec::operator bool()

2020-03-18 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-03-18T15:08:23-07:00
New Revision: 14970669ddedb07b68518ae708889e54e6d6eeb9

URL: 
https://github.com/llvm/llvm-project/commit/14970669ddedb07b68518ae708889e54e6d6eeb9
DIFF: 
https://github.com/llvm/llvm-project/commit/14970669ddedb07b68518ae708889e54e6d6eeb9.diff

LOG: [lldb/Test] Add unittest for FileSpec::operator bool()

Added: 


Modified: 
lldb/unittests/Utility/FileSpecTest.cpp

Removed: 




diff  --git a/lldb/unittests/Utility/FileSpecTest.cpp 
b/lldb/unittests/Utility/FileSpecTest.cpp
index c66edc444797..690c5ae331ee 100644
--- a/lldb/unittests/Utility/FileSpecTest.cpp
+++ b/lldb/unittests/Utility/FileSpecTest.cpp
@@ -441,3 +441,9 @@ TEST(FileSpecTest, Yaml) {
   EXPECT_EQ(deserialized.GetDirectory(), fs_windows.GetDirectory());
   EXPECT_EQ(deserialized, fs_windows);
 }
+
+TEST(FileSpecTest, OperatorBool) {
+  EXPECT_FALSE(FileSpec());
+  EXPECT_FALSE(FileSpec(""));
+  EXPECT_TRUE(FileSpec("/foo/bar"));
+}



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


[Lldb-commits] [PATCH] D76351: [lldb-vscode] Don't use SBLaunchInfo in request_attach

2020-03-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.

Yep, copy and paste error!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76351



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


[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-03-18 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin updated this revision to Diff 251181.
jarin marked 5 inline comments as done.
jarin added a comment.

Addressed reviewer comments in the code, but still have no clue how to write 
the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -31,6 +31,7 @@
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
@@ -469,6 +470,119 @@
   return register_object;
 }
 
+static llvm::Optional
+GetRegisterValue(NativeRegisterContext ®_ctx, uint32_t generic_regnum) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
+  uint32_t reg_num = reg_ctx.ConvertRegisterKindToRegisterNumber(
+  eRegisterKindGeneric, generic_regnum);
+  const RegisterInfo *const reg_info_p =
+  reg_ctx.GetRegisterInfoAtIndex(reg_num);
+
+  if (reg_info_p == nullptr || reg_info_p->value_regs != nullptr) {
+LLDB_LOGF(log, "%s failed to get register info for register index %" PRIu32,
+  __FUNCTION__, reg_num);
+return {};
+  }
+
+  RegisterValue reg_value;
+  Status error = reg_ctx.ReadRegister(reg_info_p, reg_value);
+  if (error.Fail()) {
+LLDB_LOGF(log, "%s failed to read register '%s' index %" PRIu32 ": %s",
+  __FUNCTION__,
+  reg_info_p->name ? reg_info_p->name : "",
+  reg_num, error.AsCString());
+return {};
+  }
+  return reg_value;
+}
+
+static json::Object CreateMemoryChunk(json::Array &stack_memory_chunks,
+  addr_t address,
+  std::vector &bytes) {
+  json::Object chunk;
+  chunk.try_emplace("address", static_cast(address));
+  StreamString stream;
+  for (uint8_t b : bytes)
+stream.PutHex8(b);
+  chunk.try_emplace("bytes", stream.GetString().str());
+  return chunk;
+}
+
+static json::Array GetStackMemoryAsJSON(NativeProcessProtocol &process,
+NativeThreadProtocol &thread) {
+  uint32_t address_size = process.GetArchitecture().GetAddressByteSize();
+  const size_t kStackTopMemoryInfoWordSize = 12;
+  size_t stack_top_memory_info_byte_size =
+  kStackTopMemoryInfoWordSize * address_size;
+  const size_t kMaxStackSize = 128 * 1024;
+  const size_t kMaxFrameSize = 4 * 1024;
+  size_t fp_and_ra_size = 2 * address_size;
+  const size_t kMaxFrameCount = 128;
+
+  NativeRegisterContext ®_ctx = thread.GetRegisterContext();
+
+  json::Array stack_memory_chunks;
+
+  lldb::addr_t sp_value;
+  if (llvm::Optional optional_sp_value =
+  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_SP)) {
+sp_value = optional_sp_value->GetAsUInt64();
+  } else {
+return stack_memory_chunks;
+  }
+  lldb::addr_t fp_value;
+  if (llvm::Optional optional_fp_value =
+  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_FP)) {
+fp_value = optional_fp_value->GetAsUInt64();
+  } else {
+return stack_memory_chunks;
+  }
+
+  // First, make sure we copy the top stack_top_memory_info_byte_size bytes
+  // from the stack.
+  size_t byte_count = std::min(stack_top_memory_info_byte_size,
+   static_cast(fp_value - sp_value));
+  std::vector buf(byte_count, 0);
+
+  size_t bytes_read = 0;
+  Status error = process.ReadMemoryWithoutTrap(sp_value, buf.data(), byte_count,
+   bytes_read);
+  if (error.Success() && bytes_read > 0) {
+buf.resize(bytes_read);
+stack_memory_chunks.push_back(
+std::move(CreateMemoryChunk(stack_memory_chunks, sp_value, buf)));
+  }
+
+  // Additionally, try to walk the frame pointer link chain. If the frame
+  // is too big or if the frame pointer points too far, stop the walk.
+  addr_t max_frame_pointer = sp_value + kMaxStackSize;
+  for (size_t i = 0; i < kMaxFrameCount; i++) {
+if (fp_value < sp_value || fp_value > sp_value + kMaxFrameSize ||
+fp_value > max_frame_pointer)
+  break;
+
+std::vector fp_ra_buf(fp_and_ra_size, 0);
+bytes_read = 0;
+error = process.ReadMemoryWithoutTrap(fp_value, fp_ra_buf.data(),
+  fp_and_ra_size, bytes_read);
+if (error.Fail() || bytes_read != fp_and_ra_size)
+  break;
+
+stack_memory_chunks.push_back(
+std::move(CreateMemoryChunk(stack_memory_ch

[Lldb-commits] [PATCH] D68010: [lldb] Fix string summary of an empty NSPathStore2

2020-03-18 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor updated this revision to Diff 251184.
teemperor added a comment.

- Rebased the patch.

This has been (really) delayed because of the whole unit test I wanted to 
write, but I'll just land this as-is (as having this broken even longer just 
because I haven't written that new unit test seems silly).


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

https://reviews.llvm.org/D68010

Files:
  lldb/include/lldb/DataFormatters/StringPrinter.h
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
@@ -17,6 +17,7 @@
 
 NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
+  NSString *empty = @"";
 	NSString *str0 = [[NSNumber numberWithUnsignedLongLong:0xFF] stringValue];
 	NSString *str1 = [NSString stringWithCString:"A rather short ASCII NSString object is here" encoding:NSASCIIStringEncoding];
 	NSString *str2 = [NSString stringWithUTF8String:"A rather short UTF8 NSString object is here"];
@@ -69,6 +70,7 @@
 
 	NSArray *components = @[@"usr", @"blah", @"stuff"];
 	NSString *path = [NSString pathWithComponents: components];
+  NSString *empty_path = [empty stringByDeletingPathExtension];
 
   const unichar someOfTheseAreNUL[] = {'a',' ', 'v','e','r','y',' ',
   'm','u','c','h',' ','b','o','r','i','n','g',' ','t','a','s','k',
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
@@ -76,8 +76,8 @@
 self.expect('frame variable hebrew', substrs=['לילה טוב'])
 
 def nsstring_data_formatter_commands(self):
-self.expect('frame variable str0 str1 str2 str3 str4 str5 str6 str8 str9 str10 str11 label1 label2 processName str12',
-substrs=[
+self.expect('frame variable empty str0 str1 str2 str3 str4 str5 str6 str8 str9 str10 str11 label1 label2 processName str12',
+substrs=['(NSString *) empty = ', ' @""',
  # '(NSString *) str0 = ',' @"255"',
  '(NSString *) str1 = ', ' @"A rather short ASCII NSString object is here"',
  '(NSString *) str2 = ', ' @"A rather short UTF8 NSString object is here"',
@@ -104,6 +104,8 @@
 
 self.expect('expr -d run-target -- path', substrs=['usr/blah/stuff'])
 self.expect('frame variable path', substrs=['usr/blah/stuff'])
+self.expect('expr -d run-target -- empty_path', substrs=['@""'])
+self.expect('frame variable empty_path', substrs=['@""'])
 
 def nsstring_withNULs_commands(self):
 """Check that the NSString formatter supports embedded NULs in the text"""
Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -170,6 +170,7 @@
   options.SetStream(&stream);
   options.SetQuote('"');
   options.SetSourceSize(explicit_length);
+  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==
  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -182,6 +183,7 @@
   options.SetProcessSP(process_sp);
   options.SetStream(&stream);
   options.SetSourceSize(explicit_length);
+  options.SetHasSourceSize(has_explicit_length);
   options.SetNeedsZeroTermination(false);
   options.SetIgnoreMaxLength(summary_options.GetCapping() ==
  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -199,6 +201,7 @@
 options.SetStream(&stream);
 options.SetQuote('"');
 options.SetSourceSize(explicit_length);
+options.SetHasSourceSize(has_explicit_length);
 options.SetIgnoreMaxLength(summary_options.GetCapping() ==
TypeSummaryCapping::eTypeSummaryUncapped);
 options.SetLanguage(summary_options.GetLanguage());
@@ -221,6 +224,7 @@
 options.SetStream(&stream);
 options.SetQuote('"');
 options.SetSourceSize(explicit_length);
+options.SetHasSourceSize(has_explicit_l

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

2020-03-18 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

On Fedora 31 x86_64 with LLDB using python3 I got:

  llvm-lit: .../llvm-monorepo2/llvm/utils/lit/lit/TestingConfig.py:102: fatal: 
unable to parse config file 
'.../llvm-monorepo2-clangassert/tools/lldb/test/Shell/lit.site.cfg.py', 
traceback: Traceback (most recent call last):
File ".../llvm-monorepo2/llvm/utils/lit/lit/TestingConfig.py", line 89, in 
load_from_path 
  exec(compile(data, path, 'exec'), cfg_globals, None)
File 
".../llvm-monorepo2-clangassert/tools/lldb/test/Shell/lit.site.cfg.py", line 
20, in 
  config.lldb_enable_debuginfod = TRUE
  NameError: name 'TRUE' is not defined
  make[3]: *** [tools/lldb/test/CMakeFiles/check-lldb-lit.dir/build.make:58: 
tools/lldb/test/CMakeFiles/check-lldb-lit] Error 2

It helped to change:

  -  set(Debuginfod_FOUND TRUE)
  +  set(Debuginfod_FOUND 1)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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


[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-03-18 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin added a comment.

In D74398#1929019 , @labath wrote:

> Thanks. Could you also add the other kind of test (the one inline asm) I 
> mentioned. In an ideal world we'd have a test case for every boundary 
> condition, but we're pretty far from that right now. Even so, one test case 
> like that would be nice.


Pavel, thank you for the careful review! I still do not quite understand how 
the asm test should look like and where it should live. Are you asking for 
creating a new directory with a compiled program below and building the 
machinery to stop at the right place and check the stack? If yes, that sounds 
like a fairly time-consuming undertaking, and I am afraid I cannot invest much 
more time into this. Perhaps it is better if we stick this patch only into our 
lldb branch, this one should be pretty easy for us to rebase.

As for the timings with local lldb on Linux, I see the time for jThreadsInfo of 
100 threads with fairly shallow stacks (10 entries or so) taking 20ms locally, 
as opposed to 4ms before this change. The jThreadsInfo reply size is 80kB, up 
from 11kB. While this seems excessive, I do not see any extra memory traffic 
for getting a thread list with the patch,  whereas without this patch we get 
over 100 roundtrips (each is at least 512 bytes of payload) to display the top 
of the stack of each thread.

> I am imagining the inferior doing something like:
> 
>   movabsq $0xdead, %rax
>   pushq %rax ; fake pc
>   leaq 0x1000(%rsp), %rbp ; larger than kMaxFrameSize
>   pushq %rbp
>   movq %rsp, %rbp
>   pushq $1 ; fake frame contents
>   pushq $2
>   pushq $3
>   
>   incq %rax
>   push %rax; second fake pc
>   pushq %rbp
>   movq %rsp, %rbp
>   pushq $4 ; fake frame contents
>   pushq $5
>   pushq $6
>   int3
> 
> 
> and then the test would assert that the result contains the entirety of the 
> first fake frame, the bp+pc of the second fake frame, and then would stop due 
> to hitting the kMaxFrameSize boundary.






Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:373
+# Read memory chunks from jThreadsInfo.
+memory_chunks = self.gather_threads_info_memory()
+# Check the chunks are correct.

labath wrote:
> Also assert that you have at least one chunk here?
I have asserted that for each thread now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398



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


[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251200.
wallace added a comment.

Addressed Greg's comments.

At the end I ended up not using APIs that use the format NAME=value, as they 
could be error prone. I think it's safer to use functions that work with 
explicit names and values.

Besides, I don't want to complicate this diff, so for now I'd prefer not to add 
setters for Platform and Target using the Environment class. We can add that 
later when we need it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Utility/Environment.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py

Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
===
--- /dev/null
+++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -0,0 +1,86 @@
+"""Test the SBEnvironment APIs."""
+
+
+
+from math import fabs
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+def forEachEntry(env, callback):
+for i in range(0, env.GetNumValues()):
+name = env.GetNameAtIndex(i)
+value = env.GetValueAtIndex(i)
+callback(name, value)
+
+
+class SBEnvironmentAPICase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+
+@add_test_categories(['pyapi'])
+def test_platform_environment(self):
+env = self.dbg.GetSelectedPlatform().GetEnvironment()
+# We assume at least PATH is set
+self.assertNotEqual(env.Get("PATH"), None)
+
+
+@add_test_categories(['pyapi'])
+def test_target_environment(self):
+env = self.dbg.GetSelectedTarget().GetEnvironment()
+# There is no target, so env should be empty
+self.assertEqual(env.GetNumValues(), 0)
+self.assertEqual(env.Get("PATH"), None)
+
+target = self.dbg.CreateTarget("")
+env = target.GetEnvironment()
+path = env.Get("PATH")
+# Now there's a target, so at least PATH should exist
+self.assertNotEqual(path, None)
+
+# Make sure we are getting a copy by modifying the env we just got
+env.Set("PATH", "#" + path, overwrite=True)
+self.assertEqual(target.GetEnvironment().Get("PATH"), path)
+
+@add_test_categories(['pyapi'])
+def test_creating_and_modifying_environment(self):
+env = lldb.SBEnvironment()
+
+self.assertEqual(env.Get("FOO"), None)
+self.assertEqual(env.Get("BAR"), None)
+
+# We also test empty values
+self.assertTrue(env.Set("FOO", "", overwrite=False))
+env.Set("BAR", "foo", overwrite=False)
+
+self.assertEqual(env.Get("FOO"), "")
+self.assertEqual(env.Get("BAR"), "foo")
+
+self.assertEqual(env.GetNumValues(), 2)
+
+forEachEntry(
+env, 
+lambda name, value:
+   self.assertIn(name + '=' + value, ["FOO=", "BAR=foo"])
+)
+
+# Make sure modifications work
+self.assertFalse(env.Set("FOO", "bar", overwrite=False))
+self.assertEqual(env.Get("FOO"), "")
+
+self.assertTrue(env.Set("FOO", "bar", overwrite=True))
+self.assertEqual(env.Get("FOO"), "bar")
+
+forEachEntry(
+env, 
+lambda name, value:
+   self.assertIn(name + '=' + value, ["FOO=bar", "BAR=foo"])
+)
+
+# Make sure we can unset
+self.assertTrue(env.Unset("FOO"))
+self.assertFalse(env.Unset("FOO"))
+self.assertEqual(env.Get("FOO"), None)
\ No newline at end of file
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -13,6 +13,7 @@
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBExpressionOptions.h"
 #include "lldb/API/SBFileSpec.h"
@@ -2388,6 +2389,17 @@
 m_opaque_sp->SetProcessLaunchInfo(launch_info.ref());
 }
 
+SBEnvironment SBTarget::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBTarget, GetEnvironment);
+  TargetSP target_sp(GetSP());
+
+  if (target_sp) {
+return LLDB_RECORD_RESULT(SBEnvironment(target_sp->GetEnviro

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251201.
wallace added a comment.

nit 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBPlatform.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interfaces.swig
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Utility/Environment.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py

Index: lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
===
--- /dev/null
+++ lldb/test/API/python_api/sbenvironment/TestSBEnvironment.py
@@ -0,0 +1,86 @@
+"""Test the SBEnvironment APIs."""
+
+
+
+from math import fabs
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+def forEachEntry(env, callback):
+for i in range(0, env.GetNumValues()):
+name = env.GetNameAtIndex(i)
+value = env.GetValueAtIndex(i)
+callback(name, value)
+
+
+class SBEnvironmentAPICase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+
+@add_test_categories(['pyapi'])
+def test_platform_environment(self):
+env = self.dbg.GetSelectedPlatform().GetEnvironment()
+# We assume at least PATH is set
+self.assertNotEqual(env.Get("PATH"), None)
+
+
+@add_test_categories(['pyapi'])
+def test_target_environment(self):
+env = self.dbg.GetSelectedTarget().GetEnvironment()
+# There is no target, so env should be empty
+self.assertEqual(env.GetNumValues(), 0)
+self.assertEqual(env.Get("PATH"), None)
+
+target = self.dbg.CreateTarget("")
+env = target.GetEnvironment()
+path = env.Get("PATH")
+# Now there's a target, so at least PATH should exist
+self.assertNotEqual(path, None)
+
+# Make sure we are getting a copy by modifying the env we just got
+env.Set("PATH", "#" + path, overwrite=True)
+self.assertEqual(target.GetEnvironment().Get("PATH"), path)
+
+@add_test_categories(['pyapi'])
+def test_creating_and_modifying_environment(self):
+env = lldb.SBEnvironment()
+
+self.assertEqual(env.Get("FOO"), None)
+self.assertEqual(env.Get("BAR"), None)
+
+# We also test empty values
+self.assertTrue(env.Set("FOO", "", overwrite=False))
+env.Set("BAR", "foo", overwrite=False)
+
+self.assertEqual(env.Get("FOO"), "")
+self.assertEqual(env.Get("BAR"), "foo")
+
+self.assertEqual(env.GetNumValues(), 2)
+
+forEachEntry(
+env, 
+lambda name, value:
+   self.assertIn(name + '=' + value, ["FOO=", "BAR=foo"])
+)
+
+# Make sure modifications work
+self.assertFalse(env.Set("FOO", "bar", overwrite=False))
+self.assertEqual(env.Get("FOO"), "")
+
+self.assertTrue(env.Set("FOO", "bar", overwrite=True))
+self.assertEqual(env.Get("FOO"), "bar")
+
+forEachEntry(
+env, 
+lambda name, value:
+   self.assertIn(name + '=' + value, ["FOO=bar", "BAR=foo"])
+)
+
+# Make sure we can unset
+self.assertTrue(env.Unset("FOO"))
+self.assertFalse(env.Unset("FOO"))
+self.assertEqual(env.Get("FOO"), None)
\ No newline at end of file
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -13,6 +13,7 @@
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBEnvironment.h"
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBExpressionOptions.h"
 #include "lldb/API/SBFileSpec.h"
@@ -2388,6 +2389,17 @@
 m_opaque_sp->SetProcessLaunchInfo(launch_info.ref());
 }
 
+SBEnvironment SBTarget::GetEnvironment() {
+  LLDB_RECORD_METHOD_NO_ARGS(lldb::SBEnvironment, SBTarget, GetEnvironment);
+  TargetSP target_sp(GetSP());
+
+  if (target_sp) {
+return LLDB_RECORD_RESULT(SBEnvironment(target_sp->GetEnvironment()));
+  }
+
+  return LLDB_RECORD_RESULT(SBEnvironment());
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -2643,6 +2655,7 @@
   LLDB_REGISTER_METHOD(lldb::SBInstructionList, SBTarget,
GetInstructionsWithFlavor,
(lldb::addr_t, cons

[Lldb-commits] [PATCH] D76314: [lldb-vscode] stop read loop after termination

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251204.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76314

Files:
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2821,7 +2821,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,22 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+self.create_debug_adaptor()
+# The underlying lldb-vscode process must be alive
+self.assertEqual(self.vscode.process.poll(), None)
+self.vscode.request_disconnect()
+time.sleep(1)
+# lldb-vscode process must have already finished even though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2821,7 +2821,7 @@
   }
   auto request_handlers = GetRequestHandlers();
   uint32_t packet_idx = 0;
-  while (true) {
+  while (!g_vsc.sent_terminated_event) {
 std::string json = g_vsc.ReadJSON();
 if (json.empty())
   break;
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import time
 import os
 
 
@@ -37,6 +38,22 @@
 
 @skipIfWindows
 @skipIfRemote
+def test_termination(self):
+'''
+Tests the correct termination of lldb-vscode upon a 'disconnect'
+request.
+'''
+self.create_debug_adaptor()
+# The underlying lldb-vscode process must be alive
+self.assertEqual(self.vscode.process.poll(), None)
+self.vscode.request_disconnect()
+time.sleep(1)
+# lldb-vscode process must have already finished even though
+# we didn't close the communication socket explicitly
+self.assertEqual(self.vscode.process.poll(), 0)
+
+@skipIfWindows
+@skipIfRemote
 def test_stopOnEntry(self):
 '''
 Tests the default launch of a simple program that stops at the
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D76216: Improve step over performance

2020-03-18 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin updated this revision to Diff 251208.
jarin added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76216

Files:
  lldb/source/Target/ThreadPlanStepOverRange.cpp


Index: lldb/source/Target/ThreadPlanStepOverRange.cpp
===
--- lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -171,6 +171,10 @@
   const SymbolContext &older_context =
   older_frame_sp->GetSymbolContext(eSymbolContextEverything);
   if (IsEquivalentContext(older_context)) {
+// If we have the  next-branch-breakpoint in the range, we can just
+// rely on that breakpoint to trigger once we return to the range.
+if (m_next_branch_bp_sp)
+  return false;
 new_plan_sp = m_thread.QueueThreadPlanForStepOutNoShouldStop(
 false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, 0,
 m_status, true);


Index: lldb/source/Target/ThreadPlanStepOverRange.cpp
===
--- lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -171,6 +171,10 @@
   const SymbolContext &older_context =
   older_frame_sp->GetSymbolContext(eSymbolContextEverything);
   if (IsEquivalentContext(older_context)) {
+// If we have the  next-branch-breakpoint in the range, we can just
+// rely on that breakpoint to trigger once we return to the range.
+if (m_next_branch_bp_sp)
+  return false;
 new_plan_sp = m_thread.QueueThreadPlanForStepOutNoShouldStop(
 false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, 0,
 m_status, true);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251235.
wallace added a comment.

Using the new SBEnvironment class


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636

Files:
  lldb/bindings/interface/SBLaunchInfo.i
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/test/API/tools/lldb-vscode/completions/main.cpp
  lldb/test/API/tools/lldb-vscode/environmentVariables/Makefile
  
lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
  lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -84,6 +84,11 @@
 "description": "Additional environment variables.",
 "default": []
 			},
+			"inheritEnvironment": {
+"type": "boolean",
+"description": "Inherit the debugger environment when launching a process. Only works for binaries launched directly by LLDB.",
+"default": false
+			},
 			"stopOnEntry": {
 "type": "boolean",
 "description": "Automatically stop after launch.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -40,6 +40,7 @@
 #include 
 #include 
 
+#include "lldb/API/SBEnvironment.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -1358,6 +1359,8 @@
   auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  bool launchWithDebuggerEnvironment =
+  GetBoolean(arguments, "inheritEnvironment", false);
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
@@ -1374,6 +1377,12 @@
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  // Configure the inherit environment setting
+  std::ostringstream oss;
+  oss << "settings set target.inherit-env "
+  << (launchWithDebuggerEnvironment ? "true" : "false");
+  g_vsc.RunLLDBCommands(llvm::StringRef(), {oss.str()});
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1398,10 +1407,13 @@
   if (!args.empty())
 g_vsc.launch_info.SetArguments(MakeArgv(args).data(), true);
 
-  // Pass any environment variables along that the user specified.
-  auto envs = GetStrings(arguments, "env");
-  if (!envs.empty())
-g_vsc.launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
+  // This mimics what CommandObjectProcess does when launching a process
+  lldb::SBEnvironment env = g_vsc.target.GetEnvironment();
+  for (const auto &name_and_value : GetStrings(arguments, "env")) {
+auto kv = llvm::StringRef(name_and_value).split("=");
+env.Set(kv.first.str().c_str(), kv.second.str().c_str(), true);
+  }
+  g_vsc.launch_info.SetEnvironment(env, true);
 
   auto flags = g_vsc.launch_info.GetLaunchFlags();
 
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
@@ -0,0 +1,15 @@
+#include 
+#include 
+#include 
+#include 
+
+extern char **environ;
+
+int main(int argc, char const *argv[], char const *envp[]) {
+  char **env_var_pointer = environ;
+  for (char *env_variable = *env_var_pointer; env_variable;
+   env_variable = *++env_var_pointer) {
+printf("%s\n", env_variable);
+  }
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
@@ -0,0 +1,72 @@
+"""
+Test lldb-vscode environment variables
+"""
+
+
+import lldbvscode_testcase
+import unittest2
+import vscode
+import os
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def getEnvOutputByProgram(self):
+env = {}
+for line in self.ge

[Lldb-commits] [PATCH] D74636: [lldb-vscode] Add inheritEnvironment option

2020-03-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 251236.
wallace added a comment.

improve a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636

Files:
  lldb/bindings/interface/SBLaunchInfo.i
  lldb/include/lldb/API/SBEnvironment.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/source/API/SBEnvironment.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/test/API/tools/lldb-vscode/completions/main.cpp
  lldb/test/API/tools/lldb-vscode/environmentVariables/Makefile
  
lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
  lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -84,6 +84,11 @@
 "description": "Additional environment variables.",
 "default": []
 			},
+			"inheritEnvironment": {
+"type": "boolean",
+"description": "Inherit the debugger environment when launching a process. Only works for binaries launched directly by LLDB.",
+"default": false
+			},
 			"stopOnEntry": {
 "type": "boolean",
 "description": "Automatically stop after launch.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -40,6 +40,7 @@
 #include 
 #include 
 
+#include "lldb/API/SBEnvironment.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -1358,6 +1359,8 @@
   auto launchCommands = GetStrings(arguments, "launchCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const auto debuggerRoot = GetString(arguments, "debuggerRoot");
+  bool launchWithDebuggerEnvironment =
+  GetBoolean(arguments, "inheritEnvironment", false);
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
@@ -1374,6 +1377,12 @@
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  // Configure the inherit environment setting
+  std::ostringstream oss;
+  oss << "settings set target.inherit-env "
+  << (launchWithDebuggerEnvironment ? "true" : "false");
+  g_vsc.RunLLDBCommands(llvm::StringRef(), {oss.str()});
+
   lldb::SBError status;
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
   if (status.Fail()) {
@@ -1398,10 +1407,13 @@
   if (!args.empty())
 g_vsc.launch_info.SetArguments(MakeArgv(args).data(), true);
 
-  // Pass any environment variables along that the user specified.
-  auto envs = GetStrings(arguments, "env");
-  if (!envs.empty())
-g_vsc.launch_info.SetEnvironmentEntries(MakeArgv(envs).data(), true);
+  // This mimics what CommandObjectProcess does when launching a process
+  lldb::SBEnvironment env = g_vsc.target.GetEnvironment();
+  for (const auto &name_and_value : GetStrings(arguments, "env")) {
+auto kv = llvm::StringRef(name_and_value).split("=");
+env.Set(kv.first.str().c_str(), kv.second.str().c_str(), true);
+  }
+  g_vsc.launch_info.SetEnvironment(env, true);
 
   auto flags = g_vsc.launch_info.GetLaunchFlags();
 
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/main.cpp
@@ -0,0 +1,15 @@
+#include 
+#include 
+#include 
+#include 
+
+extern char **environ;
+
+int main(int argc, char const *argv[], char const *envp[]) {
+  char **env_var_pointer = environ;
+  for (char *env_variable = *env_var_pointer; env_variable;
+   env_variable = *++env_var_pointer) {
+printf("%s\n", env_variable);
+  }
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py
@@ -0,0 +1,72 @@
+"""
+Test lldb-vscode environment variables
+"""
+
+
+import lldbvscode_testcase
+import unittest2
+import vscode
+import os
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestVSCode_variables(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def getEnvOutputByProgram(self):
+env = {}
+for line in self.get_stdout().encod

[Lldb-commits] [lldb] f0ca0a2 - [AppleObjCRuntimeV2] Rewrite GetClassDescriptor, reducing indentation.

2020-03-18 Thread Davide Italiano via lldb-commits

Author: Davide Italiano
Date: 2020-03-18T19:23:58-07:00
New Revision: f0ca0a25388066bd3605fe8ffc180e640d13c5a2

URL: 
https://github.com/llvm/llvm-project/commit/f0ca0a25388066bd3605fe8ffc180e640d13c5a2
DIFF: 
https://github.com/llvm/llvm-project/commit/f0ca0a25388066bd3605fe8ffc180e640d13c5a2.diff

LOG: [AppleObjCRuntimeV2] Rewrite GetClassDescriptor, reducing indentation.

I'm going to modify this function to account for lazily allocated
class names in the Obj-C runtime, but first I need to understand
what it does.

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 9b3dbb166b68..9fea9a217dce 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1191,33 +1191,33 @@ AppleObjCRuntimeV2::GetClassDescriptor(ValueObject 
&valobj) {
   // if we get an invalid VO (which might still happen when playing around with
   // pointers returned by the expression parser, don't consider this a valid
   // ObjC object)
-  if (valobj.GetCompilerType().IsValid()) {
-addr_t isa_pointer = valobj.GetPointerValue();
+  if (!valobj.GetCompilerType().IsValid())
+return objc_class_sp;
+  addr_t isa_pointer = valobj.GetPointerValue();
 
-// tagged pointer
-if (IsTaggedPointer(isa_pointer)) {
-  return m_tagged_pointer_vendor_up->GetClassDescriptor(isa_pointer);
-} else {
-  ExecutionContext exe_ctx(valobj.GetExecutionContextRef());
+  // tagged pointer
+  if (IsTaggedPointer(isa_pointer))
+return m_tagged_pointer_vendor_up->GetClassDescriptor(isa_pointer);
+  ExecutionContext exe_ctx(valobj.GetExecutionContextRef());
 
-  Process *process = exe_ctx.GetProcessPtr();
-  if (process) {
-Status error;
-ObjCISA isa = process->ReadPointerFromMemory(isa_pointer, error);
-if (isa != LLDB_INVALID_ADDRESS) {
-  objc_class_sp = GetClassDescriptorFromISA(isa);
-  if (isa && !objc_class_sp) {
-Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS |
-  LIBLLDB_LOG_TYPES));
-LLDB_LOGF(log,
-  "0x%" PRIx64
-  ": AppleObjCRuntimeV2::GetClassDescriptor() ISA was "
-  "not in class descriptor cache 0x%" PRIx64,
-  isa_pointer, isa);
-  }
-}
-  }
-}
+  Process *process = exe_ctx.GetProcessPtr();
+  if (!process)
+return objc_class_sp;
+
+  Status error;
+  ObjCISA isa = process->ReadPointerFromMemory(isa_pointer, error);
+  if (isa == LLDB_INVALID_ADDRESS)
+return objc_class_sp;
+
+  objc_class_sp = GetClassDescriptorFromISA(isa);
+  if (isa && !objc_class_sp) {
+Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS |
+  LIBLLDB_LOG_TYPES));
+LLDB_LOGF(log,
+  "0x%" PRIx64
+  ": AppleObjCRuntimeV2::GetClassDescriptor() ISA was "
+  "not in class descriptor cache 0x%" PRIx64,
+  isa_pointer, isa);
   }
   return objc_class_sp;
 }



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


[Lldb-commits] [lldb] 52b2bae - [lldb/testsuite] Skip TestEmptyStdModule.py if using a remote platform

2020-03-18 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-03-18T20:52:28-07:00
New Revision: 52b2bae777f2a30d1ed6e87c8812bbffc4f4feeb

URL: 
https://github.com/llvm/llvm-project/commit/52b2bae777f2a30d1ed6e87c8812bbffc4f4feeb
DIFF: 
https://github.com/llvm/llvm-project/commit/52b2bae777f2a30d1ed6e87c8812bbffc4f4feeb.diff

LOG: [lldb/testsuite] Skip TestEmptyStdModule.py if using a remote platform

The test runs `platform select host`, so it make no sense to run it
when remote debugging.

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py
index 76e79df5cd1c..2b1cb100a325 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/empty-module/TestEmptyStdModule.py
@@ -15,6 +15,7 @@ class ImportStdModule(TestBase):
 # but we still add the libc++ category so that this test is only run in
 # test configurations where libc++ is actually supposed to be tested.
 @add_test_categories(["libc++"])
+@skipIfRemote
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()



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


[Lldb-commits] [lldb] 59918d3 - [lldb/testsuite] Make TestObjCIvarStripped.py working with codesigning

2020-03-18 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-03-18T20:52:28-07:00
New Revision: 59918d3793a1136e7041b1a76f38a42cf8644474

URL: 
https://github.com/llvm/llvm-project/commit/59918d3793a1136e7041b1a76f38a42cf8644474
DIFF: 
https://github.com/llvm/llvm-project/commit/59918d3793a1136e7041b1a76f38a42cf8644474.diff

LOG: [lldb/testsuite] Make TestObjCIvarStripped.py working with codesigning

This test was stripping a binary generated by Makefile.rules which is
potentially codesigned. Stripping invalidates the code signature, so
we might need to re-sign after stripping.

Added: 


Modified: 
lldb/test/API/lang/objc/objc-ivar-stripped/Makefile

Removed: 




diff  --git a/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile 
b/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile
index 0aaa021132e1..8b63215d6d9d 100644
--- a/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile
+++ b/lldb/test/API/lang/objc/objc-ivar-stripped/Makefile
@@ -3,7 +3,10 @@ LD_EXTRAS := -lobjc -framework Foundation
 
 all:a.out.stripped
 
+include Makefile.rules
+
 a.out.stripped: a.out.dSYM
strip -o a.out.stripped a.out
-
-include Makefile.rules
+ifneq "$(CODESIGN)" ""
+   $(CODESIGN) -fs - a.out.stripped
+endif



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


[Lldb-commits] [lldb] c182be2 - [lldb/testsuite] Tweak TestBreakpointLocations.py to pass for arm64

2020-03-18 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-03-18T20:52:28-07:00
New Revision: c182be211a4d1a79390703ede8f041dcbaaf7947

URL: 
https://github.com/llvm/llvm-project/commit/c182be211a4d1a79390703ede8f041dcbaaf7947
DIFF: 
https://github.com/llvm/llvm-project/commit/c182be211a4d1a79390703ede8f041dcbaaf7947.diff

LOG: [lldb/testsuite] Tweak TestBreakpointLocations.py to pass for arm64

The test checks that we correctly set the right number of breakpoints
when breaking into an `always_inline` function. The line of this
funstion selected for this test was the return statement, but with
recent compiler, this return statement doesn't necessarily exist after
inlining, even at O0.

Switch the breakpoint to a different line of the inline function.

Added: 


Modified: 
lldb/test/API/functionalities/breakpoint/breakpoint_locations/main.c

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/main.c 
b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/main.c
index 7ec3ded67b74..f6ccb031c744 100644
--- a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/main.c
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/main.c
@@ -14,9 +14,9 @@ func_inlined (void)
 {
 static int func_inline_call_count = 0;
 printf ("Called func_inlined.\n");
-++func_inline_call_count;
+++func_inline_call_count;  // Set break point at this line.
 printf ("Returning func_inlined call count: %d.\n", 
func_inline_call_count);
-return func_inline_call_count; // Set break point at this line.
+return func_inline_call_count;
 }
 
 extern int func_inlined (void);



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


[Lldb-commits] [lldb] 71db787 - [lldb/testsuite] Rewrite TestThreadLocal.py

2020-03-18 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-03-18T20:52:28-07:00
New Revision: 71db787c4583b5b05b9066509c36eb996924aeda

URL: 
https://github.com/llvm/llvm-project/commit/71db787c4583b5b05b9066509c36eb996924aeda
DIFF: 
https://github.com/llvm/llvm-project/commit/71db787c4583b5b05b9066509c36eb996924aeda.diff

LOG: [lldb/testsuite] Rewrite TestThreadLocal.py

It was an inline test before. Clang stopped emitting line information
for the TLS initialization and the inline test didn't have a way to
break before it anymore.

This rewrites the test as a full-fldeged python test and improves the
checking of the error case to verify that the failure we are looking
for is related to the TLS setup not being complete.

Added: 


Modified: 
lldb/test/API/lang/cpp/thread_local/TestThreadLocal.py
lldb/test/API/lang/cpp/thread_local/main.cpp

Removed: 




diff  --git a/lldb/test/API/lang/cpp/thread_local/TestThreadLocal.py 
b/lldb/test/API/lang/cpp/thread_local/TestThreadLocal.py
index 5152c0010d10..e7cfa1ca14f2 100644
--- a/lldb/test/API/lang/cpp/thread_local/TestThreadLocal.py
+++ b/lldb/test/API/lang/cpp/thread_local/TestThreadLocal.py
@@ -1,6 +1,49 @@
-from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-lldbinline.MakeInlineTest(__file__, globals(),
-  lldbinline.expectedFailureAll(oslist=[
-  "windows", "linux", "netbsd"]))
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbtest
+
+
+class PlatformProcessCrashInfoTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@expectedFailureAll(oslist=["windows", "linux", "netbsd"])
+def test_thread_local(self):
+# Set a breakpoint on the first instruction of the main function,
+# before the TLS initialization has run.
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+(target, process, _, _) = \
+lldbutil.run_to_source_breakpoint(self, "Set breakpoint here",
+  lldb.SBFileSpec("main.cpp"))
+self.expect_expr("tl_local_int + 1",
+ result_type="int", result_value="323")
+self.expect_expr("*tl_local_ptr + 2",
+ result_type="int", result_value="324")
+self.expect_expr("tl_global_int",
+ result_type="int", result_value="123")
+self.expect_expr("*tl_global_ptr",
+ result_type="int", result_value="45")
+
+# Now see if we emit the correct error when the TLS is not yet
+# initialized. Let's set a breakpoint on the first instruction
+# of main.
+main_module = target.FindModule(lldb.SBFileSpec(exe))
+main_address = main_module.FindSymbol("main").GetStartAddress()
+main_bkpt = target.BreakpointCreateBySBAddress(main_address)
+
+process.Kill()
+lldbutil.run_to_breakpoint_do_run(self, target, main_bkpt)
+
+self.expect("expr tl_local_int", error=True,
+substrs=["couldn't get the value of variable tl_local_int",
+ "No TLS data currently exists for this thread"])
+self.expect("expr *tl_local_ptr", error=True,
+substrs=["couldn't get the value of variable tl_local_ptr",
+ "No TLS data currently exists for this thread"])
+

diff  --git a/lldb/test/API/lang/cpp/thread_local/main.cpp 
b/lldb/test/API/lang/cpp/thread_local/main.cpp
index 1855b7c5f344..04c7fc0ed74d 100644
--- a/lldb/test/API/lang/cpp/thread_local/main.cpp
+++ b/lldb/test/API/lang/cpp/thread_local/main.cpp
@@ -3,15 +3,9 @@ thread_local int tl_global_int = 123;
 thread_local int *tl_global_ptr = &storage;
 
 int main(int argc, char **argv) {
-  //% self.expect("expr tl_local_int", error=True, substrs=["couldn't get the 
value of variable tl_local_int"])
-  //% self.expect("expr *tl_local_ptr", error=True, substrs=["couldn't get the 
value of variable tl_local_ptr"])
   thread_local int tl_local_int = 321;
   thread_local int *tl_local_ptr = nullptr;
   tl_local_ptr = &tl_local_int;
   tl_local_int++;
-  //% self.expect("expr tl_local_int + 1", substrs=["int", "= 323"])
-  //% self.expect("expr *tl_local_ptr + 2", substrs=["int", "= 324"])
-  //% self.expect("expr tl_global_int", substrs=["int", "= 123"])
-  //% self.expect("expr *tl_global_ptr", substrs=["int", "= 45"])
-  return 0;
+  return 0; // Set breakpoint here
 }



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


[Lldb-commits] [lldb] 127b9d9 - [lldb/testsuite] Apply @skipIfDarwinEmbedded to part of TestHWBreakMultiThread

2020-03-18 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-03-18T20:52:28-07:00
New Revision: 127b9d9d774dcc593cfd50eccde307dbe96097a2

URL: 
https://github.com/llvm/llvm-project/commit/127b9d9d774dcc593cfd50eccde307dbe96097a2
DIFF: 
https://github.com/llvm/llvm-project/commit/127b9d9d774dcc593cfd50eccde307dbe96097a2.diff

LOG: [lldb/testsuite] Apply @skipIfDarwinEmbedded to part of 
TestHWBreakMultiThread

The comment in the test wrongfully claimed that we support hardware
breakpoints on darwin for arm64, but we never did.

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
index f778b8e39e72..a4a1d9effbe1 100644
--- 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
+++ 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py
@@ -32,19 +32,21 @@ def test_hw_break_set_disable_multi_thread_linux(self):
 self.setTearDownCleanup()
 self.break_multi_thread('disable', False) # llvm.org/PR44659
 
-# LLDB on darwin supports hardware breakpoints for arm, aarch64, x86_64 and
-# i386 architectures.
+# LLDB on darwin supports hardware breakpoints for x86_64 and i386
+# architectures.
 @skipUnlessDarwin
 @skipIfOutOfTreeDebugserver
+@skipIfDarwinEmbedded
 def test_hw_break_set_delete_multi_thread_macos(self):
 self.build()
 self.setTearDownCleanup()
 self.break_multi_thread('delete')
 
-# LLDB on darwin supports hardware breakpoints for arm, aarch64, x86_64 and
-# i386 architectures.
+# LLDB on darwin supports hardware breakpoints for x86_64 and i386
+# architectures.
 @skipUnlessDarwin
 @skipIfOutOfTreeDebugserver
+@skipIfDarwinEmbedded
 def test_hw_break_set_disable_multi_thread_macos(self):
 self.build()
 self.setTearDownCleanup()



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


[Lldb-commits] [lldb] acd641c - [lldb/testsuite] Slightly rework TestHiddenIvars.py

2020-03-18 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-03-18T20:52:28-07:00
New Revision: acd641c19d687c7117b08cdd568a91a381043ebb

URL: 
https://github.com/llvm/llvm-project/commit/acd641c19d687c7117b08cdd568a91a381043ebb
DIFF: 
https://github.com/llvm/llvm-project/commit/acd641c19d687c7117b08cdd568a91a381043ebb.diff

LOG: [lldb/testsuite] Slightly rework TestHiddenIvars.py

The test was stripping the binaries from the Python
code. Unfortunately, if running on darwin embedded in a context that
requires code signing, the stripping was invalidating the signature,
thus breaking the test.

This patch moves the stripping to the Makefile and resigns the
stripped binaries if required.

Added: 


Modified: 
lldb/test/API/lang/objc/hidden-ivars/Makefile
lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py

Removed: 




diff  --git a/lldb/test/API/lang/objc/hidden-ivars/Makefile 
b/lldb/test/API/lang/objc/hidden-ivars/Makefile
index 0664769456ef..283e8a118fb1 100644
--- a/lldb/test/API/lang/objc/hidden-ivars/Makefile
+++ b/lldb/test/API/lang/objc/hidden-ivars/Makefile
@@ -4,4 +4,24 @@ OBJC_SOURCES := main.m
 
 LD_EXTRAS = -framework Foundation
 
+all: a.out libInternalDefiner.dylib stripped
+
 include Makefile.rules
+
+ifeq "$(MAKE_DSYM)" "YES"
+stripped: a.out.dSYM
+endif
+
+stripped: a.out libInternalDefiner.dylib
+   mkdir stripped
+   strip -Sx a.out -o stripped/a.out
+   strip -Sx libInternalDefiner.dylib -o stripped/libInternalDefiner.dylib
+ifneq "$(CODESIGN)" ""
+   $(CODESIGN) -fs - stripped/a.out
+endif
+ifneq "$(CODESIGN)" ""
+   $(CODESIGN) -fs - stripped/libInternalDefiner.dylib
+endif
+ifeq "$(MAKE_DSYM)" "YES"
+   cp -r a.out.dSYM stripped/a.out.dSYM
+endif

diff  --git a/lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py 
b/lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py
index 03a325ac49c6..5930ffdc958a 100644
--- a/lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py
+++ b/lldb/test/API/lang/objc/hidden-ivars/TestHiddenIvars.py
@@ -80,20 +80,11 @@ def test_frame_variable_across_modules(self):
 def common_setup(self, strip):
 
 if strip:
-self.assertTrue(subprocess.call(
-['/usr/bin/strip', '-Sx',
- self.getBuildArtifact('libInternalDefiner.dylib')]) == 0,
-'stripping dylib succeeded')
-self.assertTrue(subprocess.call(
-['/bin/rm', '-rf',
- self.getBuildArtifact('libInternalDefiner.dylib.dSYM')]) == 0,
-'remove dylib dSYM file succeeded')
-self.assertTrue(subprocess.call(['/usr/bin/strip', '-Sx',
- self.getBuildArtifact("a.out")
-]) == 0,
-'stripping a.out succeeded')
+exe = self.getBuildArtifact("stripped/a.out")
+else:
+exe = self.getBuildArtifact("a.out")
 # Create a target by the debugger.
-target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+target = self.dbg.CreateTarget(exe)
 self.assertTrue(target, VALID_TARGET)
 
 # Create the breakpoint inside function 'main'.
@@ -110,7 +101,6 @@ def common_setup(self, strip):
 None, environment, self.get_process_working_directory())
 self.assertTrue(process, PROCESS_IS_VALID)
 
-exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
 # Break inside the foo function which takes a bar_ptr argument.



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


[Lldb-commits] [PATCH] D76406: [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision.
friss added reviewers: labath, jingham.
Herald added subscribers: danielkiss, kristof.beyls.
Herald added a project: LLDB.

TestInlineStepping tests LLDB's ability to step in the presence of
inline frames. The testcase source has a number of functions and some
of them are marked `always_inline`.

The test is built around the assumption that the inline function will
be fully represented once inlined, but this is not true with the
current arm64 code generation. For example:

void caller() {

  always_inline_function(); // Step here

}

When stppeing into `caller()` above, you might immediatly end up in
the inlines frame for `always_inline_function()`, because there might
literally be no code associated with `caller()` itself.

This patch hacks around the issue by adding an `asm volatile("nop")`
on some lines with inlined calls where we expect to be able to
step. Like so:

void caller() {

  asm volatile("nop"); always_inline_function(); // Step here

}

This guarantees there is always going to be one instruction for this
line in the caller.

Any other (better) idea how to deal with this?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76406

Files:
  lldb/test/API/functionalities/inline-stepping/calling.cpp


Index: lldb/test/API/functionalities/inline-stepping/calling.cpp
===
--- lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -75,7 +75,7 @@
 void
 caller_trivial_2 ()
 {
-inline_trivial_1 (); // In caller_trivial_2.
+asm volatile ("nop"); inline_trivial_1 (); // In caller_trivial_2.
 inline_value += 1;  // At increment in caller_trivial_2.
 }
 
@@ -88,7 +88,7 @@
 void
 inline_trivial_1 ()
 {
-inline_trivial_2(); // In inline_trivial_1.
+asm volatile ("nop"); inline_trivial_2(); // In inline_trivial_1.
 inline_value += 1;  // At increment in inline_trivial_1.
 }
 


Index: lldb/test/API/functionalities/inline-stepping/calling.cpp
===
--- lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -75,7 +75,7 @@
 void
 caller_trivial_2 ()
 {
-inline_trivial_1 (); // In caller_trivial_2.
+asm volatile ("nop"); inline_trivial_1 (); // In caller_trivial_2.
 inline_value += 1;  // At increment in caller_trivial_2.
 }
 
@@ -88,7 +88,7 @@
 void
 inline_trivial_1 ()
 {
-inline_trivial_2(); // In inline_trivial_1.
+asm volatile ("nop"); inline_trivial_2(); // In inline_trivial_1.
 inline_value += 1;  // At increment in inline_trivial_1.
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision.
friss added reviewers: mib, jingham.
Herald added a project: LLDB.

The crash info annotations are not necessarily indicative of a
crash. Some APIs will set them before doing something risk and
clean them up after, while some others might just leave a breadcrumb
there for the whole dureation of the process.

The latter happens for dyld on iOS. Dependeing on how the process is
launched, it will leave some information it its crash_info annotations
about its configuration.

This makes the test fail because it expects to find an empty crash
info struct when the process is running happily. This patch just
deletes this test, but I'm happy to consider other alternatives.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76407

Files:
  lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py


Index: lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
===
--- lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
+++ lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
@@ -69,21 +69,3 @@
 
 self.assertIn("pointer being freed was not allocated", 
stream.GetData())
 
-def test_on_sane_process(self):
-"""Test that lldb doesn't fetch the extended crash information
-dictionnary from a 'sane' stopped process."""
-self.build()
-target, _, _, _ = lldbutil.run_to_line_breakpoint(self, 
lldb.SBFileSpec(self.source),
-self.line)
-
-stream = lldb.SBStream()
-self.assertTrue(stream)
-
-process = target.GetProcess()
-self.assertTrue(process)
-
-crash_info = process.GetExtendedCrashInformation()
-
-error = crash_info.GetAsJSON(stream)
-self.assertFalse(error.Success())
-self.assertIn("No structured data.", error.GetCString())


Index: lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
===
--- lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
+++ lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
@@ -69,21 +69,3 @@
 
 self.assertIn("pointer being freed was not allocated", stream.GetData())
 
-def test_on_sane_process(self):
-"""Test that lldb doesn't fetch the extended crash information
-dictionnary from a 'sane' stopped process."""
-self.build()
-target, _, _, _ = lldbutil.run_to_line_breakpoint(self, lldb.SBFileSpec(self.source),
-self.line)
-
-stream = lldb.SBStream()
-self.assertTrue(stream)
-
-process = target.GetProcess()
-self.assertTrue(process)
-
-crash_info = process.GetExtendedCrashInformation()
-
-error = crash_info.GetAsJSON(stream)
-self.assertFalse(error.Success())
-self.assertIn("No structured data.", error.GetCString())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D76408: [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision.
friss added a reviewer: labath.
Herald added subscribers: danielkiss, kristof.beyls.
Herald added a project: LLDB.

TestBuiltinTrap fail on darwin embedded because the `__builin_trap`
builtin doesn't get any line info attached to it by clang when
building for arm64.

The test was already XFailed for linux arm(64), I presume for the same
reasons. This patch just XFails it independently of the platform.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76408

Files:
  lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py


Index: lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
===
--- lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
+++ lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
@@ -23,7 +23,7 @@
 
 # gcc generates incorrect linetable
 @expectedFailureAll(archs="arm", compiler="gcc", triple=".*-android")
-@expectedFailureAll(oslist=['linux'], archs=['arm', 'aarch64'])
+@expectedFailureAll(archs=['arm', 'aarch64'])
 @skipIfWindows
 def test_with_run_command(self):
 """Test that LLDB handles a function with __builtin_trap correctly."""


Index: lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
===
--- lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
+++ lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
@@ -23,7 +23,7 @@
 
 # gcc generates incorrect linetable
 @expectedFailureAll(archs="arm", compiler="gcc", triple=".*-android")
-@expectedFailureAll(oslist=['linux'], archs=['arm', 'aarch64'])
+@expectedFailureAll(archs=['arm', 'aarch64'])
 @skipIfWindows
 def test_with_run_command(self):
 """Test that LLDB handles a function with __builtin_trap correctly."""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 5d881dd - Update so debugserver can be built on macos again with xcodebuild.

2020-03-18 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2020-03-18T21:46:20-07:00
New Revision: 5d881dd8a8b8ea6f80bf4ef5b900ca006dacd9bf

URL: 
https://github.com/llvm/llvm-project/commit/5d881dd8a8b8ea6f80bf4ef5b900ca006dacd9bf
DIFF: 
https://github.com/llvm/llvm-project/commit/5d881dd8a8b8ea6f80bf4ef5b900ca006dacd9bf.diff

LOG: Update so debugserver can be built on macos again with xcodebuild.

Added: 


Modified: 
lldb/tools/debugserver/debugserver.xcodeproj/project.pbxproj

Removed: 




diff  --git a/lldb/tools/debugserver/debugserver.xcodeproj/project.pbxproj 
b/lldb/tools/debugserver/debugserver.xcodeproj/project.pbxproj
index f4267b7633a2..1c7a55f7108a 100644
--- a/lldb/tools/debugserver/debugserver.xcodeproj/project.pbxproj
+++ b/lldb/tools/debugserver/debugserver.xcodeproj/project.pbxproj
@@ -7,131 +7,165 @@
objects = {
 
 /* Begin PBXBuildFile section */
-   23562ED61D342A5A00AB2BD4 /* ActivityStore.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 23562ED51D342A5A00AB2BD4 /* ActivityStore.cpp 
*/; };
-   23562ED71D342A5A00AB2BD4 /* ActivityStore.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 23562ED51D342A5A00AB2BD4 /* ActivityStore.cpp 
*/; };
-   26CE05C5115C36590022F371 /* CFBundle.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 2695DD910D3EBFF6007E4CA2 /* CFBundle.cpp */; };
-   456F67641AD46CE9002850C2 /* CFBundle.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 2695DD910D3EBFF6007E4CA2 /* CFBundle.cpp */; };
-   26CE05C3115C36580022F371 /* CFString.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 2695DD9B0D3EC160007E4CA2 /* CFString.cpp */; };
-   456F67621AD46CE9002850C2 /* CFString.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 2695DD9B0D3EC160007E4CA2 /* CFString.cpp */; };
-   26CE05CF115C36F70022F371 /* CoreFoundation.framework in 
Frameworks */ = {isa = PBXBuildFile; fileRef = 26ACA3340D3E956300A2120B /* 
CoreFoundation.framework */; settings = {ATTRIBUTES = (Required, ); }; };
-   456F676B1AD46CE9002850C2 /* CoreFoundation.framework in 
Frameworks */ = {isa = PBXBuildFile; fileRef = 26ACA3340D3E956300A2120B /* 
CoreFoundation.framework */; settings = {ATTRIBUTES = (Required, ); }; };
-   26CE05B7115C363B0022F371 /* DNB.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 26C637D60C71334A0024798E /* DNB.cpp */; };
-   456F67551AD46CE9002850C2 /* DNB.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 26C637D60C71334A0024798E /* DNB.cpp */; };
-   264D5D581293835600ED4C01 /* DNBArch.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 264D5D571293835600ED4C01 /* DNBArch.cpp */; };
-   456F67671AD46CE9002850C2 /* DNBArch.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 264D5D571293835600ED4C01 /* DNBArch.cpp */; };
-   26CE05C1115C36510022F371 /* DNBArchImpl.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 2675D4220CCEB705000F49AF /* DNBArchImpl.cpp */; 
};
-   26CE05C2115C36550022F371 /* DNBArchImpl.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26C637FB0C71334A0024798E /* DNBArchImpl.cpp */; 
};
-   456F67601AD46CE9002850C2 /* DNBArchImpl.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 2675D4220CCEB705000F49AF /* DNBArchImpl.cpp */; 
};
-   456F67611AD46CE9002850C2 /* DNBArchImpl.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26C637FB0C71334A0024798E /* DNBArchImpl.cpp */; 
};
-   266B5ED11460A68200E43F0A /* DNBArchImplARM64.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 266B5ECF1460A68200E43F0A /* 
DNBArchImplARM64.cpp */; };
-   456F67691AD46CE9002850C2 /* DNBArchImplARM64.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 266B5ECF1460A68200E43F0A /* 
DNBArchImplARM64.cpp */; };
-   26CE05C0115C364F0022F371 /* DNBArchImplI386.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26C637EA0C71334A0024798E /* DNBArchImplI386.cpp 
*/; };
-   456F675F1AD46CE9002850C2 /* DNBArchImplI386.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26C637EA0C71334A0024798E /* DNBArchImplI386.cpp 
*/; };
-   26CE05BF115C364D0022F371 /* DNBArchImplX86_64.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 26CF99A21142EB7400011AAB /* 
DNBArchImplX86_64.cpp */; };
-   456F675E1AD46CE9002850C2 /* DNBArchImplX86_64.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 26CF99A21142EB7400011AAB /* 
DNBArchImplX86_64.cpp */; };
-   26CE05B8115C363C0022F371 /* DNBBreakpoint.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26C637D90C71334A0024798E /* DNBBreakpoint.cpp 
*/; };
-   456F67571AD46CE9002850C2 /* DNBBreakpoint.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26C637D90C71334A0024798E /* DNBBreakpoint.cpp 
*/; };
-   26CE05B9115C363D0022F371 /* DNBDataRef.cpp in Sources */ = {isa 
= PBX

[Lldb-commits] [PATCH] D76411: [debugserver] Implement hardware breakpoints for ARM64

2020-03-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added subscribers: danielkiss, kristof.beyls.

Add support for hardware breakpoints on ARM64


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D76411

Files:
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h

Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.h
@@ -26,10 +26,12 @@
 
   DNBArchMachARM64(MachThread *thread)
   : m_thread(thread), m_state(), m_disabled_watchpoints(),
-m_watchpoint_hw_index(-1), m_watchpoint_did_occur(false),
+m_disabled_breakpoints(), m_watchpoint_hw_index(-1),
+m_watchpoint_did_occur(false),
 m_watchpoint_resume_single_step_enabled(false),
 m_saved_register_states() {
 m_disabled_watchpoints.resize(16);
+m_disabled_breakpoints.resize(16);
 memset(&m_dbg_save, 0, sizeof(m_dbg_save));
   }
 
@@ -62,7 +64,13 @@
   static const uint8_t *SoftwareBreakpointOpcode(nub_size_t byte_size);
   static uint32_t GetCPUType();
 
+  virtual uint32_t NumSupportedHardwareBreakpoints();
   virtual uint32_t NumSupportedHardwareWatchpoints();
+
+  virtual uint32_t EnableHardwareBreakpoint(nub_addr_t addr, nub_size_t size,
+bool also_set_on_task);
+  virtual bool DisableHardwareBreakpoint(uint32_t hw_break_index,
+ bool also_set_on_task);
   virtual uint32_t EnableHardwareWatchpoint(nub_addr_t addr, nub_size_t size,
 bool read, bool write,
 bool also_set_on_task);
@@ -229,10 +237,11 @@
   State m_state;
   arm_debug_state64_t m_dbg_save;
 
-  // arm64 doesn't keep the disabled watchpoint values in the debug register
-  // context like armv7;
+  // arm64 doesn't keep the disabled watchpoint and breakpoint values in the
+  // debug register context like armv7;
   // we need to save them aside when we disable them temporarily.
   std::vector m_disabled_watchpoints;
+  std::vector m_disabled_breakpoints;
 
   // The following member variables should be updated atomically.
   int32_t m_watchpoint_hw_index;
Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -666,6 +666,112 @@
   return g_num_supported_hw_watchpoints;
 }
 
+uint32_t DNBArchMachARM64::NumSupportedHardwareBreakpoints() {
+  // Set the init value to something that will let us know that we need to
+  // autodetect how many breakpoints are supported dynamically...
+  static uint32_t g_num_supported_hw_breakpoints = UINT_MAX;
+  if (g_num_supported_hw_breakpoints == UINT_MAX) {
+// Set this to zero in case we can't tell if there are any HW breakpoints
+g_num_supported_hw_breakpoints = 0;
+
+size_t len;
+uint32_t n = 0;
+len = sizeof(n);
+if (::sysctlbyname("hw.optional.breakpoint", &n, &len, NULL, 0) == 0) {
+  g_num_supported_hw_breakpoints = n;
+  DNBLogThreadedIf(LOG_THREAD, "hw.optional.breakpoint=%u", n);
+} else {
+// For AArch64 we would need to look at ID_AA64DFR0_EL1 but debugserver runs in
+// EL0 so it can't access that reg.  The kernel should have filled in the
+// sysctls based on it though.
+#if defined(__arm__)
+  uint32_t register_DBGDIDR;
+
+  asm("mrc p14, 0, %0, c0, c0, 0" : "=r"(register_DBGDIDR));
+  uint32_t numWRPs = bits(register_DBGDIDR, 31, 28);
+  // Zero is reserved for the WRP count, so don't increment it if it is zero
+  if (numWRPs > 0)
+numWRPs++;
+  g_num_supported_hw_breakpoints = numWRPs;
+  DNBLogThreadedIf(LOG_THREAD,
+   "Number of supported hw breakpoint via asm():  %d",
+   g_num_supported_hw_breakpoints);
+#endif
+}
+  }
+  return g_num_supported_hw_breakpoints;
+}
+
+uint32_t DNBArchMachARM64::EnableHardwareBreakpoint(nub_addr_t addr,
+nub_size_t size,
+bool also_set_on_task) {
+  DNBLogThreadedIf(LOG_WATCHPOINTS,
+   "DNBArchMachARM64::EnableHardwareBreakpoint(addr = "
+   "0x%8.8llx, size = %zu)",
+   (uint64_t)addr, size);
+
+  const uint32_t num_hw_breakpoints = NumSupportedHardwareBreakpoints();
+
+  nub_addr_t aligned_bp_address = addr;
+  uint32_t control_value = 0;
+
+  switch (size) {
+  case 2:
+control_value = (0x3 << 5) | 7;
+aligned_bp_address &= ~1;
+break;
+  case 4:
+control_value 

[Lldb-commits] [PATCH] D76216: Improve step over performance

2020-03-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Jim is the one that really needs to mark this as accepted as the thread plans 
are one of his many areas of expertise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76216



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