[Lldb-commits] [PATCH] D81209: Move GetXcode*Directory into HostInfo (NFC)

2020-06-04 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl created this revision.
aprantl added reviewers: JDevlieghere, friss, jingham.
aprantl added a child revision: D81210: Teach GetXcodeSDK to look in the Xcode 
that contains LLDB.

These functions really don't belong into PlatformDarwin, since they actually 
query state of the Host and not of the remote platform.


https://reviews.llvm.org/D81209

Files:
  lldb/include/lldb/Host/HostInfoBase.h
  lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
  lldb/include/lldb/Utility/XcodeSDK.h
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.cpp
  lldb/source/Utility/XcodeSDK.cpp
  lldb/unittests/Platform/PlatformDarwinTest.cpp
  lldb/unittests/Utility/XcodeSDKTest.cpp

Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- lldb/unittests/Utility/XcodeSDKTest.cpp
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -204,3 +204,38 @@
   EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("i386-unknown-netbsd")),
 XcodeSDK::Type::unknown);
 }
+
+TEST(XcodeSDKTest, FindXcodeContentsDirectoryInPath) {
+  std::string standard =
+  "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/"
+  "Developer/SDKs/MacOSX.sdk";
+  EXPECT_EQ("/Applications/Xcode.app/Contents",
+XcodeSDK::FindXcodeContentsDirectoryInPath(standard));
+
+  std::string standard_version =
+  "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/"
+  "Developer/SDKs/MacOSX10.15.sdk";
+  EXPECT_EQ("/Applications/Xcode.app/Contents",
+XcodeSDK::FindXcodeContentsDirectoryInPath(standard_version));
+
+  std::string beta = "/Applications/Xcode-beta.app/Contents/Developer/"
+ "Platforms/MacOSX.platform/"
+ "Developer/SDKs/MacOSX10.15.sdk";
+  EXPECT_EQ("/Applications/Xcode-beta.app/Contents",
+XcodeSDK::FindXcodeContentsDirectoryInPath(beta));
+
+  std::string no_app =
+  "/Applications/Xcode/Contents/Developer/Platforms/MacOSX.platform/"
+  "Developer/SDKs/MacOSX10.15.sdk";
+  EXPECT_EQ("", XcodeSDK::FindXcodeContentsDirectoryInPath(no_app));
+
+  std::string no_contents =
+  "/Applications/Xcode.app/Developer/Platforms/MacOSX.platform/"
+  "Developer/SDKs/MacOSX10.15.sdk";
+  EXPECT_EQ("", XcodeSDK::FindXcodeContentsDirectoryInPath(no_contents));
+
+  std::string no_capitalization =
+  "/Applications/Xcode.app/contents/Developer/Platforms/MacOSX.platform/"
+  "Developer/SDKs/MacOSX10.15.sdk";
+  EXPECT_EQ("", XcodeSDK::FindXcodeContentsDirectoryInPath(no_capitalization));
+}
Index: lldb/unittests/Platform/PlatformDarwinTest.cpp
===
--- lldb/unittests/Platform/PlatformDarwinTest.cpp
+++ lldb/unittests/Platform/PlatformDarwinTest.cpp
@@ -20,7 +20,6 @@
 struct PlatformDarwinTester : public PlatformDarwin {
 public:
   using PlatformDarwin::FindComponentInPath;
-  using PlatformDarwin::FindXcodeContentsDirectoryInPath;
 };
 
 TEST(PlatformDarwinTest, TestParseVersionBuildDir) {
@@ -51,44 +50,6 @@
   EXPECT_EQ(llvm::VersionTuple(3, 4, 5), V);
 }
 
-TEST(PlatformDarwinTest, FindXcodeContentsDirectoryInPath) {
-  std::string standard =
-  "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/"
-  "Developer/SDKs/MacOSX.sdk";
-  EXPECT_EQ("/Applications/Xcode.app/Contents",
-PlatformDarwinTester::FindXcodeContentsDirectoryInPath(standard));
-
-  std::string standard_version =
-  "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/"
-  "Developer/SDKs/MacOSX10.15.sdk";
-  EXPECT_EQ(
-  "/Applications/Xcode.app/Contents",
-  PlatformDarwinTester::FindXcodeContentsDirectoryInPath(standard_version));
-
-  std::string beta = "/Applications/Xcode-beta.app/Contents/Developer/"
- "Platforms/MacOSX.platform/"
- "Developer/SDKs/MacOSX10.15.sdk";
-  EXPECT_EQ("/Applications/Xcode-beta.app/Contents",
-PlatformDarwinTester::FindXcodeContentsDirectoryInPath(beta));
-
-  std::string no_app =
-  "/Applications/Xcode/Contents/Developer/Platforms/MacOSX.platform/"
-  "Developer/SDKs/MacOSX10.15.sdk";
-  EXPECT_EQ("", PlatformDarwinTester::FindXcodeContentsDirectoryInPath(no_app));
-
-  std::string no_contents =
-  "/Applications/Xcode.app/Developer/Platforms/MacOSX.platform/"
-  "Developer/SDKs/MacOSX10.15.sdk";
-

[Lldb-commits] [PATCH] D81210: Teach GetXcodeSDK to look in the Xcode that contains LLDB

2020-06-04 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl created this revision.
aprantl added reviewers: JDevlieghere, friss, jingham.
aprantl added a parent revision: D81209: Move GetXcode*Directory into HostInfo 
(NFC).

Teach GetXcodeSDK to look in the Xcode that contains LLDB instead of preferring 
the one chosen with xcode-select. Because we're using xcrun to find matching 
SDK's you can now get into a situation where LLDB, when run from a 
non-xcode-selected Xcode will find a matching SDK in the xcode-selected Xcode, 
which can cause anything from mild performance degradation to really confusing 
Clang compile errors, if the other Xcode is, for example, older, or missing an 
SDK.

  

rdar://problem/64000666


https://reviews.llvm.org/D81210

Files:
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -297,46 +297,47 @@
   }
 }
 
-FileSpec HostInfoMacOSX::GetXcodeContentsDirectory() {
-  static FileSpec g_xcode_contents_path;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, [&]() {
-// Try the shlib dir first.
-if (FileSpec fspec = HostInfo::GetShlibDir()) {
-  if (FileSystem::Instance().Exists(fspec)) {
-std::string xcode_contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-if (!xcode_contents_dir.empty()) {
-  g_xcode_contents_path = FileSpec(xcode_contents_dir);
-  return;
-}
-  }
+static FileSpec GetXcodeContentsDirectory(bool use_xcrun) {
+  // Try the shlib dir first.
+  if (FileSpec fspec = HostInfo::GetShlibDir()) {
+if (FileSystem::Instance().Exists(fspec)) {
+  std::string xcode_contents_dir =
+  XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
+  if (!xcode_contents_dir.empty())
+return FileSpec(xcode_contents_dir);
 }
+  }
 
-if (const char *developer_dir_env_var = getenv("DEVELOPER_DIR")) {
-  FileSpec fspec(developer_dir_env_var);
-  if (FileSystem::Instance().Exists(fspec)) {
-// FIXME: This looks like it couldn't possibly work!
-std::string xcode_contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-if (!xcode_contents_dir.empty()) {
-  g_xcode_contents_path = FileSpec(xcode_contents_dir);
-  return;
-}
-  }
+  if (const char *developer_dir_env_var = getenv("DEVELOPER_DIR")) {
+FileSpec fspec(developer_dir_env_var);
+if (FileSystem::Instance().Exists(fspec)) {
+  // FIXME: This looks like it couldn't possibly work!
+  std::string xcode_contents_dir =
+  XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
+  if (!xcode_contents_dir.empty())
+return FileSpec(xcode_contents_dir);
 }
+  }
 
-FileSpec fspec(HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS()));
-if (fspec) {
-  if (FileSystem::Instance().Exists(fspec)) {
-std::string xcode_contents_dir =
-XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
-if (!xcode_contents_dir.empty()) {
-  g_xcode_contents_path = FileSpec(xcode_contents_dir);
-  return;
-}
-  }
+  if (!use_xcrun)
+return {};
+  FileSpec fspec(HostInfo::GetXcodeSDKPath(XcodeSDK::GetAnyMacOS()));
+  if (fspec) {
+if (FileSystem::Instance().Exists(fspec)) {
+  std::string xcode_contents_dir =
+  XcodeSDK::FindXcodeContentsDirectoryInPath(fspec.GetPath());
+  if (!xcode_contents_dir.empty())
+return FileSpec(xcode_contents_dir);
 }
+  }
+  return {};
+}
+
+FileSpec HostInfoMacOSX::GetXcodeContentsDirectory() {
+  static FileSpec g_xcode_contents_path;
+  static std::once_flag g_once_flag;
+  std::call_once(g_once_flag, [&]() {
+g_xcode_contents_path = ::GetXcodeContentsDirectory(true);
   });
   return g_xcode_contents_path;
 }
@@ -358,7 +359,19 @@
   XcodeSDK::Info info = sdk.Parse();
   std::string sdk_name = XcodeSDK::GetCanonicalName(info);
   auto find_sdk = [](std::string sdk_name) -> std::string {
-std::string xcrun_cmd = "xcrun --show-sdk-path --sdk " + sdk_name;
+std::string xcrun_cmd;
+Environment env = Host::GetEnvironment();
+std::string developer_dir = env.lookup("DEVELOPER_DIR");
+if (developer_dir.empty()) {
+  // Avoid infinite recursion GetXcodeContentsDirectory calling GetXcodeSDK.
+  FileSpec path = ::GetXcodeContentsDirectory(false);
+  if (path.RemoveLastPathComponent())
+developer_dir = path.GetPath();
+}
+if (!developer_dir.empty())
+  xcrun_cmd = "/usr/bin/env DEVELOPER_DIR=" + developer_dir + " ";
+xcrun_cmd += "xcrun --show-sdk-path --sdk " + sdk_name;
+
 int status = 0;
 int signo = 0;
 std::string output_str;
___
lldb-commits mailing

[Lldb-commits] [PATCH] D81200: [vscode] set default values for terminateDebuggee for the disconnect request

2020-06-04 Thread walter erquinigo via Phabricator via lldb-commits 
wallace marked 2 inline comments as done.
wallace added a comment.

Well, the problem that i've seen happens mostly with long running processes 
like services that just don't die. So this fixes those issues anyway because 
those processes are not dying when they should. I tried an older version of the 
IDE and it sends all the time the terminateDebuggee flag. 
Anyway, what you mention made me notice something. This is the existing flow 
without this change

> vscode sends a disconnect request. At this point the UI might stop showing 
> the debug session, but it's still running

< lldb-vscode receives it and doesn't terminate the inferior nor terminates 
itself
< the inferior keeps running and whenever there's output, it's sent to vscode
< if the process terminates, it sends a terminate event

> vscode then finishes the debug session

A problem that I see here is that lldb-vscode was still alive. I'll investigate 
that tomorrow.

Anyway,wWe still need to investigate why some other processes don't die under 
normal circumstances, but I'll do it in a different patch.




Comment at: lldb/test/API/tools/lldb-vscode/disconnect/main.c:10
+  int x;
+  // We wait for a signal to proceed
+  scanf("%d", );

clayborg wrote:
> This comment seems off? You can call "pause()" to wait for a signal, but that 
> won't work on non unix based systems. Is the comment just left over?
ah yes, it's a left over. I don't want to do something too posix dependant. 
Good catch



Comment at: lldb/tools/lldb-vscode/VSCode.h:92
   bool stop_at_entry;
+  bool is_attach;
   // Keep track of the last stop thread index IDs as threads won't go away

clayborg wrote:
> This should be initialized in the VSCode::VSCode constructor? Surely we are 
> initializing the other values right?
 oh thank you, i didn't notice that constructor :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81200



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

In D80112#2075097 , @jingham wrote:

> Adding a ShouldStop to that if test doesn't seem right.  You should still run 
> the stop actions even if the thread doesn't want to stop.
>
> If I understand the problem you are describing, it is that you suspended a 
> thread as a user-level suspend, so it's stop reason got stuck.  That seems 
> okay, its probably useful to keep the stop reason of the thread at what it 
> was when you suspended it.  But that means  when gets asked to do its action 
> again, the action is stale and does the wrong thing.  If that's what's going 
> wrong, then it would make more sense short-circuit suspended threads earlier 
> on in the iteration, like:
>
>   lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx);
>   // This thread was suspended by the user during this run, so it's actions 
> are stale:
>   if (thread_sp->GetResumeState() == eStateSuspended)
>   continue;
>   
>   
>
> That mirrors what the ThreadList::ShouldStop does.  You actually probably 
> don't want to do it quite this way, because it is possible that one thread's 
> breakpoint action could resume another thread.  So you should probably first 
> run through the thread list and get all the suspended threads, then run 
> through again doing the actions and skipping the suspended threads you found 
> in the first iteration.


Actually, you can more easily do that.  The thread iteration where we call 
PerformAction etc. works on a copy of the thread list (since one thread's 
action could cause the thread list to change).  So if you just change the copy 
operation to only copy over threads which aren't user-suspended, then you 
should be set.

> It would be good to write a end-to-end test that mirrors this behavior.  It 
> would be pretty straightforward to write an API test that runs a process, 
> sets a breakpoint, hits it on one thread, suspends that thread, then hits it 
> on another thread and make sure the action didn't run twice on the second 
> stop.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D81200: [vscode] set default values for terminateDebuggee for the disconnect request

2020-06-04 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.

So the main question is why was lldb-vscode not exiting. Do we know why? Some 
thread wasn't exiting I would guess, but usually as long as the main thread 
exits, all other threads will just be terminated.  Our main thread should be 
the loop that waits for packets, gets one, executes what it needs to and sends 
a response. When the "terminate" packet comes in, that read loop should exit 
and this should cause the lldb-vscode to exit. So I am questioning if this 
works around the issue. Does "disconnect" always get sent for both launch and 
attach? Then "terminate"?

The changes look good for other reasons, but I am wondering if this will fix 
all of the lldb-vscode binaries that we have sitting around. A quick "sample 
lldb-vscode" should show where these lldb-vscode binaries are stuck. Can we 
verify what is going on to make sure this will fix this?

Marking as needs changes due to initialization of the "VSCode::is_attach" not 
being done in constructor.




Comment at: lldb/test/API/tools/lldb-vscode/disconnect/main.c:10
+  int x;
+  // We wait for a signal to proceed
+  scanf("%d", );

This comment seems off? You can call "pause()" to wait for a signal, but that 
won't work on non unix based systems. Is the comment just left over?



Comment at: lldb/tools/lldb-vscode/VSCode.h:92
   bool stop_at_entry;
+  bool is_attach;
   // Keep track of the last stop thread index IDs as threads won't go away

This should be initialized in the VSCode::VSCode constructor? Surely we are 
initializing the other values right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81200



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

Adding a ShouldStop to that if test doesn't seem right.  You should still run 
the stop actions even if the thread doesn't want to stop.

If I understand the problem you are describing, it is that you suspended a 
thread as a user-level suspend, so it's stop reason got stuck.  That seems 
okay, its probably useful to keep the stop reason of the thread at what it was 
when you suspended it.  But that means  when gets asked to do its action again, 
the action is stale and does the wrong thing.  If that's what's going wrong, 
then it would make more sense short-circuit suspended threads earlier on in the 
iteration, like:

  lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx);
  // This thread was suspended by the user during this run, so it's actions are 
stale:
  if (thread_sp->GetResumeState() == eStateSuspended)
  continue;

That mirrors what the ThreadList::ShouldStop does.  You actually probably don't 
want to do it quite this way, because it is possible that one thread's 
breakpoint action could resume another thread.  So you should probably first 
run through the thread list and get all the suspended threads, then run through 
again doing the actions and skipping the suspended threads you found in the 
first iteration.

It would be good to write a end-to-end test that mirrors this behavior.  It 
would be pretty straightforward to write an API test that runs a process, sets 
a breakpoint, hits it on one thread, suspends that thread, then hits it on 
another thread and make sure the action didn't run twice on the second stop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Ilya Bukonkin via Phabricator via lldb-commits 
fallkrum added a comment.

Didn't notice that we use in Process::ProcessEventData::DoOnRemoval:

  this_thread_wants_to_stop = stop_info_sp->ShouldStop(event_ptr);

and yes it is probably correct to fix the issue by this:

  if (stop_info_sp && stop_info_sp->IsValid() && 
thread_sp->ShouldStop(event_ptr))

But again not sure if all this correct. Because further in the code we call  
stop_info_sp->ShouldStop(event_ptr) what looks like a duplicate of  
thread_sp->ShouldStop(event_ptr). Need your further comments :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Ilya Bukonkin via Phabricator via lldb-commits 
fallkrum added a comment.

As far as I see the problem lies in Process::ProcessEventData::DoOnRemoval:

  StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
if (stop_info_sp && stop_info_sp->IsValid()) {
  does_anybody_have_an_opinion = true;
  bool this_thread_wants_to_stop;
  if (stop_info_sp->GetOverrideShouldStop()) {
this_thread_wants_to_stop =
stop_info_sp->GetOverriddenShouldStopValue();
  } else {
stop_info_sp->PerformAction(event_ptr);
// The stop action might restart the target.  If it does, then we
// want to mark that in the event so that whoever is receiving it
// will know to wait for the running event and reflect that state
// appropriately. We also need to stop processing actions, since 
they
// aren't expecting the target to be running.
  
// FIXME: we might have run.
if (stop_info_sp->HasTargetRunSinceMe()) {
  SetRestarted(true);
  break;
}
  
this_thread_wants_to_stop = stop_info_sp->ShouldStop(event_ptr);
  }
  
  if (!still_should_stop)
still_should_stop = this_thread_wants_to_stop;
}
  }

As you can see we get StopInfo from all the the threads available even 
suspended (note that all thread's stop_info are valid at this moment due to 
GetPrivateStopInfo gets called prior to DoOnRemoval). As a result we have a 
situation when suspended thread's stop_info tells we should stop even when the 
thread that is a real reason of stop says we should not.  Maybe you are right 
and the right place for the fix is inside 
Process::ProcessEventData::DoOnRemoval, something like this:

  if (stop_info_sp && stop_info_sp->IsValid() && thread_sp->ShouldStop()) {
  .
  }
  }

But you know, I don't know if it possible to apply it, semantics of 
Thread::ShouldStop is Thread::ShouldStop(Event *) and it is unclear what kind 
of event to pass in. In any case, maybe I don't see the whole picture of what's 
going on yet but I don't see any reason to hold on stop_info of suspended 
thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D81200: [vscode] set default values for terminateDebuggee for the disconnect request

2020-06-04 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
wallace added reviewers: clayborg, labath, aadsm.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace added a reviewer: kusmour.

Recently I've noticed that VSCode sometimes doesn't send the terminateDebuggee 
flag within the disconnectRequest,
even though lldb-vscode sets the terminateDebuggee capability correctly.
This has been causing that inferiors don't die after the debug session ends, 
and many users have reported issues because of this.

An easy way to mitigate this is to set better default values for the 
terminateDebuggee field in the disconnect request.
I'm assuming that for a launch request, the default will be true, and for 
attach it'll be false.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81200

Files:
  lldb/test/API/tools/lldb-vscode/disconnect/Makefile
  lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
  lldb/test/API/tools/lldb-vscode/disconnect/main.c
  lldb/tools/lldb-vscode/VSCode.h
  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
@@ -514,6 +514,7 @@
 //   }]
 // }
 void request_attach(const llvm::json::Object ) {
+  g_vsc.is_attach = true;
   llvm::json::Object response;
   lldb::SBError error;
   FillResponse(request, response);
@@ -769,7 +770,9 @@
   FillResponse(request, response);
   auto arguments = request.getObject("arguments");
 
-  bool terminateDebuggee = GetBoolean(arguments, "terminateDebuggee", false);
+  bool defaultTerminateDebuggee = g_vsc.is_attach ? false : true;
+  bool terminateDebuggee =
+  GetBoolean(arguments, "terminateDebuggee", defaultTerminateDebuggee);
   lldb::SBProcess process = g_vsc.target.GetProcess();
   auto state = process.GetState();
 
@@ -1357,6 +1360,7 @@
 //   }]
 // }
 void request_launch(const llvm::json::Object ) {
+  g_vsc.is_attach = false;
   llvm::json::Object response;
   lldb::SBError error;
   FillResponse(request, response);
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -89,6 +89,7 @@
   lldb::tid_t focus_tid;
   bool sent_terminated_event;
   bool stop_at_entry;
+  bool is_attach;
   // Keep track of the last stop thread index IDs as threads won't go away
   // unless we send a "thread" event to indicate the thread exited.
   llvm::DenseSet thread_ids;
Index: lldb/test/API/tools/lldb-vscode/disconnect/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/disconnect/main.c
@@ -0,0 +1,21 @@
+#include 
+
+void for_launch() {
+  int x = 0; // breakpoint 1
+  printf("%d\n", x);
+}
+
+void for_attach() {
+  int x;
+  // We wait for a signal to proceed
+  scanf("%d", );
+  printf("%d\n", x); // breakpoint 2
+}
+
+int main(int argc, char **args) {
+  if (argc == 1)
+for_launch();
+  else
+for_attach();
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
@@ -0,0 +1,68 @@
+"""
+Test lldb-vscode disconnect request
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import subprocess
+import time
+import os
+
+
+class TestVSCode_launch(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+source = 'main.c'
+
+def disconnect_and_check_no_output(self):
+self.vscode.request_disconnect()
+# verify we didn't get any input after disconnect
+time.sleep(2)
+output = self.get_stdout()
+self.assertTrue(output is None or len(output) == 0)
+
+@skipIfWindows
+@skipIfRemote
+def test_launch(self):
+"""
+This test launches a process that would output some text to the stdout
+if we let it run after disconnection.
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+
+self.set_source_breakpoints(self.source, [line_number(self.source, '// breakpoint 1')])
+self.continue_to_next_stop()
+
+self.disconnect_and_check_no_output()
+
+
+@skipIfWindows
+@skipIfRemote
+def test_attach(self):
+"""
+This test attaches to a running process that would output some text to the stdout
+if we let it run after disconnection.
+"""
+self.build_and_create_debug_adaptor()
+program = self.getBuildArtifact("a.out")
+
+self.process = subprocess.Popen([program, "--attach"],
+

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-04 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

Hi Jan, I noticed our sanitizer bot started getting failures in

Failing Tests (3):

  lldb-api :: commands/expression/unwind_expression/TestUnwindExpression.py
  lldb-unit :: Utility/./UtilityTests/DataExtractorTest.GetSLEB128_bit63
  lldb-shell :: SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s

/Users/buildslave/jenkins/workspace/lldb-cmake-sanitized/llvm-project/lldb/source/Utility/DataExtractor.cpp:934:17:
 runtime error: negation of -9223372036854775808 cannot be represented in type 
'int64_t' (aka 'long long'); cast to an unsigned type to negate this value to 
itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
/Users/buildslave/jenkins/workspace/lldb-cmake-sanitized/llvm-project/lldb/source/Utility/DataExtractor.cpp:934:17
 in 
http://green.lab.llvm.org/green/blue/organizations/jenkins/lldb-cmake-sanitized/detail/lldb-cmake-sanitized/1509/pipeline

TestUnwindExpression might be unrelated, but it looks like 
DataExtractorTest.GetSLEB128_bit63 and DW_TAG_variable-DW_AT_const_value.s are 
hitting this ubsan error after this commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119



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

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-04 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 268600.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D81058

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Stream.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/source/Utility/Stream.cpp

Index: lldb/source/Utility/Stream.cpp
===
--- lldb/source/Utility/Stream.cpp
+++ lldb/source/Utility/Stream.cpp
@@ -22,13 +22,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order)
+Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order,
+   bool color)
 : m_flags(flags), m_addr_size(addr_size), m_byte_order(byte_order),
-  m_indent_level(0), m_forwarder(*this) {}
+  m_indent_level(0), m_color_enabled(color), m_forwarder(*this) {}
 
 Stream::Stream()
 : m_flags(0), m_addr_size(4), m_byte_order(endian::InlHostByteOrder()),
-  m_indent_level(0), m_forwarder(*this) {}
+  m_indent_level(0), m_color_enabled(false), m_forwarder(*this) {}
 
 // Destructor
 Stream::~Stream() {}
@@ -174,6 +175,13 @@
 m_indent_level = 0;
 }
 
+bool Stream::HasColors() const { return m_color_enabled; }
+
+void Stream::SetColors(bool colors) {
+  m_color_enabled = colors;
+  m_forwarder.enable_colors(colors);
+}
+
 // Get the address size in bytes
 uint32_t Stream::GetAddressByteSize() const { return m_addr_size; }
 
Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -28,9 +28,12 @@
 strm.EOL();
 }
 
-CommandReturnObject::CommandReturnObject()
+CommandReturnObject::CommandReturnObject(bool colors)
 : m_out_stream(), m_err_stream(), m_status(eReturnStatusStarted),
-  m_did_change_process_state(false), m_interactive(true) {}
+  m_did_change_process_state(false), m_interactive(true) {
+  m_out_stream.SetColors(colors);
+  m_err_stream.SetColors(colors);
+}
 
 CommandReturnObject::~CommandReturnObject() {}
 
@@ -45,9 +48,10 @@
 
   const std::string  = std::string(sstrm.GetString());
   if (!s.empty()) {
-Stream _strm = GetErrorStream();
-error_strm.PutCString("error: ");
-DumpStringToStreamWithNewline(error_strm, s);
+llvm::WithColor(GetErrorStream().AsRawOstream(),
+llvm::HighlightColor::Error)
+<< "error: ";
+DumpStringToStreamWithNewline(GetErrorStream(), s);
   }
 }
 
@@ -72,7 +76,9 @@
   sstrm.PrintfVarArg(format, args);
   va_end(args);
 
-  GetErrorStream() << "warning: " << sstrm.GetString();
+  llvm::WithColor(GetErrorStream().AsRawOstream(),
+  llvm::HighlightColor::Warning)
+  << "warning: " << sstrm.GetString();
 }
 
 void CommandReturnObject::AppendMessage(llvm::StringRef in_string) {
@@ -84,7 +90,9 @@
 void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
   if (in_string.empty())
 return;
-  GetErrorStream() << "warning: " << in_string << "\n";
+  llvm::WithColor(GetErrorStream().AsRawOstream(),
+  llvm::HighlightColor::Warning)
+  << "warning: " << in_string << '\n';
 }
 
 // Similar to AppendWarning, but do not prepend 'warning: ' to message, and
@@ -99,7 +107,8 @@
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   if (in_string.empty())
 return;
-  GetErrorStream() << "error: " << in_string << "\n";
+  llvm::WithColor(GetErrorStream().AsRawOstream(), llvm::HighlightColor::Error)
+  << "error: " << in_string << '\n';
 }
 
 void CommandReturnObject::SetError(const Status ,
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -209,7 +209,7 @@
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
 
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
 
   LoadCommandDictionary();
 
@@ -2792,7 +2792,7 @@
 
   StartHandlingCommand();
 
-  lldb_private::CommandReturnObject result;
+  lldb_private::CommandReturnObject result(m_debugger.GetUseColor());
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
   // Now emit the command output text from the command we just executed
Index: lldb/include/lldb/Utility/Stream.h
===
--- lldb/include/lldb/Utility/Stream.h
+++ lldb/include/lldb/Utility/Stream.h
@@ -56,7 +56,8 @@
   ///
   /// Construct with dump flags \a flags and the default address size. \a
   /// flags can be any of the above enumeration logical OR'ed together.
-

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

Sorry, now that I'm thinking about this more, I'm confused as to why you are 
seeing the symptoms you describe.  Thread::ShouldStop starts with:

  if (GetResumeState() == eStateSuspended) {
LLDB_LOGF(log,
  "Thread::%s for tid = 0x%4.4" PRIx64 " 0x%4.4" PRIx64
  ", should_stop = 0 (ignore since thread was suspended)",
  __FUNCTION__, GetID(), GetProtocolID());
return false;
  }

So a suspended thread should always return false from "ShouldStop" and should 
never make a process stop.  Why isn't this effective in your case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

It bugs me a little bit that we are doing work masking the stop info when in 
fact the ShouldStop mechanism is the one that shouldn't be consulting threads 
that were suspended by the user during the last run.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Ilya Bukonkin via Phabricator via lldb-commits 
fallkrum marked an inline comment as done.
fallkrum added inline comments.



Comment at: lldb/source/Target/Thread.cpp:382
+if (m_stop_info_sp->IsValid() ||
+(IsStillAtLastBreakpointHit() &&
+ m_resume_state != eStateSuspended) ||

fallkrum wrote:
> What is historical need for this check? How is it possible for a breakpoint 
> to stop a thread that was already stopped second time even while stepping in 
> multithreaded programs?
Any thoughts on this? Maybe it is better to get rid of 
IsStillAtLastBreakpointHit at all? In this case there will be no need to check 
wether thread was suspended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Ilya Bukonkin via Phabricator via lldb-commits 
fallkrum added a comment.

@jingham tried test you added, got the following output:

Ilyas-Mac-mini:Ninja ilya$ ./bin/lldb-dotest -p TestStateAfterExpression.py
/Library/Frameworks/Python.framework/Versions/3.8/bin/python3.8 
/Users/ilya/Documents/Projects/llvm-project/lldb/test/API/dotest.py --arch 
x86_64 -s 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/lldb-test-traces -S nm 
-u CXXFLAGS -u CFLAGS --codesign-identity - --server 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/debugserver 
--build-dir 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/lldb-test-build.noindex 
--executable /Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/lldb 
--compiler /Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/clang 
--dsymutil 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/dsymutil 
--filecheck 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/FileCheck 
--lldb-libs-dir /Users/ilya/Documents/Projects/llvm-project/build/Ninja/./lib 
-p TestStateAfterExpression.py
LLDB library dir: /Users/ilya/Documents/Projects/llvm-project/build/Ninja/bin
LLDB import library dir: 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/./lib
lldb version 11.0.0

  clang revision c0b351fea652442190b5c890cbd23297762a5d64
  llvm revision c0b351fea652442190b5c890cbd23297762a5d64

libstdcxx tests will not be run because: Don't know how to build with libstdcxx 
on macosx
Skipping following debug info categories: ['dwo']

Session logs for test failures/errors/unexpected successes will go into 
directory 
'/Users/ilya/Documents/Projects/llvm-project/build/Ninja/lldb-test-traces'
Command invoked: 
/Users/ilya/Documents/Projects/llvm-project/lldb/test/API/dotest.py --arch 
x86_64 -s 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/lldb-test-traces -S nm 
-u CXXFLAGS -u CFLAGS --codesign-identity - --server 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/debugserver 
--build-dir 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/lldb-test-build.noindex 
--executable /Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/lldb 
--compiler /Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/clang 
--dsymutil 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/dsymutil 
--filecheck 
/Users/ilya/Documents/Projects/llvm-project/build/Ninja/./bin/FileCheck 
--lldb-libs-dir /Users/ilya/Documents/Projects/llvm-project/build/Ninja/./lib 
-p TestStateAfterExpression.py
PASS: LLDB 
(/Users/ilya/Documents/Projects/llvm-project/build/Ninja/bin/clang-11-x86_64) 
:: test_thread_state_after_expr_dsym 
(TestStateAfterExpression.TestStopReasonAfterExpression)
PASS: LLDB 
(/Users/ilya/Documents/Projects/llvm-project/build/Ninja/bin/clang-11-x86_64) 
:: test_thread_state_after_expr_dwarf 
(TestStateAfterExpression.TestStopReasonAfterExpression)
UNSUPPORTED: LLDB 
(/Users/ilya/Documents/Projects/llvm-project/build/Ninja/bin/clang-11-x86_64) 
:: test_thread_state_after_expr_dwo 
(TestStateAfterExpression.TestStopReasonAfterExpression) (test case does not 
fall in any category of interest for this run)

PASS: LLDB 
(/Users/ilya/Documents/Projects/llvm-project/build/Ninja/bin/clang-11-x86_64) 
:: test_thread_state_after_expr_gmodules 
(TestStateAfterExpression.TestStopReasonAfterExpression)
--

Ran 4 tests in 28.206s

RESULT: PASSED (3 passes, 0 failures, 0 errors, 1 skipped, 0 expected failures, 
0 unexpected successes)

If I understand everything correctly the patch passes the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-04 Thread Jan Kratochvil via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG476f520a0bd2: [lldb] Fix SLEB128 decoding (authored by 
jankratochvil).

Changed prior to commit:
  https://reviews.llvm.org/D81119?vs=268489=268535#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119

Files:
  lldb/source/Utility/DataExtractor.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s
  lldb/unittests/Utility/DataExtractorTest.cpp

Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -261,3 +261,41 @@
   EXPECT_EQ(0x0102030405060708U, BE.GetMaxU64_unchecked(, 8));
   EXPECT_EQ(8U, offset);
 }
+
+TEST(DataExtractorTest, GetSLEB128_bit63) {
+  uint8_t buffer[] = {0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0x7f};
+
+  DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
+
+  lldb::offset_t offset;
+
+  int64_t expected =
+  0b000111000111000111000111;
+  offset = 0;
+  EXPECT_EQ(expected, LE.GetSLEB128());
+  EXPECT_EQ(9U, offset);
+  offset = 0;
+  EXPECT_EQ(expected, BE.GetSLEB128());
+  EXPECT_EQ(9U, offset);
+}
+
+TEST(DataExtractorTest, GetULEB128_bit63) {
+  uint8_t buffer[] = {0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0x7f};
+
+  DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
+
+  lldb::offset_t offset;
+
+  uint64_t expected =
+  0b0111000111000111000111000111;
+  offset = 0;
+  EXPECT_EQ(expected, LE.GetULEB128());
+  EXPECT_EQ(9U, offset);
+  offset = 0;
+  EXPECT_EQ(expected, BE.GetULEB128());
+  EXPECT_EQ(9U, offset);
+}
Index: lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s
@@ -0,0 +1,85 @@
+# This tests that lldb is able to print DW_TAG_variable using DW_AT_const_value.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -triple x86_64-unknown-linux-gnu %s -filetype=obj > %t.o
+# RUN: %lldb %t.o -o "p/x magic64" -o exit | FileCheck %s
+
+# CHECK: (const long) $0 = 0xed9a924c00011151
+
+# The DW_TAG_variable using DW_AT_const_value. can be produced from:
+# static const long magic64 = 0xed9a924c00011151;
+# int main(void) { return magic64; }
+
+	.text
+	.globl	main# -- Begin function main
+	.type	main,@function
+main:   # @main
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+# -- End function
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	8   # DW_FORM_string
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	8   # DW_FORM_string
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	28  # DW_AT_const_value
+#	.byte	15  # DW_FORM_udata
+	.byte	13  # DW_FORM_sdata
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	38  # DW_TAG_const_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	4   # Abbreviation Code
+	.byte	36  # DW_TAG_base_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	8   # DW_FORM_string
+	.byte	62  # DW_AT_encoding
+	.byte	11  # DW_FORM_data1
+	.byte	11  # DW_AT_byte_size
+	.byte	11  # DW_FORM_data1
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	0   # EOM(3)
+	.section

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-04 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil marked 5 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:6
+# RUN: llvm-mc -g -dwarf-version=5 -triple x86_64-unknown-linux-gnu %s 
-filetype=obj > %t.o
+# RUN: ld.lld -m elf_x86_64 %t.o -o %t 
+# RUN: %lldb %t -o "p/x magic64" -o exit | FileCheck %s

jankratochvil wrote:
> labath wrote:
> > It's kinda weird (though very useful for tests) but lldb can open .o files 
> > too. You should be able to drop the linker and then the main function too.
> I admit it works with `.o` now but when I was testing it before it did not 
> (despite GDB could open such `.o` file).
I have thus removed the `REQUIRES: lld`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119



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

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-04 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil marked 12 inline comments as done.
jankratochvil added a comment.

In D81119#2073973 , @labath wrote:

> PS: You can still just drop the test if you think that's too much hassle. :)


It is sure nice to learn more the LLDB high standards, thanks for the review.




Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:6
+# RUN: llvm-mc -g -dwarf-version=5 -triple x86_64-unknown-linux-gnu %s 
-filetype=obj > %t.o
+# RUN: ld.lld -m elf_x86_64 %t.o -o %t 
+# RUN: %lldb %t -o "p/x magic64" -o exit | FileCheck %s

labath wrote:
> It's kinda weird (though very useful for tests) but lldb can open .o files 
> too. You should be able to drop the linker and then the main function too.
I admit it works with `.o` now but when I was testing it before it did not 
(despite GDB could open such `.o` file).



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:11-13
+# It is built from this source with a few #-marked patched lines:
+# static const long magic64 = 0xed9a924c00011151;
+# int main(void) { return magic64; }

labath wrote:
> At this point the file is far enough from the original, that this and the 
> "patched" lines are not useful.
I disagree, this is a minimal C code to produce DWARF containing 
`DW_TAG_variable` + `DW_AT_const_value`. I have reworded the comment.



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:24-28
+   .asciz  "clang version 10.0.0" # string offset=0
+.Linfo_string3:
+   .asciz  "magic64"   # string offset=74
+.Linfo_string4:
+   .asciz  "long int"  # string offset=82

labath wrote:
> The offset comments aren't correct anymore. Just delete them. (I usually also 
> inline my strings via DW_FORM_string forms, but you don't need to do that if 
> you don't want to).
I just do not think it is worth the development time to make even testcases so 
optimal but OK.



Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:270
+   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void 
*));
+

labath wrote:
> I'm not sure how useful it is to have both big and little versions of these. 
> If you think they add value, you can keep them, but I'd personally just use 
> one (maybe the big one as it's more likely to flush out errors).
I have kept them, what if someone makes there some endianity dependency, dunno.



Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:292
+
+  int64_t expected = 
0b0111000111000111000111000111;
+  offset = 0;

labath wrote:
> Should this be uint64_t ?
Yes, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119



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

[Lldb-commits] [lldb] a976a7f - Disable this test for Windows.

2020-06-04 Thread Jim Ingham via lldb-commits 

Author: Jim Ingham
Date: 2020-06-04T10:51:01-07:00
New Revision: a976a7fcae44ed5ca9e4f13735a4f91378169282

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

LOG: Disable this test for Windows.

The printf expression crashes with the message:

Attempted to dereference an invalid pointer

Someone who knows more about Windows should suggest how to fix this.

Added: 


Modified: 

lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
 
b/lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
index 082b556dbdce..b108d53c9654 100644
--- 
a/lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
+++ 
b/lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
@@ -8,12 +8,13 @@
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test.lldbtest import *
-
+from lldbsuite.test.decorators import *
 
 class TestStopReasonAfterExpression(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfWindows
 def test_thread_state_after_expr(self):
 self.build()
 self.main_source_file = lldb.SBFileSpec("main.cpp")



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

[Lldb-commits] [lldb] 476f520 - [lldb] Fix SLEB128 decoding

2020-06-04 Thread Jan Kratochvil via lldb-commits 

Author: Jan Kratochvil
Date: 2020-06-04T19:41:24+02:00
New Revision: 476f520a0bd29d74f559ea1151ac8d0b428d9150

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

LOG: [lldb] Fix SLEB128 decoding

Bug 46181 shows SLEB128 0xED9A924C00011151 decoded as 0x80011151.
LLDB show a wrong value for function argument
https://bugs.llvm.org/show_bug.cgi?id=46181

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s

Modified: 
lldb/source/Utility/DataExtractor.cpp
lldb/unittests/Utility/DataExtractorTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/DataExtractor.cpp 
b/lldb/source/Utility/DataExtractor.cpp
index 40819f107052..5f4abb82163e 100644
--- a/lldb/source/Utility/DataExtractor.cpp
+++ b/lldb/source/Utility/DataExtractor.cpp
@@ -931,7 +931,7 @@ int64_t DataExtractor::GetSLEB128(offset_t *offset_ptr) 
const {
 
 // Sign bit of byte is 2nd high order bit (0x40)
 if (shift < size && (byte & 0x40))
-  result |= -(1 << shift);
+  result |= -(static_cast(1) << shift);
 
 *offset_ptr += bytecount;
 return result;

diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s 
b/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s
new file mode 100644
index ..a6d4f199a647
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s
@@ -0,0 +1,85 @@
+# This tests that lldb is able to print DW_TAG_variable using 
DW_AT_const_value.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -triple x86_64-unknown-linux-gnu %s -filetype=obj > %t.o
+# RUN: %lldb %t.o -o "p/x magic64" -o exit | FileCheck %s
+
+# CHECK: (const long) $0 = 0xed9a924c00011151
+
+# The DW_TAG_variable using DW_AT_const_value. can be produced from:
+# static const long magic64 = 0xed9a924c00011151;
+# int main(void) { return magic64; }
+
+   .text
+   .globl  main# -- Begin function main
+   .type   main,@function
+main:   # @main
+.Lfunc_end0:
+   .size   main, .Lfunc_end0-main
+# -- End function
+   .section.debug_abbrev,"",@progbits
+   .byte   1   # Abbreviation Code
+   .byte   17  # DW_TAG_compile_unit
+   .byte   1   # DW_CHILDREN_yes
+   .byte   37  # DW_AT_producer
+   .byte   8   # DW_FORM_string
+   .byte   19  # DW_AT_language
+   .byte   5   # DW_FORM_data2
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   2   # Abbreviation Code
+   .byte   52  # DW_TAG_variable
+   .byte   0   # DW_CHILDREN_no
+   .byte   3   # DW_AT_name
+   .byte   8   # DW_FORM_string
+   .byte   73  # DW_AT_type
+   .byte   19  # DW_FORM_ref4
+   .byte   28  # DW_AT_const_value
+#  .byte   15  # DW_FORM_udata
+   .byte   13  # DW_FORM_sdata
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   3   # Abbreviation Code
+   .byte   38  # DW_TAG_const_type
+   .byte   0   # DW_CHILDREN_no
+   .byte   73  # DW_AT_type
+   .byte   19  # DW_FORM_ref4
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   4   # Abbreviation Code
+   .byte   36  # DW_TAG_base_type
+   .byte   0   # DW_CHILDREN_no
+   .byte   3   # DW_AT_name
+   .byte   8   # DW_FORM_string
+   .byte   62  # DW_AT_encoding
+   .byte   11  # DW_FORM_data1
+   .byte   11  # DW_AT_byte_size
+   .byte   11  # DW_FORM_data1
+   .byte   0   # EOM(1)
+   .byte   0   # EOM(2)
+   .byte   0   # EOM(3)
+   .section.debug_info,"",@progbits
+.Lcu_begin0:
+   .long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+   .short  4   # DWARF version number
+   .long   .debug_abbrev   # Offset Into Abbrev. Section
+   .byte   8

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-04 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. The fix and the unittest look good. I have a bunch of comments on the 
.s file. As I note in the inline comments, I don't think that having a test 
like that for specifically handling sleb encoding has value. But if we rephrase 
it to test printing of "const_values" then I think it is ok. I also note some 
additional simplifications opportunities.

PS: You can still just drop the test if you think that's too much hassle. :)




Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:1
+# This tests that lldb is able to process 64-bit wide negative SLEB128.
+

For me, the interesting aspect of this test is that we have a very targeted 
test for the handling of const_values of variables. The fact that it uses a 
sleb128 (much less a wide negative sleb128) is not important here as that can 
be more easily tested elsewhere. So, I'd rename this test to something like 
`DW_TAG_variable-const_value`, and rephrase this comment to reflect that.



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:5
+
+# RUN: llvm-mc -g -dwarf-version=5 -triple x86_64-unknown-linux-gnu %s 
-filetype=obj > %t.o
+# RUN: ld.lld -m elf_x86_64 %t.o -o %t 

I don't think `-g -dwarf-version=5 ` is needed here.



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:6
+# RUN: llvm-mc -g -dwarf-version=5 -triple x86_64-unknown-linux-gnu %s 
-filetype=obj > %t.o
+# RUN: ld.lld -m elf_x86_64 %t.o -o %t 
+# RUN: %lldb %t -o "p/x magic64" -o exit | FileCheck %s

It's kinda weird (though very useful for tests) but lldb can open .o files too. 
You should be able to drop the linker and then the main function too.



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:11-13
+# It is built from this source with a few #-marked patched lines:
+# static const long magic64 = 0xed9a924c00011151;
+# int main(void) { return magic64; }

At this point the file is far enough from the original, that this and the 
"patched" lines are not useful.



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:24-28
+   .asciz  "clang version 10.0.0" # string offset=0
+.Linfo_string3:
+   .asciz  "magic64"   # string offset=74
+.Linfo_string4:
+   .asciz  "long int"  # string offset=82

The offset comments aren't correct anymore. Just delete them. (I usually also 
inline my strings via DW_FORM_string forms, but you don't need to do that if 
you don't want to).



Comment at: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s:84
+#  .ascii  "\321\242\204\200\300\311\244\315\355\001" # DW_AT_const_value
+   .ascii  "\321\242\204\200\300\311\244\315m" # DW_AT_const_value - sdata
+.Lconst:

`.sleb128 0xed9a924c00011151`



Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:270
+   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void 
*));
+

I'm not sure how useful it is to have both big and little versions of these. If 
you think they add value, you can keep them, but I'd personally just use one 
(maybe the big one as it's more likely to flush out errors).



Comment at: lldb/unittests/Utility/DataExtractorTest.cpp:292
+
+  int64_t expected = 
0b0111000111000111000111000111;
+  offset = 0;

Should this be uint64_t ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119



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

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

2020-06-04 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin updated this revision to Diff 268497.
jarin added a comment.

Exclude UUID strings ending with "-".


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

https://reviews.llvm.org/D80755

Files:
  lldb/include/lldb/Utility/UUID.h
  lldb/source/Interpreter/OptionValueUUID.cpp
  lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Utility/UUID.cpp
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
  
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-4.yaml
  
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
  lldb/unittests/Target/ModuleCacheTest.cpp
  lldb/unittests/Utility/UUIDTest.cpp

Index: lldb/unittests/Utility/UUIDTest.cpp
===
--- lldb/unittests/Utility/UUIDTest.cpp
+++ lldb/unittests/Utility/UUIDTest.cpp
@@ -45,7 +45,7 @@
   from_str.SetFromStringRef("----");
   UUID opt_from_str;
   opt_from_str.SetFromOptionalStringRef("----");
-  
+
   EXPECT_FALSE(empty);
   EXPECT_TRUE(a16);
   EXPECT_TRUE(a20);
@@ -57,25 +57,30 @@
 
 TEST(UUIDTest, SetFromStringRef) {
   UUID u;
-  EXPECT_EQ(32u, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f"));
+  EXPECT_TRUE(u.SetFromStringRef("404142434445464748494a4b4c4d4e4f"));
   EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
 
-  EXPECT_EQ(36u, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
+  EXPECT_TRUE(u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
   EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
 
-  EXPECT_EQ(45u, u.SetFromStringRef(
- "40-41-42-43-4445464748494a4b4c4d4e4f-50515253", 20));
+  EXPECT_TRUE(
+  u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f-50515253"));
   EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u);
 
-  EXPECT_EQ(0u, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f", 20));
-  EXPECT_EQ(0u, u.SetFromStringRef("40x"));
-  EXPECT_EQ(0u, u.SetFromStringRef(""));
-  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u)
+  EXPECT_TRUE(u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
+
+  EXPECT_FALSE(u.SetFromStringRef("40x"));
+  EXPECT_FALSE(u.SetFromStringRef(""));
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u)
   << "uuid was changed by failed parse calls";
 
-  EXPECT_EQ(
-  32u, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253", 16));
-  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
+  EXPECT_TRUE(u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253"));
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u);
+
+  EXPECT_TRUE(u.SetFromStringRef("40414243"));
+  EXPECT_EQ(UUID::fromData("@ABCD", 4), u);
+
+  EXPECT_FALSE(u.SetFromStringRef("4"));
 }
 
 TEST(UUIDTest, StringConverion) {
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/ModuleCacheTest.cpp
+++ lldb/unittests/Target/ModuleCacheTest.cpp
@@ -41,7 +41,6 @@
 static const char module_name[] = "TestModule.so";
 static const char module_uuid[] =
 "F4E7E991-9B61-6AD4-0073-561AC3D9FA10-C043A476";
-static const uint32_t uuid_bytes = 20;
 static const size_t module_size = 5602;
 
 static FileSpec GetDummyRemotePath() {
@@ -87,7 +86,7 @@
   ModuleCache mc;
   ModuleSpec module_spec;
   module_spec.GetFileSpec() = GetDummyRemotePath();
-  module_spec.GetUUID().SetFromStringRef(module_uuid, uuid_bytes);
+  module_spec.GetUUID().SetFromStringRef(module_uuid);
   module_spec.SetObjectSize(module_size);
   ModuleSP module_sp;
   bool did_create;
Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -156,7 +156,7 @@
   ModuleSpec Spec;
   ASSERT_TRUE(Specs.GetModuleSpecAtIndex(0, Spec)) ;
   UUID Uuid;
-  Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9", 20);
+  Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9");
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
 
@@ -284,4 +284,4 @@
 
   auto entry_point_addr = module_sp->GetObjectFile()->GetEntryPointAddress();
   ASSERT_EQ(entry_point_addr.GetAddressClass(), AddressClass::eCode);
-}
\ No newline at end of file
+}
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-uuids-match.yaml
@@ -0,0 +1,15 @@
+--- !minidump
+Streams: 
+  - Type:

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-04 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil updated this revision to Diff 268489.
jankratochvil added a comment.

Added unit test, simplified the `.s` test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119

Files:
  lldb/source/Utility/DataExtractor.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s
  lldb/unittests/Utility/DataExtractorTest.cpp

Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -261,3 +261,39 @@
   EXPECT_EQ(0x0102030405060708U, BE.GetMaxU64_unchecked(, 8));
   EXPECT_EQ(8U, offset);
 }
+
+TEST(DataExtractorTest, GetSLEB128_bit63) {
+  uint8_t buffer[] = {0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0x7f};
+
+  DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
+
+  lldb::offset_t offset;
+
+  int64_t expected = 0b000111000111000111000111;
+  offset = 0;
+  EXPECT_EQ(expected, LE.GetSLEB128());
+  EXPECT_EQ(9U, offset);
+  offset = 0;
+  EXPECT_EQ(expected, BE.GetSLEB128());
+  EXPECT_EQ(9U, offset);
+}
+
+TEST(DataExtractorTest, GetULEB128_bit63) {
+  uint8_t buffer[] = {0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0xff, 0x80, 0x7f};
+
+  DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
+   sizeof(void *));
+  DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig, sizeof(void *));
+
+  lldb::offset_t offset;
+
+  int64_t expected = 0b0111000111000111000111000111;
+  offset = 0;
+  EXPECT_EQ(expected, LE.GetULEB128());
+  EXPECT_EQ(9U, offset);
+  offset = 0;
+  EXPECT_EQ(expected, BE.GetULEB128());
+  EXPECT_EQ(9U, offset);
+}
Index: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s
@@ -0,0 +1,94 @@
+# This tests that lldb is able to process 64-bit wide negative SLEB128.
+
+# REQUIRES: lld, x86
+
+# RUN: llvm-mc -g -dwarf-version=5 -triple x86_64-unknown-linux-gnu %s -filetype=obj > %t.o
+# RUN: ld.lld -m elf_x86_64 %t.o -o %t 
+# RUN: %lldb %t -o "p/x magic64" -o exit | FileCheck %s
+
+# CHECK: (const long) $0 = 0xed9a924c00011151
+
+# It is built from this source with a few #-marked patched lines:
+# static const long magic64 = 0xed9a924c00011151;
+# int main(void) { return magic64; }
+
+	.text
+	.globl	main# -- Begin function main
+	.type	main,@function
+main:   # @main
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+# -- End function
+	.section	.debug_str,"MS",@progbits,1
+.Linfo_string0:
+	.asciz	"clang version 10.0.0" # string offset=0
+.Linfo_string3:
+	.asciz	"magic64"   # string offset=74
+.Linfo_string4:
+	.asciz	"long int"  # string offset=82
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	14  # DW_FORM_strp
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	28  # DW_AT_const_value
+#	.byte	15  # DW_FORM_udata
+	.byte	13  # DW_FORM_sdata
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	38  # DW_TAG_const_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	4   # Abbreviation Code
+	.byte	36  # DW_TAG_base_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	62  # DW_AT_encoding
+	.byte	11  # DW_FORM_data1
+	.byte	11  # DW_AT_byte_size
+	.byte	11  # DW_FORM_data1
+	.byte	0   # EOM(1)
+	.byte	0

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-04 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

The fix is good (thanks for tracking that down), but the test is way to 
complicated for what the fix does. A generic "can we show the DW_AT_const_value 
of a global variable" test might be useful -- I don't believe we have anything 
exactly like that  right now -- but it ought to be reduced as it contains a 
bunch of unrelated/unneeded stuff. However, that's strictly optional..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81119



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

[Lldb-commits] [PATCH] D80775: [lldb] tab completion for `command script delete'

2020-06-04 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2e8f304f5ea5: [lldb] tab completion for `command script 
delete (authored by MrHate, committed by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80775

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -371,6 +371,10 @@
 self.complete_from_to("watchpoint set variable foo --watch w", 
"watchpoint set variable foo --watch write")
 self.complete_from_to('watchpoint set variable foo -w read_', 
'watchpoint set variable foo -w read_write')
 
+def test_command_script_delete(self):
+self.runCmd("command script add -h test_desc -f none -s current 
usercmd1")
+self.check_completion_with_desc('command script delete ', 
[['usercmd1', 'test_desc']])
+
 def test_completion_description_commands(self):
 """Test descriptions of top-level command completions"""
 self.check_completion_with_desc("", [
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1767,6 +1767,19 @@
 
   ~CommandObjectCommandsScriptDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+
+if (!m_interpreter.HasCommands() || request.GetCursorIndex() != 0)
+  return;
+const auto _dict = m_interpreter.GetUserCommands();
+
+for (auto pos = user_dict.begin(); pos != user_dict.end(); ++pos) {
+  request.TryCompleteCurrentArg(pos->first, pos->second->GetHelp());
+}
+  }
+
 protected:
   bool DoExecute(Args , CommandReturnObject ) override {
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -494,6 +494,10 @@
   bool GetEchoCommentCommands() const;
   void SetEchoCommentCommands(bool enable);
 
+  const CommandObject::CommandMap () const {
+return m_user_dict;
+  }
+
   /// Specify if the command interpreter should allow that the user can
   /// specify a custom exit code when calling 'quit'.
   void AllowExitCodeOnQuit(bool allow);


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -371,6 +371,10 @@
 self.complete_from_to("watchpoint set variable foo --watch w", "watchpoint set variable foo --watch write")
 self.complete_from_to('watchpoint set variable foo -w read_', 'watchpoint set variable foo -w read_write')
 
+def test_command_script_delete(self):
+self.runCmd("command script add -h test_desc -f none -s current usercmd1")
+self.check_completion_with_desc('command script delete ', [['usercmd1', 'test_desc']])
+
 def test_completion_description_commands(self):
 """Test descriptions of top-level command completions"""
 self.check_completion_with_desc("", [
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1767,6 +1767,19 @@
 
   ~CommandObjectCommandsScriptDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+
+if (!m_interpreter.HasCommands() || request.GetCursorIndex() != 0)
+  return;
+const auto _dict = m_interpreter.GetUserCommands();
+
+for (auto pos = user_dict.begin(); pos != user_dict.end(); ++pos) {
+  request.TryCompleteCurrentArg(pos->first, pos->second->GetHelp());
+}
+  }
+
 protected:
   bool DoExecute(Args , CommandReturnObject ) override {
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -494,6 +494,10 @@
   bool GetEchoCommentCommands() const;
   void SetEchoCommentCommands(bool enable);
 
+  const CommandObject::CommandMap ()

[Lldb-commits] [lldb] 2ebe30c - [lldb][NFC] Address some review feedback for D80775 ('command script delete' completion)

2020-06-04 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-06-04T10:30:27+02:00
New Revision: 2ebe30c6e4ab1d65fc8605051eb528fc26ffc621

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

LOG: [lldb][NFC] Address some review feedback for D80775 ('command script 
delete' completion)

In the similar review D81128, Jonas pointed out some style errors that also
apply to D80775 (which is already committed). Also applying the changes
suggested there to this code.

Added: 


Modified: 
lldb/source/Commands/CommandObjectCommands.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index 8f72ed7177e0..8c78803ab159 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -1770,14 +1770,11 @@ class CommandObjectCommandsScriptDelete : public 
CommandObjectParsed {
   void
   HandleArgumentCompletion(CompletionRequest ,
OptionElementVector _element_vector) override {
-
 if (!m_interpreter.HasCommands() || request.GetCursorIndex() != 0)
   return;
-const auto _dict = m_interpreter.GetUserCommands();
 
-for (auto pos = user_dict.begin(); pos != user_dict.end(); ++pos) {
-  request.TryCompleteCurrentArg(pos->first, pos->second->GetHelp());
-}
+for (const auto  : m_interpreter.GetUserCommands())
+  request.TryCompleteCurrentArg(c.first, c.second->GetHelp());
   }
 
 protected:



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

[Lldb-commits] [lldb] 2e8f304 - [lldb] tab completion for `command script delete'

2020-06-04 Thread Raphael Isemann via lldb-commits 

Author: Gongyu Deng
Date: 2020-06-04T10:19:03+02:00
New Revision: 2e8f304f5ea5f7b2bd4f5689eae23d4399cd5120

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

LOG: [lldb] tab completion for `command script delete'

Summary: Added the tab completion for `command script delete`.

Reviewers: teemperor, JDevlieghere

Reviewed By: teemperor

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Commands/CommandObjectCommands.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index a0a9bcb49969..8a9dce7a19bc 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -494,6 +494,10 @@ class CommandInterpreter : public Broadcaster,
   bool GetEchoCommentCommands() const;
   void SetEchoCommentCommands(bool enable);
 
+  const CommandObject::CommandMap () const {
+return m_user_dict;
+  }
+
   /// Specify if the command interpreter should allow that the user can
   /// specify a custom exit code when calling 'quit'.
   void AllowExitCodeOnQuit(bool allow);

diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index b12377d71512..8f72ed7177e0 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -1767,6 +1767,19 @@ class CommandObjectCommandsScriptDelete : public 
CommandObjectParsed {
 
   ~CommandObjectCommandsScriptDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+
+if (!m_interpreter.HasCommands() || request.GetCursorIndex() != 0)
+  return;
+const auto _dict = m_interpreter.GetUserCommands();
+
+for (auto pos = user_dict.begin(); pos != user_dict.end(); ++pos) {
+  request.TryCompleteCurrentArg(pos->first, pos->second->GetHelp());
+}
+  }
+
 protected:
   bool DoExecute(Args , CommandReturnObject ) override {
 

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index 7c674dc872ed..4a548ad77083 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -371,6 +371,10 @@ def test_command_argument_completion(self):
 self.complete_from_to("watchpoint set variable foo --watch w", 
"watchpoint set variable foo --watch write")
 self.complete_from_to('watchpoint set variable foo -w read_', 
'watchpoint set variable foo -w read_write')
 
+def test_command_script_delete(self):
+self.runCmd("command script add -h test_desc -f none -s current 
usercmd1")
+self.check_completion_with_desc('command script delete ', 
[['usercmd1', 'test_desc']])
+
 def test_completion_description_commands(self):
 """Test descriptions of top-level command completions"""
 self.check_completion_with_desc("", [



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