[Lldb-commits] [PATCH] D128780: [lldb] [test] Use raise(SIGSTOP) instead of trap in fork tests

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

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jingham.
Herald added subscribers: kristof.beyls, arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Replace the use of "trap" with a new "stop" command in fork tests,
that maps to `raise(SIGSTOP)`.  Since traps do not increment PC on some
architectures (notably ARM), using traps would require special logic
to increment it while testing.  Using SIGSTOP avoids the problem
and is probably more logical, given that the purpose of the "trap"s
was to simply stop the inferior at a synchronization point.  This fixes
tests on AArch64 (and possibly ARM, I'll update XFAILs when it is
confirmed by the buildbot).

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D128780

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/fork_testbase.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -353,6 +353,10 @@
   printf("%s\n", value ? value : "__unset__");
 } else if (consume_front(arg, "trap")) {
   trap();
+#if !defined(_WIN32)
+} else if (consume_front(arg, "stop")) {
+  raise(SIGSTOP);
+#endif
 } else {
   // Treat the argument as text for stdout.
   printf("%s\n", argv[i]);
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -100,16 +100,12 @@
 self.vkill_test(kill_parent=True, kill_child=True, nonstop=True)
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_interspersed_nonstop(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],
  nonstop=True)
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_interspersed_nonstop(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -154,73 +154,53 @@
 self.vkill_test(kill_parent=True, kill_child=True)
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_parent(self):
 self.resume_one_test(run_order=["parent", "parent"])
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_child(self):
 self.resume_one_test(run_order=["child", "child"])
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"])
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"])
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
 @expectedFailureAll(archs=["arm"])  # TODO
-@expectedFailureAll(archs=["aarch64"],
-bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_parent(self):
 self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
 
 @expectedFailureAll(archs=["arm"])  # TODO

[Lldb-commits] [lldb] 7fc12da - [lldb] [test] Split TestGdbRemoteFork in two

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-29T06:57:38+02:00
New Revision: 7fc12da898e05e68ec59fbe852c4402378938efa

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

LOG: [lldb] [test] Split TestGdbRemoteFork in two

Split the test that's gotten very long in two, in hope that it will
resolve the arm/aarch64 buildbot failures.  Even if it does not, it
could help pinpointing where the problem lies.

Sponsored by: The FreeBSD Foundation

Added: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/fork_testbase.py
lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py

Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/fork_testbase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/fork_testbase.py
new file mode 100644
index ..42833a15318f
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/fork_testbase.py
@@ -0,0 +1,204 @@
+import gdbremote_testcase
+
+
+class GdbRemoteForkTestBase(gdbremote_testcase.GdbRemoteTestCaseBase):
+fork_regex = ("[$]T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_regex_nonstop = ("%Stop:T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_capture = {1: "parent_pid", 2: "parent_tid",
+3: "child_pid", 4: "child_tid"}
+
+def start_fork_test(self, args, variant="fork", nonstop=False):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=args)
+self.add_qSupported_packets(["multiprocess+",
+ "{}-events+".format(variant)])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+if nonstop:
+self.test_sequence.add_log_lines([
+"read packet: $QNonStop:1#00",
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": self.fork_regex_nonstop.format(variant),
+ "capture": self.fork_capture},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format(variant),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+return tuple(ret[x] for x in ("parent_pid", "parent_tid",
+  "child_pid", "child_tid"))
+
+def fork_and_detach_test(self, variant, nonstop=False):
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test([variant], variant, nonstop=nonstop))
+
+# detach the forked child
+self.test_sequence.add_log_lines([
+"read packet: $D;{}#00".format(child_pid),
+"send packet: $OK#00",
+# verify that the current process is correct
+"read packet: $qC#00",
+"send packet: $QCp{}.{}#00".format(parent_pid, parent_tid),
+# verify that the correct processes are detached/available
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $Eff#00",
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
+], True)
+self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+return parent_pid, parent_tid
+
+def fork_and_follow_test(self, variant, nonstop=False):
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test([variant], variant, nonstop=nonstop))
+
+# switch to the forked child
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",
+"read packet: $Hcp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",
+# detach the parent
+"read packet: $D;{}#00".format(parent_pid),
+"send packet: $OK#00",
+# verify that the correct processes are detached/available
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $Eff#00",
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",

[Lldb-commits] [PATCH] D128775: [trace] Fix errors when handling command arguments

[Sujin Park via Phabricator via lldb-commits]

persona0220 added a comment.

lgtm. Thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128775

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

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

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

mgorny added a comment.

I am trying to but unfortunately the server seems to be down. Unfortunately, I 
can't reproduce this on my aarch64 test machine but maybe the test is taking 
too long now (we've been suspecting that might be the case). I'm going to split 
it and see if that helps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128638

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

[Lldb-commits] [PATCH] D128576: [trace] Make events first class items in the trace cursor and rework errors

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/include/lldb/Target/Trace.h:171
+  /// information failed to load, an \a llvm::Error is returned.
+  virtual llvm::Expected
+  CreateNewCursor(Thread ) = 0;

jj10306 wrote:
> Do we want to keep the cursor as a UP or take this refactor before creating 
> the public bindings to make it a SP? IMO a UP might make sense because I 
> can't really think of many cases where you would want multiple users of the 
> same cursor.
> wdyt?
It seems that we can use unique_ptrs! SBMemoryRegionInfoList is an example of a 
SB wrapper of a unique_ptr. It seems that python reuses the same reference of 
the SB object with all the variables that use it. On the other hand, if you use 
c++, the unique_ptr would be enforced strictly



Comment at: lldb/include/lldb/Target/TraceCursor.h:44-46
+/// The cursor can also point at events in the trace, which aren't errors
+/// nor instructions. An example of an event could be a context switch in
+/// between two instructions.

jj10306 wrote:
> do you want to include pointers to the methods you can use to check/get 
> events similar to what you did for errors above?
good idea!



Comment at: lldb/include/lldb/Target/TraceCursor.h:232-246
+  virtual const char *GetError() const = 0;
 
-  /// Get the corresponding error message if the cursor points to an error in
-  /// the trace.
-  ///
   /// \return
-  /// \b nullptr if the cursor is not pointing to an error in
-  /// the trace. Otherwise return the actual error message.
-  virtual const char *GetError() = 0;
+  /// Whether the cursor points to an event or not.
+  virtual bool IsEvent() const = 0;
 
   /// \return

jj10306 wrote:
> For the getters, what are your thoughts on returning an optional instead of 
> using designated value/variants (ie nullptr, LLDB_INVALID_INSTRUCTION, 
> eTraceEventNone`) to indicate that the current item does not match what `Get` 
> method is being used?
> 
> Along the same lines, but a slightly bigger change:
> With the introduction of `Events` as first class citizens of the trace cursor 
> API, it may make sense to introduce the notion of of a trace "item" or 
> something similar that encapsulates (instructions, events, errors, etc). I'm 
> thinking of a tagged union type structure (ie a Rust enum)  that enforces the 
> correctness of the access to the underlying item.
> wdyt?
check the new version of the code. I'm now using a tagged union for the low 
level storage, and then using an enum for the item kinds in the TraceCursor. I 
think this version introduces more safety and it's easier to handle the low 
level storage, not to mention that we now use less memory!



Comment at: lldb/include/lldb/Target/TraceCursor.h:258
+  virtual llvm::Optional
+  GetCounter(lldb::TraceCounter counter_type) const = 0;
 

jj10306 wrote:
> Now that we are introducing the notion of an `Event`? wdyt about combining 
> Events and Counters since a counter value in a trace really is just a type of 
> event? In this case, `Counter` could just become a variant of `TraceEvent`. 
> This may make more sense because even in the case of TSCs in an IntelPT trace 
> because, strictly speaking, these aren't associated with instructions, 
> correct? Instead the TSC values are emitted with PSBs and then we "attribute" 
> these values to the nearest instruction, right?
I really like your idea, but that's not that trivial, so let's try to come up 
with a nice design. What about the following?

- We create a new event `TraceEventHWClockTick`. This will help use identify 
quickly anytime we see a different TSC. That way we don't need to keep asking 
every instruction for its TSC even when it doesn't change. The new name also 
makes it more generic.
- In order to access the TSC value, the user could invoke `uint64_t 
GetEventValueHWClockTick()`. I'd imagine that each event type would have its 
own data accessor. 
- We create another method `optional GetEventValueLastHWClockTick()`, 
which returns the most recent value for this event type that happened on or 
before the cursor's current point. I wouldn't make this kind of functionality 
generic for all event kinds because for some it might not make sense at all. 
Anyway, the reason is the following code

In the dumper, we would like to support this flow:
   cursor->GoTo(starting_id)
   hw_timestamp = cursor->GetEventValueLastHWClockTick();

   for (; cursor->HasValue(); cursor->Next()) {
 if (cursor->GetItemKind() == eTraceItemKindEvent && cursor->GetEventKind() 
== eTraceEventHWClockTick)
   hw_timestamp = cursor->GetEventValueHWClockTick();

 cout << "current clock (tsc) " << hw_timestamp << endl;
  }

so, basically we need a way to know the starting tsc for a given instruction 
id, and then to identify changes.

Later, we can implement a new method `nanoseconds 

[Lldb-commits] [PATCH] D128776: Handle a stop when another thread has hit a breakpoint with a failed condition

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Threads which hit a breakpoint but fail the condition are considered
not to be hit.  But another thread might be hit at the same time and
actually stop.  So we have to be sure to switch the first thread's
stop info to eStopReasonNone or we'll report a hit when the condition
failed, which is confusing.

I have a test for this, but it only tests the actual fix on systems that report 
multiple threads hitting breakpoints on a given stop.  Darwin 
behaves this way, and w/o this fix the test fails on the first stop.  But I 
give it 
20 tries so this is likely to test the condition on systems that report multiple
hits less frequently.  And for a system that serializes all stops, all this 
code is
moot anyway, and this is just testing breakpoint conditions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128776

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/two_hits_one_actual/Makefile
  
lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
  lldb/test/API/functionalities/breakpoint/two_hits_one_actual/main.cpp

Index: lldb/test/API/functionalities/breakpoint/two_hits_one_actual/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/two_hits_one_actual/main.cpp
@@ -0,0 +1,22 @@
+//#include 
+#include 
+#include 
+
+void usleep_helper(useconds_t usec) {
+  usleep(usec); // Break here in the helper
+}
+
+void *background_thread(void *arg) {
+(void) arg;
+for (;;) {
+usleep_helper(2);
+}
+}
+
+int main(void) {
+  useconds_t main_usec = 1;
+  std::thread main_thread(background_thread, nullptr); // Set bkpt here to get started
+  for (;;) {
+usleep_helper(main_usec);
+  }
+}
Index: lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
@@ -0,0 +1,62 @@
+"""
+Test that if we hit a breakpoint on two threads at the 
+same time, one of which passes the condition, one not,
+we only have a breakpoint stop reason for the one that
+passed the condition.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestTwoHitsOneActual(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_two_hits_one_actual(self):
+"""There can be many tests in a test case - describe this test here."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+self.sample_test()
+
+def sample_test(self):
+"""You might use the test implementation in several ways, say so here."""
+
+(target, process, main_thread, _) = lldbutil.run_to_source_breakpoint(self,
+   "Set bkpt here to get started", self.main_source_file)
+# This is working around a separate bug.  If you hit a breakpoint and
+# run an expression and it is the first expression you've ever run, on
+# Darwin that will involve running the ObjC runtime parsing code, and we'll
+# be in the middle of that when we do PerformAction on the other thread,
+# which will cause the condition expression to fail.  Calling another
+# expression first works around this.
+val_obj = main_thread.frame[0].EvaluateExpression("main_usec==1")
+self.assertSuccess(val_obj.GetError(), "Ran our expression successfully")
+self.assertEqual(val_obj.value, "true", "Value was true.")
+# Set two breakpoints just to test the multiple location logic:
+bkpt1 = target.BreakpointCreateBySourceRegex("Break here in the helper", self.main_source_file);
+bkpt2 = target.BreakpointCreateBySourceRegex("Break here in the helper", self.main_source_file);
+
+# This one will never be hit:
+bkpt1.SetCondition("usec == 100")
+# This one will only be hit on the main thread:
+bkpt2.SetCondition("usec == 1")
+
+# This is hard to test definitively, becuase it requires hitting
+# a breakpoint on multiple threads at the same time.  On Darwin, this
+# will happen pretty much ever time we continue.  What we are really
+# asserting is that we only ever stop on one thread, so we approximate that
+# by continuing 20 times and assert we only ever hit the first thread.  Either
+# this is a platform that only reports one hit at a time, in which case all
+# this code is unused, or we actually didn't hit the other thread.
+
+for idx in range(0, 20):
+process.Continue()
+for thread in 

[Lldb-commits] [PATCH] D128775: [trace] Fix errors when handling command arguments

[Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6564ca188a12: [trace] Fix errors when handling command 
arguments (authored by Walter Erquinigo wall...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128775

Files:
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThreadUtil.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h


Index: lldb/source/Commands/CommandObjectThreadUtil.h
===
--- lldb/source/Commands/CommandObjectThreadUtil.h
+++ lldb/source/Commands/CommandObjectThreadUtil.h
@@ -80,7 +80,9 @@
 /// an action on multiple threads at once instead of iterating over each 
thread.
 class CommandObjectMultipleThreads : public CommandObjectParsed {
 public:
-  using CommandObjectParsed::CommandObjectParsed;
+  CommandObjectMultipleThreads(CommandInterpreter ,
+   const char *name, const char *help,
+   const char *syntax, uint32_t flags);
 
   bool DoExecute(Args , CommandReturnObject ) override;
 
Index: lldb/source/Commands/CommandObjectThreadUtil.cpp
===
--- lldb/source/Commands/CommandObjectThreadUtil.cpp
+++ lldb/source/Commands/CommandObjectThreadUtil.cpp
@@ -25,6 +25,15 @@
   m_arguments.push_back({thread_arg});
 }
 
+CommandObjectMultipleThreads::CommandObjectMultipleThreads(
+CommandInterpreter , const char *name, const char *help,
+const char *syntax, uint32_t flags)
+: CommandObjectParsed(interpreter, name, help, syntax, flags) {
+  // These commands all take thread ID's as arguments.
+  CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatStar};
+  m_arguments.push_back({thread_arg});
+}
+
 bool CommandObjectIterateOverThreads::DoExecute(Args ,
 CommandReturnObject ) {
   result.SetStatus(m_success_return);
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -2219,7 +2219,10 @@
 nullptr,
 eCommandRequiresProcess | eCommandRequiresThread |
 eCommandTryTargetAPILock | eCommandProcessMustBeLaunched |
-eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {}
+eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {
+CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatOptional};
+m_arguments.push_back({thread_arg});
+  }
 
   ~CommandObjectTraceDumpInstructions() override = default;
 


Index: lldb/source/Commands/CommandObjectThreadUtil.h
===
--- lldb/source/Commands/CommandObjectThreadUtil.h
+++ lldb/source/Commands/CommandObjectThreadUtil.h
@@ -80,7 +80,9 @@
 /// an action on multiple threads at once instead of iterating over each thread.
 class CommandObjectMultipleThreads : public CommandObjectParsed {
 public:
-  using CommandObjectParsed::CommandObjectParsed;
+  CommandObjectMultipleThreads(CommandInterpreter ,
+   const char *name, const char *help,
+   const char *syntax, uint32_t flags);
 
   bool DoExecute(Args , CommandReturnObject ) override;
 
Index: lldb/source/Commands/CommandObjectThreadUtil.cpp
===
--- lldb/source/Commands/CommandObjectThreadUtil.cpp
+++ lldb/source/Commands/CommandObjectThreadUtil.cpp
@@ -25,6 +25,15 @@
   m_arguments.push_back({thread_arg});
 }
 
+CommandObjectMultipleThreads::CommandObjectMultipleThreads(
+CommandInterpreter , const char *name, const char *help,
+const char *syntax, uint32_t flags)
+: CommandObjectParsed(interpreter, name, help, syntax, flags) {
+  // These commands all take thread ID's as arguments.
+  CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatStar};
+  m_arguments.push_back({thread_arg});
+}
+
 bool CommandObjectIterateOverThreads::DoExecute(Args ,
 CommandReturnObject ) {
   result.SetStatus(m_success_return);
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -2219,7 +2219,10 @@
 nullptr,
 eCommandRequiresProcess | eCommandRequiresThread |
 eCommandTryTargetAPILock | eCommandProcessMustBeLaunched |
-eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {}
+

[Lldb-commits] [lldb] 6564ca1 - [trace] Fix errors when handling command arguments

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-28T17:54:30-07:00
New Revision: 6564ca188a122c79c36ad7e4b3d362fb22fbd0b5

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

LOG: [trace] Fix errors when handling command arguments

https://reviews.llvm.org/D128453 recently added some safety checks for
command arguments. Unfortunately, some few commands started failing due
to that, and this diff fixes it. But fortunately, the fix is trivial, which is
simply declaring the argument that these commands will receive.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/CommandObjectThreadUtil.cpp
lldb/source/Commands/CommandObjectThreadUtil.h

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 65512a051ab4..7dd89e601af8 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -2219,7 +2219,10 @@ class CommandObjectTraceDumpInstructions : public 
CommandObjectParsed {
 nullptr,
 eCommandRequiresProcess | eCommandRequiresThread |
 eCommandTryTargetAPILock | eCommandProcessMustBeLaunched |
-eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {}
+eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {
+CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatOptional};
+m_arguments.push_back({thread_arg});
+  }
 
   ~CommandObjectTraceDumpInstructions() override = default;
 

diff  --git a/lldb/source/Commands/CommandObjectThreadUtil.cpp 
b/lldb/source/Commands/CommandObjectThreadUtil.cpp
index cac1bef3e9df..504d5fa0118d 100644
--- a/lldb/source/Commands/CommandObjectThreadUtil.cpp
+++ b/lldb/source/Commands/CommandObjectThreadUtil.cpp
@@ -25,6 +25,15 @@ 
CommandObjectIterateOverThreads::CommandObjectIterateOverThreads(
   m_arguments.push_back({thread_arg});
 }
 
+CommandObjectMultipleThreads::CommandObjectMultipleThreads(
+CommandInterpreter , const char *name, const char *help,
+const char *syntax, uint32_t flags)
+: CommandObjectParsed(interpreter, name, help, syntax, flags) {
+  // These commands all take thread ID's as arguments.
+  CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatStar};
+  m_arguments.push_back({thread_arg});
+}
+
 bool CommandObjectIterateOverThreads::DoExecute(Args ,
 CommandReturnObject ) {
   result.SetStatus(m_success_return);

diff  --git a/lldb/source/Commands/CommandObjectThreadUtil.h 
b/lldb/source/Commands/CommandObjectThreadUtil.h
index 289ffdfc1894..c8f51eabc043 100644
--- a/lldb/source/Commands/CommandObjectThreadUtil.h
+++ b/lldb/source/Commands/CommandObjectThreadUtil.h
@@ -80,7 +80,9 @@ class CommandObjectIterateOverThreads : public 
CommandObjectParsed {
 /// an action on multiple threads at once instead of iterating over each 
thread.
 class CommandObjectMultipleThreads : public CommandObjectParsed {
 public:
-  using CommandObjectParsed::CommandObjectParsed;
+  CommandObjectMultipleThreads(CommandInterpreter ,
+   const char *name, const char *help,
+   const char *syntax, uint32_t flags);
 
   bool DoExecute(Args , CommandReturnObject ) override;
 



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

[Lldb-commits] [PATCH] D128775: [trace] Fix errors when handling command arguments

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: jj10306, persona0220.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

https://reviews.llvm.org/D128453 recently added some safety checks for
command arguments. Unfortunately, some few commands started failing due
to that, and this diff fixes it. But fortunately, the fix is trivial, which is
simply declaring the argument that these commands will receive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128775

Files:
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThreadUtil.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h


Index: lldb/source/Commands/CommandObjectThreadUtil.h
===
--- lldb/source/Commands/CommandObjectThreadUtil.h
+++ lldb/source/Commands/CommandObjectThreadUtil.h
@@ -80,7 +80,9 @@
 /// an action on multiple threads at once instead of iterating over each 
thread.
 class CommandObjectMultipleThreads : public CommandObjectParsed {
 public:
-  using CommandObjectParsed::CommandObjectParsed;
+  CommandObjectMultipleThreads(CommandInterpreter ,
+   const char *name, const char *help,
+   const char *syntax, uint32_t flags);
 
   bool DoExecute(Args , CommandReturnObject ) override;
 
Index: lldb/source/Commands/CommandObjectThreadUtil.cpp
===
--- lldb/source/Commands/CommandObjectThreadUtil.cpp
+++ lldb/source/Commands/CommandObjectThreadUtil.cpp
@@ -25,6 +25,15 @@
   m_arguments.push_back({thread_arg});
 }
 
+CommandObjectMultipleThreads::CommandObjectMultipleThreads(
+CommandInterpreter , const char *name, const char *help,
+const char *syntax, uint32_t flags)
+: CommandObjectParsed(interpreter, name, help, syntax, flags) {
+  // These commands all take thread ID's as arguments.
+  CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatStar};
+  m_arguments.push_back({thread_arg});
+}
+
 bool CommandObjectIterateOverThreads::DoExecute(Args ,
 CommandReturnObject ) {
   result.SetStatus(m_success_return);
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -2219,7 +2219,10 @@
 nullptr,
 eCommandRequiresProcess | eCommandRequiresThread |
 eCommandTryTargetAPILock | eCommandProcessMustBeLaunched |
-eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {}
+eCommandProcessMustBePaused | eCommandProcessMustBeTraced) {
+CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatOptional};
+m_arguments.push_back({thread_arg});
+  }
 
   ~CommandObjectTraceDumpInstructions() override = default;
 


Index: lldb/source/Commands/CommandObjectThreadUtil.h
===
--- lldb/source/Commands/CommandObjectThreadUtil.h
+++ lldb/source/Commands/CommandObjectThreadUtil.h
@@ -80,7 +80,9 @@
 /// an action on multiple threads at once instead of iterating over each thread.
 class CommandObjectMultipleThreads : public CommandObjectParsed {
 public:
-  using CommandObjectParsed::CommandObjectParsed;
+  CommandObjectMultipleThreads(CommandInterpreter ,
+   const char *name, const char *help,
+   const char *syntax, uint32_t flags);
 
   bool DoExecute(Args , CommandReturnObject ) override;
 
Index: lldb/source/Commands/CommandObjectThreadUtil.cpp
===
--- lldb/source/Commands/CommandObjectThreadUtil.cpp
+++ lldb/source/Commands/CommandObjectThreadUtil.cpp
@@ -25,6 +25,15 @@
   m_arguments.push_back({thread_arg});
 }
 
+CommandObjectMultipleThreads::CommandObjectMultipleThreads(
+CommandInterpreter , const char *name, const char *help,
+const char *syntax, uint32_t flags)
+: CommandObjectParsed(interpreter, name, help, syntax, flags) {
+  // These commands all take thread ID's as arguments.
+  CommandArgumentData thread_arg{eArgTypeThreadIndex, eArgRepeatStar};
+  m_arguments.push_back({thread_arg});
+}
+
 bool CommandObjectIterateOverThreads::DoExecute(Args ,
 CommandReturnObject ) {
   result.SetStatus(m_success_return);
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -2219,7 +2219,10 @@
 nullptr,
 eCommandRequiresProcess | eCommandRequiresThread |
 

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

nice job!!




Comment at: lldb/source/Commands/Options.td:304
+  def disassemble_options_kind : Option<"kind", "k">,
+Desc<"Show instruction control flow type.">;
   def disassemble_options_context : Option<"context", "C">, Arg<"NumLines">,

mention here the enum to inspect for documentation. Besides that, you can 
quickly mention here that far jumps and far returns often indicate a kernel 
calls and returns. That might be enough because the other categories are pretty 
intuitive



Comment at: lldb/source/Commands/Options.td:1156
+  def thread_trace_dump_instructions_show_kind : Option<"kind", "k">, Group<1>,
+Desc<"For each instruction, print the instruction type">;
   def thread_trace_dump_instructions_show_tsc : Option<"tsc", "t">, Group<1>,

same here



Comment at: lldb/source/Core/Disassembler.cpp:680
+Instruction::GetControlFlowInstructionKind(const ExecutionContext *exe_ctx) {
+  uint8_t *opcode_bytes = (uint8_t *)m_opcode.GetOpcodeBytes();
+  uint8_t opcode, modrm, map;

you have to check here that the triple of the current architecture of the 
current target (you can get that from exe_ctx) is x86, otherwise directly 
return eInstructionControlFlowTypeUnknown. You can just rely on the check you 
are doing in line 685 because that check might fail in the future



Comment at: lldb/source/Core/Disassembler.cpp:681
+  uint8_t *opcode_bytes = (uint8_t *)m_opcode.GetOpcodeBytes();
+  uint8_t opcode, modrm, map;
+  int op_idx = 0;

from this point on, the code is specifically meant for x86, so I recommend 
creating a x86 namespace in this file and include a 
GetControlFlowInstructionKind method along with the instruction_decode method 
there. That way we organize the code in a more cleaner manner



Comment at: lldb/source/Core/Disassembler.cpp:681
+  uint8_t *opcode_bytes = (uint8_t *)m_opcode.GetOpcodeBytes();
+  uint8_t opcode, modrm, map;
+  int op_idx = 0;

wallace wrote:
> from this point on, the code is specifically meant for x86, so I recommend 
> creating a x86 namespace in this file and include a 
> GetControlFlowInstructionKind method along with the instruction_decode method 
> there. That way we organize the code in a more cleaner manner
add a comment explaining what these variables are



Comment at: lldb/source/Core/Disassembler.cpp:692
+  // Get opcode and modrm
+  map = PTI_MAP_INVALID;
+  while (!prefix_done) {

add a comment explaining what this prefix handling is actually doing



Comment at: lldb/source/Core/Disassembler.cpp:715
+case 0x40 ... 0x4f:
+  if (exe_ctx->GetTargetRef().GetArchitecture().GetMachine() ==
+  llvm::Triple::ArchType::x86_64)

you can move this to the beginning of the function and assume that this mapping 
is x86 only, which is how it should be



Comment at: lldb/source/Core/Disassembler.cpp:773
+
+  if (opcode == 0x0F) {
+if (opcode_bytes[op_idx + 1] == 0x38) {

you can also mention here how this mapping is actually generated and were you 
got inspiration from, like libipt



Comment at: lldb/source/Core/Disassembler.cpp:846
+  if (show_kind) {
+// TODO-SUJIN
+switch (GetControlFlowInstructionKind(exe_ctx)) {

should you remove this?



Comment at: lldb/source/Core/Disassembler.cpp:849
+case eInstructionControlFlowTypeUnknown:
+  ss.Printf("%-12s", "unknwon");
+  break;

unknown



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:63
+InstructionControlFlowType
 DecodedThread::GetInstructionControlFlowType(size_t insn_index) const {
   if (IsInstructionAnError(insn_index))

remove this, because we won't use it anymore


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

https://reviews.llvm.org/D128477

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

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/include/lldb/Core/Disassembler.h:82-86
+  lldb::InstructionControlFlowType
+  instruction_decode(uint8_t opcode, uint8_t modrm, uint8_t map);
+
+  lldb::InstructionControlFlowType
+  GetControlFlowInstructionKind(const ExecutionContext *exe_ctx);

as this code is new, please write documentation and function names should be in 
PascalCase. Try to have a more descriptive name as well, because 
`instruction_decode` is very vague. 



Comment at: lldb/include/lldb/Core/Disassembler.h:152
   virtual void Dump(Stream *s, uint32_t max_opcode_byte_size, bool 
show_address,
-bool show_bytes, const ExecutionContext *exe_ctx,
+bool show_bytes, bool show_kind,
+const ExecutionContext *exe_ctx,

let's try to be more explicit with the naming



Comment at: lldb/include/lldb/Core/Disassembler.h:226-233
+  enum {
+PTI_MAP_0,
+PTI_MAP_1,
+PTI_MAP_2,
+PTI_MAP_3,
+PTI_MAP_AMD3DNOW,
+PTI_MAP_INVALID

add documentation. Also, if this mapping is intel-specific, I recommend moving 
this to the cpp file so that it's not exposed publicly. The Disassembler is 
architecture agnostic, so its API should try to remain that way



Comment at: lldb/include/lldb/Core/Disassembler.h:341
 
-  void Dump(Stream *s, bool show_address, bool show_bytes,
+  void Dump(Stream *s, bool show_address, bool show_bytes, bool show_kind,
 const ExecutionContext *exe_ctx);

name



Comment at: lldb/include/lldb/Core/Disassembler.h:398
+(1u << 3), // Mark the disassembly line the contains the PC
+eOptionShowKind = (1u << 4),
   };





Comment at: lldb/include/lldb/Target/TraceCursor.h:35
 ///  point at them. The consumer should invoke \a TraceCursor::GetError() to
 ///  check if the cursor is pointing to either a valid instruction or an error.
 ///

this file has changed, could you rebase from upstream/main. 



Comment at: lldb/include/lldb/Target/TraceCursor.h:230-234
   /// \return
-  /// The \a lldb::TraceInstructionControlFlowType categories the
+  /// The \a lldb::InstructionControlFlowType categories the
   /// instruction the cursor is pointing at falls into. If the cursor 
points
   /// to an error in the trace, return \b 0.
+  virtual lldb::InstructionControlFlowType GetInstructionControlFlowType() = 0;

when you rebase, could you delete any references to this method and its 
implementation as well? We won't use it anymore



Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:38
+  /// For each instruction, print the instruction type.
+  bool show_kind = false;
   /// Optional custom id to start traversing from.

name



Comment at: lldb/include/lldb/lldb-enumerations.h:973
 /// A single instruction can match one or more of these categories.
-FLAGS_ENUM(TraceInstructionControlFlowType){
-/// Any instruction.
-eTraceInstructionControlFlowTypeInstruction = (1u << 1),
-/// A conditional or unconditional branch/jump.
-eTraceInstructionControlFlowTypeBranch = (1u << 2),
-/// A conditional or unconditional branch/jump that changed
-/// the control flow of the program.
-eTraceInstructionControlFlowTypeTakenBranch = (1u << 3),
-/// A call to a function.
-eTraceInstructionControlFlowTypeCall = (1u << 4),
-/// A return from a function.
-eTraceInstructionControlFlowTypeReturn = (1u << 5)};
-
-LLDB_MARK_AS_BITMASK_ENUM(TraceInstructionControlFlowType)
+FLAGS_ENUM(InstructionControlFlowType){
+/// The instruction could not be classified.

this doesn't need to be a FLAGS_ENUM anymore (i.e. bitmask), you can use a 
simple enum instead, like the `ExpressionEvaluationPhase` above



Comment at: lldb/source/API/SBInstruction.cpp:244
 FormatEntity::Parse("${addr}: ", format);
-inst_sp->Dump((), 0, true, false, nullptr, , nullptr, , 0);
+inst_sp->Dump((), 0, true, false, false, nullptr, , nullptr,
+  , 0);

include this kind of comment wherever you have added this parameter



Comment at: lldb/source/Commands/CommandObjectDisassemble.h:49
 bool show_bytes;
+bool show_kind;
 uint32_t num_lines_context = 0;

ame


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

https://reviews.llvm.org/D128477

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

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf91d82816ff5: [trace] Improve the TraceCursor iteration API 
(authored by Walter Erquinigo wall...@fb.com).

Changed prior to commit:
  https://reviews.llvm.org/D128543?vs=439827=440810#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128543

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -261,33 +261,20 @@
 : m_cursor_up(std::move(cursor_up)), m_options(options),
   m_writer_up(CreateWriter(s, m_options)) {
 
-  if (m_options.id) {
-if (!m_cursor_up->GoToId(*m_options.id)) {
-  m_writer_up->InfoMessage("invalid instruction id");
-  SetNoMoreData();
-}
-  } else if (m_options.forwards) {
+  if (m_options.id)
+m_cursor_up->GoToId(*m_options.id);
+  else if (m_options.forwards)
 m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
-  } else {
+  else
 m_cursor_up->Seek(0, TraceCursor::SeekType::End);
-  }
 
   m_cursor_up->SetForwards(m_options.forwards);
   if (m_options.skip) {
-uint64_t to_skip = *m_options.skip;
-if (m_cursor_up->Seek((m_options.forwards ? 1 : -1) * to_skip,
-  TraceCursor::SeekType::Current) < to_skip) {
-  // This happens when the skip value was more than the number of
-  // available instructions.
-  SetNoMoreData();
-}
+m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
+  TraceCursor::SeekType::Current);
   }
 }
 
-void TraceInstructionDumper::SetNoMoreData() { m_no_more_data = true; }
-
-bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
-
 TraceInstructionDumper::InstructionEntry
 TraceInstructionDumper::CreatRawInstructionEntry() {
   InstructionEntry insn;
@@ -375,11 +362,8 @@
   ExecutionContext exe_ctx;
   thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx);
 
-  for (size_t i = 0; i < count; i++) {
-if (!HasMoreData()) {
-  m_writer_up->InfoMessage("no more data");
-  break;
-}
+  for (size_t i = 0; i < count && m_cursor_up->HasValue();
+   m_cursor_up->Next(), i++) {
 last_id = m_cursor_up->GetId();
 
 if (m_options.forwards) {
@@ -418,9 +402,8 @@
   // makes sense.
   PrintEvents();
 }
-
-if (!m_cursor_up->Next())
-  SetNoMoreData();
   }
+  if (!m_cursor_up->HasValue())
+m_writer_up->InfoMessage("no more data");
   return last_id;
 }
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -21,15 +21,6 @@
   return m_exe_ctx_ref;
 }
 
-void TraceCursor::SetGranularity(
-lldb::TraceInstructionControlFlowType granularity) {
-  m_granularity = granularity;
-}
-
-void TraceCursor::SetIgnoreErrors(bool ignore_errors) {
-  m_ignore_errors = ignore_errors;
-}
-
 void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; }
 
 bool TraceCursor::IsForwards() const { return m_forwards; }
Index: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
===
--- lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
+++ lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
@@ -154,7 +154,8 @@
   // TODO: Make distinction between errors by storing the error messages.
   // Currently, all errors are treated the same.
   m_instruction_layer_up->AppendInstruction(0);
-  more_data_in_trace = cursor.Next();
+  cursor.Next();
+  more_data_in_trace = cursor.HasValue();
 } else {
   lldb::addr_t current_instruction_load_address = cursor.GetLoadAddress();
   lldb::TraceInstructionControlFlowType current_instruction_type =
@@ -162,7 +163,8 @@
 
   m_instruction_layer_up->AppendInstruction(
   current_instruction_load_address);
-  more_data_in_trace = cursor.Next();
+  cursor.Next();
+  more_data_in_trace = cursor.HasValue();
   if (current_instruction_type &
   lldb::eTraceInstructionControlFlowTypeCall) {
 if (more_data_in_trace && !cursor.IsError()) {
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- 

[Lldb-commits] [lldb] f91d828 - [trace] Improve the TraceCursor iteration API

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-28T16:50:12-07:00
New Revision: f91d82816ff5a88a59e86b54a4d63547776d4854

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

LOG: [trace] Improve the TraceCursor iteration API

The current way ot traversing the cursor is a bit uncommon and it can't handle 
empty traces, in fact, its invariant is that it shold always point to a valid 
item. This diff simplifies the cursor API and allows it to point to invalid 
items, thus being able to handle empty traces or to know it ran out of data.

- Removed all the granularity functionalities, because we are not actually 
making use of that. We can bring them back when they are actually needed.
- change the looping logic to the following:

```
  for (; cursor->HasValue(); cursor->Next()) {
 if (cursor->IsError()) {
   .. do something for error
   continue;
 }
 .. do something for instruction
  }

```

- added a HasValue method that can be used to identify if the cursor ran out of 
data, the trace is empty, or the user tried to move to an invalid position via 
SetId() or Seek()
- made several simplifications to severals parts of the code.

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

Added: 


Modified: 
lldb/include/lldb/Target/TraceCursor.h
lldb/include/lldb/Target/TraceInstructionDumper.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Target/TraceCursor.cpp
lldb/source/Target/TraceInstructionDumper.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/TraceCursor.h 
b/lldb/include/lldb/Target/TraceCursor.h
index 79c4cd371e4f4..7640fed93b99f 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -25,51 +25,66 @@ namespace lldb_private {
 /// Live processes:
 ///  In the case of a live process trace, an instance of a \a TraceCursor 
should
 ///  point to the trace at the moment it was collected. If the process is later
-///  resumed and new trace data is collected, that should leave that old cursor
-///  unaffected.
+///  resumed and new trace data is collected, then it's up to each trace 
plug-in
+///  to decide whether to leave the old cursor unaffected or not.
 ///
 /// Errors in the trace:
 ///  As there could be errors when reconstructing the instructions of a trace,
 ///  these errors are represented as failed instructions, and the cursor can
-///  point at them. The consumer should invoke \a TraceCursor::GetError() to
-///  check if the cursor is pointing to either a valid instruction or an error.
+///  point at them. The consumer should invoke \a TraceCursor::IsError() to
+///  check if the cursor is pointing to either a valid instruction or an error,
+///  and then \a TraceCursor::GetError() can return the actual error message.
 ///
 /// Instructions:
 ///  A \a TraceCursor always points to a specific instruction or error in the
 ///  trace.
 ///
 /// Defaults:
-///   By default, the cursor points at the end item of the trace, moves
-///   backwards, has a move granularity of \a
-///   eTraceInstructionControlFlowTypeInstruction (i.e. visit every 
instruction)
-///   and stops at every error (the "ignore errors" flag is \b false). See the
-///   \a TraceCursor::Next() method for more documentation.
+///   By default, the cursor points at the most recent item in the trace and is
+///   set up to iterate backwards. See the \a TraceCursor::Next() method for
+///   more documentation.
 ///
 /// Sample usage:
 ///
 ///  TraceCursorUP cursor = trace.GetTrace(thread);
 ///
-///  cursor->SetGranularity(eTraceInstructionControlFlowTypeCall |
-///eTraceInstructionControlFlowTypeReturn);
+///  for (; cursor->HasValue(); cursor->Next()) {
+/// if (cursor->IsError()) {
+///   cout << "error found at: " << cursor->GetError() << endl;
+///   continue;
+/// }
 ///
-///  do {
-/// if (llvm::Error error = cursor->GetError())
-///   cout << "error found at: " << llvm::toString(error) << endl;
-/// else if (cursor->GetInstructionControlFlowType() &
-/// eTraceInstructionControlFlowTypeCall)
-///   std::cout << "call found at " << cursor->GetLoadAddress() <<
-///   std::endl;
-/// else if (cursor->GetInstructionControlFlowType() &
-/// eTraceInstructionControlFlowTypeReturn)
-///   std::cout << "return found at " << cursor->GetLoadAddress() <<
-///   std::endl;
-///  } while(cursor->Next());
+/// switch (cursor->GetInstructionControlFlowType()) {
+///   eTraceInstructionControlFlowTypeCall:
+///

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:38
+void TraceCursorIntelPT::Next() {
+  m_pos += IsForwards() ? 1 : -1;
 

jj10306 wrote:
> should only do this increment or decrement if `HasValue()` is true? otherwise 
> (in theory) this value could wrap around if it's incremented/decremented too 
> many times?
i think that's a very extreme case =P



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:40
 
+  {
+// We try to go to a neighbor tsc range that might contain the current pos

jj10306 wrote:
> why is this new scope introduced here?
i'll remove it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128543

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

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[Sujin Park via Phabricator via lldb-commits]

persona0220 updated this revision to Diff 440808.

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

https://reviews.llvm.org/D128477

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectDisassemble.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -151,6 +151,7 @@
 insn.symbol_info->instruction->Dump(_s, /*max_opcode_byte_size=*/0,
 /*show_address=*/false,
 /*show_bytes=*/false,
+/*show_kind=*/m_options.show_kind,
 _info->exe_ctx,
 _info->sc,
 /*prev_sym_ctx=*/nullptr,
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -21,15 +21,6 @@
   return m_exe_ctx_ref;
 }
 
-void TraceCursor::SetGranularity(
-lldb::TraceInstructionControlFlowType granularity) {
-  m_granularity = granularity;
-}
-
-void TraceCursor::SetIgnoreErrors(bool ignore_errors) {
-  m_ignore_errors = ignore_errors;
-}
-
 void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; }
 
 bool TraceCursor::IsForwards() const { return m_forwards; }
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -170,12 +170,13 @@
   if (instruction_list.GetSize()) {
 const bool show_bytes = true;
 const bool show_address = true;
+const bool show_kind = true;
 Instruction *instruction =
 instruction_list.GetInstructionAtIndex(0).get();
 const FormatEntity::Entry *disassemble_format =
 m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
 instruction->Dump(stream, max_opcode_byte_size, show_address,
-  show_bytes, nullptr, nullptr, nullptr,
+  show_bytes, show_kind, nullptr, nullptr, nullptr,
   disassemble_format, 0);
   }
 }
Index: lldb/source/Symbol/Symbol.cpp
===
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -558,8 +558,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
+const bool show_kind = false;
 disassembler_sp->GetInstructionList().Dump(, show_address, show_bytes,
-   _ctx);
+   show_kind, _ctx);
 return true;
   }
   return false;
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -440,8 +440,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
+const bool show_kind = false;
 disassembler_sp->GetInstructionList().Dump(, show_address, show_bytes,
-   _ctx);
+   show_kind, _ctx);
 return true;
   }
   return false;
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -83,6 +83,7 @@
   const uint32_t 

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[walter erquinigo via Phabricator via lldb-commits]

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

remove unwanted files


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

https://reviews.llvm.org/D128477

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

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[Sujin Park via Phabricator via lldb-commits]

persona0220 updated this revision to Diff 440806.
persona0220 added a comment.
Herald added subscribers: Michael137, MaskRay.

This new diff attach additional metadata 'control flow kind' for instruction.
Add '-k / --kind' flag for both `disassemble` and `thread trace dump` command.

Thanks to @jingham for the suggestion!

Sample output:

  (lldb) disas -k
  a.out`main:
  0x4005b6 <+0>:  other pushq  %rbp
  0x4005b7 <+1>:  other movq   %rsp, %rbp
  0x4005ba <+4>:  other movl   $0x400668, %edi   ; imm = 
0x400668
  0x4005bf <+9>:  call  callq  0x4004c0  ; symbol stub 
for: puts
  ->  0x4005c4 <+14>: other movl   $0x0, %eax
  0x4005c9 <+19>: other popq   %rbp
  0x4005ca <+20>: returnretq
  
  (lldb) thread trace dump instruction --kind
  thread #1: tid = 1893259
libc.so.6`write + 30
  2027: 0x771fa86econd jump ja 0x1208c8  ; 
<+120>
  2026: 0x771fa868other cmpq   $-0x1000, %rax; 
imm = 0xF000
  2025: 0x771fa866far call  syscall
  2024: 0x771fa861other movl   $0x1, %eax
  2023: 0x771fa85fcond jump jne0x120878  ; 
<+40>
  2022: 0x771fa85dother testl  %eax, %eax
  2021: 0x771fa85bother movl   (%rax), %eax
  2020: 0x771fa854other leaq   0x2a4d35(%rip), %rax  ; 
__libc_multiple_threads
  2019: 0x771fa850other endbr64


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

https://reviews.llvm.org/D128477

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/aarch64-target-as-march.s
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-ias-Wa.s
  clang/test/Preprocessor/aarch64-target-features.c
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectDisassemble.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -151,6 +151,7 @@
 insn.symbol_info->instruction->Dump(_s, /*max_opcode_byte_size=*/0,
 /*show_address=*/false,
 /*show_bytes=*/false,
+/*show_kind=*/m_options.show_kind,
 _info->exe_ctx,
 _info->sc,
 /*prev_sym_ctx=*/nullptr,
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -21,15 +21,6 @@
   return m_exe_ctx_ref;
 }
 
-void TraceCursor::SetGranularity(
-lldb::TraceInstructionControlFlowType granularity) {
-  m_granularity = granularity;
-}
-
-void TraceCursor::SetIgnoreErrors(bool ignore_errors) {
-  m_ignore_errors = ignore_errors;
-}
-
 void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; }
 
 bool TraceCursor::IsForwards() const { return m_forwards; }
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -170,12 +170,13 @@
   if (instruction_list.GetSize()) {
 const bool show_bytes = true;
 const bool show_address = true;
+const bool show_kind = true;
 Instruction *instruction =
 instruction_list.GetInstructionAtIndex(0).get();
 const FormatEntity::Entry *disassemble_format =
 m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
 instruction->Dump(stream, 

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.
Herald added a subscriber: JDevlieghere.

Hi @mgorny,

There are some random test failures following this patch on 
`lldb-debian-x86_64`: https://lab.llvm.org/buildbot/#/builders/68/builds/34883

  
  Unresolved Tests (1):
lldb-api :: tools/lldb-server/TestGdbRemoteFork.py

Would you mind taking a look at this ? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128638

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

[Lldb-commits] [lldb] 44a114f - [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2022-06-28T15:34:17-07:00
New Revision: 44a114fec715bd9c9b725c939a09de4b746108e3

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

LOG: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

This patch changes the C++ `std::string` dataformatter to reflect
internal layout changes following D128285.

Now, in short-mode strings, in order to access the `__size_` and
`__is_long_` attributes, we need to access a packed anonymous struct,
which introduces another indirection.

We need to do the same in order to access the `__cap_` field for
long-mode strings.

This should fix the various test failures that are happening on
GreenDragon:

https://green.lab.llvm.org/green/job/lldb-cmake/44918/

rdar://96010248

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index cd09c3e7d1e9b..18bca3b43406a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -583,8 +583,18 @@ ExtractLibcxxStringInfo(ValueObject ) {
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (ValueObjectSP is_long = dataval_sp->GetChildAtNamePath(
-  {ConstString("__s"), ConstString("__is_long_")})) {
+  ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
+  if (!short_sp)
+return {};
+
+  // After D128285, we need to access the `__is_long_` and `__size_` fields 
from
+  // a packed anonymous struct
+  ValueObjectSP packed_fields_sp = short_sp->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
+
+  if (ValueObjectSP is_long = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__is_long_"), true)) {
 using_bitmasks = false;
 short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
 if (ValueObjectSP size_member =
@@ -621,14 +631,11 @@ ExtractLibcxxStringInfo(ValueObject ) {
   }
 
   if (short_mode) {
-ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
-if (!short_sp)
-  return {};
-ValueObjectSP location_sp = short_sp->GetChildAtIndex(
+ValueObjectSP location_sp = packed_fields_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
 // After D125496, there is a flat layout.
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildMemberWithName(g_data_name, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
@@ -650,13 +657,19 @@ ExtractLibcxxStringInfo(ValueObject ) {
   ValueObjectSP l(dataval_sp->GetChildAtIndex(0, true));
   if (!l)
 return {};
+
+  // After D128285, we need to access the `__cap_` field from a packed 
anonymous
+  // struct
+  packed_fields_sp = l->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
   // we can use the layout_decider object as the data pointer
   ValueObjectSP location_sp =
   l->GetChildMemberWithName(ConstString("__data_"), /*can_create=*/true);
   ValueObjectSP size_vo =
   l->GetChildMemberWithName(ConstString("__size_"), /*can_create=*/true);
-  ValueObjectSP capacity_vo =
-  l->GetChildMemberWithName(ConstString("__cap_"), /*can_create=*/true);
+  ValueObjectSP capacity_vo = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__cap_"), /*can_create=*/true);
   if (!size_vo || !location_sp || !capacity_vo)
 return {};
   size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);



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

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG44a114fec715: [lldb/Dataformatters] Adapt C++ std::string 
dataformatter for D128285 (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128694

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -583,8 +583,18 @@
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (ValueObjectSP is_long = dataval_sp->GetChildAtNamePath(
-  {ConstString("__s"), ConstString("__is_long_")})) {
+  ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
+  if (!short_sp)
+return {};
+
+  // After D128285, we need to access the `__is_long_` and `__size_` fields 
from
+  // a packed anonymous struct
+  ValueObjectSP packed_fields_sp = short_sp->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
+
+  if (ValueObjectSP is_long = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__is_long_"), true)) {
 using_bitmasks = false;
 short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
 if (ValueObjectSP size_member =
@@ -621,14 +631,11 @@
   }
 
   if (short_mode) {
-ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
-if (!short_sp)
-  return {};
-ValueObjectSP location_sp = short_sp->GetChildAtIndex(
+ValueObjectSP location_sp = packed_fields_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
 // After D125496, there is a flat layout.
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildMemberWithName(g_data_name, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
@@ -650,13 +657,19 @@
   ValueObjectSP l(dataval_sp->GetChildAtIndex(0, true));
   if (!l)
 return {};
+
+  // After D128285, we need to access the `__cap_` field from a packed 
anonymous
+  // struct
+  packed_fields_sp = l->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
   // we can use the layout_decider object as the data pointer
   ValueObjectSP location_sp =
   l->GetChildMemberWithName(ConstString("__data_"), /*can_create=*/true);
   ValueObjectSP size_vo =
   l->GetChildMemberWithName(ConstString("__size_"), /*can_create=*/true);
-  ValueObjectSP capacity_vo =
-  l->GetChildMemberWithName(ConstString("__cap_"), /*can_create=*/true);
+  ValueObjectSP capacity_vo = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__cap_"), /*can_create=*/true);
   if (!size_vo || !location_sp || !capacity_vo)
 return {};
   size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -583,8 +583,18 @@
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (ValueObjectSP is_long = dataval_sp->GetChildAtNamePath(
-  {ConstString("__s"), ConstString("__is_long_")})) {
+  ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
+  if (!short_sp)
+return {};
+
+  // After D128285, we need to access the `__is_long_` and `__size_` fields from
+  // a packed anonymous struct
+  ValueObjectSP packed_fields_sp = short_sp->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
+
+  if (ValueObjectSP is_long = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__is_long_"), true)) {
 using_bitmasks = false;
 short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
 if (ValueObjectSP size_member =
@@ -621,14 +631,11 @@
   }
 
   if (short_mode) {
-ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
-if (!short_sp)
-  return {};
-ValueObjectSP location_sp = short_sp->GetChildAtIndex(
+ValueObjectSP location_sp = packed_fields_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
 // After D125496, there is a flat layout.
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildMemberWithName(g_data_name, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
@@ -650,13 +657,19 @@
   ValueObjectSP l(dataval_sp->GetChildAtIndex(0, true));
   if (!l)
 return {};
+
+  // After D128285, we need to access the 

[Lldb-commits] [PATCH] D128768: [lldb/Core] Fix finite progress event reporting

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should fix event handling for finite progress reports.

Previously, the event handler would get stuck when receiving a finite
progress report, and stop displaying upcoming reports.
This was due to the fact that we were checking if the progress event was
completed by calling `GetCompleted` but returns the completion amount
instead of saying whether it's completed.

That caused the current event id to remain the same, preventing all the
following progress reports to be shown to the user.

This patch also adds some logging to facilitate debugging progress events.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128768

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1827,6 +1827,12 @@
 }
 
 void Debugger::HandleProgressEvent(const lldb::EventSP _sp) {
+  Log *log = GetLog(LLDBLog::Events);
+
+  LLDB_LOGF(log, "%p Debugger('%llu')::HandleProgressEvent (event_sp = {%p})",
+static_cast(this), m_uid,
+static_cast(event_sp.get()));
+
   auto *data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
   if (!data)
 return;
@@ -1835,10 +1841,22 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+if (data->IsFinite())
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, completed = %llu, "
+"total = %llu})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data(), data->GetCompleted(),
+data->GetTotal());
+else
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, infinite = true})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data());
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
-  m_current_event_id.reset();
   } else {
 m_current_event_id = id;
   }
@@ -1860,8 +1878,9 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
+m_current_event_id.reset();
 output->Printf("\x1B[2K");
 output->Flush();
 return;


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1827,6 +1827,12 @@
 }
 
 void Debugger::HandleProgressEvent(const lldb::EventSP _sp) {
+  Log *log = GetLog(LLDBLog::Events);
+
+  LLDB_LOGF(log, "%p Debugger('%llu')::HandleProgressEvent (event_sp = {%p})",
+static_cast(this), m_uid,
+static_cast(event_sp.get()));
+
   auto *data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
   if (!data)
 return;
@@ -1835,10 +1841,22 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+if (data->IsFinite())
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, completed = %llu, "
+"total = %llu})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data(), data->GetCompleted(),
+data->GetTotal());
+else
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, infinite = true})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data());
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
-  m_current_event_id.reset();
   } else {
 m_current_event_id = id;
   }
@@ -1860,8 +1878,9 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
+m_current_event_id.reset();
 output->Printf("\x1B[2K");
 output->Flush();
 return;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Jonas Devlieghere via Phabricator via lldb-commits]

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

I agree with Adrian that we should try to support both the old and new layout. 
Is there a way we can fall back? I think it's fine to land the patch as is to 
unblock the bots and add the fallback later.


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

https://reviews.llvm.org/D128694

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D128678#3616685 , @mstorsjo wrote:

> Thanks, this looks more complete and consistent to me now!
>
> In D128678#3615531 , @labath wrote:
>
>> It seems like this is not actually running the code. Can we make it such 
>> that these tests are conditional on the appropriate llvm targets being 
>> enabled (instead of the host being of a specific type)?
>
> Well it does need being able to link executables for these targets, which you 
> generally can't rely on I think? (I don't think the `%build` macro in general 
> works as a cross compiler?)

We can compile the test on linux as it doesnt require any libraries i-e 
--nodefaultlib. However its MS DIA SDK based PDB test which is only available 
on system windows. NATIVE PDB reader tests are already being run independently.


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Thanks, this looks more complete and consistent to me now!

In D128678#3615531 , @labath wrote:

> It seems like this is not actually running the code. Can we make it such that 
> these tests are conditional on the appropriate llvm targets being enabled 
> (instead of the host being of a specific type)?

Well it does need being able to link executables for these targets, which you 
generally can't rely on I think? (I don't think the `%build` macro in general 
works as a cross compiler?)


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3614661 , @labath wrote:

> In D128268#3611053 , @mstorsjo 
> wrote:
>
>> The odd thing about the second one is the added hardcoded 
>> `AddArch(ArchSpec("i686-pc-windows"));` and 
>> `AddArch(ArchSpec("i386-pc-windows"));` at the top of `PlatformWindows.cpp`.
>>
>> Anyway, it does seem to work fine in my quick test to get rid of this 
>> duality, so I'll post a patch that does that.
>
> I think those hardcoded lines are there precisely because of the duality 
> (i.e. wanting to support both i386 and i686 regardless of what the host layer 
> reports).

Both that, and because the i386+i686 fat binary arches from ObjectFile 
triggered the more elaborate arch validation, these had to be present to make 
it work. Anyway, after getting rid of the duality, those hardcoded ones don't 
seem to be needed either.

> If that is removed, maybe all of that can be changed to  the pattern used in 
> other platform plugins 
> .

That looks reasonable!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128639: [lldb] [llgs] Fix premature server exit if multiprocess+nonstop

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG261d0033500e: [lldb] [llgs] Fix premature server exit if 
multiprocess+nonstop (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D128639?vs=440187=440733#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128639

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -392,17 +392,17 @@
 def test_vkill_both_nonstop(self):
 self.vkill_test(kill_parent=True, kill_child=True, nonstop=True)
 
-def resume_one_test(self, run_order, use_vCont=False):
+def resume_one_test(self, run_order, use_vCont=False, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test(["fork", "trap"]))
+self.start_fork_test(["fork", "trap"], nonstop=nonstop))
 
 parent_expect = [
-"[$]T05thread:p{}.{};.*".format(parent_pid, parent_tid),
-"[$]W00;process:{}#.*".format(parent_pid),
+"T05thread:p{}.{};.*".format(parent_pid, parent_tid),
+"W00;process:{}#.*".format(parent_pid),
 ]
 child_expect = [
-"[$]T05thread:p{}.{};.*".format(child_pid, child_tid),
-"[$]W00;process:{}#.*".format(child_pid),
+"T05thread:p{}.{};.*".format(child_pid, child_tid),
+"W00;process:{}#.*".format(child_pid),
 ]
 
 for x in run_order:
@@ -427,9 +427,17 @@
 "send packet: $OK#00",
 "read packet: $c#00",
 ], True)
-self.test_sequence.add_log_lines([
-{"direction": "send", "regex": expect},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:" + expect},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+{"direction": "send", "regex": "[$]" + expect},
+], True)
 # if at least one process remained, check both PIDs
 if parent_expect or child_expect:
 self.test_sequence.add_log_lines([
@@ -475,6 +483,14 @@
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
+@add_test_categories(["fork"])
+def test_c_interspersed_nonstop(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ nonstop=True)
+
 @expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
@@ -513,6 +529,14 @@
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
+@add_test_categories(["fork"])
+def test_vCont_interspersed_nonstop(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ use_vCont=True, nonstop=True)
+
 @add_test_categories(["fork"])
 def test_vCont_two_processes(self):
 parent_pid, parent_tid, child_pid, child_tid = (
Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3860,9 +3860,9 @@
   m_stop_notification_queue.pop_front();
   if (!m_stop_notification_queue.empty())
 return SendPacketNoLock(m_stop_notification_queue.front());
-  // If this was the last notification and the process exited, terminate
-  // the server.
-  if (m_inferior_prev_state == eStateExited) {
+  // If this was the last notification and all the processes exited,
+  // terminate the server.
+  if (m_debugged_processes.empty()) {
 

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb415f8e3dfb8: [lldb] [llgs] Add base nonstop fork/vfork 
tests (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128638

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -9,10 +9,12 @@
 
 fork_regex = ("[$]T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
   "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_regex_nonstop = ("%Stop:T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
 
-def start_fork_test(self, args, variant="fork"):
+def start_fork_test(self, args, variant="fork", nonstop=False):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=args)
 self.add_qSupported_packets(["multiprocess+",
@@ -22,11 +24,24 @@
 self.reset_test_sequence()
 
 # continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"read packet: $QNonStop:1#00",
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": self.fork_regex_nonstop.format(variant),
+ "capture": self.fork_capture},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format(variant),
+ "capture": self.fork_capture},
+], True)
 ret = self.expect_gdbremote_sequence()
 self.reset_test_sequence()
 
@@ -45,9 +60,9 @@
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_detach_test(self, variant):
+def fork_and_detach_test(self, variant, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test([variant], variant))
+self.start_fork_test([variant], variant, nonstop=nonstop))
 
 # detach the forked child
 self.test_sequence.add_log_lines([
@@ -77,6 +92,20 @@
 ], True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["fork"])
+def test_fork_nonstop(self):
+parent_pid, _ = self.fork_and_detach_test("fork", nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+"send packet: %Stop:W00;process:{}#00".format(parent_pid),
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+self.expect_gdbremote_sequence()
+
 @add_test_categories(["fork"])
 def test_vfork(self):
 parent_pid, parent_tid = self.fork_and_detach_test("vfork")
@@ -93,9 +122,32 @@
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_follow_test(self, variant):
+@add_test_categories(["fork"])
+def test_vfork_nonstop(self):
+parent_pid, parent_tid = self.fork_and_detach_test("vfork",
+   nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": r"%Stop:T05thread:p{}[.]{}.*vforkdone.*".format(
+ parent_pid, parent_tid),
+ },
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+"send packet: %Stop:W00;process:{}#00".format(parent_pid),
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+self.expect_gdbremote_sequence()
+
+def fork_and_follow_test(self, variant, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test([variant], variant))
+self.start_fork_test([variant], variant, nonstop=nonstop))
 
 # switch to the forked child
 self.test_sequence.add_log_lines([
@@ -113,18 +165,37 @@
  

[Lldb-commits] [lldb] 261d003 - [lldb] [llgs] Fix premature server exit if multiprocess+nonstop

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T21:49:17+02:00
New Revision: 261d0033500eaa281b74c9d54ae240257a0ea00c

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

LOG: [lldb] [llgs] Fix premature server exit if multiprocess+nonstop

Fix lldb-server in the non-stop + multiprocess mode to exit on vStopped
only if all processes have exited, rather than when the first one exits.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128639

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 49125f91d743a..63174ef552196 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3860,9 +3860,9 @@ GDBRemoteCommunicationServerLLGS::Handle_vStopped(
   m_stop_notification_queue.pop_front();
   if (!m_stop_notification_queue.empty())
 return SendPacketNoLock(m_stop_notification_queue.front());
-  // If this was the last notification and the process exited, terminate
-  // the server.
-  if (m_inferior_prev_state == eStateExited) {
+  // If this was the last notification and all the processes exited,
+  // terminate the server.
+  if (m_debugged_processes.empty()) {
 m_exit_now = true;
 m_mainloop.RequestTermination();
   }

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 8b8e30c9b7379..c58f0fc381ebb 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -392,17 +392,17 @@ def test_vkill_both(self):
 def test_vkill_both_nonstop(self):
 self.vkill_test(kill_parent=True, kill_child=True, nonstop=True)
 
-def resume_one_test(self, run_order, use_vCont=False):
+def resume_one_test(self, run_order, use_vCont=False, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test(["fork", "trap"]))
+self.start_fork_test(["fork", "trap"], nonstop=nonstop))
 
 parent_expect = [
-"[$]T05thread:p{}.{};.*".format(parent_pid, parent_tid),
-"[$]W00;process:{}#.*".format(parent_pid),
+"T05thread:p{}.{};.*".format(parent_pid, parent_tid),
+"W00;process:{}#.*".format(parent_pid),
 ]
 child_expect = [
-"[$]T05thread:p{}.{};.*".format(child_pid, child_tid),
-"[$]W00;process:{}#.*".format(child_pid),
+"T05thread:p{}.{};.*".format(child_pid, child_tid),
+"W00;process:{}#.*".format(child_pid),
 ]
 
 for x in run_order:
@@ -427,9 +427,17 @@ def resume_one_test(self, run_order, use_vCont=False):
 "send packet: $OK#00",
 "read packet: $c#00",
 ], True)
-self.test_sequence.add_log_lines([
-{"direction": "send", "regex": expect},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:" + expect},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+{"direction": "send", "regex": "[$]" + expect},
+], True)
 # if at least one process remained, check both PIDs
 if parent_expect or child_expect:
 self.test_sequence.add_log_lines([
@@ -475,6 +483,14 @@ def test_c_child_then_parent(self):
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
+@add_test_categories(["fork"])
+def test_c_interspersed_nonstop(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ nonstop=True)
+
 @expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
@@ -513,6 +529,14 @@ def test_vCont_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", 

[Lldb-commits] [PATCH] D128698: [lldb] Add a NativeProcessProtocol::Threads() iterable

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe095cddb7622: [lldb] Add a NativeProcessProtocol::Threads() 
iterable (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128698

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -724,17 +724,12 @@
   json::Array threads_array;
 
   // Ensure we can get info on the given thread.
-  uint32_t thread_idx = 0;
-  for (NativeThreadProtocol *thread;
-   (thread = process.GetThreadAtIndex(thread_idx)) != nullptr;
-   ++thread_idx) {
-
-lldb::tid_t tid = thread->GetID();
-
+  for (NativeThreadProtocol  : process.Threads()) {
+lldb::tid_t tid = thread.GetID();
 // Grab the reason this thread stopped.
 struct ThreadStopInfo tid_stop_info;
 std::string description;
-if (!thread->GetStopReason(tid_stop_info, description))
+if (!thread.GetStopReason(tid_stop_info, description))
   return llvm::make_error(
   "failed to get stop reason", llvm::inconvertibleErrorCode());
 
@@ -751,7 +746,7 @@
 json::Object thread_obj;
 
 if (!abridged) {
-  if (llvm::Optional registers = GetRegistersAsJSON(*thread))
+  if (llvm::Optional registers = GetRegistersAsJSON(thread))
 thread_obj.try_emplace("registers", std::move(*registers));
 }
 
@@ -760,7 +755,7 @@
 if (signum != 0)
   thread_obj.try_emplace("signal", signum);
 
-const std::string thread_name = thread->GetName();
+const std::string thread_name = thread.GetName();
 if (!thread_name.empty())
   thread_obj.try_emplace("name", thread_name);
 
@@ -856,14 +851,12 @@
   if (m_list_threads_in_stop_reply) {
 response.PutCString("threads:");
 
-uint32_t thread_index = 0;
-NativeThreadProtocol *listed_thread;
-for (listed_thread = process.GetThreadAtIndex(thread_index); listed_thread;
- ++thread_index,
-listed_thread = process.GetThreadAtIndex(thread_index)) {
-  if (thread_index > 0)
+uint32_t thread_num = 0;
+for (NativeThreadProtocol _thread : process.Threads()) {
+  if (thread_num > 0)
 response.PutChar(',');
-  response.Printf("%" PRIx64, listed_thread->GetID());
+  response.Printf("%" PRIx64, listed_thread.GetID());
+  ++thread_num;
 }
 response.PutChar(';');
 
@@ -872,7 +865,7 @@
 // is hex ascii JSON that contains the thread IDs thread stop info only for
 // threads that have stop reasons. Only send this if we have more than one
 // thread otherwise this packet has all the info it needs.
-if (thread_index > 1) {
+if (thread_num > 1) {
   const bool threads_with_valid_stop_info_only = true;
   llvm::Expected threads_info = GetJSONThreadsInfo(
   *m_current_process, threads_with_valid_stop_info_only);
@@ -889,12 +882,10 @@
   }
 }
 
-uint32_t i = 0;
 response.PutCString("thread-pcs");
 char delimiter = ':';
-for (NativeThreadProtocol *thread;
- (thread = process.GetThreadAtIndex(i)) != nullptr; ++i) {
-  NativeRegisterContext _ctx = thread->GetRegisterContext();
+for (NativeThreadProtocol  : process.Threads()) {
+  NativeRegisterContext _ctx = thread.GetRegisterContext();
 
   uint32_t reg_to_read = reg_ctx.ConvertRegisterKindToRegisterNumber(
   eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC);
@@ -1024,12 +1015,10 @@
   if (!m_non_stop)
 return;
 
-  uint32_t thread_index = 0;
-  while (NativeThreadProtocol *listed_thread =
- m_current_process->GetThreadAtIndex(thread_index++)) {
-if (listed_thread->GetID() != thread_to_skip)
+  for (NativeThreadProtocol _thread : m_current_process->Threads()) {
+if (listed_thread.GetID() != thread_to_skip)
   m_stop_notification_queue.push_back(
-  PrepareStopReplyPacketForThread(*listed_thread).GetString().str());
+  PrepareStopReplyPacketForThread(listed_thread).GetString().str());
   }
 }
 
@@ -1990,15 +1979,10 @@
 return;
 
   LLDB_LOG(log, "iterating over threads of process {0}", process.GetID());
-  NativeThreadProtocol *thread;
-  uint32_t thread_index;
-  for (thread_index = 0, thread = process.GetThreadAtIndex(thread_index);
-   thread;
-   ++thread_index, thread = process.GetThreadAtIndex(thread_index)) {
-LLDB_LOG(log, "iterated thread {0} (tid={1})", thread_index,
-  

[Lldb-commits] [lldb] e095cdd - [lldb] Add a NativeProcessProtocol::Threads() iterable

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T21:49:16+02:00
New Revision: e095cddb763fde72b577f3fab5e039cea4c3

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

LOG: [lldb] Add a NativeProcessProtocol::Threads() iterable

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128698

Added: 


Modified: 
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h 
b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 63dbb3a62ea5f..37dacca6ff813 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -15,6 +15,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Iterable.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/TraceGDBRemotePackets.h"
 #include "lldb/Utility/UnimplementedError.h"
@@ -48,6 +49,16 @@ class NativeProcessProtocol {
 public:
   virtual ~NativeProcessProtocol() = default;
 
+  typedef std::vector> thread_collection;
+  template 
+  static NativeThreadProtocol _list_adapter(I ) {
+assert(*iter);
+return **iter;
+  }
+  typedef LockingAdaptedIterable
+  ThreadIterable;
+
   virtual Status Resume(const ResumeActionList _actions) = 0;
 
   virtual Status Halt() = 0;
@@ -210,6 +221,10 @@ class NativeProcessProtocol {
 return GetThreadByID(m_current_thread_id);
   }
 
+  ThreadIterable Threads() const {
+return ThreadIterable(m_threads, m_threads_mutex);
+  }
+
   // Access to inferior stdio
   virtual int GetTerminalFileDescriptor() { return m_terminal_fd; }
 

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index f7990f2d394fc..d038c78a59941 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -98,8 +98,8 @@ Error IntelPTCollector::TraceStart(const 
TraceIntelPTStartRequest ) {
   }
 } else {
   std::vector process_threads;
-  for (size_t i = 0; m_process.GetThreadAtIndex(i); i++)
-process_threads.push_back(m_process.GetThreadAtIndex(i)->GetID());
+  for (NativeThreadProtocol  : m_process.Threads())
+process_threads.push_back(thread.GetID());
 
   // per-thread process tracing
   if (Expected trace =

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
index ef4c5ae46ed53..f538ef7776491 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
@@ -107,9 +107,9 @@ void IntelPTMultiCoreTrace::ProcessWillResume() {
 TraceIntelPTGetStateResponse IntelPTMultiCoreTrace::GetState() {
   TraceIntelPTGetStateResponse state;
 
-  for (size_t i = 0; m_process.GetThreadAtIndex(i); i++)
+  for (NativeThreadProtocol  : m_process.Threads())
 state.traced_threads.push_back(
-TraceThreadState{m_process.GetThreadAtIndex(i)->GetID(), {}});
+TraceThreadState{thread.GetID(), {}});
 
   state.cpus.emplace();
   ForEachCore([&](lldb::cpu_id_t cpu_id,

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index ae8b8ef8ae156..49125f91d743a 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -724,17 +724,12 @@ GetJSONThreadsInfo(NativeProcessProtocol , bool 
abridged) {
   json::Array threads_array;
 
   // Ensure we can get info on the given thread.
-  uint32_t thread_idx = 0;
-  for (NativeThreadProtocol *thread;
-   (thread = process.GetThreadAtIndex(thread_idx)) != nullptr;
-   ++thread_idx) {
-
-lldb::tid_t tid = thread->GetID();
-
+  for (NativeThreadProtocol  : process.Threads()) {
+lldb::tid_t tid = thread.GetID();
 // Grab the reason this thread stopped.
 struct ThreadStopInfo tid_stop_info;
 std::string description;
-if (!thread->GetStopReason(tid_stop_info, description))
+if (!thread.GetStopReason(tid_stop_info, description))
   return llvm::make_error(
   "failed to get stop reason", llvm::inconvertibleErrorCode());
 
@@ -751,7 +746,7 @@ GetJSONThreadsInfo(NativeProcessProtocol , bool 
abridged) {
 json::Object 

[Lldb-commits] [lldb] b415f8e - [lldb] [llgs] Add base nonstop fork/vfork tests

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T21:49:16+02:00
New Revision: b415f8e3dfb839a886b5290bbebbdd42344c07a9

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

LOG: [lldb] [llgs] Add base nonstop fork/vfork tests

Extend the most of baseline fork tests to run in nonstop mode as well.
For more cases, we're just testing one example scenario to save time.
This patch does not cover tests that rely on correct exit handling,
as fixing that is addressed in a followup patch.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128638

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 70cea2f85f6ec..8b8e30c9b7379 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -9,10 +9,12 @@ class 
TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 
 fork_regex = ("[$]T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
   "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_regex_nonstop = ("%Stop:T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
 
-def start_fork_test(self, args, variant="fork"):
+def start_fork_test(self, args, variant="fork", nonstop=False):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=args)
 self.add_qSupported_packets(["multiprocess+",
@@ -22,11 +24,24 @@ def start_fork_test(self, args, variant="fork"):
 self.reset_test_sequence()
 
 # continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"read packet: $QNonStop:1#00",
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": self.fork_regex_nonstop.format(variant),
+ "capture": self.fork_capture},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format(variant),
+ "capture": self.fork_capture},
+], True)
 ret = self.expect_gdbremote_sequence()
 self.reset_test_sequence()
 
@@ -45,9 +60,9 @@ def test_fork_multithreaded(self):
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_detach_test(self, variant):
+def fork_and_detach_test(self, variant, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test([variant], variant))
+self.start_fork_test([variant], variant, nonstop=nonstop))
 
 # detach the forked child
 self.test_sequence.add_log_lines([
@@ -77,6 +92,20 @@ def test_fork(self):
 ], True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["fork"])
+def test_fork_nonstop(self):
+parent_pid, _ = self.fork_and_detach_test("fork", nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+"send packet: %Stop:W00;process:{}#00".format(parent_pid),
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+self.expect_gdbremote_sequence()
+
 @add_test_categories(["fork"])
 def test_vfork(self):
 parent_pid, parent_tid = self.fork_and_detach_test("vfork")
@@ -93,9 +122,32 @@ def test_vfork(self):
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_follow_test(self, variant):
+@add_test_categories(["fork"])
+def test_vfork_nonstop(self):
+parent_pid, parent_tid = self.fork_and_detach_test("vfork",
+   nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": r"%Stop:T05thread:p{}[.]{}.*vforkdone.*".format(
+ parent_pid, parent_tid),
+ },
+   

[Lldb-commits] [lldb] f7bf9d1 - TestIgnoredExceptions.py needs support from debugserver, so it

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-06-28T11:49:55-07:00
New Revision: f7bf9d13d50d785889952deb18cc93de0176cb96

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

LOG: TestIgnoredExceptions.py needs support from debugserver, so it
needs to be marked skip if out of tree debugserver.

Added: 


Modified: 
lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py

Removed: 




diff  --git a/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py 
b/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
index e90b2ef8dbb3..f8112b6803e1 100644
--- a/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
+++ b/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
@@ -12,6 +12,7 @@ class TestDarwinSignalHandlers(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfOutOfTreeDebugserver
 @skipUnlessDarwin
 def test_ignored_thread(self):
 """It isn't possible to convert an EXC_BAD_ACCESS to a signal when



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

[Lldb-commits] [PATCH] D128707: [lldb] Fix build on older Linux kernel versions

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

@kongyi thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128707

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

[Lldb-commits] [PATCH] D128707: [lldb] Fix build on older Linux kernel versions

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.
Herald added a subscriber: JDevlieghere.

thanks a good point, Jakob. Let's keep it in mind. Maybe we can bootcamp that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128707

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

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

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

mgorny added a comment.

Ok, I'll commit as-is then and will revisit if we start seeing timeouts.


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

https://reviews.llvm.org/D128638

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

[Lldb-commits] [PATCH] D128576: [trace] Make events first class items in the trace cursor and rework errors

[Jakob Johnson via Phabricator via lldb-commits]

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

review- part 1




Comment at: lldb/include/lldb/Target/Trace.h:171
+  /// information failed to load, an \a llvm::Error is returned.
+  virtual llvm::Expected
+  CreateNewCursor(Thread ) = 0;

Do we want to keep the cursor as a UP or take this refactor before creating the 
public bindings to make it a SP? IMO a UP might make sense because I can't 
really think of many cases where you would want multiple users of the same 
cursor.
wdyt?



Comment at: lldb/include/lldb/Target/TraceCursor.h:44-46
+/// The cursor can also point at events in the trace, which aren't errors
+/// nor instructions. An example of an event could be a context switch in
+/// between two instructions.

do you want to include pointers to the methods you can use to check/get events 
similar to what you did for errors above?



Comment at: lldb/include/lldb/Target/TraceCursor.h:232-246
+  virtual const char *GetError() const = 0;
 
-  /// Get the corresponding error message if the cursor points to an error in
-  /// the trace.
-  ///
   /// \return
-  /// \b nullptr if the cursor is not pointing to an error in
-  /// the trace. Otherwise return the actual error message.
-  virtual const char *GetError() = 0;
+  /// Whether the cursor points to an event or not.
+  virtual bool IsEvent() const = 0;
 
   /// \return

For the getters, what are your thoughts on returning an optional instead of 
using designated value/variants (ie nullptr, LLDB_INVALID_INSTRUCTION, 
eTraceEventNone`) to indicate that the current item does not match what `Get` 
method is being used?

Along the same lines, but a slightly bigger change:
With the introduction of `Events` as first class citizens of the trace cursor 
API, it may make sense to introduce the notion of of a trace "item" or 
something similar that encapsulates (instructions, events, errors, etc). I'm 
thinking of a tagged union type structure (ie a Rust enum)  that enforces the 
correctness of the access to the underlying item.
wdyt?



Comment at: lldb/include/lldb/Target/TraceCursor.h:258
+  virtual llvm::Optional
+  GetCounter(lldb::TraceCounter counter_type) const = 0;
 

Now that we are introducing the notion of an `Event`? wdyt about combining 
Events and Counters since a counter value in a trace really is just a type of 
event? In this case, `Counter` could just become a variant of `TraceEvent`. 
This may make more sense because even in the case of TSCs in an IntelPT trace 
because, strictly speaking, these aren't associated with instructions, correct? 
Instead the TSC values are emitted with PSBs and then we "attribute" these 
values to the nearest instruction, right?



Comment at: lldb/include/lldb/Target/TraceDumper.h:59
+  /// Helper struct that holds all the information we know about a trace item
+  struct TraceItem {
 lldb::user_id_t id;

oh looks like you already introduced a `TraceItem` structure!
Similar to my comment above related to introducing a trace item type structure 
for the trace cursor, would it make sense to have a separate type for each of 
the different "items". With the way the TraceItem struct is currently, a lot of 
this data is mutually exclusive, hence the many optional fields, correct?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145-147
   /// \return
-  /// The control flow categories, or \b 0 if the instruction is an error.
+  /// The control flow categories, or an undefined vlaue if the item is not
+  /// an instruction.

Why not use an optional here instead?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:236-239
+  enum class ItemKind : uint64_t {
+Instruction = 0,
+Error = LLDB_INVALID_ADDRESS - 1,
+Event = LLDB_INVALID_ADDRESS - 2,

Continuing the theme of my comments related to the notion of a `TraceItem` - 
would it be possible to unite the notion of a trace item? currently there is an 
Item enum here, a item struct in the Dumper and I feel there is a need for a 
Item at the trace cursor level as well.
wdyt?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:241
+  };
+  ItemKind LowestNonInstructionItemKind = ItemKind::Event;
+

why is this needed?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:253-258
+  /// The low level storage of all instruction addresses, error and events. 
Each
+  /// trace item has an index in this vector and this index is used throughout
+  /// the code. As an storage optimization, values of \b ItemKind::Error
+  /// indicate that this item is an error, \b ItemKind::Event if the item is an
+  /// event, and otherwise the 

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

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

mgorny added a comment.

Unfortunately, I wasn't able to get ARM to work (some triple mismatch) but I've 
masked the tests xfail for now, and both buildbots seem green.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

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

[Lldb-commits] [lldb] 25f4608 - [lldb] [test] XFAIL llgs tests failing on arm

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T17:02:59+02:00
New Revision: 25f46084d8e11894edc0b71b737f45e0f9115b45

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

LOG: [lldb] [test] XFAIL llgs tests failing on arm

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index aa97ccd51458..70cea2f85f6e 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -317,48 +317,56 @@ def resume_one_test(self, run_order, use_vCont=False):
 ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_parent(self):
 self.resume_one_test(run_order=["parent", "parent"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_child(self):
 self.resume_one_test(run_order=["child", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_parent(self):
 self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_child(self):
 self.resume_one_test(run_order=["child", "child"], use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
@@ -366,6 +374,7 @@ def test_vCont_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
@@ -373,6 +382,7 @@ def test_vCont_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
@@ -415,6 +425,7 @@ def test_vCont_all_processes_implicit(self):
 ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @add_test_categories(["fork"])
 def test_threadinfo(self):
 parent_pid, parent_tid, child_pid, child_tid = (
@@ -460,6 +471,7 @@ def test_threadinfo(self):
  ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @add_test_categories(["fork"])
 def test_memory_read_write(self):
 self.build()
@@ -547,6 +559,7 @@ def test_memory_read_write(self):
 self.assertEqual(data, name + "\0")
 

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:637
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildAtIndex(2, true);
 if (using_bitmasks)

mib wrote:
> mib wrote:
> > aprantl wrote:
> > > Let me know if I',m misunderstanding what the code is doing, but this 
> > > looks like it is replacing the previous implementation? Ideally we would 
> > > detect the layout and then parse it correctly depending on which version 
> > > we're dealing with. Otherwise we risk breaking the matrix bot that checks 
> > > that LLDB can debug C++ produced by older versions of LLVM (and by 
> > > extension libcxx).
> > I've look at D12828 and D123580, and I don't see any way of versioning 
> > these changes ... may be @ldionne have an idea on how we could do this 
> > properly ?
> > 
> > Also, in D124113, @labath mentions that this data formatter uses mostly 
> > indices to parse and access the various fields of the type data structure 
> > (because it uses some anonymous structs). This makes it very fragile on our 
> > end because our data formatter break every time they make a change in the 
> > layout ...
> > 
> > @aprantl, I'll update the line your pointed at to the the field identifier 
> > instead of using changing the index while waiting for a better way to 
> > version this.
> @aprantl, I'll update the line you pointed at to *use* the field identifier 
> instead of using changing the index, while waiting for a better way to 
> version this.
I don't see a way to version this. You don't have access to the value of macros 
that were defined when the executable was compiled, right? If you did, you 
could check `_LIBCPP_VERSION` (1400 = old implementation, 1500 = current 
implementation). I'm very naive when it comes to debuggers but I assume that's 
not possible.


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

https://reviews.llvm.org/D128694

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Guy Benyei via Phabricator via lldb-commits]

gbenyei updated this revision to Diff 440579.
gbenyei marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest  : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, false, true}},
Index: llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
===
--- llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
+++ llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
@@ -33,6 +33,12 @@
 # RUN: llvm-objcopy -I binary -O elf64-littleriscv %t.txt %t.rv64.o
 # RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,LE,RISCV64,64
 
+# RUN: llvm-objcopy -I binary -O elf32-bigriscv %t.txt %t.rv32.o
+# RUN: llvm-readobj --file-headers %t.rv32.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV32,32
+
+# RUN: llvm-objcopy -I binary -O elf64-bigriscv %t.txt %t.rv64.o
+# RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV64,64
+
 # RUN: llvm-objcopy -I binary -O elf32-sparc %t.txt %t.sparc.o
 # RUN: llvm-readobj --file-headers %t.sparc.o | FileCheck %s --check-prefixes=CHECK,BE,SPARC,32
 
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ 

[Lldb-commits] [PATCH] D128611: [libc++] Implement `std::ranges::merge`

[Hui via Phabricator via lldb-commits]

huixie90 updated this revision to Diff 440493.
huixie90 added a comment.
Herald added subscribers: cfe-commits, llvm-commits, libc-commits, 
openmp-commits, lldb-commits, Sanitizers, steakhal, nlopes, mtrofin, jsji, 
Enna1, bzcheeseman, kosarev, jsilvanus, mattd, gchakrabarti, ThomasRaoux, 
pmatos, asb, pcwang-thead, yota9, ayermolo, awarzynski, arjunp, sdasgup3, 
luke957, asavonic, carlosgalvezp, jeroen.dobbelaere, Groverkss, wenzhicui, 
wrengr, armkevincheng, ormris, foad, jsmolens, eric-k256, dcaballe, ChuanqiXu, 
cota, mravishankar, teijeong, frasercrmck, rdzhabarov, ecnelises, tatianashp, 
wenlei, mehdi_amini, okura, jdoerfert, bmahjour, msifontes, sstefan1, jurahul, 
kuter, Kayjukh, grosul1, martong, Joonsoo, stephenneuendorffer, kerbowa, 
liufengdb, aartbik, mgester, arpith-jacob, csigg, nicolasvasilache, 
antiagainst, shauheen, rriddle, luismarques, apazos, sameer.abuasal, usaxena95, 
pengfei, s.egerton, Jim, asbirlea, mstorsjo, miyuki, kadircet, jocewei, 
rupprecht, PkmX, arphaman, george.burgess.iv, the_o, brucehoult, MartinMosbeck, 
rogfer01, steven_wu, mgrang, edward-jones, zzheng, MaskRay, jrtc27, gbedwell, 
delcypher, niosHD, sabuasal, simoncook, haicheng, johnrusso, rbar, javed.absar, 
fedor.sergeev, kbarton, aheejin, hiraditya, jgravelle-google, arichardson, 
sbc100, nhaehnle, jvesely, nemanjai, emaste, dylanmckay, jyknight, dschuff, 
arsenm, qcolombet, MatzeB, jholewinski.
Herald added a reviewer: deadalnix.
Herald added a reviewer: bollu.
Herald added a reviewer: andreadb.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jhenderson.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: sscalpone.
Herald added a reviewer: jpienaar.
Herald added a reviewer: ftynse.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: awarzynski.
Herald added a reviewer: sjarus.
Herald added a reviewer: clementval.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: paulkirth.
Herald added projects: clang, Sanitizers, LLDB, OpenMP, libc-project, 
libunwind, MLIR, LLVM, lld-macho, clang-tools-extra, Flang.
Herald added a reviewer: libunwind.
Herald added a reviewer: lld-macho.

reorder includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128611

Files:
  bolt/README.md
  bolt/docs/OptimizingClang.md
  bolt/include/bolt/Passes/SplitFunctions.h
  bolt/lib/Core/BinaryFunctionProfile.cpp
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Passes/IndirectCallPromotion.cpp
  bolt/lib/Passes/LongJmp.cpp
  bolt/lib/Passes/SplitFunctions.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  bolt/test/X86/Inputs/dwarf5-call-pc-helper.s
  bolt/test/X86/Inputs/dwarf5-call-pc-main.s
  bolt/test/X86/Inputs/dwarf5-return-pc-helper.s
  bolt/test/X86/Inputs/dwarf5-return-pc-main.s
  bolt/test/X86/bug-reorder-bb-jrcxz.s
  bolt/test/X86/dwarf5-call-pc.test
  bolt/test/X86/dwarf5-return-pc.test
  bolt/test/X86/jump-table-icp.test
  bolt/test/X86/shared_object_entry.s
  bolt/test/X86/unreachable.test
  bolt/test/runtime/X86/exceptions-instrumentation.test
  bolt/test/runtime/X86/pie-exceptions-split.test
  bolt/test/runtime/meta-merge-fdata.test
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/FeatureModule.cpp
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/DumpASTTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/LSPClient.cpp
  clang-tools-extra/clangd/unittests/Matchers.h
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/test/lr-build-basic.test
  

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D128465#3612948 , @phosek wrote:

> I think this patch should be broken into at least two:
>
> 1. Refactor `llvm/include/llvm/Support/Compression.h` and 
> `llvm/lib/Support/Compression.cpp` to introduce a generic interface and use 
> it throughout the codebase.
> 2. zstd support in `llvm/include/llvm/Support/Compression.h` including the 
> CMake bits.
>
> When uploading future changes, please also make sure to include full context.

Agree. As soon as the namespace refactoring is in a good enough shape, I think 
you may land the refactoring part before the rest of zstd patches.

Note: if you have a deep stacked patches, it may be useful to have a branch 
somewhere (e.g. your llvm-project fork on Github) so that interested folks can 
get the whole picture more easily.
(`arc patch Dx` can technically apply a deep stack, but it often fails to 
apply changes cleanly.)


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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

I think this patch should be broken into at least two:

1. Refactor `llvm/include/llvm/Support/Compression.h` and 
`llvm/lib/Support/Compression.cpp` to introduce a generic interface and use it 
throughout the codebase.
2. zstd support in `llvm/include/llvm/Support/Compression.h` including the 
CMake bits.

When uploading future changes, please also make sure to include full context.




Comment at: llvm/include/llvm/Support/Compression.h:67-87
+#if LLVM_ENABLE_ZSTD
+namespace internal = llvm::compression::zstd;
+#else
+namespace internal = llvm::compression::zlib;
+#endif
+
+namespace elf = llvm::compression::zlib;

I think think that this should be a runtime rather than compile time selection. 
Many (if not most) toolchain vendors would likely want to enable both zstd and 
zlib and choose the algorithm based an option or file content.

For example, in the case of profiles, we want to be able to read existing 
profiles that use zlib, but for newly created profiles we may want to use zstd 
if available.

To make it possible, I suspect we may want to wrap the existing functions into 
a class that implements a common interface. I expect we would also need a 
function to detect the compression type based on the buffer content.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440325.
ckissane added a comment.

more context


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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,54 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
+  const char* StringString = strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
   EXPECT_EQ(
   0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- 

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440327.
ckissane added a comment.

format


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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,53 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
-  EXPECT_EQ(
-  0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  const char *StringString =
+  strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
+  EXPECT_EQ(0x414FA339U, llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- 

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440298.
ckissane added a comment.

fix type for FindZstd cmake usage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,54 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
+  const char* StringString = strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
   EXPECT_EQ(
   0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: llvm/unittests/ProfileData/InstrProfTest.cpp

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Jessica Clarke via Phabricator via lldb-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.h:113
+if (Triple.isLittleEndian())
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32-S128");
+else

And please avoid repeating the whole data layout, just make the e/E a variable



Comment at: clang/lib/Driver/ToolChains/FreeBSD.cpp:235
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32briscv");
+break;

-X to match LE. Or ditch these (and I can ditch riscv32...) since FreeBSD only 
supports riscv64.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1700
 {M.gccSuffix(),
  "/../../../../riscv64-unknown-elf/lib" + M.gccSuffix(),
  "/../../../../riscv32-unknown-elf/lib" + M.gccSuffix()});

Just shove a `+ Be +` in the middle of these two rather than introducing a 
whole new set and picking between them?



Comment at: llvm/cmake/config-ix.cmake:463
   set(LLVM_NATIVE_ARCH RISCV)
+elseif (LLVM_NATIVE_ARCH MATCHES "riscv32be")
+  set(LLVM_NATIVE_ARCH RISCV)

I believe these need to come before the unsuffixed versions to do anything, but 
also the unsuffixed versions already handle the suffixed versions correctly so 
this isn't needed?



Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp:554
 .buildGraph();
-  } else {
-assert((*ELFObj)->getArch() == Triple::riscv32 &&
-   "Invalid triple for RISCV ELF object file");
+  } else if ((*ELFObj)->getArch() == Triple::riscv64be) {
+auto  = cast>(**ELFObj);

Why switch to this order when before you've used 32, 64, 32be, 64be as the order



Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:65
+
+return "E-m:e-p:64:64-i64:64-i128:128-n64-S128";
+  }

As with Clang


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440288.
ckissane added a comment.

Fix missing cmake findZstd, remove unused crc32 from compression namespaces and 
simplify some code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,54 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
+  const char* StringString = strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
   EXPECT_EQ(
   0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: 

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[James Henderson via Phabricator via lldb-commits]

jhenderson added inline comments.



Comment at: llvm/include/llvm/ADT/Triple.h:864
 
   /// Tests whether the target is RISC-V (32- and 64-bit).
   bool isRISCV() const {

Perhaps worth updating to mention big and little endian here, like `isPPC64` 
above?



Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:304-305
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC

We need llvm-objcopy testing for these new targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Craig Topper via Phabricator via lldb-commits]

craig.topper added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.h:111
 SizeType = UnsignedInt;
-resetDataLayout("e-m:e-p:32:32-i64:64-n32-S128");
   }

Instead of creating new classes, could we have a branch on the Arch or 
isLittleEndian portion of the triple to decide what to pass to resetDataLayout? 
That's what PPC32TargetInfo does for example.



Comment at: lld/ELF/Arch/RISCV.cpp:160
+  if (config->is64) {
+if (config->isLE)
+  write64le(buf, mainPart->dynamic->getVA());

Is it possible to use write64 instead of write64le/be? It looks like it checks 
the endianness internally.



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:628
+  // For big endian cores, data fixup should be swapped.
+  bool swapValue = (Endian == support::big) && isDataFixup(Kind);
   for (unsigned i = 0; i != NumBytes; ++i) {

Capitalize `swapValue`



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h:34
   const MCTargetOptions )
-  : MCAsmBackend(support::little), STI(STI), OSABI(OSABI), 
Is64Bit(Is64Bit),
+  : MCAsmBackend(IsLittleEndian ? support::little : support::big), 
STI(STI), OSABI(OSABI), Is64Bit(Is64Bit),
 TargetOptions(Options) {

Is this longer than 80 columns?



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp:24
+  if (TT.getArch() == Triple::riscv32be || TT.getArch() == Triple::riscv64be)
+IsLittleEndian = false;
+

Could we do IsLittleEndian = TT.isLittleEndian()?



Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:64
+  return "e-m:e-p:64:64-i64:64-i128:128-n64-S128";
+else
+  return "E-m:e-p:64:64-i64:64-i128:128-n64-S128";

No else after return


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Guy Benyei via Phabricator via lldb-commits]

gbenyei updated this revision to Diff 440196.
gbenyei added a comment.

Thanks, Craig. Updated the patch with your remarks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest  : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, false, true}},
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -479,7 +479,8 @@
   bool IsMIPS64 = TargetTriple.isMIPS64();
   bool IsArmOrThumb = TargetTriple.isARM() || TargetTriple.isThumb();
   bool IsAArch64 = TargetTriple.getArch() == Triple::aarch64;
-  bool IsRISCV64 = TargetTriple.getArch() == Triple::riscv64;
+  bool IsRISCV64 = TargetTriple.getArch() == Triple::riscv64 ||
+   TargetTriple.getArch() == Triple::riscv64be;
   bool IsWindows = TargetTriple.isOSWindows();
   bool IsFuchsia = TargetTriple.isOSFuchsia();
   bool IsEmscripten = TargetTriple.isOSEmscripten();
Index: llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
===
--- llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
+++ llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
@@ -15,6 +15,8 @@
 
 Target ();
 Target ();
+Target ();
+Target ();
 
 } // namespace llvm
 
Index: 

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Guy Benyei via Phabricator via lldb-commits]

gbenyei created this revision.
gbenyei added a reviewer: asb.
gbenyei added projects: clang, LLVM, lld.
Herald added subscribers: Enna1, sunshaoce, VincentWu, luke957, StephenFan, 
vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, ormris, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, 
rbar, hiraditya, arichardson, mgorny, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
gbenyei requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, pcwang-thead.
Herald added a project: LLDB.

Implement riscv32be and riscv64be targets.
The RISC-V big- and bi-endian targets are discussed in the RISC-V spec  Version 
20191213, but some aspects, like ABI are still unclear.
The instruction encoding is little endian in both big- and little-endian modes. 
ISA spec Volume 1 1.15: "Instructions are stored in memory as a sequence
of 16-bit little-endian parcels, regardless of memory system endianness".

RISC-V Big-endian cores are already supported by GCC. Where spec is unclear, we 
aim to be compatible with GCC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest  : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, 

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

Emmmer updated this revision to Diff 439955.
Emmmer marked an inline comment as done.
Emmmer added a comment.

This patch change:

- add `lldb/source/Plugins/Architecture/RISCV64`
- add `lldb/source/Plugins/Architecture/RISCV32`
- update `lldb/source/Utility/ArchSpec.cpp`
- remove 
`lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64`

> No DWARF numbers for the FPU regs?

I would like to add support for GPR first, after making sure they work well, 
FPU regs are a simple copy-paste.

> AArch64 linux build doesn't handle AArch32 mode, you'd need to use an AArch32 
> built binary for that.

Thanks for your suggestion, I split them out.

> and PAC means? (please add it to the comment)
> I don't see an implementation of this yet, not one that actually reads 
> anything. Intentional? It'll just see 0 breakpoints I guess.

At first, I referenced the AArch64 code to add riscv64 support because their 
arch is similar, but I overlooked that riscv64 doesn't have a Debug reg yet, so 
it was a mistake on my part.


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

https://reviews.llvm.org/D128250

Files:
  lldb/source/Plugins/Architecture/CMakeLists.txt
  lldb/source/Plugins/Architecture/RISCV32/ArchitectureRISCV32.cpp
  lldb/source/Plugins/Architecture/RISCV32/ArchitectureRISCV32.h
  lldb/source/Plugins/Architecture/RISCV32/CMakeLists.txt
  lldb/source/Plugins/Architecture/RISCV64/ArchitectureRISCV64.cpp
  lldb/source/Plugins/Architecture/RISCV64/ArchitectureRISCV64.h
  lldb/source/Plugins/Architecture/RISCV64/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/source/Utility/RISCV64_DWARF_Registers.h

Index: lldb/source/Utility/RISCV64_DWARF_Registers.h
===
--- /dev/null
+++ lldb/source/Utility/RISCV64_DWARF_Registers.h
@@ -0,0 +1,56 @@
+//===-- RISCV64_DWARF_Registers.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+#define LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+
+#include "lldb/lldb-private.h"
+
+namespace riscv64_dwarf {
+
+enum {
+  x0 = 0,
+  x1,
+  x2,
+  x3,
+  x4,
+  x5,
+  x6,
+  x7,
+  x8,
+  x9,
+  x10,
+  x11,
+  x12,
+  x13,
+  x14,
+  x15,
+  x16,
+  x17,
+  x18,
+  x19,
+  x20,
+  x21,
+  x22,
+  x23,
+  x24,
+  x25,
+  x26,
+  x27,
+  x28,
+  x29,
+  x30,
+  x31,
+  ra = x1,
+  sp = x2,
+  fp = x8,
+};
+
+} // namespace riscv64_dwarf
+
+#endif // LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -724,6 +724,8 @@
   case llvm::Triple::systemz:
   case llvm::Triple::xcore:
   case llvm::Triple::arc:
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
 return false;
   }
 }
@@ -1376,6 +1378,20 @@
 }
 break;
 
+  case ArchSpec::eCore_riscv32:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_riscv32)
+return true;
+}
+break;
+
+  case ArchSpec::eCore_riscv64:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_riscv64)
+return true;
+}
+break;
+
   default:
 break;
   }
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -0,0 +1,153 @@
+//===-- RegisterInfos_riscv64.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifdef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
+
+#include 
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private.h"
+
+#include "Utility/RISCV64_DWARF_Registers.h"
+
+#ifndef GPR_OFFSET
+#error GPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef GPR_OFFSET_NAME
+#error GPR_OFFSET_NAME must be defined 

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

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



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h:9
+
+#if defined(__riscv) || __riscv_xlen == 64
+

DavidSpickett wrote:
> Should this be `&&`? I'm assuming that `__riscv` is also defined for riscv32.
WIP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I tried to fix this error with gdb, but the debugging process was too slow, 
> so I'm sorry for not being able to provide a test patch for verification of 
> my commit.

Oh and if you want help with this please ask on discord/discourse. This stuff 
is a bit tedious with so many exchanges on first connection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Overall this looks fine. Without the ability to test it I don't think we would 
accept it yet, so I didn't look into the details too much.

> To use gdbserver by lldb requires the participation of lldb-server, so my 
> current plan is to allow lldb to co-op with gdbserver, then let lldb be able 
> to use lldb-server, pass the unit tests, and finally add riscv32 support.

Sounds good. If those lldb changes can be tested by existing bots then they can 
go in first. For anything else it's probably better to build a tree of your 
changes and when you've got something that ends in a testable lldb-server then 
put them into review.

I appreciate it's difficult to test a lot of the enabling code e.g. the 
breakpoint code in this change. That'll be exercised by the test suite in 
general once more things are working. So it would be fine to say here is a 
series that ends in an lldb/lldb-server that can run a large amount of existing 
tests.

If you find anything non-riscv specific e.g. some gdb protocol difference that 
can go into review whenever. Anything to improve lldb-gdb compatibility is 
welcome.

> We use Open Build Server as the riscv build bot.

I'm not familiar with this infrastructure. In a future where your changes are 
in llvm `main` will developers be notified when they break the riscv lldb build?




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:249
+bool NativeRegisterContextLinux_riscv64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_riscv64::GPRegSet)

```
  return GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) == 
RegisterInfoPOSIX_riscv64::GPRegSet;
```



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:256
+bool NativeRegisterContextLinux_riscv64::IsFPR(unsigned reg) const {
+  return false;
+}

Seems like this should do a lookup?



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:323
+
+void NativeRegisterContextLinux_riscv64::InvalidateAllRegisters() {
+  m_gpr_is_valid = false;

Minor thing but if you call this in the constructor it's one less place to `= 
false` all the things.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:341
+llvm::Error NativeRegisterContextLinux_riscv64::ReadHardwareDebugInfo() {
+  return llvm::Error::success();
+}

This also a placeholder I assume (I'm not sure what this does for other targets 
tbh).



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:345
+NativeRegisterContextLinux_riscv64::DREGType hwbType) {
+  return llvm::Error::success();
+}

This is a placeholder?



Comment at: 
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.cpp:19
+constexpr uint32_t g_enable_bit = 1;
+// PAC (bits 2:1): 0b10
+constexpr uint32_t g_pac_bits = (2 << 1);

and PAC means? (please add it to the comment)



Comment at: 
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.cpp:34
+  Log *log = GetLog(LLDBLog::Breakpoints);
+  llvm::Error error = ReadHardwareDebugInfo();
+  if (error) {

I don't see an implementation of this yet, not one that actually reads 
anything. Intentional? It'll just see 0 breakpoints I guess.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp:51
+  switch (target_arch.GetMachine()) {
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:

So the way lldb-server works, at least at the moment, is that one binary does 
one architecture. So for example the AArch64 linux build doesn't handle AArch32 
mode, you'd need to use an AArch32 built binary for that.

Maybe you can make this work with riscv but I'd start with just looking for 
riscv64 here.



Comment at: lldb/source/Utility/RISCV64_DWARF_Registers.h:54
+
+} // namespace riscv64_dwarf
+

No DWARF numbers for the FPU regs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128465: [WIP] Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane created this revision.
ckissane added reviewers: phosek, leonardchan.
ckissane added a project: LLVM.
Herald added subscribers: Enna1, abrachet, wenlei, usaxena95, kadircet, 
arphaman, hiraditya, arichardson, mgorny, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added projects: Flang, All.
ckissane requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, lldb-commits, Sanitizers, 
StephenFan, jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, clang-tools-extra.

All work in progress POC implementation of the [RFC] Zstandard as a second 
compression method to LLVM.

Note:

- An interesting unresolved caveat crc32 implementation in the compression 
namespace requires ZLIB even if only enabling ZSTD




Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -18,6 +18,7 @@
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -70,4 +71,53 @@
 
 #endif
 
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
+}
+
+TEST(CompressionTest, ZstdCRC32) {
+  EXPECT_EQ(
+  0x414FA339U,
+  

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I'll give this a more thorough read later just one comment for now.

In the meantime, what's your plan for testing this config going forward? This 
patch would be fine as the first of a series but I'd like to see the follow ups 
that make it usable.

Will there be a build bot? You could at least enable building lldb on an 
existing riscv bot, while you iron out all the inevitable test suite issues.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h:9
+
+#if defined(__riscv) || __riscv_xlen == 64
+

Should this be `&&`? I'm assuming that `__riscv` is also defined for riscv32.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

Emmmer added a comment.

In D128250#3598223 , @DavidSpickett 
wrote:

> I'll give this a more thorough read later just one comment for now.
>
> In the meantime, what's your plan for testing this config going forward? This 
> patch would be fine as the first of a series but I'd like to see the follow 
> ups that make it usable.
>
> Will there be a build bot? You could at least enable building lldb on an 
> existing riscv bot, while you iron out all the inevitable test suite issues.

To use gdbserver by lldb requires the participation of lldb-server, so my 
current plan is to allow lldb to co-op with gdbserver, then let lldb be able to 
use lldb-server, pass the unit tests, and finally add riscv32 support. 
We use `Open Build Server` as the riscv build bot 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

Emmmer created this revision.
Emmmer added reviewers: DavidSpickett, craig.topper.
Emmmer added a project: LLDB.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, 
frasercrmck, evandro, luismarques, apazos, sameer.abuasal, JDevlieghere, 
s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, arichardson, mgorny.
Herald added a project: All.
Emmmer requested review of this revision.
Herald added subscribers: lldb-commits, pcwang-thead, eopXD, MaskRay.

I added some initial support for lldb-server on **riscv64** platform, now it 
can be built on riscv64.
But when I use lldb for debugging, it reports:

  error: 'A' packet returned an error: -1

I tried to fix this error with gdb, but the debugging process was too slow, so 
I'm sorry for not being able to provide a test patch for verification of my 
commit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128250

Files:
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.cpp
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
  lldb/source/Utility/RISCV64_DWARF_Registers.h

Index: lldb/source/Utility/RISCV64_DWARF_Registers.h
===
--- /dev/null
+++ lldb/source/Utility/RISCV64_DWARF_Registers.h
@@ -0,0 +1,56 @@
+//===-- RISCV64_DWARF_Registers.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+#define LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+
+#include "lldb/lldb-private.h"
+
+namespace riscv64_dwarf {
+
+enum {
+  x0 = 0,
+  x1,
+  x2,
+  x3,
+  x4,
+  x5,
+  x6,
+  x7,
+  x8,
+  x9,
+  x10,
+  x11,
+  x12,
+  x13,
+  x14,
+  x15,
+  x16,
+  x17,
+  x18,
+  x19,
+  x20,
+  x21,
+  x22,
+  x23,
+  x24,
+  x25,
+  x26,
+  x27,
+  x28,
+  x29,
+  x30,
+  x31,
+  ra = x1,
+  sp = x2,
+  fp = x8,
+};
+
+} // namespace riscv64_dwarf
+
+#endif // LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -0,0 +1,156 @@
+//===-- RegisterInfos_riscv64.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifdef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
+
+#include 
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private.h"
+
+#include "Utility/RISCV64_DWARF_Registers.h"
+
+#ifndef GPR_OFFSET
+#error GPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef GPR_OFFSET_NAME
+#error GPR_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef FPU_OFFSET
+#error FPU_OFFSET must be defined before including this header file
+#endif
+
+#ifndef FPU_OFFSET_NAME
+#error FPU_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef DBG_OFFSET_NAME
+#error DBG_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef DEFINE_DBG
+#error DEFINE_DBG must be defined before including this header file
+#endif
+
+// Offsets for a little-endian layout of the register context
+#define GPR_W_PSEUDO_REG_ENDIAN_OFFSET 0
+#define FPU_S_PSEUDO_REG_ENDIAN_OFFSET 0
+#define FPU_D_PSEUDO_REG_ENDIAN_OFFSET 0
+
+enum {
+  gpr_x0 = 0,
+  gpr_x1,
+  gpr_x2,
+  gpr_x3,
+  gpr_x4,
+  gpr_x5,
+  gpr_x6,
+  gpr_x7,
+  gpr_x8,
+  gpr_x9,
+  gpr_x10,
+  gpr_x11,
+  gpr_x12,
+  gpr_x13,
+  gpr_x14,
+  gpr_x15,
+  gpr_x16,
+  gpr_x17,
+  gpr_x18,
+  gpr_x19,
+  gpr_x20,
+  gpr_x21,
+  gpr_x22,
+  gpr_x23,
+  gpr_x24,
+  gpr_x25,
+  gpr_x26,
+  gpr_x27,
+  gpr_x28,
+  gpr_x29,
+  gpr_x30,
+  gpr_x31,
+  gpr_ra = gpr_x1,
+  gpr_sp = gpr_x2,
+  gpr_fp = gpr_x8,
+
+  k_num_registers
+};
+
+// Generates register kinds 

[Lldb-commits] [PATCH] D128166: ManagedStatic: Destroy from destructor

[Nicolai Hähnle via Phabricator via lldb-commits]

nhaehnle created this revision.
Herald added a reviewer: deadalnix.
Herald added subscribers: bzcheeseman, ayermolo, sdasgup3, wenzhicui, wrengr, 
dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, 
grosul1, jvesely, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, mstorsjo, hiraditya, mgorny.
Herald added a reviewer: bollu.
Herald added a reviewer: MaskRay.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, yota9, StephenFan, 
stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, LLDB, MLIR, LLVM.

This allows LLVM to be safely loaded and unloaded as a shared library
without leaking memory:

Consider the scenario where shared library Foo links against LLVM as
a shared library, and Foo may be loaded and unloaded multiple times
in the same process. Should Foo call llvm_shutdown or not? If it does
not, and LLVM is also loaded and unloaded multiple times, then
memory is leaked. If it does call llvm_shutdown and LLVM *isn't*
also unloaded, the state of LLVM's global statics is corrupted because
while the ManagedStatics may be re-initialized, *other* global
constructors aren't re-run.

Keep in mind that Foo itself may use ManagedStatic. If Foo is unloaded
but LLVM isn't, those statics should be destroyed.

The safe solution is to skip llvm_shutdown() altogether and destroy
managed statics from their destructor.

LLD relies on being able to call _exit() and still get some side-effects
of managed static destructors. We make this expectation much more
explicit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128166

Files:
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/merge-fdata/merge-fdata.cpp
  clang/include/clang/Frontend/CompilerInstance.h
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp
  libclc/utils/prepare-builtins.cpp
  lld/Common/ErrorHandler.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/LogTest.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/docs/ProgrammersManual.rst
  llvm/examples/BrainF/BrainFDriver.cpp
  llvm/examples/HowToUseJIT/HowToUseJIT.cpp
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/ADT/Statistic.h
  llvm/include/llvm/PassRegistry.h
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/include/llvm/Support/FastShutdown.h
  llvm/include/llvm/Support/InitLLVM.h
  llvm/include/llvm/Support/ManagedStatic.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Pass.cpp
  llvm/lib/IR/PassRegistry.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/FastShutdown.cpp
  llvm/lib/Support/InitLLVM.cpp
  llvm/lib/Support/ManagedStatic.cpp
  llvm/lib/Support/Parallel.cpp
  llvm/lib/Support/Statistic.cpp
  llvm/lib/Support/Unix/DynamicLibrary.inc
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/DynamicLibrary.inc
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
  llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
  llvm/utils/KillTheDoctor/KillTheDoctor.cpp
  mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -587,7 +587,6 @@
 
 	delete sema;
 	delete Clang;
-	llvm::llvm_shutdown();
 
 	if (Diags.hasErrorOccurred())
 		return EXIT_FAILURE;
Index: mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
===
--- mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
+++ mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
@@ -61,7 +61,6 @@
 }
 
 int main(int argc, char **argv) {
-  llvm::llvm_shutdown_obj x;
   registerPassManagerCLOptions();
 
   llvm::InitLLVM y(argc, argv);
Index: llvm/utils/KillTheDoctor/KillTheDoctor.cpp
===
--- llvm/utils/KillTheDoctor/KillTheDoctor.cpp
+++ llvm/utils/KillTheDoctor/KillTheDoctor.cpp
@@ -297,7 +297,6 @@
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
-  llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   ToolName = argv[0];
 
Index: llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
===
--- llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ 

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster added a comment.

In D127741#3583559 , @DavidSpickett 
wrote:

> 



> I agree that objdump should have some kind of "max" setting, even a default. 
> However there will still need to be a way to test what I referred to.

As you said, ` instruction requires bla` is part of assembler, not 
disassembler, so it is not affected by the patch. The problem here is that 
tests do `CHECK-UNKNOWN: 83 a9 91 c0 ` which basically checks that 
objdump cannot disassemble instruction that was assembled in the very same 
test. If we change that, we can make `+all` default attribute for objdump and 
it will disassemble everything the same way GNU objdump and lldb do.

> What might be possible is to write the testing such that it doesn't use 
> llvm-objdump. I haven't seen the structure of the tests perhaps this would 
> mean duplicating tests all over the place.

Why not use objdump? If we only check that instructions get disassembled, it 
will work fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Stephen Hines via Phabricator via lldb-commits]

srhines added a comment.

We had some internal Google folks hit this exact issue recently. I don't think 
that the same "default" CPU should be used for debugging tools like 
`llvm-objdump`, and that is the crux of the matter here. Perhaps updating the 
test to specify "generic" as the CPU when passed to `llvm-objdump` for those 
tests is the right pattern? Then the default for disassembly should be to have 
all attributes/features enabled unless the developer specifies on the command 
line that they want a different target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Why not use objdump? If we only check that instructions get disassembled, it 
> will work fine.

I just remember some tests using llvm-mc to disassemble and some using objdump. 
Istr that using llvm-mc for assemble and disassemble usually meant having two 
test files instead of one. Anyway, not important.

objdump with a generic cpu or with the all feature removed sgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster added a comment.

For now i'd like to hear more about possible clashing instruction encodings 
from @labrinea, and if it's not a concern, i think we can fix the tests if we 
remove `CHECK-UNKNOWN:` and only call objdump with `CHECK-INST`. What do you 
think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Now I think about it, tests could do something like "-mattr=-all" to remove it, 
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

I like the idea of `all` for llvm-objdump, but it should be a separate change, 
with `-mattr=-all` tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added subscribers: labrinea, DavidSpickett.
DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.
Herald added a subscriber: Michael137.

> conflicting instructions with the same encoding in different extensions, but 
> it doesn't seem possible within one archeticture

I seem to remember some system register names changing between architecture 
*profiles* not version. @labrinea may know more about that.

> Well, it broke a lot of tests that were checking that instructions are not 
> disassembled without explicitly specified extensions, but i don't quite get 
> the point of such tests.

Without looking at them in detail I assume they are checking that the proper 
extension name is printed when it says "requires bla" and that each instruction 
is properly tied to the correct feature name. This is for when you are 
assembling as opposed to disassembling.

I agree that objdump should have some kind of "max" setting, even a default. 
However there will still need to be a way to test what I referred to.

What might be possible is to write the testing such that it doesn't use 
llvm-objdump. I haven't seen the structure of the tests perhaps this would mean 
duplicating tests all over the place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the `LLVM.org` URLs

[Shao-Ce SUN via Phabricator via lldb-commits]

sunshaoce abandoned this revision.
sunshaoce added a comment.

Thanks for comments!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the `LLVM.org` URLs

[Jonas Devlieghere via Phabricator via lldb-commits]

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

+1 on breaking this down per sub-project and creating separate patches for 
review.

I sampled the patch and about a quarter or so of the links I tried didn't work, 
even before the change. There's no point in generating churn by changing those, 
but there's also little value in keeping them around, and fixing them is beyond 
the scope of this patch. I'm inclined to say let's leave them be, but I'm 
curious to hear what others think about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster added a comment.

Well, it broke a lot of tests that very explicitly checking that instructions 
are not disassembled without explicitly specified extensions, but i don't quite 
get the point of such tests. If new instructions were added by default in lldb, 
why can't we do the same with objdump?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster created this revision.
treapster added a reviewer: rafaelauler.
Herald added subscribers: jsji, ayermolo, pengfei, rupprecht, hiraditya, 
kristof.beyls.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
treapster requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, yota9, StephenFan.
Herald added projects: LLDB, LLVM.

It think it's reasonable to enable all supported extensions when we are 
disassembling something because we don't want to 
[hardcode](hhtps://reviews.llvm.org/D121999) it in gdb, objdump, bolt and 
whatever other places use disassembler in llvm. The only reason not to do it I 
can think of is if there are conflicting instructions with the same encoding in 
different extensions, but it doesn't seem possible within one archeticture. We 
probably should use +all on x86 and other platforms? I'm not sure how test for 
this should look like, because disassembler is used by at least three tools I 
know of(gdb, objdump, BOLT), so it's not clear where should we put it, and how 
to make sure that all new instructions are added to it. Waiting for your 
feedback.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127741

Files:
  bolt/lib/Core/BinaryContext.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/lib/MC/MCSubtargetInfo.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp


Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1693,6 +1693,10 @@
   if (!MAttrs.empty())
 for (unsigned I = 0; I != MAttrs.size(); ++I)
   Features.AddFeature(MAttrs[I]);
+  else {
+if (Obj->getArch() == llvm::Triple::aarch64)
+  Features.AddFeature("+all");
+  }
 
   std::unique_ptr MRI(
   TheTarget->createMCRegInfo(TripleName));
Index: llvm/lib/MC/MCSubtargetInfo.cpp
===
--- llvm/lib/MC/MCSubtargetInfo.cpp
+++ llvm/lib/MC/MCSubtargetInfo.cpp
@@ -198,6 +198,8 @@
   Help(ProcDesc, ProcFeatures);
 else if (Feature == "+cpuhelp")
   cpuHelp(ProcDesc);
+else if (Feature == "+all")
+  Bits.set();
 else
   ApplyFeatureFlag(Bits, Feature, ProcFeatures);
   }
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1179,16 +1179,9 @@
   features_str += "+dspr2,";
   }
 
-  // If any AArch64 variant, enable latest ISA with all extensions.
+  // If any AArch64 variant, enable all features.
   if (triple.isAArch64()) {
-features_str += "+v9.3a,";
-std::vector features;
-// Get all possible features
-llvm::AArch64::getExtensionFeatures(-1, features);
-features_str += llvm::join(features, ",");
-
-if (triple.getVendor() == llvm::Triple::Apple)
-  cpu = "apple-latest";
+features_str += "+all";
   }
 
   if (triple.isRISCV()) {
Index: bolt/lib/Core/BinaryContext.cpp
===
--- bolt/lib/Core/BinaryContext.cpp
+++ bolt/lib/Core/BinaryContext.cpp
@@ -124,8 +124,7 @@
 break;
   case llvm::Triple::aarch64:
 ArchName = "aarch64";
-FeaturesStr = "+fp-armv8,+neon,+crypto,+dotprod,+crc,+lse,+ras,+rdm,"
-  "+fullfp16,+spe,+fuse-aes,+rcpc";
+FeaturesStr = "+all";
 break;
   default:
 return createStringError(std::errc::not_supported,


Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1693,6 +1693,10 @@
   if (!MAttrs.empty())
 for (unsigned I = 0; I != MAttrs.size(); ++I)
   Features.AddFeature(MAttrs[I]);
+  else {
+if (Obj->getArch() == llvm::Triple::aarch64)
+  Features.AddFeature("+all");
+  }
 
   std::unique_ptr MRI(
   TheTarget->createMCRegInfo(TripleName));
Index: llvm/lib/MC/MCSubtargetInfo.cpp
===
--- llvm/lib/MC/MCSubtargetInfo.cpp
+++ llvm/lib/MC/MCSubtargetInfo.cpp
@@ -198,6 +198,8 @@
   Help(ProcDesc, ProcFeatures);
 else if (Feature == "+cpuhelp")
   cpuHelp(ProcDesc);
+else if (Feature == "+all")
+  Bits.set();
 else
   ApplyFeatureFlag(Bits, Feature, ProcFeatures);
   }
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ 

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls

[Eli Friedman via Phabricator via lldb-commits]

efriedma added a comment.

A couple notes skimming the changes:

1. If you're going to update URLs, please verify the new URLs are actually 
valid.
2. Some of the "URLs" you're updating aren't actually URLs. For example, the 
`xmlns` attribute in SVG files must be the exact string written in the 
standard; changing it could make the file unreadable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls

[Nikolas Klauser via Phabricator via lldb-commits]

philnik added a comment.

Could you do this on a per-project basis? More than 1000 files is way to much 
to sift through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls

[Shao-Ce SUN via Phabricator via lldb-commits]

sunshaoce created this revision.
Herald added subscribers: libc-commits, libcxx-commits, jsji, Enna1, 
bzcheeseman, kosarev, jsilvanus, mattd, gchakrabarti, hsmhsm, ThomasRaoux, 
pmatos, asb, ayermolo, awarzynski, sdasgup3, asavonic, carlosgalvezp, 
jeroen.dobbelaere, abrachet, wenzhicui, wrengr, Chia-hungDuan, lebedev.ri, 
ormris, foad, smeenai, dcaballe, cota, mravishankar, teijeong, frasercrmck, 
rdzhabarov, tatianashp, wenlei, okura, jdoerfert, msifontes, jurahul, kuter, 
Kayjukh, grosul1, jvesely, martong, Joonsoo, phosek, kerbowa, liufengdb, 
aartbik, mgester, arpith-jacob, csigg, antiagainst, shauheen, rriddle, 
mehdi_amini, luismarques, apazos, sameer.abuasal, usaxena95, pengfei, 
s.egerton, dmgreen, Jim, asbirlea, mstorsjo, kadircet, jocewei, rupprecht, 
PkmX, arphaman, the_o, brucehoult, MartinMosbeck, rogfer01, steven_wu, 
atanasyan, edward-jones, george.burgess.iv, zzheng, jrtc27, delcypher, niosHD, 
sabuasal, simoncook, johnrusso, rbar, fedor.sergeev, kbarton, hiraditya, 
jgravelle-google, whisperity, arichardson, sbc100, mgorny, nhaehnle, nemanjai, 
sdardis, emaste, dylanmckay, jyknight, dschuff, arsenm.
Herald added a reviewer: deadalnix.
Herald added a reviewer: bollu.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: ldionne.
Herald added a reviewer: jhenderson.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: sscalpone.
Herald added a reviewer: ftynse.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: aartbik.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: libunwind, libc-project, Flang, All.
Herald added a reviewer: libunwind.
sunshaoce requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, openmp-commits, 
lldb-commits, Sanitizers, pcwang-thead, yota9, StephenFan, sstefan1, 
stephenneuendorffer, nicolasvasilache, aheejin, jholewinski.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: nicolasvasilache.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
MLIR, LLVM, clang-tools-extra.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127684

Files:
  .github/PULL_REQUEST_TEMPLATE.md
  .github/workflows/repo-lockdown.yml
  .gitignore
  README.md
  bolt/docs/CMakeLists.txt
  bolt/docs/OptimizingClang.md
  bolt/docs/doxygen.cfg.in
  clang-tools-extra/.gitignore
  clang-tools-extra/README.txt
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/assets/clang-doc-default-stylesheet.css
  
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.py
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationAdditionCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationComparisonCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationConversionCastCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationSubtractionCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
  clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
  clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.h
  clang-tools-extra/clang-tidy/abseil/NoNamespaceCheck.h
  clang-tools-extra/clang-tidy/abseil/RedundantStrcatCallsCheck.h
  clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.h
  clang-tools-extra/clang-tidy/abseil/TimeComparisonCheck.h
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.h
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
  clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
  clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
  clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
  clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.h
  clang-tools-extra/clang-tidy/android/CloexecAccept4Check.h
  clang-tools-extra/clang-tidy/android/CloexecAcceptCheck.h
  clang-tools-extra/clang-tidy/android/CloexecCreatCheck.h
  

[Lldb-commits] [PATCH] D127224: [flang][OpenMP] Lowering support for atomic capture

[Nimish Mishra via Phabricator via lldb-commits]

NimishMishra updated this revision to Diff 435060.
NimishMishra added a comment.
Herald added subscribers: cfe-commits, llvm-commits, libc-commits, 
libcxx-commits, openmp-commits, lldb-commits, Sanitizers, jsji, Enna1, kosarev, 
mravishankar, jsilvanus, mattd, gchakrabarti, hsmhsm, pmatos, asb, 
pcwang-thead, yota9, ayermolo, awarzynski, arjunp, luke957, asavonic, 
carlosgalvezp, abrachet, Groverkss, armkevincheng, ormris, foad, jsmolens, 
eric-k256, mstorsjo, frasercrmck, ecnelises, ThomasRaoux, vkmr, martong, 
phosek, kerbowa, csigg, luismarques, apazos, sameer.abuasal, pengfei, 
s.egerton, dmgreen, Jim, jocewei, PkmX, arphaman, the_o, brucehoult, 
MartinMosbeck, rogfer01, steven_wu, atanasyan, mgrang, edward-jones, zzheng, 
MaskRay, jrtc27, gbedwell, niosHD, cryptoad, sabuasal, simoncook, johnrusso, 
rbar, fedor.sergeev, kbarton, aheejin, hiraditya, jgravelle-google, 
arichardson, sbc100, mgorny, nhaehnle, jvesely, nemanjai, sdardis, dylanmckay, 
jyknight, dschuff, arsenm, jholewinski.
Herald added a reviewer: bollu.
Herald added a reviewer: andreadb.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: antiagainst.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: aartbik.
Herald added a reviewer: ftynse.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: sjarus.
Herald added a reviewer: zuban32.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: clang, Sanitizers, LLDB, libc++, libc-project, 
libc++abi, libunwind, LLVM, lld-macho, clang-tools-extra.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: libunwind.
Herald added a reviewer: lld-macho.

[flang][OpenMP] Added lowering support for atomic capture


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127224

Files:
  bolt/lib/Rewrite/DWARFRewriter.cpp
  bolt/lib/Rewrite/MachORewriteInstance.cpp
  bolt/lib/Rewrite/RewriteInstance.cpp
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/pseudo/lib/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
  clang-tools-extra/pseudo/test/cxx/keyword.cpp
  clang-tools-extra/pseudo/test/cxx/parameter-decl-clause.cpp
  clang-tools-extra/pseudo/test/cxx/predefined-identifier.cpp
  clang-tools-extra/pseudo/test/cxx/template-empty-type-parameter.cpp
  clang-tools-extra/pseudo/test/cxx/unsized-array.cpp
  clang-tools-extra/pseudo/test/glr.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Headers/unwind.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CodeGen/builtins-memcpy-inline.c
  clang/test/CodeGenCXX/externc-used-not-replaced.cpp
  clang/test/Driver/cl-zc.cpp
  clang/test/Driver/sanitizer-ld.c
  clang/test/Driver/windows-exceptions.cpp
  clang/test/Modules/Inputs/gmodules-deduction-guide.h
  clang/test/Modules/gmodules-deduction-guide.cpp
  clang/test/Sema/aarch64-sve-vector-arith-ops.c
  clang/test/Sema/aarch64-sve-vector-bitwise-ops.c
  clang/test/Sema/aarch64-sve-vector-compare-ops.c
  clang/test/Sema/builtins-memcpy-inline.cpp
  clang/test/SemaCXX/aarch64-sve-vector-conditional-op.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/www/c_status.html
  compiler-rt/lib/asan/asan_allocator.h
  compiler-rt/lib/asan/asan_errors.cpp

[Lldb-commits] [PATCH] D125860: [clang] Only use major version in resource dir

[Timm Bäder via Phabricator via lldb-commits]

tbaeder updated this revision to Diff 435876.

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

https://reviews.llvm.org/D125860

Files:
  clang/include/clang/Basic/Version.inc.in
  clang/lib/Driver/Driver.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/runtime/src/CMakeLists.txt
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -109,7 +109,7 @@
 	llvm::cl::value_desc("name"));
 
 static const char *ResourceDir =
-	CLANG_PREFIX "/lib/clang/" CLANG_VERSION_STRING;
+CLANG_PREFIX "/lib/clang/" CLANG_VERSION_MAJOR_STRING;
 
 /* Does decl have an attribute of the following form?
  *
Index: openmp/runtime/src/CMakeLists.txt
===
--- openmp/runtime/src/CMakeLists.txt
+++ openmp/runtime/src/CMakeLists.txt
@@ -352,8 +352,8 @@
 if(${OPENMP_STANDALONE_BUILD})
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION})
+  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION_MAJOR}/include")
 endif()
 if(WIN32)
   install(TARGETS omp RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -255,9 +255,9 @@
 set(llvm_config_path ${LLVM_CONFIG_PATH})
 
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION}")
+  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -37,10 +37,12 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir = "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir = "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
 #endif
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -55,7 +55,7 @@
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
   // in $install_dir/lib{,64}/clang/$clang_version.
-  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_LIBDIR_SUFFIX.
Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -38,14 +38,14 @@
 
 if (LLVM_TREE_AVAILABLE)
   # Compute the Clang version from the LLVM version.
-  # FIXME: We should be able to reuse CLANG_VERSION variable calculated
+  # FIXME: We should be able to reuse CLANG_VERSION_MAJOR variable calculated
   #in Clang cmake files, instead of copying the rules here.
-  string(REGEX MATCH 

[Lldb-commits] [PATCH] D125860: [clang] Only use major version in resource dir

[Timm Bäder via Phabricator via lldb-commits]

tbaeder updated this revision to Diff 435850.
Herald added a subscriber: Enna1.

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

https://reviews.llvm.org/D125860

Files:
  clang/include/clang/Basic/Version.inc.in
  clang/lib/Driver/Driver.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/runtime/src/CMakeLists.txt
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -109,7 +109,7 @@
 	llvm::cl::value_desc("name"));
 
 static const char *ResourceDir =
-	CLANG_PREFIX "/lib/clang/" CLANG_VERSION_STRING;
+CLANG_PREFIX "/lib/clang/" CLANG_VERSION_MAJOR_STRING;
 
 /* Does decl have an attribute of the following form?
  *
Index: openmp/runtime/src/CMakeLists.txt
===
--- openmp/runtime/src/CMakeLists.txt
+++ openmp/runtime/src/CMakeLists.txt
@@ -352,8 +352,8 @@
 if(${OPENMP_STANDALONE_BUILD})
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION})
+  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION_MAJOR}/include")
 endif()
 if(WIN32)
   install(TARGETS omp RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -255,9 +255,9 @@
 set(llvm_config_path ${LLVM_CONFIG_PATH})
 
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION}")
+  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -37,10 +37,12 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir = "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir = "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
 #endif
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -55,7 +55,7 @@
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
   // in $install_dir/lib{,64}/clang/$clang_version.
-  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_LIBDIR_SUFFIX.
Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -38,14 +38,16 @@
 
 if (LLVM_TREE_AVAILABLE)
   # Compute the Clang version from the LLVM version.
-  # FIXME: We should be able to reuse CLANG_VERSION variable calculated
+  # FIXME: We should be able to reuse CLANG_VERSION_MAJOR variable calculated
   #in Clang cmake files, instead of copying the rules 

[Lldb-commits] [PATCH] D121876: [BOLT][DWARF] Implement monolithic DWARF5

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added a comment.

In D121876#3542766 , @Amir wrote:

> Hi,
> The test started to fail on Ubuntu buildbot: 
> https://lab.llvm.org/buildbot/#/builders/215/builds/6376
>
>   # PRECHECK: include_directories[ 0] = .debug_line_str[0x]
>   ^
>   :8:12: note: scanning from here
>version: 5
>  ^
>   :30:1: note: possible intended match here
>   include_directories[ 0] = .debug_line_str[0x0009] = "."
>
> Can you please take a look?

Fixed in: D126733 . Was due to LLD change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121876

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

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

[Luís Marques via Phabricator via lldb-commits]

luismarques added a comment.

In D62732#3157524 , @kasper81 wrote:

> Hi Luis, are you planning on adding plugin architecture support (in 
> `lldb/source/Plugins/Architecture`) as part of this work?

I commandeered this patch from Simon Cook, the original author, but I haven't 
been a good steward. For a while I seemed to always have something higher 
priority in my queue and eventually I kinda stopped thinking about it. So don't 
rely on my plans for this patch. Feel free to adopt it and continue this line 
of work. My apologies to the community.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D121876: [BOLT][DWARF] Implement monolithic DWARF5

[Amir Ayupov via Phabricator via lldb-commits]

Amir added a comment.

Hi,
The test started to fail on Ubuntu buildbot: 
https://lab.llvm.org/buildbot/#/builders/215/builds/6376

  # PRECHECK: include_directories[ 0] = .debug_line_str[0x]
  ^
  :8:12: note: scanning from here
   version: 5
 ^
  :30:1: note: possible intended match here
  include_directories[ 0] = .debug_line_str[0x0009] = "."

Can you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121876

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

[Lldb-commits] [PATCH] D125860: [clang] Only use major version in resource dir

[Timm Bäder via Phabricator via lldb-commits]

tbaeder created this revision.
tbaeder added a reviewer: tstellar.
Herald added a subscriber: mgorny.
Herald added a reviewer: bollu.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, lldb-commits, 
Sanitizers, cfe-commits, MaskRay.
Herald added projects: clang, Sanitizers, LLDB, OpenMP, LLVM.

As discussed in 
https://discourse.llvm.org/t/should-we-continue-embed-the-full-llvm-version-in-lib-clang/62094/8

Release note update still missing of course.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125860

Files:
  clang/include/clang/Basic/Version.inc.in
  clang/lib/Driver/Driver.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/runtime/src/CMakeLists.txt
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -109,7 +109,7 @@
 	llvm::cl::value_desc("name"));
 
 static const char *ResourceDir =
-	CLANG_PREFIX "/lib/clang/" CLANG_VERSION_STRING;
+CLANG_PREFIX "/lib/clang/" CLANG_VERSION_MAJOR_STRING;
 
 /* Does decl have an attribute of the following form?
  *
Index: openmp/runtime/src/CMakeLists.txt
===
--- openmp/runtime/src/CMakeLists.txt
+++ openmp/runtime/src/CMakeLists.txt
@@ -353,7 +353,7 @@
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
   string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION_MAJOR}/include")
 endif()
 if(WIN32)
   install(TARGETS omp RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -259,7 +259,7 @@
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION}")
+  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -37,10 +37,12 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir = "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir = "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
 #endif
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -54,8 +54,8 @@
 
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
-  // in $install_dir/lib{,64}/clang/$clang_version.
-  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  // in $install_dir/lib{,64}/clang/$clang_major_version.
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_LIBDIR_SUFFIX.
Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -43,9 +43,9 @@
   string(REGEX 

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Maksim Panchenko via Phabricator via lldb-commits]

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

Awesome!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

Mordante wrote:
> ldionne wrote:
> > philnik wrote:
> > > labath wrote:
> > > > philnik wrote:
> > > > > dblaikie wrote:
> > > > > > philnik wrote:
> > > > > > > jgorbe wrote:
> > > > > > > > Mordante wrote:
> > > > > > > > > philnik wrote:
> > > > > > > > > > Mordante wrote:
> > > > > > > > > > > philnik wrote:
> > > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > > > Probably. Would be nice to have a test runner for that.
> > > > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > > > Do you know where I would have to look to know what the 
> > > > > > > > > > LLDB pretty printers do?
> > > > > > > > > Unfortunately no. @jingham seems to be the Data formatter 
> > > > > > > > > code owner.
> > > > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > > > similar change to string: 
> > > > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > > > 
> > > > > > > > If the gdb prettyprinter needed fixing for this change, chances 
> > > > > > > > are that lldb will need a similar update too.
> > > > > > > Could someone from #lldb help me figure out what to change in the 
> > > > > > > pretty printers? I looked at the file, but I don't really 
> > > > > > > understand how it works and TBH I don't really feel like spending 
> > > > > > > a lot of time figuring it out. If nobody says anything I'll land 
> > > > > > > this in a week.
> > > > > > > 
> > > > > > > As a side note: it would be really nice if there were a few more 
> > > > > > > comments inside `LibCxx.cpp` to explain what happens there. That 
> > > > > > > would make fixing the pretty printer a lot easier. The code is 
> > > > > > > probably not very hard (at least it doesn't look like it), but I 
> > > > > > > am looking for a few things that I can't find and I have no idea 
> > > > > > > what some of the things mean.
> > > > > > Looks like something around 
> > > > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > > > >  (& the similar masking in the `else` block a few lines down) - I 
> > > > > > guess a specific lookup for the new field would be needed, rather 
> > > > > > than the bitmasking.
> > > > > Yes, but what do the numbers in `size_mode_locations` mean? Why is 
> > > > > there no checking if it's big or little endian? Is it unsupported 
> > > > > maybe? Does it work because of something else? Is there a reason that 
> > > > > `g_data_name` exists instead of comparing directly? Should I add 
> > > > > another layout style or should I just update the code for the new 
> > > > > layout?
> > > > > I don't know anything about the LLDB codebase, so I don't understand 
> > > > > the code and I don't know how I should change it.
> > > > I don't think there's been any official policy decision either way, but 
> > > > historically we haven't been asking libc++ authors to update lldb 
> > > > pretty printers -- we would just fix them up on the lldb side when we 
> > > > noticed the change. The thing that has changed recently is that google 
> > > > started relying (and testing) more on lldb, which considerably 
> > > > shortened the time it takes to notice this change, and also makes it 
> > > > difficult for some people to make progress while we are in this state. 
> > > > But I don't think that means that updating the pretty printer is 
> > > > suddenly your responsibility.
> > > > 
> > > > As for your questions, I'll try to answer them as best as I can:
> > > > > what do the numbers in size_mode_locations mean?
> > > > These are the indexes of fields in the string object. For some reason 
> > > > (unknown to me), the pretty printer uses indexes rather than field 
> > > > names for its work. Prompted by the previous patch, I've been trying to 
> > > > change that, but I haven't done it yet, as I was trying to improve the 
> > > > testing story (more on that later).
> > > > > Why is there no checking if it's big or little endian? Is it 
> > > > > unsupported maybe?
> > > > Most likely yes. Although most parts of lldb support big endian, I am 
> > > > not aware of anyone testing it on a regular basis, so it's quite likely 
> > > > that a lot of things are in fact broken.
> > > > > Is there a reason that g_data_name exists instead of comparing 
> > > > > directly?
> > > > LLDB uses a global string pool, so this is an attempt to reduce the 
> > > > number of string pool queries. The pattern is not consistently used 
> > > > everywhere, and overall, I wouldn't be too worried about it.
> > > > > Should I add another layout style or should I just update the code 
> > > > > for the new 

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added inline comments.



Comment at: bolt/include/bolt/Core/DebugData.h:375-377
+if (Optional DWOId = Unit.getDWOId())
+  return *DWOId;
+return Unit.getOffset();

dblaikie wrote:
> ayermolo wrote:
> > ayermolo wrote:
> > > dblaikie wrote:
> > > > That seems like a somewhat problematic API - returning two very 
> > > > different kinds of data (the DWO_ID or the unit offset) seems quite 
> > > > easy to misuse this?
> > > The idea I had behind this APIS is for it to return unique ID 
> > > representing the CU. As it applies to .debug_addr. For monolithic case is 
> > > its offset with .debug_info for Split Dwarf case is its DWO ID. At least 
> > > in my head viewed in that context the return data is the same. It's just 
> > > something that uniquely identifies this CU, and logic is encapsulated in 
> > > it.
> > > 
> > @dblaikie What do you think?
> Could you use the offset consistently? The debug_addr section is always in 
> the executable - the offset of the skeleton CU would be unique?
In theory yes, if I always use offset of the CU/Skeleton CU in the main binary. 
For that I still need to do a check of CU that is passed in is DWO CU, get it 
skeleton CU, then get it's offset. At the end what we get is offset all the 
time.  Is it really better? Sure we always getting an offset, but intent of 
this API is not really get an offset but the unique ID of the CU. Are you 
concern that offset and DWO ID will collide?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Mark de Wever via Phabricator via lldb-commits]

Mordante added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

labath wrote:
> Mordante wrote:
> > ldionne wrote:
> > > philnik wrote:
> > > > labath wrote:
> > > > > philnik wrote:
> > > > > > dblaikie wrote:
> > > > > > > philnik wrote:
> > > > > > > > jgorbe wrote:
> > > > > > > > > Mordante wrote:
> > > > > > > > > > philnik wrote:
> > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > philnik wrote:
> > > > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > > > > Probably. Would be nice to have a test runner for 
> > > > > > > > > > > > > that.
> > > > > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > > > > Do you know where I would have to look to know what the 
> > > > > > > > > > > LLDB pretty printers do?
> > > > > > > > > > Unfortunately no. @jingham seems to be the Data formatter 
> > > > > > > > > > code owner.
> > > > > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > > > > similar change to string: 
> > > > > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > > > > 
> > > > > > > > > If the gdb prettyprinter needed fixing for this change, 
> > > > > > > > > chances are that lldb will need a similar update too.
> > > > > > > > Could someone from #lldb help me figure out what to change in 
> > > > > > > > the pretty printers? I looked at the file, but I don't really 
> > > > > > > > understand how it works and TBH I don't really feel like 
> > > > > > > > spending a lot of time figuring it out. If nobody says anything 
> > > > > > > > I'll land this in a week.
> > > > > > > > 
> > > > > > > > As a side note: it would be really nice if there were a few 
> > > > > > > > more comments inside `LibCxx.cpp` to explain what happens 
> > > > > > > > there. That would make fixing the pretty printer a lot easier. 
> > > > > > > > The code is probably not very hard (at least it doesn't look 
> > > > > > > > like it), but I am looking for a few things that I can't find 
> > > > > > > > and I have no idea what some of the things mean.
> > > > > > > Looks like something around 
> > > > > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > > > > >  (& the similar masking in the `else` block a few lines down) - I 
> > > > > > > guess a specific lookup for the new field would be needed, rather 
> > > > > > > than the bitmasking.
> > > > > > Yes, but what do the numbers in `size_mode_locations` mean? Why is 
> > > > > > there no checking if it's big or little endian? Is it unsupported 
> > > > > > maybe? Does it work because of something else? Is there a reason 
> > > > > > that `g_data_name` exists instead of comparing directly? Should I 
> > > > > > add another layout style or should I just update the code for the 
> > > > > > new layout?
> > > > > > I don't know anything about the LLDB codebase, so I don't 
> > > > > > understand the code and I don't know how I should change it.
> > > > > I don't think there's been any official policy decision either way, 
> > > > > but historically we haven't been asking libc++ authors to update lldb 
> > > > > pretty printers -- we would just fix them up on the lldb side when we 
> > > > > noticed the change. The thing that has changed recently is that 
> > > > > google started relying (and testing) more on lldb, which considerably 
> > > > > shortened the time it takes to notice this change, and also makes it 
> > > > > difficult for some people to make progress while we are in this 
> > > > > state. But I don't think that means that updating the pretty printer 
> > > > > is suddenly your responsibility.
> > > > > 
> > > > > As for your questions, I'll try to answer them as best as I can:
> > > > > > what do the numbers in size_mode_locations mean?
> > > > > These are the indexes of fields in the string object. For some reason 
> > > > > (unknown to me), the pretty printer uses indexes rather than field 
> > > > > names for its work. Prompted by the previous patch, I've been trying 
> > > > > to change that, but I haven't done it yet, as I was trying to improve 
> > > > > the testing story (more on that later).
> > > > > > Why is there no checking if it's big or little endian? Is it 
> > > > > > unsupported maybe?
> > > > > Most likely yes. Although most parts of lldb support big endian, I am 
> > > > > not aware of anyone testing it on a regular basis, so it's quite 
> > > > > likely that a lot of things are in fact broken.
> > > > > > Is there a reason that g_data_name exists instead of comparing 
> > > > > > directly?
> > > > > LLDB uses a global string pool, so this is an attempt to reduce the 
> > > > > number of string pool queries. The pattern 

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added inline comments.



Comment at: bolt/include/bolt/Core/DebugData.h:375-377
+if (Optional DWOId = Unit.getDWOId())
+  return *DWOId;
+return Unit.getOffset();

ayermolo wrote:
> dblaikie wrote:
> > That seems like a somewhat problematic API - returning two very different 
> > kinds of data (the DWO_ID or the unit offset) seems quite easy to misuse 
> > this?
> The idea I had behind this APIS is for it to return unique ID representing 
> the CU. As it applies to .debug_addr. For monolithic case is its offset with 
> .debug_info for Split Dwarf case is its DWO ID. At least in my head viewed in 
> that context the return data is the same. It's just something that uniquely 
> identifies this CU, and logic is encapsulated in it.
> 
@dblaikie What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added inline comments.



Comment at: bolt/include/bolt/Core/DebugData.h:375-377
+if (Optional DWOId = Unit.getDWOId())
+  return *DWOId;
+return Unit.getOffset();

dblaikie wrote:
> That seems like a somewhat problematic API - returning two very different 
> kinds of data (the DWO_ID or the unit offset) seems quite easy to misuse this?
The idea I had behind this APIS is for it to return unique ID representing the 
CU. As it applies to .debug_addr. For monolithic case is its offset with 
.debug_info for Split Dwarf case is its DWO ID. At least in my head viewed in 
that context the return data is the same. It's just something that uniquely 
identifies this CU, and logic is encapsulated in it.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Mark de Wever via Phabricator via lldb-commits]

Mordante added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

ldionne wrote:
> philnik wrote:
> > labath wrote:
> > > philnik wrote:
> > > > dblaikie wrote:
> > > > > philnik wrote:
> > > > > > jgorbe wrote:
> > > > > > > Mordante wrote:
> > > > > > > > philnik wrote:
> > > > > > > > > Mordante wrote:
> > > > > > > > > > philnik wrote:
> > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > > Probably. Would be nice to have a test runner for that.
> > > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > > Do you know where I would have to look to know what the LLDB 
> > > > > > > > > pretty printers do?
> > > > > > > > Unfortunately no. @jingham seems to be the Data formatter code 
> > > > > > > > owner.
> > > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > > similar change to string: 
> > > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > > 
> > > > > > > If the gdb prettyprinter needed fixing for this change, chances 
> > > > > > > are that lldb will need a similar update too.
> > > > > > Could someone from #lldb help me figure out what to change in the 
> > > > > > pretty printers? I looked at the file, but I don't really 
> > > > > > understand how it works and TBH I don't really feel like spending a 
> > > > > > lot of time figuring it out. If nobody says anything I'll land this 
> > > > > > in a week.
> > > > > > 
> > > > > > As a side note: it would be really nice if there were a few more 
> > > > > > comments inside `LibCxx.cpp` to explain what happens there. That 
> > > > > > would make fixing the pretty printer a lot easier. The code is 
> > > > > > probably not very hard (at least it doesn't look like it), but I am 
> > > > > > looking for a few things that I can't find and I have no idea what 
> > > > > > some of the things mean.
> > > > > Looks like something around 
> > > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > > >  (& the similar masking in the `else` block a few lines down) - I 
> > > > > guess a specific lookup for the new field would be needed, rather 
> > > > > than the bitmasking.
> > > > Yes, but what do the numbers in `size_mode_locations` mean? Why is 
> > > > there no checking if it's big or little endian? Is it unsupported 
> > > > maybe? Does it work because of something else? Is there a reason that 
> > > > `g_data_name` exists instead of comparing directly? Should I add 
> > > > another layout style or should I just update the code for the new 
> > > > layout?
> > > > I don't know anything about the LLDB codebase, so I don't understand 
> > > > the code and I don't know how I should change it.
> > > I don't think there's been any official policy decision either way, but 
> > > historically we haven't been asking libc++ authors to update lldb pretty 
> > > printers -- we would just fix them up on the lldb side when we noticed 
> > > the change. The thing that has changed recently is that google started 
> > > relying (and testing) more on lldb, which considerably shortened the time 
> > > it takes to notice this change, and also makes it difficult for some 
> > > people to make progress while we are in this state. But I don't think 
> > > that means that updating the pretty printer is suddenly your 
> > > responsibility.
> > > 
> > > As for your questions, I'll try to answer them as best as I can:
> > > > what do the numbers in size_mode_locations mean?
> > > These are the indexes of fields in the string object. For some reason 
> > > (unknown to me), the pretty printer uses indexes rather than field names 
> > > for its work. Prompted by the previous patch, I've been trying to change 
> > > that, but I haven't done it yet, as I was trying to improve the testing 
> > > story (more on that later).
> > > > Why is there no checking if it's big or little endian? Is it 
> > > > unsupported maybe?
> > > Most likely yes. Although most parts of lldb support big endian, I am not 
> > > aware of anyone testing it on a regular basis, so it's quite likely that 
> > > a lot of things are in fact broken.
> > > > Is there a reason that g_data_name exists instead of comparing directly?
> > > LLDB uses a global string pool, so this is an attempt to reduce the 
> > > number of string pool queries. The pattern is not consistently used 
> > > everywhere, and overall, I wouldn't be too worried about it.
> > > > Should I add another layout style or should I just update the code for 
> > > > the new layout?
> > > As the pretty printers ship with lldb, they are expected to support not 
> > > just the current format, but also the past ones (within reason). This is 
> > > what makes 

Re: [Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

[David M. Lary via lldb-commits]

On Fri, Jun 17, 2022 at 14:32 Jonas Devlieghere via Phabricator
 wrote:
>
> JDevlieghere added a comment.
>
> In D128069#3592733 , @dmlary wrote:
>
> > I went through the existing tests for SBSection, and there is only one test 
> > case for all the getters & props, and that is for `target_byte_size`.  
> > Based on that lack, and the simplicity of the getter, I didn't think a test 
> > case was warranted here.
>
> I'm less concerned about the getter and the property and more about the 
> underlying functionality. It doesn't look like it's being tested, and adding 
> it to the SB API allows to change that.

Just to clarify, are you asking me to add a test for the underlying
functionality, `Section.GetLog2Align()`?  Or is the goal to add the
test at the SBSection level to ensure we detect if someone ever
changes the underlying api?
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a1df636 - [lldb] [test] Skip llgs tests broken due to #56268 on aarch64

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T16:24:38+02:00
New Revision: a1df636a8b51a660d58708d82a5bc060ee5f57d3

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

LOG: [lldb] [test] Skip llgs tests broken due to #56268 on aarch64

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index d8c93b84d0ce5..aa97ccd514583 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -317,44 +317,64 @@ def resume_one_test(self, run_order, use_vCont=False):
 ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_parent(self):
 self.resume_one_test(run_order=["parent", "parent"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_child(self):
 self.resume_one_test(run_order=["child", "child"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_parent(self):
 self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_child(self):
 self.resume_one_test(run_order=["child", "child"], use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268;)
 @add_test_categories(["fork"])
 def test_vCont_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],



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

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Michael Daniels via Phabricator via lldb-commits]

mdaniels added a comment.

I do not have commit access, so I would need someone to commit for me.


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

https://reviews.llvm.org/D127999

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

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Cool, thanks. So do you have commit access, or you need someone to commit this 
for you?


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

https://reviews.llvm.org/D127999

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

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Pavel Labath via Phabricator via lldb-commits]

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

@mgorny, ping. Do you maybe want to try this out on freebsd?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128264

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

[Lldb-commits] [PATCH] D128410: [lldb] Add a testcase for nested exceptions on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D128410#3609185 , @mstorsjo wrote:

> In D128410#3608190 , @labath wrote:
>
>> In D128410#3604927 , @alvinhochun 
>> wrote:
>>
>>> It may be possible to stuff a pointer to an `EXCEPTION_RECORD` into another 
>>> `EXCEPTION_RECORD` and use `RtlRaiseException` to generate the exception, 
>>> but you'll have to test how it actually works.
>>
>> That would be pretty cool.
>
> Yeah - I guess it's two separate kinds of testcases; this one would be more 
> of a macro-testcase, "does this real-world case work - whichever way lldb 
> happens to handle it" (nested exception or not?) while that would be more of 
> a clinical unit test for specifically testing nested exceptions.

That's true. However, if I had to choose between the two, I would always go for 
the one with the fewest moving parts. Lldb has a lot of problems with 
reproducibility of tests, so I am always looking for ways to make tests more 
specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128410

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It seems like this is not actually running the code. Can we make it such that 
these tests are conditional on the appropriate llvm targets being enabled 
(instead of the host being of a specific type)?


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

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

mgorny added a comment.

Ok, it seems that LLDB is hitting some deadloop on `brk #0xf000` in code. I've 
filed https://github.com/llvm/llvm-project/issues/56268 for it and I'm going to 
skip the tests relying on traps.

I'm going to try if I manage to get 32-bit ARM container to work as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

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

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

[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 D128638#3611932 , @mgorny wrote:

> BTW this test is getting a bit long as well but I don't have a good idea how 
> to split it, except for just moving some functions into a second file and 
> making some reusable base class.

I don't know if that's necessary right now, but the general approach seems fine 
to me.


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

https://reviews.llvm.org/D128638

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

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The problem is most likely due to the arm64 version having slightly different 
line number sequences due to different instruction scheduling. You might be 
able to gain some insight just by comparing the debug line content, without 
actually running it.

It might be helpful to make the test more hermetic by removing the calls to 
printf (a library function coming from the environment).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

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

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

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

mgorny added a comment.

Thanks for letting me know. I'll set up an AArch64 development environment in a 
minute and try to figure it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

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