[Lldb-commits] [PATCH] D77043: Fix process gdb-remote usage of value_regs/invalidate_regs

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid marked 2 inline comments as done.
omjavaid added a comment.

In D77043#2031339 , @labath wrote:

> The invalidate_regs part looks as I would expect. I think it ought to be 
> implemented a bit differently, but that can wait until the bigger issue is 
> resolved.
>
> The bigger issue being the fact that this patch now renumbers all numbers 
> going in/out of the stub (qRegisterInfo, p,P, etc.). It seems I got more than 
> what I bargained for there, though in retrospect, that does not seem totally 
> unexpected, as you were mentioning the problem of the extra dbg registers in 
> the middle of the register list. The part which I didn't get is that this 
> does not only apply to the "lldb" register list, but that these registers 
> also make their way to 
> `NativeRegisterContextLinux_arm64::GetRegisterInfoAtIndex`. Currently that 
> class was culling them (in the same way as other register contexts do for 
> "optional" registers) by returning a smaller value via GetUserRegisterCount. 
> That is, of course, not possible if you have other registers in the list that 
> you want to make available.


I have tried other solution like overriding GetRegisterInfoAtIndex for 
NativeRegisterContextLinux_arm64 and doing all this register number magic their 
but then this culminated into lots of changes related to register sets and the 
currently presented solution looked cleaner than others.
In case of all registers other than SVE registers UserRegIndexToRegInfosIndex 
is going to do nothing but return the input as ouput. So using this approach 
doesnt really impact anything else other than AArch64+SVE.

> So, IIUC, what the current patch does is expose the registers through 
> GetRegisterInfoAtIndex, but then ensures that the 
> `UserRegIndexToRegInfosIndex` conversion skips over them. That wouldn't be 
> too bad were it not for the fact that accessing the register "the right way" 
> is very hard now.

The problem we have is that qRegisterInfo packets uses its own register 
numbering scheme and we dont have mechanism to push those register numbers 
through to registerinfos list. lldb register numbers are actually indices and 
not register number so to speak.

> So here's my current doubt. Basically, the main thing this patch does is 
> "hide" the uninteresting (debug) registers from the client. However, those 
> registers still remain exposed through the 
> NativeRegisterContext<->GDBRemoteCommunicationServerLLGS boundary via the 
> `GetRegisterInfoAtIndex` function. What if we took this one step further, and 
> made is so that the debug registers are not exposed from 
> NativeRegisterContextLinux_arm64 at all?

The only problem/hurdle we have here is the way we construct our register sets 
from the indices of registerinfos array which is static. This will require some 
reshuffling of register sets a revamp of how sets are created in the register 
context. Any solution that requires changes to register sets will be more 
extensive than this.

> One way to achieve that would be to handle the renumbering inside 
> `GetRegisterInfoAtIndex`. Another would be to ensure that these registers 
> don't even appear in the RegisterInfo array that backs this function (then 
> the current implementation of `GetRegisterInfoAtIndex` would "just work"). 
> All other things being equal, I would prefer the second approach. It seems 
> like that could be easily achieved as you're defining a custom RegisterInfo 
> array for SVE anyway (`g_register_infos_arm64_sve_le` in D77047 
> ). We could just exclude dbg registers from 
> that list.

This solution requires quite a lot of register numbering magic in 
RegisterInfos_arm64 and I was reluctant to do that as they are used all over 
the code with a consideration of a static array with lldb_reg_num as indices.

> That would still leave us with the question of what does `invalidate_regs` 
> refer to. If it still makes sense for it to refer to "lldb" register numbers, 
> then we can remap it as we discussed previously. But it may turn out that 
> after no remapping is needed -- in which case, even better.






Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:467
+reg_index = reg_context.RegInfosIndexToUserRegIndex(*reg_num);
 if (i > 0)
   response.PutChar(',');

Just to explain this call to RegInfosIndexToUserRegIndex: It is more relevant 
for host's needs to have correct register number living in value_regs list in 
order to access the correct parent for all the pseudo registers.

As far as invalidate_regs are concerned they actually do not matter much as far 
as process gdb remote + aarch64 is concerned. Mainly because if we write a 
pseudo reg we ll actually be writing its parent from value_regs list. This 
parent will automatically be invalidated by the call to writeregister. When any 
pseudo register having the same value_r

[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yeah, harbormaster is a bit temperamental these days. I wouldn't worry too much 
about it reporting problems clearly unrelated to your patch.

Btw, do you have commit access, or need someone to commit this for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79559



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

[Lldb-commits] [PATCH] D79825: [lldb/Reproducers] Add test-specific API to set the test CWD

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

It's definitely a hack, but it seems to be fairly well contained.

As an aside, I've always wondered, how much we really need to switch the cwd. 
Now that we build tests out-of-tree, I would expect most of the file references 
to be cwd-independent (`getBuildArtifact`). The only remaining references are 
probably the ones where we work with source files, which shouldn't be too hard 
to fix, and also those where we still accidentally create files in the source 
tree (which this would help eliminate).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D79825



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

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In general, I think this is a good direction to go in. We can run into the same 
kinds of problems even with non-artificial functions -- either the application 
can have real ODR violations, or we can introduce ODR violations during 
expression eval by playing fast-and-loose with anonymous namespaces.

For all of those cases we need a more general approach to handling the "I have 
two versions of a class, and they don't really match" problem.

As for a test case where this would fail, I haven't had first hand experience 
with this, but I would guess it involves two shared libraries, one which 
contains the definition of this artificial constructor, and one which doesn't. 
And then pulling both of these class definitions into the same expression...




Comment at: lldb/test/API/lang/cpp/constructors/main.cpp:11
+  int x=10;
+  ClassForSideEffect cse = ClassForSideEffect(100);
 };

Just for my own education, how would this fail without the patch? During 
expression evaluation, we would synthesize a call to the default constructor of 
`ClassForSideEffect`, instead of the one taking int=100? Or is it something 
else?


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

https://reviews.llvm.org/D79811



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

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Cool. This looks good now.

As an aside, it would be nice if adb actually started supporting ipv6 -- it's 
over 20 years old now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757



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

[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

[Mathias LANG via Phabricator via lldb-commits]

Geod24 added a comment.

Good to know, thanks! It's also my first time using `arc`, so I had to poke 
around a bit. In any case I didn't seem to include that whitespace fix even 
after squashing the commits together. If this is fine as is, then great.

No I don't have commit access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79559



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

[Lldb-commits] [lldb] e16111c - [lldb] Also recognize DWARF UTF base types using their size

[Pavel Labath via lldb-commits]

Author: Mathias LANG
Date: 2020-05-13T12:56:13+02:00
New Revision: e16111ce2fce7fd86c10d3f1dfe3e3b62b76d73b

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

LOG: [lldb] Also recognize DWARF UTF base types using their size

Summary:
The D programming language has 'char', 'wchar', and 'dchar' as base types,
which are defined as UTF-8, UTF-16, and UTF-32, respectively.

It also has type constructors (e.g. 'const' and 'immutable'),
that leads to D compilers emitting DW_TAG_base_type with DW_ATE_UTF
and name 'char', 'immutable(wchar)', 'const(char)', etc...

Before this patch, DW_ATE_UTF would only recognize types that
followed the C/C++ naming, and emit an error message for the rest, e.g.:
```
error: need to add support for DW_TAG_base_type 'immutable(char)'
encoded with DW_ATE = 0x10, bit_size = 8
```

The code was changed to check the byte size first,
then fall back to the old name-based check.

Reviewers: clayborg, labath

Reviewed By: labath

Subscribers: labath, lldb-commits

Tags: #lldb

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/DW_TAG_basic_type_DW_ATE_UTF_nonC.ll

Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 970d4161899e..6e8946e23104 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1130,13 +1130,22 @@ CompilerType 
TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
 break;
 
   case DW_ATE_UTF:
-if (!type_name.empty()) {
-  if (type_name == "char16_t")
-return GetType(ast.Char16Ty);
-  if (type_name == "char32_t")
-return GetType(ast.Char32Ty);
-  if (type_name == "char8_t")
-return GetType(ast.Char8Ty);
+switch (bit_size) {
+case 8:
+  return GetType(ast.Char8Ty);
+case 16:
+  return GetType(ast.Char16Ty);
+case 32:
+  return GetType(ast.Char32Ty);
+default:
+  if (!type_name.empty()) {
+if (type_name == "char16_t")
+  return GetType(ast.Char16Ty);
+if (type_name == "char32_t")
+  return GetType(ast.Char32Ty);
+if (type_name == "char8_t")
+  return GetType(ast.Char8Ty);
+  }
 }
 break;
   }
@@ -8885,7 +8894,7 @@ void 
TypeSystemClang::DumpTypeDescription(lldb::opaque_compiler_type_t type,
 if (clang::TagDecl *tag_decl = tag_type->getDecl()) {
   if (level == eDescriptionLevelVerbose)
 tag_decl->dump(llvm_ostrm);
-  else 
+  else
 tag_decl->print(llvm_ostrm, 0);
 }
   } else {

diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_basic_type_DW_ATE_UTF_nonC.ll 
b/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_basic_type_DW_ATE_UTF_nonC.ll
new file mode 100644
index ..c52309aa85e8
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_basic_type_DW_ATE_UTF_nonC.ll
@@ -0,0 +1,82 @@
+;
+; This test verifies that DWARF DIE of type DW_TAG_basic_type with DW_ATE_UTF
+; are matched based on their bit size (8, 16, 32) in addition to their name.
+;
+; This is used by languages which don't use the C(++) naming of
+; `char{8,16,32}_t`, e.g. the D programming language uses `char`, `wchar`, 
`dchar`.
+;
+; The D code used to generate this IR is:
+; ```
+; // Compiled with `ldc2 --mtriple=x86_64-pc-linux -betterC -g -c --output-ll 
utftypes.d`
+; __gshared string utf8 = "Hello";
+; __gshared wstring utf16 = "Dlang"w;
+; __gshared dstring utf32 = "World"d;
+; ```
+;
+; Note: lldb will print types 
diff erently before and after 'run'.
+;
+; RUN: %clang --target=x86_64-pc-linux -c -g -o %t %s
+; RUN: %lldb %t -o 'type lookup string' -o 'type lookup wstring' \
+; RUN:   -o 'type lookup dstring' -o exit | FileCheck %s
+;
+; CHECK: struct string {
+; CHECK: unsigned long length;
+; CHECK: char8_t *ptr;
+; CHECK: }
+; CHECK: struct wstring {
+; CHECK: unsigned long length;
+; CHECK: char16_t *ptr;
+; CHECK: }
+; CHECK: struct dstring {
+; CHECK: unsigned long length;
+; CHECK: char32_t *ptr;
+; CHECK: }
+
+$_D8utftypes4utf8Aya = comdat any
+$_D8utftypes5utf16Ayu = comdat any
+$_D8utftypes5utf32Ayw = comdat any
+
+@_D8utftypes4utf8Aya = global { i64, i8* } { i64 5, i8* getelementptr inbounds 
([6 x i8], [6 x i8]* @.str, i32 0, i32 0) }, comdat, align 8, !dbg !0 ; [#uses 
= 0]
+@.str = private unnamed_addr constant [6 x i8] c"Hello\00" ; [#uses = 1]
+@_D8utftypes5utf16Ayu = global { i64, i16* } { i64 5, i16* getelementptr 
inbounds ([6 x i16], [6 x i16]* @.str.1, i32 0, i32 0) }, comdat, align 8, !dbg 
!11 ; [#uses = 0]
+@.str.1 = priva

[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

[Pavel Labath via Phabricator via lldb-commits]

labath closed this revision.
labath added a comment.

Committed as e16111ce2f 
. I've 
also added checks for the dstring/wstring types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79559



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

[Lldb-commits] [PATCH] D79726: Add terminateCommands to lldb-vscode protocol

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D79726#2031817 , @aadsm wrote:

> > I gotta say I don't understand the difference between "exit" and 
> > "terminate" commands, as both seem to happen pretty at once. Is the 
> > difference that "exit" commands are run only when the inferior exits freely 
> > on its own, and not when we forcefully kill it? And that "terminate" cmds 
> > are run in both cases?
>
> Yeah, just like said, the exit event (and commands) only happen when the 
> program normally exits (with an exit code). However, I want to run commands 
> when the debugging session is over, which might not necessarily mean the 
> debuggee has ended (forcefully or not), only that we stopped debugging it. My 
> main goal is to collect stats of the debugging session that has just ended.


Aha, intereseting. So the "exit" commands would not run if we detach from the 
process, but the "terminate" commands still would. That makes sense to me.




Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py:250-251
 def cleanup():
-self.vscode.request_disconnect(terminateDebuggee=True)
+if disconnect:
+self.vscode.request_disconnect(terminateDebuggee=True)
 self.vscode.terminate()

aadsm wrote:
> aadsm wrote:
> > labath wrote:
> > > aadsm wrote:
> > > > labath wrote:
> > > > > What's the purpose of this argument? To ensure a clean shutdown? 
> > > > > Would it be possible to make the function smart enough to detect the 
> > > > > right thing to do when cleaning up?
> > > > it indicates if the process should be killed or not when lldb-vscode 
> > > > disconnects from it.
> > > I meant the `disconnect` argument to the `attach` function in the test 
> > > suite , not the `terminateDebuggee` argument of the `disconnect` command 
> > > to lldb-vscode.
> > > 
> > > My point is that, given the way the cleanup functions work, you cannot 
> > > assume that you're going to be invoked in any particular moment. For 
> > > example they can called if an assertion fails, to ensure any external 
> > > resources/processes are terminated. Since you don't know which assertion 
> > > will fail, it's best if you don't assume anything about the the state of 
> > > the inferior and just do your best to clean up. I'm not sure what "doing 
> > > your best" would mean here. For example it may mean sending a "terminate" 
> > > request and ignoring errors. Or maybe a better solution is possible...
> > oh yeah of course, the argument I added 😅. Oh, you know what, I misread 
> > when the cleanup function was going to be called. I don't need this at all, 
> > will remove it.
> sorry about this, I remember now (I've been working on something else not 
> related to this when I replied).
> 
> I did it this way so I can explicitly say that I don't want the test to 
> automatically disconnect when it's teared down. I need to disconnect manually 
> so I can test the terminateCommands, and and 2 disconnects won't work well 
> because we'll be waiting for that answer that will never arrive, since it has 
> already been terminated.
> 
> Another way to do this, that I thought at the time, was to track if the test 
> is connected or not and only disconnect in that situation. I opted to not do 
> that in the end because I wanted this behaviour to be intentional in the 
> test, to prevent something from disconnecting (unintentionally, like a bug) 
> and then the teardown will skip it and that error would never be surfaced. 
> This might be a bit overzealous though...
> 
> what do you think?
> 
> 
> 
Yeah, I think it would be better if this tracked when it is necessary to 
terminate the child automatically, or just sent the terminate request blindly 
(ignoring errors). The main purpose of the clean up commands is to "clean up" 
after the test, generating an exception here can just prevent other cleanups 
from happening. That said, I don't think this is too important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79726



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

[Lldb-commits] [lldb] e072b20 - [lldb] Merge PlatformXXX::ResolveExecutable

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-05-13T13:28:19+02:00
New Revision: e072b20bdea5629d0bc7a7c2216bdc7762dcb564

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

LOG: [lldb] Merge PlatformXXX::ResolveExecutable

The near-identical implementations of this function for posix-y
platforms were merged in r293910. PlatformWindows was left out of this
merge because at the time we did not have a suitable base class to sink
the code into. That is no longer true, so this commit finishes the job
by moving the code into RemoteAwarePlatform::ResolveExecutable.

Added: 


Modified: 
lldb/include/lldb/Target/RemoteAwarePlatform.h
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.h
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
lldb/source/Plugins/Platform/Windows/PlatformWindows.h
lldb/source/Target/RemoteAwarePlatform.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h 
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index 55d5ff673f1d..5741dbe027b7 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -22,6 +22,10 @@ class RemoteAwarePlatform : public Platform {
   bool GetModuleSpec(const FileSpec &module_file_spec, const ArchSpec &arch,
  ModuleSpec &module_spec) override;
 
+  Status
+  ResolveExecutable(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
+const FileSpecList *module_search_paths_ptr) override;
+
   lldb::user_id_t OpenFile(const FileSpec &file_spec, File::OpenOptions flags,
uint32_t mode, Status &error) override;
 

diff  --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp 
b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 4bd6509ec2ec..180ea1d2cfd1 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -11,7 +11,6 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
-#include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/FunctionCaller.h"
@@ -64,148 +63,6 @@ lldb_private::OptionGroupOptions 
*PlatformPOSIX::GetConnectionOptions(
   return m_options.at(&interpreter).get();
 }
 
-Status
-PlatformPOSIX::ResolveExecutable(const ModuleSpec &module_spec,
- lldb::ModuleSP &exe_module_sp,
- const FileSpecList *module_search_paths_ptr) {
-  Status error;
-  // Nothing special to do here, just use the actual file and architecture
-
-  char exe_path[PATH_MAX];
-  ModuleSpec resolved_module_spec(module_spec);
-
-  if (IsHost()) {
-// If we have "ls" as the exe_file, resolve the executable location based
-// on the current path variables
-if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec())) {
-  resolved_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
-  resolved_module_spec.GetFileSpec().SetFile(exe_path,
- FileSpec::Style::native);
-  FileSystem::Instance().Resolve(resolved_module_spec.GetFileSpec());
-}
-
-if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
-  FileSystem::Instance().ResolveExecutableLocation(
-  resolved_module_spec.GetFileSpec());
-
-// Resolve any executable within a bundle on MacOSX
-Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
-
-if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
-  error.Clear();
-else {
-  const uint32_t permissions = FileSystem::Instance().GetPermissions(
-  resolved_module_spec.GetFileSpec());
-  if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
-error.SetErrorStringWithFormat(
-"executable '%s' is not readable",
-resolved_module_spec.GetFileSpec().GetPath().c_str());
-  else
-error.SetErrorStringWithFormat(
-"unable to find executable for '%s'",
-resolved_module_spec.GetFileSpec().GetPath().c_str());
-}
-  } else {
-if (m_remote_platform_sp) {
-  error =
-  GetCachedExecutable(resolved_module_spec, exe_module_sp,
-  module_search_paths_ptr, *m_remote_platform_sp);
-} else {
-  // We may connect to a process and use the provided executable (Don't use
-  // local $PATH).
-
-  // Resolve any executable within a bundle on MacOSX
-  Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
-
-  if (FileSyste

[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

[Mathias LANG via Phabricator via lldb-commits]

Geod24 added a comment.

Thanks! 
I suppose this won't be in the next patch release (10.0.1) but instead in the 
next major ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79559



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

[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79559



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

[Lldb-commits] [lldb] 2fe6672 - [lldb][NFC] Don't specify a default argument when creating a TextDiagnosticPrinter

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-05-13T15:55:51+02:00
New Revision: 2fe6672498d60074fd34e2770659d5526e53e093

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

LOG: [lldb][NFC] Don't specify a default argument when creating a 
TextDiagnosticPrinter

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index e7afbe5c3031..8885cbc85b2c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -162,8 +162,7 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
 m_options->ShowPresumedLoc = true;
 m_options->ShowLevel = false;
 m_os.reset(new llvm::raw_string_ostream(m_output));
-m_passthrough.reset(
-new clang::TextDiagnosticPrinter(*m_os, m_options, false));
+m_passthrough.reset(new clang::TextDiagnosticPrinter(*m_os, m_options));
   }
 
   void ResetManager(DiagnosticManager *manager = nullptr) {



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

[Lldb-commits] [PATCH] D79789: [lldb] Don't dissasemble large functions by default

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 263697.
labath marked an inline comment as done.
labath added a comment.

Remove commas, add --force.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79789

Files:
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectDisassemble.h
  lldb/source/Commands/Options.td
  lldb/test/Shell/Commands/Inputs/command-disassemble.lldbinit
  lldb/test/Shell/Commands/command-disassemble-process.yaml
  lldb/test/Shell/Commands/command-disassemble.s

Index: lldb/test/Shell/Commands/command-disassemble.s
===
--- lldb/test/Shell/Commands/command-disassemble.s
+++ lldb/test/Shell/Commands/command-disassemble.s
@@ -51,8 +51,19 @@
 # CHECK-NEXT: command-disassemble.s.tmp[0x8] <+8>:  int$0x14
 # CHECK-NEXT: command-disassemble.s.tmp[0xa] <+10>: int$0x15
 # CHECK-NEXT: command-disassemble.s.tmp[0xc] <+12>: int$0x16
-# CHECK-NEXT: (lldb) disassemble --address 0xdead
-# CHECK-NEXT: error: Could not find function bounds for address 0xdead
+# CHECK-NEXT: (lldb) disassemble --address 0xdeadb
+# CHECK-NEXT: error: Could not find function bounds for address 0xdeadb
+# CHECK-NEXT: (lldb) disassemble --address 0x100
+# CHECK-NEXT: error: Not disassembling the function because it is very large [0x0040-0x2040). To disassemble specify an instruction count limit, start/stop addresses or use the --force option.
+# CHECK-NEXT: (lldb) disassemble --address 0x100 --count 3
+# CHECK-NEXT: command-disassemble.s.tmp`very_long:
+# CHECK-NEXT: command-disassemble.s.tmp[0x40] <+0>: int$0x2a
+# CHECK-NEXT: command-disassemble.s.tmp[0x42] <+2>: int$0x2a
+# CHECK-NEXT: command-disassemble.s.tmp[0x44] <+4>: int$0x2a
+# CHECK-NEXT: (lldb) disassemble --address 0x100 --force
+# CHECK-NEXT: command-disassemble.s.tmp`very_long:
+# CHECK-NEXT: command-disassemble.s.tmp[0x40]   <+0>:int$0x2a
+# CHECK:  command-disassemble.s.tmp[0x203e] <+8190>: int$0x2a
 # CHECK-NEXT: (lldb) disassemble --start-address 0x0 --count 7
 # CHECK-NEXT: command-disassemble.s.tmp`foo:
 # CHECK-NEXT: command-disassemble.s.tmp[0x0] <+0>:  int$0x10
@@ -64,8 +75,32 @@
 # CHECK-NEXT: command-disassemble.s.tmp[0xc] <+12>: int$0x16
 # CHECK-NEXT: (lldb) disassemble --start-address 0x0 --end-address 0x20 --count 7
 # CHECK-NEXT: error: invalid combination of options for the given command
-# CHECK-NEXT: (lldb) disassemble --address 0x0 --count 7
-# CHECK-NEXT: error: invalid combination of options for the given command
+# CHECK-NEXT: (lldb) disassemble --name case1
+# CHECK-NEXT: command-disassemble.s.tmp`n1::case1:
+# CHECK-NEXT: command-disassemble.s.tmp[0x2040] <+0>: int$0x30
+# CHECK-EMPTY:
+# CHECK-NEXT: command-disassemble.s.tmp`n2::case1:
+# CHECK-NEXT: command-disassemble.s.tmp[0x2042] <+0>: int$0x31
+# CHECK-EMPTY:
+# CHECK-NEXT: (lldb) disassemble --name case2
+# CHECK-NEXT: command-disassemble.s.tmp`n1::case2:
+# CHECK-NEXT: command-disassemble.s.tmp[0x2044] <+0>: int$0x32
+# CHECK-NEXT: warning: Not disassembling a range because it is very large [0x2046-0x4046). To disassemble specify an instruction count limit, start/stop addresses or use the --force option.
+# CHECK-NEXT: (lldb) disassemble --name case3
+# CHECK-NEXT: error: Not disassembling a range because it is very large [0x4046-0x6046). To disassemble specify an instruction count limit, start/stop addresses or use the --force option.
+# CHECK-NEXT: Not disassembling a range because it is very large [0x6046-0x8046). To disassemble specify an instruction count limit, start/stop addresses or use the --force option.
+# CHECK-NEXT: (lldb) disassemble --name case3 --count 3
+# CHECK-NEXT: command-disassemble.s.tmp`n1::case3:
+# CHECK-NEXT: command-disassemble.s.tmp[0x4046] <+0>: int$0x2a
+# CHECK-NEXT: command-disassemble.s.tmp[0x4048] <+2>: int$0x2a
+# CHECK-NEXT: command-disassemble.s.tmp[0x404a] <+4>: int$0x2a
+# CHECK-EMPTY:
+# CHECK-NEXT: command-disassemble.s.tmp`n2::case3:
+# CHECK-NEXT: command-disassemble.s.tmp[0x6046] <+0>: int$0x2a
+# CHECK-NEXT: command-disassemble.s.tmp[0x6048] <+2>: int$0x2a
+# CHECK-NEXT: command-disassemble.s.tmp[0x604a] <+4>: int$0x2a
+# CHECK-EMPTY:
+
 
 .text
 foo:
@@ -102,3 +137,32 @@
 int $0x2d
 int $0x2e
 int $0x2f
+
+very_long:
+.rept 0x1000
+int $42
+.endr
+
+_ZN2n15case1Ev:
+int $0x30
+
+_ZN2n25case1Ev:
+int $0x31
+
+_ZN2n15case2Ev:
+int $0x32
+
+_ZN2n25case2Ev:
+.rept 0x1000
+int $42
+.endr
+
+_ZN2n15case3Ev:
+.rept 0x1000
+int $42
+.endr
+
+_ZN2n25case3Ev:
+.rept 0x1000
+int $42
+.endr
Index: lldb/test/Shell/Commands/command-disassemble-process.y

[Lldb-commits] [PATCH] D79789: [lldb] Don't dissasemble large functions by default

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jingham.
labath marked 3 inline comments as done.
labath added a comment.

In D79789#2032008 , @jingham wrote:

> Instead of just printing out the range, can we print out the disassemble 
> command you would run to actually disassemble the range?
>
> Also, I think we should add a --force option since if you were using this in 
> a script you wouldn't get a chance to respond to the error.


Yes, I've been thinking about a --force option too. You're right that this is a 
friendlier interface for scripting purposes.

The reason I wanted to print the range was to give the user a chance to pause 
and say "yes, I really don't want to disassemble all of *that*". With a --force 
option, do you think it's necessary to print the repeat command? It seems that 
doing a "--force would be more straight-forward than copy-pasting a 
chunk of text (and I don't believe we currently have a way to fetch the 
original command line from within a CommandObject implementation).




Comment at: lldb/test/Shell/Commands/command-disassemble-process.yaml:93
 Value:   0x4002
-Size:0x0008
+Size:[[MAIN_SIZE]]
 ProgramHeaders:

JDevlieghere wrote:
> Cool, I didn't realize this was possible. 
It's a very recent addition. It runs as a textual pre-processing step, so you 
can use it to do all the evi^H^H^Hnice things you can with a c preprocessor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79789



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

[Lldb-commits] [lldb] 5f7a5e3 - [lldb][NFC] Early-exit in SetupDeclVendor

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-05-13T16:54:38+02:00
New Revision: 5f7a5e3bdba3663c8ec9704e28f75f6b2850f9f8

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

LOG: [lldb][NFC] Early-exit in SetupDeclVendor

Also removed the unnecessary element-by-element copy of the std::vector
hand_imported_modules to modules_for_macros.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 2b75c4f75c63..b2c6cba3bd58 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -348,37 +348,37 @@ bool 
ClangUserExpression::SetupPersistentState(DiagnosticManager &diagnostic_man
 }
 
 static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target) {
-  if (ClangModulesDeclVendor *decl_vendor =
-  target->GetClangModulesDeclVendor()) {
-auto *persistent_state = llvm::cast(
-target->GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
-if (!persistent_state)
-  return;
-const ClangModulesDeclVendor::ModuleVector &hand_imported_modules =
-persistent_state->GetHandLoadedClangModules();
-ClangModulesDeclVendor::ModuleVector modules_for_macros;
+  ClangModulesDeclVendor *decl_vendor = target->GetClangModulesDeclVendor();
+  if (!decl_vendor)
+return;
 
-for (ClangModulesDeclVendor::ModuleID module : hand_imported_modules) {
-  modules_for_macros.push_back(module);
-}
+  if (!target->GetEnableAutoImportClangModules())
+return;
+
+  auto *persistent_state = llvm::cast(
+  target->GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
+  if (!persistent_state)
+return;
 
-if (target->GetEnableAutoImportClangModules()) {
-  if (StackFrame *frame = exe_ctx.GetFramePtr()) {
-if (Block *block = frame->GetFrameBlock()) {
-  SymbolContext sc;
+  StackFrame *frame = exe_ctx.GetFramePtr();
+  if (!frame)
+return;
+
+  Block *block = frame->GetFrameBlock();
+  if (!block)
+return;
+  SymbolContext sc;
 
-  block->CalculateSymbolContext(&sc);
+  block->CalculateSymbolContext(&sc);
 
-  if (sc.comp_unit) {
-StreamString error_stream;
+  if (!sc.comp_unit)
+return;
+  StreamString error_stream;
 
-decl_vendor->AddModulesForCompileUnit(
-*sc.comp_unit, modules_for_macros, error_stream);
-  }
-}
-  }
-}
-  }
+  ClangModulesDeclVendor::ModuleVector modules_for_macros =
+  persistent_state->GetHandLoadedClangModules();
+  decl_vendor->AddModulesForCompileUnit(*sc.comp_unit, modules_for_macros,
+error_stream);
 }
 
 void ClangUserExpression::UpdateLanguageForExpr() {



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

[Lldb-commits] [lldb] 6671a81 - [lldb/Reproducers] Add test-specific API to set the test CWD

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-05-13T09:00:07-07:00
New Revision: 6671a81bc71cc2635c5a10d6f688fea46ca4e5d6

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

LOG: [lldb/Reproducers] Add test-specific API to set the test CWD

The reproducers' working directory is set to the current working
directory when they are initialized. While this is not optimal, as the
cwd can change during a debug session, it has been sufficient so far.

The current approach doesn't work for the API test suite however because
dotest temporarily changes the directory to where the test's Python file
lives.

This patch adds an API to tell the reproducers what to set the CWD to.
This is a NO-OP in every mode but capture.

Differential revision: https://reviews.llvm.org/D79825

Added: 


Modified: 
lldb/bindings/interface/SBReproducer.i
lldb/include/lldb/API/SBReproducer.h
lldb/include/lldb/Utility/Reproducer.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/API/SBReproducer.cpp

Removed: 




diff  --git a/lldb/bindings/interface/SBReproducer.i 
b/lldb/bindings/interface/SBReproducer.i
index e976863f15c5..7c9b007db3c0 100644
--- a/lldb/bindings/interface/SBReproducer.i
+++ b/lldb/bindings/interface/SBReproducer.i
@@ -13,5 +13,6 @@ class SBReproducer
 static const char *Capture(const char *path);
 static const char *PassiveReplay(const char *path);
 static bool SetAutoGenerate(bool b);
+static void SetWorkingDirectory(const char *path);
 };
 }

diff  --git a/lldb/include/lldb/API/SBReproducer.h 
b/lldb/include/lldb/API/SBReproducer.h
index 5a298d1bcf43..78044e9acbc3 100644
--- a/lldb/include/lldb/API/SBReproducer.h
+++ b/lldb/include/lldb/API/SBReproducer.h
@@ -26,6 +26,13 @@ class LLDB_API SBReproducer {
   static const char *GetPath();
   static bool SetAutoGenerate(bool b);
   static bool Generate();
+
+  /// The working directory is set to the current working directory when the
+  /// reproducers are initialized. This method allows setting a 
diff erent
+  /// working directory. This is used by the API test suite  which temporarily
+  /// changes the directory to where the test lives. This is a NO-OP in every
+  /// mode but capture.
+  static void SetWorkingDirectory(const char *path);
 };
 
 } // namespace lldb

diff  --git a/lldb/include/lldb/Utility/Reproducer.h 
b/lldb/include/lldb/Utility/Reproducer.h
index 499243b41202..5d810460a0c5 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -147,6 +147,9 @@ class WorkingDirectoryProvider : public 
Provider {
   return;
 m_cwd = std::string(cwd.str());
   }
+
+  void Update(llvm::StringRef path) { m_cwd = std::string(path); }
+
   struct Info {
 static const char *name;
 static const char *file;

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 3cea39ddcd57..9d119e08b6c3 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -526,6 +526,7 @@ def setUpClass(cls):
 if traceAlways:
 print("Change dir to:", full_dir, file=sys.stderr)
 os.chdir(full_dir)
+lldb.SBReproducer.SetWorkingDirectory(full_dir)
 
 # Set platform context.
 cls.platformContext = lldbplatformutil.createPlatformContext()

diff  --git a/lldb/source/API/SBReproducer.cpp 
b/lldb/source/API/SBReproducer.cpp
index 329c1b55d16d..703d613c713f 100644
--- a/lldb/source/API/SBReproducer.cpp
+++ b/lldb/source/API/SBReproducer.cpp
@@ -231,6 +231,12 @@ const char *SBReproducer::GetPath() {
   return path.c_str();
 }
 
+void SBReproducer::SetWorkingDirectory(const char *path) {
+  if (auto *g = lldb_private::repro::Reproducer::Instance().GetGenerator()) {
+g->GetOrCreate().Update(path);
+  }
+}
+
 char lldb_private::repro::SBProvider::ID = 0;
 const char *SBProvider::Info::name = "sbapi";
 const char *SBProvider::Info::file = "sbapi.bin";



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

[Lldb-commits] [PATCH] D79825: [lldb/Reproducers] Add test-specific API to set the test CWD

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6671a81bc71c: [lldb/Reproducers] Add test-specific API to 
set the test CWD (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79825

Files:
  lldb/bindings/interface/SBReproducer.i
  lldb/include/lldb/API/SBReproducer.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/API/SBReproducer.cpp


Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -231,6 +231,12 @@
   return path.c_str();
 }
 
+void SBReproducer::SetWorkingDirectory(const char *path) {
+  if (auto *g = lldb_private::repro::Reproducer::Instance().GetGenerator()) {
+g->GetOrCreate().Update(path);
+  }
+}
+
 char lldb_private::repro::SBProvider::ID = 0;
 const char *SBProvider::Info::name = "sbapi";
 const char *SBProvider::Info::file = "sbapi.bin";
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -526,6 +526,7 @@
 if traceAlways:
 print("Change dir to:", full_dir, file=sys.stderr)
 os.chdir(full_dir)
+lldb.SBReproducer.SetWorkingDirectory(full_dir)
 
 # Set platform context.
 cls.platformContext = lldbplatformutil.createPlatformContext()
Index: lldb/include/lldb/Utility/Reproducer.h
===
--- lldb/include/lldb/Utility/Reproducer.h
+++ lldb/include/lldb/Utility/Reproducer.h
@@ -147,6 +147,9 @@
   return;
 m_cwd = std::string(cwd.str());
   }
+
+  void Update(llvm::StringRef path) { m_cwd = std::string(path); }
+
   struct Info {
 static const char *name;
 static const char *file;
Index: lldb/include/lldb/API/SBReproducer.h
===
--- lldb/include/lldb/API/SBReproducer.h
+++ lldb/include/lldb/API/SBReproducer.h
@@ -26,6 +26,13 @@
   static const char *GetPath();
   static bool SetAutoGenerate(bool b);
   static bool Generate();
+
+  /// The working directory is set to the current working directory when the
+  /// reproducers are initialized. This method allows setting a different
+  /// working directory. This is used by the API test suite  which temporarily
+  /// changes the directory to where the test lives. This is a NO-OP in every
+  /// mode but capture.
+  static void SetWorkingDirectory(const char *path);
 };
 
 } // namespace lldb
Index: lldb/bindings/interface/SBReproducer.i
===
--- lldb/bindings/interface/SBReproducer.i
+++ lldb/bindings/interface/SBReproducer.i
@@ -13,5 +13,6 @@
 static const char *Capture(const char *path);
 static const char *PassiveReplay(const char *path);
 static bool SetAutoGenerate(bool b);
+static void SetWorkingDirectory(const char *path);
 };
 }


Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -231,6 +231,12 @@
   return path.c_str();
 }
 
+void SBReproducer::SetWorkingDirectory(const char *path) {
+  if (auto *g = lldb_private::repro::Reproducer::Instance().GetGenerator()) {
+g->GetOrCreate().Update(path);
+  }
+}
+
 char lldb_private::repro::SBProvider::ID = 0;
 const char *SBProvider::Info::name = "sbapi";
 const char *SBProvider::Info::file = "sbapi.bin";
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -526,6 +526,7 @@
 if traceAlways:
 print("Change dir to:", full_dir, file=sys.stderr)
 os.chdir(full_dir)
+lldb.SBReproducer.SetWorkingDirectory(full_dir)
 
 # Set platform context.
 cls.platformContext = lldbplatformutil.createPlatformContext()
Index: lldb/include/lldb/Utility/Reproducer.h
===
--- lldb/include/lldb/Utility/Reproducer.h
+++ lldb/include/lldb/Utility/Reproducer.h
@@ -147,6 +147,9 @@
   return;
 m_cwd = std::string(cwd.str());
   }
+
+  void Update(llvm::StringRef path) { m_cwd = std::string(path); }
+
   struct Info {
 static const char *name;
 static const char *file;
Index: lldb/include/lldb/API/SBReproducer.h
===
--- lldb/include/lldb/API/SBReproducer.h
+++ lldb/include/ll

[Lldb-commits] [PATCH] D79825: [lldb/Reproducers] Add test-specific API to set the test CWD

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D79825#2033405 , @labath wrote:

> It's definitely a hack, but it seems to be fairly well contained.
>
> As an aside, I've always wondered, how much we really need to switch the cwd. 
> Now that we build tests out-of-tree, I would expect most of the file 
> references to be cwd-independent (`getBuildArtifact`). The only remaining 
> references are probably the ones where we work with source files, which 
> shouldn't be too hard to fix, and also those where we still accidentally 
> create files in the source tree (which this would help eliminate).


Sounds good. About half the API tests are failing without changing the working 
directory, though I suspect it's mostly the same issue. I've added it to my 
list of nice-to-haves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79825



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

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked an inline comment as done.
shafik added a comment.

In D79811#2033399 , @labath wrote:

> In general, I think this is a good direction to go in. We can run into the 
> same kinds of problems even with non-artificial functions -- either the 
> application can have real ODR violations, or we can introduce ODR violations 
> during expression eval by playing fast-and-loose with anonymous namespaces.
>
> For all of those cases we need a more general approach to handling the "I 
> have two versions of a class, and they don't really match" problem.
>
> As for a test case where this would fail, I haven't had first hand experience 
> with this, but I would guess it involves two shared libraries, one which 
> contains the definition of this artificial constructor, and one which 
> doesn't. And then pulling both of these class definitions into the same 
> expression...


Yes, in some offline conversations with @clayborg that was the scenario he had 
seen this come up before. I was not able to synthesize a test case that ran 
into this issue but that does not mean it is not possible. Perhaps I just 
wasn't creative enough ;-)




Comment at: lldb/test/API/lang/cpp/constructors/main.cpp:11
+  int x=10;
+  ClassForSideEffect cse = ClassForSideEffect(100);
 };

labath wrote:
> Just for my own education, how would this fail without the patch? During 
> expression evaluation, we would synthesize a call to the default constructor 
> of `ClassForSideEffect`, instead of the one taking int=100? Or is it 
> something else?
So it would not initialize `x` to `10` and it would not call 
`ClassForSideEffect(100)` it would just default initialize it. I don't quite 
understand why, I plan to dig into that some more but I wanted to get the 
conversation rolling.


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

https://reviews.llvm.org/D79811



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

[Lldb-commits] [PATCH] D79789: [lldb] Don't dissasemble large functions by default

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

If you have --force, then the address range is informational, in which case 
having the full command would be more confusing than anything.  So this is 
fine, thanks for adding it.

I thought briefly about whether we should also add a setting for this.  But if 
it is something that you come across often you can just re-alias `di` to 
`disassemble --force`.  I used to think we needed to always provide global 
settings for this sort of behavior, but if the setting just controls the 
behavior of one command, I'm beginning to think using aliases is better than 
adding more and more stuff to the settings.

I agree you want to print out the range.  Many of the times you get huge 
functions, they aren't actually huge, they just look that way because we're 
just missing the subsequent symbols.  So the user might be disassembling 
something they know is a small function, and we need to give them some clue to 
why we think that's not so.

This looks fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79789



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

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The error where two of the same classes conflicted usually only  happened in 
complex cases that were hard to reduce down.

If I understand this correctly, the compiler should be able to auto synthesize 
these functions at will since the original class definition should have all of 
the information it needs to create them. So why do we need these to be added? 
Seems like the wrong approach IMHO. I would like the hear the justification for 
needing to add artificial functions to a class definition before we entertain 
this more. Can you elaborate on why you think these are needed?


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

https://reviews.llvm.org/D79811



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

[Lldb-commits] [PATCH] D79726: Add terminateCommands to lldb-vscode protocol

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

might be nice to have "disconnectCommands" at some point if we ever need them 
too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79726



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

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D79811#2034842 , @clayborg wrote:

> The error where two of the same classes conflicted usually only  happened in 
> complex cases that were hard to reduce down.
>
> If I understand this correctly, the compiler should be able to auto 
> synthesize these functions at will since the original class definition should 
> have all of the information it needs to create them. So why do we need these 
> to be added? Seems like the wrong approach IMHO. I would like the hear the 
> justification for needing to add artificial functions to a class definition 
> before we entertain this more. Can you elaborate on why you think these are 
> needed?


Yes, the test case below shows that if we don't add the method then during 
expression evaluation of let's say:

  A{}

The `default member initializer` won't be done. So `x` won't be initialized to 
`10` and the default constructor for `cse` will be called instead of 
`ClassForSideEffect(int)`.


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

https://reviews.llvm.org/D79811



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

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

WeakODR requires that the symbol actually be discarded if not referenced.  This 
will preserve the symbol even if unreferenced will it not?  One approach might 
be to just create a `DenseMap` and check for any references and mark is as 
preserved otherwise just drop it.  However, it _is_ ODR, which means that if it 
ever gets instantiated, we are well within our rights to give you the second 
definition rather than the first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78972



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

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D79811#2035078 , @shafik wrote:

> In D79811#2034842 , @clayborg wrote:
>
> > The error where two of the same classes conflicted usually only  happened 
> > in complex cases that were hard to reduce down.
> >
> > If I understand this correctly, the compiler should be able to auto 
> > synthesize these functions at will since the original class definition 
> > should have all of the information it needs to create them. So why do we 
> > need these to be added? Seems like the wrong approach IMHO. I would like 
> > the hear the justification for needing to add artificial functions to a 
> > class definition before we entertain this more. Can you elaborate on why 
> > you think these are needed?
>
>
> Yes, the test case below shows that if we don't add the method then during 
> expression evaluation of let's say:
>
>   A{}
>
>
> The `default member initializer` won't be done. So `x` won't be initialized 
> to `10` and the default constructor for `cse` will be called instead of 
> `ClassForSideEffect(int)`.


So that sounds like a compiler bug or something that we are not setting up 
right in the class type when we convert it from DWARF. Can you compile a quick 
example and dump the AST somehow and then compare it to the class we generate 
from debug info and see how they differ?


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

https://reviews.llvm.org/D79811



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

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757



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

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

There was a patch previous to this that enabled IPv6. We need IPv6 on some 
computers here at Facebook. Can you check this file's log and look at the IPv6 
patch and make sure this doesn't just undo that patch prior to committing this? 
If it does undo that patch, maybe we can check to see if IPv6 works somehow in 
LLDB one time and then enable IPv6 if it is supported and disable it if not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757



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

[Lldb-commits] [lldb] 20db891 - Fix typo in error message

[via lldb-commits]

Author: Gabor Greif
Date: 2020-05-14T07:23:59+02:00
New Revision: 20db891cef97df752ff8d2a249c155e5605ba06d

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

LOG: Fix typo in error message

Added: 


Modified: 
lldb/source/Commands/CommandObjectGUI.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectGUI.cpp 
b/lldb/source/Commands/CommandObjectGUI.cpp
index 31b68881b52f..3f45a26de228 100644
--- a/lldb/source/Commands/CommandObjectGUI.cpp
+++ b/lldb/source/Commands/CommandObjectGUI.cpp
@@ -47,7 +47,7 @@ bool CommandObjectGUI::DoExecute(Args &args, 
CommandReturnObject &result) {
   }
   return true;
 #else
-  result.AppendError("lldb was not build with gui support");
+  result.AppendError("lldb was not built with gui support");
   return false;
 #endif
 }



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