[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D75711#1920425 , @jingham wrote:

> Is this one okay as well?


All my concerns have been addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75711



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

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

[James Henderson via Phabricator via lldb-commits]

jhenderson added a comment.

I'm not really up to speed with the .debug_*_index sections, so I haven't 
looked really at the overall approach. I've just provided some basic stylistic 
comments.




Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:22
 
+// Pre-standard implementation of package files defined a number of section
+// identifiers with values that clash definitions in the DWARFv5 standard.

I'd guess this should probably use doxygen-style comments?

Same goes for below.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:66
+// pre-standard GNU proposal or 5 for DWARFv5 package file.
+uint32_t SerializeSectionKind(DWARFSectionKind Kind, unsigned IndexVersion);
+

lowerCamelCase for function names. Same below.



Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:116
+  // This is a parallel array of raw section IDs for columns of unknown kinds.
+  // This array is created only if there are items in columns ColumnKinds with
+  // DW_SECT_EXT_unknown and the only initialized items here are those with

"items in columns ColumnKinds" doesn't read well to me. I'm not sure if its 
missing punctuation, an extra word, or what.



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp:101
+if (Version != 5)
+  return false;
+*OffsetPtr += 2; // Skip padding.

Probably out of scope for this change, but this should return an llvm::Error 
instead to say why parsing failed.



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

also lay -> are



Comment at: llvm/test/DebugInfo/X86/dwp-v2-cu-index.s:1
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
+# RUN:   llvm-dwarfdump -debug-cu-index - | \

Might be worth a brief comment at the top of this test saying this is the 
pre-standard GCC version.



Comment at: llvm/test/DebugInfo/X86/dwp-v2-loc.s:1
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
+# RUN:   llvm-dwarfdump -debug-info -debug-loc - | \

I might have missed something, but is this relevant? I thought this patch was 
for supporting .debug_cu_index?



Comment at: llvm/test/DebugInfo/X86/dwp-v2-tu-index.s:1
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
+# RUN:   llvm-dwarfdump -debug-tu-index - | \

Add a comment again about "v2".



Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:614
+if (CUIndex.getVersion() != 2)
+  return make_error("Unsupported cu_index version");
 

I see the above error message starts with a capital letter, but more generally 
I think we try to use lower-case for error messages. Maybe worth doing it right 
here and changing the above line in a separate change?



Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:653
+  if (TUIndex.getVersion() != 2)
+return make_error("Unsupported tu_index version");
   addAllTypesFromDWP(

Same comment as above.


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

https://reviews.llvm.org/D75929



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

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

[Igor Kudrin via Phabricator via lldb-commits]

ikudrin added a comment.

In D75929#1920691 , @dblaikie wrote:

> > This is almost what you are doing right now -- the only difference is that 
> > the "internal" enum would no longer be internal -- it would actually match 
> > the on-disk format of a v5 index. This v5 enum would contain the official 
> > DWARFv5 constants as well as the new extensions we want to introduce for 
> > mixed 5+4 indices.
>
> Yep, this would be the direction I would suggest/encourage. It seems that if 
> the goal is to have one index in a DWP file (which seems reasonable) then all 
> future index versions will have to support column indexes all previous DWARF 
> sections - the DWARFvN enum can then be used to describe all the previous 
> versions as well.


So, what are the differences with the last update, apart from that 
`DWARFSectionKind` is still internal? I believe we should not put the 
extensions into the official part before they are approved.


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

https://reviews.llvm.org/D75929



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

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

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

> This is almost what you are doing right now -- the only difference is that 
> the "internal" enum would no longer be internal -- it would actually match 
> the on-disk format of a v5 index. This v5 enum would contain the official 
> DWARFv5 constants as well as the new extensions we want to introduce for 
> mixed 5+4 indices.

Yep, this would be the direction I would suggest/encourage. It seems that if 
the goal is to have one index in a DWP file (which seems reasonable) then all 
future index versions will have to support column indexes all previous DWARF 
sections - the DWARFvN enum can then be used to describe all the previous 
versions as well.

All we'd do is store a "what's the highest DWARF version we have seen in 
parsing all the inputs (either input CUs in .dwo files, or input cu_indexes in 
.dwp files)" and use that number to determine what version index we produce (& 
knowing that those inputs can only contain the limited columns expressable in 
that DWARF/index version, so we won't have undescribable columns in our 
internal representation when we're going to emit the index).


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

https://reviews.llvm.org/D75929



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm not sure that this is the right API to represent an environment. The 
environment is more like a dictionary/map than an array. (The internal 
Environment object *is* a map, though this does not immediately mean the SB one 
should be too).

Even for your most immediate use case, you'll want to use the map-like 
properties of the environment (to "merge" the inherited environment and the 
user-provided values). With an api like this you'd have to implement all of 
that by yourself.

So, I am wondering if we shouldn't provide a more map-like interface here too. 
Rough proposal:

  const char *GetEntry(const char *name); // nullptr if not present
  void PutEntry(const char *string); // modeled on putenv(3)
  void SetEntry(const char *name, const char *value, bool overwrite); //modeled 
on setenv(3), maybe we can skip it if it's not needed now
  SBStringList GetEntries(); // if someone wants to enumerate all of them, 
maybe also skippable

If we don't want a map-like interface, then maybe we don't actually need a 
separate class, and an SBStringList would work just fine?

Maybe you could also refactor the other patch to use this new functionality 
(whatever it ends up being), so that we can see whether it makes sense at least 
for that use case..




Comment at: lldb/include/lldb/API/SBEnvironment.h:26-28
+  int Size();
+
+  const char *GetEntryAtIndex(int idx);

s/int/size_t



Comment at: lldb/source/API/SBEnvironment.cpp:1-2
+//===-- SBEnvironment.cpp
+//---===//
+//

fix formatting



Comment at: lldb/source/API/SBEnvironment.cpp:30-31
+SBEnvironment::SBEnvironment(const Environment &rhs)
+: m_opaque_up(new Environment()) {
+  *m_opaque_up = rhs;
+}

new(Environment(rhs))



Comment at: lldb/source/API/SBEnvironment.cpp:53
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetEntryAtIndex, (int), idx);
+  return LLDB_RECORD_RESULT(m_opaque_up->getEnvp().get()[idx]);
+}

This will return a dangling pointer as Envp is a temporary object. It's 
intended to be used to pass the environment object to execve-like function. 
The current "state of the art" is to ConstStringify all temporary strings that 
go out of the SB api. (Though I think we should start using std::string).

Also, constructing the Envp object just to fetch a single string out of it is 
pretty expensive. You can fetch the desired result from the Environment object 
directly.

Putting it all together, this kind of an API would be implemented via something 
like:
```
  if (idx >= m_opaque_up->size())
return nullptr;
  return ConstString(Environment::compose(*std::next(m_opaque_up.begin(), 
idx)).AsCString();
```

... but I am not sure this is really the right api. More on that in a separate 
comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D76105#1920753 , @friss wrote:

> Yeah, this shortcoming was pretty obvious while writing the patch. I don't 
> like it very much, it seems like the inherit behavior should be handled 
> closer to the point we launch, Or at least the inherited environment should 
> be stored separately from the one you set manually. Comparing values would 
> solve one class of issues, but what about explicitly setting one variable to 
> the same value it has in your environment. Then you would delete it when 
> changing the inherit-env property. Also not very intuitive.


Yeah, I've been thinking about that too. Is it now too late to do anything 
about that?

In my ideal world the "env-vars" setting would only hold the values that the 
user has set, and nothing more. And there would be a separate way to get the 
effective enviroment used for launches and what not. Maybe something like:

- `platform environment` => get the current platform's environment -- this is 
the thing that would get inherited
- `process environment` => get the effective environment -- if a process is 
running it's the environment the process was launched with, if it's not yet 
started, it's the (merged) environment it would be launched with. Or maybe 
these should be separate commands?

We wouldn't have to add all of these commands, if we don't think they are 
useful. My main point is about avoiding merging user-provided and inherited 
variables so early in the process..

One gotcha with this flow is that it becomes hard to unset a specific 
environment variable but inherit the rest. But maybe that doesn't matter and 
setting the variable to an empty string is enough? Or we can have a "target 
populate-environment" command which would fill in the host environment into the 
"target.env-vars" setting and set "inherit-env=false" (I've seen this kind of 
flow when launching processes from visual studio (through a gui), and I kind of 
liked it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76105



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

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D76009#1920548 , @friss wrote:

> I put up a "fix" for the inherit-env issue mentioned here: 
> https://reviews.llvm.org/D76105
>  It is mostly orthogonal to this patch as Jim showed the behavior today is 
> already broken.


Yeah, I agree the current behavior is pretty bad, and this patch at least 
cleans up one aspect of it.

I don't have an immediate use case for the current env-vars behavior. If this 
helps you in any way, then I don't want to block it just because of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76009



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

[Lldb-commits] [PATCH] D75877: [lldb/Reproducers] Fix replay for process attach workflows

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:35
+
+reproducer_patch = tempfile.NamedTemporaryFile().name
+

s/patch/path



Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:53-54
+outs, errs = capture.communicate()
+self.assertTrue('Process {} stopped'.format(pid) in outs)
+self.assertTrue('Reproducer written' in outs)
+

self.assertIn(needle, haystack)



Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:66-75
+replay = subprocess.Popen([
+lldbtest_config.lldbExec, '-b', '-o',
+'reproducer dump -f {} -p process'.format(reproducer_patch)
+],
+  stdin=subprocess.PIPE,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)

Would it be possible to run this in the context of the current process (via 
self.expect?)



Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:74-75
+outs, errs = replay.communicate()
+self.assertTrue('name = {}'.format(exe))
+self.assertTrue('pid = {}'.format(pid))
+

I guess you meant assertIn here too



Comment at: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py:77-82
+# Remove the reproducer but don't complain in case the directory was
+# never created.
+try:
+shutil.rmtree(reproducer_patch)
+except OSError:
+pass

The reproducer dir will still linger on if the test fails for any reason. If 
you just put this in the build directory (by (ab)using 
`self.getBuildArtifact`), then maybe you don't need to clean it up, as it will 
be automatically deleted the next time the test suite runs...


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

https://reviews.llvm.org/D75877



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

[Lldb-commits] [lldb] 2451cbf - [lldb/Reproducers] Intercept the FindProcesses API

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-13T09:31:35-07:00
New Revision: 2451cbf07bbc500718c30a9e9447385f7235707b

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

LOG: [lldb/Reproducers] Intercept the FindProcesses API

This patch extends the reproducers to intercept calls to FindProcesses.
During capture it serializes the ProcessInstanceInfoList returned by the
API. During replay, it returns the serialized data instead of querying
the host.

The motivation for this patch is supporting the process attach workflow
during replay. Without this change it would incorrectly look for the
inferior on the host during replay and failing if no matching process
was found.

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

Added: 
lldb/test/API/functionalities/reproducers/attach/Makefile
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
lldb/test/API/functionalities/reproducers/attach/main.cpp

Modified: 
lldb/include/lldb/Host/Host.h
lldb/include/lldb/Utility/ProcessInfo.h
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Host/common/Host.cpp
lldb/source/Host/linux/Host.cpp
lldb/source/Host/macosx/objcxx/Host.mm
lldb/source/Host/netbsd/Host.cpp
lldb/source/Host/openbsd/Host.cpp
lldb/source/Utility/ProcessInfo.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 269bb18571b0..f19cb85d2329 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -232,6 +232,10 @@ class Host {
 
   static std::unique_ptr
   CreateDefaultConnection(llvm::StringRef url);
+
+protected:
+  static uint32_t FindProcessesImpl(const ProcessInstanceInfoMatch &match_info,
+ProcessInstanceInfoList &proc_infos);
 };
 
 } // namespace lldb_private

diff  --git a/lldb/include/lldb/Utility/ProcessInfo.h 
b/lldb/include/lldb/Utility/ProcessInfo.h
index 0d631d06d2df..ec91060cda54 100644
--- a/lldb/include/lldb/Utility/ProcessInfo.h
+++ b/lldb/include/lldb/Utility/ProcessInfo.h
@@ -14,6 +14,7 @@
 #include "lldb/Utility/Environment.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/NameMatches.h"
+#include "lldb/Utility/Reproducer.h"
 #include "llvm/Support/YAMLTraits.h"
 #include 
 
@@ -215,6 +216,42 @@ class ProcessInstanceInfoMatch {
   bool m_match_all_users;
 };
 
+namespace repro {
+class ProcessInfoRecorder : public AbstractRecorder {
+public:
+  ProcessInfoRecorder(const FileSpec &filename, std::error_code &ec)
+  : AbstractRecorder(filename, ec) {}
+
+  static llvm::Expected>
+  Create(const FileSpec &filename);
+
+  void Record(const ProcessInstanceInfoList &process_infos);
+};
+
+class ProcessInfoProvider : public repro::Provider {
+public:
+  struct Info {
+static const char *name;
+static const char *file;
+  };
+
+  ProcessInfoProvider(const FileSpec &directory) : Provider(directory) {}
+
+  ProcessInfoRecorder *GetNewProcessInfoRecorder();
+
+  void Keep() override;
+  void Discard() override;
+
+  static char ID;
+
+private:
+  std::unique_ptr m_stream_up;
+  std::vector> m_process_info_recorders;
+};
+
+llvm::Optional GetReplayProcessInstanceInfoList();
+
+} // namespace repro
 } // namespace lldb_private
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::ProcessInstanceInfo)

diff  --git a/lldb/source/Commands/CommandObjectReproducer.cpp 
b/lldb/source/Commands/CommandObjectReproducer.cpp
index d73fa78231a5..104130b70b2b 100644
--- a/lldb/source/Commands/CommandObjectReproducer.cpp
+++ b/lldb/source/Commands/CommandObjectReproducer.cpp
@@ -8,13 +8,14 @@
 
 #include "CommandObjectReproducer.h"
 
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Host/OptionParser.h"
-#include "lldb/Utility/GDBRemote.h"
-#include "lldb/Utility/Reproducer.h"
-
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Utility/GDBRemote.h"
+#include "lldb/Utility/ProcessInfo.h"
+#include "lldb/Utility/Reproducer.h"
 
 #include 
 
@@ -27,6 +28,7 @@ enum ReproducerProvider {
   eReproducerProviderCommands,
   eReproducerProviderFiles,
   eReproducerProviderGDB,
+  eReproducerProviderProcessInfo,
   eReproducerProviderVersion,
   eReproducerProviderWorkingDirectory,
   eReproducerProviderNone
@@ -48,6 +50,11 @@ static constexpr OptionEnumValueElement 
g_reproducer_provider_type[] = {
 "gdb",
 "GDB Remote Packets",
 },
+{
+eReproducerProviderProcessInfo,
+"processes",
+"Process Info",
+},
 {
 eReproducerProviderVersion,
 "version",
@@ -97,6 +104,24 @@ static constexpr OptionEnumValues ReproducerSignalType() {
 #define LLDB_OPTIONS_reproducer_xcrash
 #include 

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Yes, a key/value approach would be a better API.  Looks like the Environment 
class would make that pretty easy to wrap up, as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

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

[Adrian Prantl via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:224
+if (auto *rd = llvm::dyn_cast(tag_decl))
+  for (auto *f : rd->fields())
+TypeSystemClang::SetOwningModule(f, owning_module);

aprantl wrote:
> labath wrote:
> > Maybe spell out the type here? I have no idea what's the type of this..
> Yeah, this should either be a Visitor or in our ASTImporterDelegate in lldb.
Another comment from an in-person discussion with @teemperor: We probably 
should not attach owning module information in the scratch/expression context.


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

https://reviews.llvm.org/D75488



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

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

[Adrian Prantl via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:224
+if (auto *rd = llvm::dyn_cast(tag_decl))
+  for (auto *f : rd->fields())
+TypeSystemClang::SetOwningModule(f, owning_module);

labath wrote:
> Maybe spell out the type here? I have no idea what's the type of this..
Yeah, this should either be a Visitor or in our ASTImporterDelegate in lldb.


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

https://reviews.llvm.org/D75488



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

[Lldb-commits] [PATCH] D75877: [lldb/Reproducers] Fix replay for process attach workflows

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rG2451cbf07bbc: [lldb/Reproducers] Intercept the FindProcesses 
API (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D75877?vs=249764&id=250236#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75877

Files:
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/linux/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Host/netbsd/Host.cpp
  lldb/source/Host/openbsd/Host.cpp
  lldb/source/Utility/ProcessInfo.cpp
  lldb/test/API/functionalities/reproducers/attach/Makefile
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
  lldb/test/API/functionalities/reproducers/attach/main.cpp

Index: lldb/test/API/functionalities/reproducers/attach/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/attach/main.cpp
@@ -0,0 +1,24 @@
+#include 
+#include 
+#include 
+
+using std::chrono::seconds;
+
+int main(int argc, char const *argv[]) {
+  lldb_enable_attach();
+
+  // Create the synchronization token.
+  FILE *f;
+  if (f = fopen(argv[1], "wx")) {
+fputs("\n", f);
+fflush(f);
+fclose(f);
+  } else
+return 1;
+
+  while (true) {
+std::this_thread::sleep_for(seconds(1));
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -0,0 +1,71 @@
+"""
+Test reproducer attach.
+"""
+
+import lldb
+import tempfile
+from lldbsuite.test import lldbtest_config
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class CreateAfterAttachTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfFreeBSD
+@skipIfNetBSD
+@skipIfWindows
+@skipIfRemote
+@skipIfiOSSimulator
+def test_create_after_attach_with_fork(self):
+"""Test thread creation after process attach."""
+exe = '%s_%d' % (self.testMethodName, os.getpid())
+
+token = self.getBuildArtifact(exe + '.token')
+if os.path.exists(token):
+os.remove(token)
+
+reproducer = self.getBuildArtifact(exe + '.reproducer')
+if os.path.exists(reproducer):
+try:
+shutil.rmtree(reproducer)
+except OSError:
+pass
+
+self.build(dictionary={'EXE': exe})
+self.addTearDownHook(self.cleanupSubprocesses)
+
+inferior = self.spawnSubprocess(self.getBuildArtifact(exe), [token])
+pid = inferior.pid
+
+lldbutil.wait_for_file_on_target(self, token)
+
+# Use Popen because pexpect is overkill and spawnSubprocess is
+# asynchronous.
+capture = subprocess.Popen([
+lldbtest_config.lldbExec, '-b', '--capture', '--capture-path',
+reproducer, '-o', 'proc att -n {}'.format(exe), '-o',
+'reproducer generate'
+],
+   stdin=subprocess.PIPE,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.PIPE)
+outs, errs = capture.communicate()
+self.assertIn('Process {} stopped'.format(pid), outs)
+self.assertIn('Reproducer written', outs)
+
+# Check that replay works.
+replay = subprocess.Popen(
+[lldbtest_config.lldbExec, '-replay', reproducer],
+stdin=subprocess.PIPE,
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE)
+outs, errs = replay.communicate()
+self.assertIn('Process {} stopped'.format(pid), outs)
+
+# We can dump the reproducer in the current context.
+self.expect('reproducer dump -f {} -p process'.format(reproducer),
+substrs=['pid = {}'.format(pid), 'name = {}'.format(exe)])
Index: lldb/test/API/functionalities/reproducers/attach/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/reproducers/attach/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -18,6 +18,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::repro;
 
 ProcessInfo::ProcessInfo()
 : m_executable(), m_arguments(),

[Lldb-commits] [lldb] 17bdb7a - [lldb/Test] Convert stdout to str by calling decode('utf-8') on it.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-13T09:50:41-07:00
New Revision: 17bdb7a17912bb4d961cf292c035b08c9d0c13ba

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

LOG: [lldb/Test] Convert stdout to str by calling decode('utf-8') on it.

Make sure both arguments to assertIn are of type str. This should fix
the following error:

TypeError: a bytes-like object is required, not 'str'.

Added: 


Modified: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py 
b/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
index 48659e46ebd7..9bc123a5d051 100644
--- a/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ b/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -53,7 +53,8 @@ def test_create_after_attach_with_fork(self):
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
-outs, errs = capture.communicate()
+outs, _ = capture.communicate()
+outs = outs.decode('utf-8')
 self.assertIn('Process {} stopped'.format(pid), outs)
 self.assertIn('Reproducer written', outs)
 
@@ -63,7 +64,8 @@ def test_create_after_attach_with_fork(self):
 stdin=subprocess.PIPE,
 stdout=subprocess.PIPE,
 stderr=subprocess.PIPE)
-outs, errs = replay.communicate()
+outs, _ = replay.communicate()
+outs = outs.decode('utf-8')
 self.assertIn('Process {} stopped'.format(pid), outs)
 
 # We can dump the reproducer in the current context.



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

[Lldb-commits] [lldb] 20e36f3 - [lldb/Host] s/FindProcesses/FindProcessesImpl/ in windows/Host.cpp

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-13T10:07:15-07:00
New Revision: 20e36f31dfc1bb079dc6e6db5f692a4e90aa0c9d

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

LOG: [lldb/Host] s/FindProcesses/FindProcessesImpl/ in windows/Host.cpp

Fix the Windows build.

Added: 


Modified: 
lldb/source/Host/windows/Host.cpp

Removed: 




diff  --git a/lldb/source/Host/windows/Host.cpp 
b/lldb/source/Host/windows/Host.cpp
index 29c96560af25..50617a8c4bb0 100644
--- a/lldb/source/Host/windows/Host.cpp
+++ b/lldb/source/Host/windows/Host.cpp
@@ -131,8 +131,8 @@ FileSpec Host::GetModuleFileSpecForHostAddress(const void 
*host_addr) {
   return module_filespec;
 }
 
-uint32_t Host::FindProcesses(const ProcessInstanceInfoMatch &match_info,
- ProcessInstanceInfoList &process_infos) {
+uint32_t Host::FindProcessesImpl(const ProcessInstanceInfoMatch &match_info,
+ ProcessInstanceInfoList &process_infos) {
   process_infos.clear();
 
   AutoHandle snapshot(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0));



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

[Lldb-commits] [lldb] 01387c4 - [lldb/Test] Temporarily skip TestReproducerAttach on Linux

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-03-13T10:07:15-07:00
New Revision: 01387c44d05287bd330e67af68fc265f01b8d2ed

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

LOG: [lldb/Test] Temporarily skip TestReproducerAttach on Linux

The test is failing with an unexpected packet during replay. Temporarily
disabling the test while I setup and environment to investigate.

Added: 


Modified: 
lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py 
b/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
index 9bc123a5d051..e9a570b50976 100644
--- a/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ b/lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -10,17 +10,18 @@
 from lldbsuite.test import lldbutil
 
 
-class CreateAfterAttachTestCase(TestBase):
+class ReproducerAttachTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfLinux # Reproducer received unexpected packet.
 @skipIfFreeBSD
 @skipIfNetBSD
 @skipIfWindows
 @skipIfRemote
 @skipIfiOSSimulator
-def test_create_after_attach_with_fork(self):
+def test_reproducer_attach(self):
 """Test thread creation after process attach."""
 exe = '%s_%d' % (self.testMethodName, os.getpid())
 



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

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D76105#1921273 , @labath wrote:

> In D76105#1920753 , @friss wrote:
>
> > Yeah, this shortcoming was pretty obvious while writing the patch. I don't 
> > like it very much, it seems like the inherit behavior should be handled 
> > closer to the point we launch, Or at least the inherited environment should 
> > be stored separately from the one you set manually. Comparing values would 
> > solve one class of issues, but what about explicitly setting one variable 
> > to the same value it has in your environment. Then you would delete it when 
> > changing the inherit-env property. Also not very intuitive.
>
>
> Yeah, I've been thinking about that too. Is it now too late to do anything 
> about that?
>
> In my ideal world the "env-vars" setting would only hold the values that the 
> user has set, and nothing more. And there would be a separate way to get the 
> effective enviroment used for launches and what not. Maybe something like:
>
> - `platform environment` => get the current platform's environment -- this is 
> the thing that would get inherited
> - `process environment` => get the effective environment -- if a process is 
> running it's the environment the process was launched with, if it's not yet 
> started, it's the (merged) environment it would be launched with. Or maybe 
> these should be separate commands?


This seems like a better set-up to me.  I like having "platform environment" 
because it is useful to see "If I were to launch something, when environment 
would it get" before mucking with it.  Particularly if we get around to 
fetching the starting environment from the remote target, since that's hard to 
see otherwise.

It's a little awkward that you use commands to see the original and resultant 
environments, but a "settings show" to see your changes to the original.  You 
could have "target environment" which be the environment YOU have set in the 
target (same as env-vars), then it would make sense for process to show the 
aggregate and platform to show the original.  That's a little more symmetrical, 
but so long as you have to use the setting to change it, I'm not so sure this 
is valuable.

> We wouldn't have to add all of these commands, if we don't think they are 
> useful. My main point is about avoiding merging user-provided and inherited 
> variables so early in the process..

Yes, I agree there is value in showing these things separately.

> One gotcha with this flow is that it becomes hard to unset a specific 
> environment variable but inherit the rest. But maybe that doesn't matter and 
> setting the variable to an empty string is enough? Or we can have a "target 
> populate-environment" command which would fill in the host environment into 
> the "target.env-vars" setting and set "inherit-env=false" (I've seen this 
> kind of flow when launching processes from visual studio (through a gui), and 
> I kind of liked it)

Setting them to an empty string won't do, I think.  There are too many uses of 
environment variables that just check that the variable exists, and don't care 
about the value.

As we think about a better way of doing this, I'd like to make it possible to 
fetch the environment from a remote system.  Filling in the host environment 
for a remote session is not the right thing to do.  Filling it with the 
connected platform's environment is what actually makes some sense.

But if we do it that way you won't have a valid environment till you've 
connected to some remote platform.  That would mean the model where you 
populate the environment and then excise unwanted entries can only happen 
properly after targets get created and connected to a platform.  That makes the 
populate then excise model less workable.

This makes me think that maybe settings are not a rich enough interface for 
environment variables.  Maybe it would be better to have "target environment 
{set/show/remove}" where set tracks with the env-vars, and remove means remove 
these entries from the composite environment.  Then "platform env" and "process 
env" would be show only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76105



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

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

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Thanks a lot for the feedback! I was thinking about adding features to this 
class as needed, but definitely I should make this API be more like a map


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [lldb] f82b32a - Revert "Reland "[DebugInfo] Enable the debug entry values feature by default""

[Nico Weber via lldb-commits]

Author: Nico Weber
Date: 2020-03-13T15:37:44-04:00
New Revision: f82b32a51e22cc56d20f695772797127d3f9d85a

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

LOG: Revert "Reland "[DebugInfo] Enable the debug entry values feature by 
default""

This reverts commit 5aa5c943f7da155b95564058cd5d50a93eabfc89.
Causes clang to assert, see
https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c4
for a repro.

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/CC1Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
lldb/packages/Python/lldbsuite/test/decorators.py

lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
llvm/include/llvm/CodeGen/CommandFlags.inc
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
llvm/lib/CodeGen/LiveDebugValues.cpp
llvm/lib/CodeGen/TargetOptionsImpl.cpp
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
llvm/lib/Target/ARM/ARMTargetMachine.cpp
llvm/lib/Target/X86/X86TargetMachine.cpp
llvm/test/CodeGen/MIR/Hexagon/bundled-call-site-info.mir
llvm/test/CodeGen/MIR/X86/call-site-info-error4.mir
llvm/test/CodeGen/X86/call-site-info-output.ll
llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir
llvm/test/DebugInfo/MIR/ARM/call-site-info-vmovd.mir
llvm/test/DebugInfo/MIR/ARM/call-site-info-vmovs.mir
llvm/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir
llvm/test/DebugInfo/MIR/Hexagon/dbgcall-site-instr-before-bundled-call.mir
llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
llvm/test/DebugInfo/MIR/X86/DW_OP_entry_value.mir
llvm/test/DebugInfo/MIR/X86/call-site-gnu-vs-dwarf5-attrs.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-copy-super-sub.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-lea-interpretation.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reference.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir
llvm/test/DebugInfo/MIR/X86/dbgcall-site-two-fwd-reg-defs.mir
llvm/test/DebugInfo/MIR/X86/dbginfo-entryvals.mir
llvm/test/DebugInfo/MIR/X86/debug-call-site-param.mir
llvm/test/DebugInfo/MIR/X86/entry-value-of-modified-param.mir
llvm/test/DebugInfo/MIR/X86/entry-values-diamond-bbs.mir
llvm/test/DebugInfo/MIR/X86/propagate-entry-value-cross-bbs.mir
llvm/test/DebugInfo/MIR/X86/unreachable-block-call-site.mir
llvm/test/DebugInfo/X86/dbg-value-range.ll
llvm/test/DebugInfo/X86/dbg-value-regmask-clobber.ll
llvm/test/DebugInfo/X86/dbgcall-site-64-bit-imms.ll
llvm/test/DebugInfo/X86/dbgcall-site-zero-valued-imms.ll
llvm/test/DebugInfo/X86/loclists-dwp.ll
llvm/test/tools/llvm-locstats/locstats.ll

Removed: 
llvm/test/DebugInfo/X86/no-entry-values-with-O0.ll



diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index e047054447f3..3c8b0eeb47a5 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -63,6 +63,7 @@ CODEGENOPT(ExperimentalNewPassManager, 1, 0) ///< Enables the 
new, experimental
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(EnableDebugEntryValues, 1, 0) ///< Emit call site parameter dbg info
 CODEGENOPT(EmitCallSiteInfo, 1, 0) ///< Emit call site info only in the case of
///< '-g' + 'O>0' level.
 CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs

diff  --git a/clang/include/clang/Driver/CC1Options.td 
b/clang/include/clang/Driver/CC1Options.td
index cc30893703df..b7a2826d8fcb 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -388,6 +388,8 @@ def flto_visibility_public_std:
 def flto_unit: Flag<["-"], "flto-unit">,
 HelpText<"Emit IR to support LTO unit features (CFI, whole program vtable 
opt)">;
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
+def femit_debug_entry_values : Flag<["-"], "femit-debug-entry-values">,
+HelpText<"Enables debug info about call site parameter's entry values">;
 def 

[Lldb-commits] [PATCH] D75975: [lldb] Copy m_behaves_like_zeroth_frame on stack frame update

[Jason Molenda via Phabricator via lldb-commits]

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

Looks good to me, nice catch.  Thanks for writing a test dase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75975



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

Reduced repro at 
https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c7

Scrolling up, it looks like this was reverted yesterday already. I'm confused 
why this wasn't reverted yesterday; all work I did in bisecting and creducing 
was a duplicate of what happened yesterday :/


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

This makes clang crash, repro at 
https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c4 I'll revert 
for now.

I'm also seeing timeouts and use-after-frees on other bots; maybe they're 
related (or maybe not).


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

https://reviews.llvm.org/D73534



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

[Lldb-commits] [PATCH] D76163: [lldb/Reproducers] Decode run-length encoding in GDB replay server.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 250305.
JDevlieghere added a comment.

Share RLE-decoding logic.


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

https://reviews.llvm.org/D76163

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py

Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -15,7 +15,6 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipIfLinux # Reproducer received unexpected packet.
 @skipIfFreeBSD
 @skipIfNetBSD
 @skipIfWindows
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
@@ -131,22 +131,26 @@
 GDBRemotePacket entry = m_packet_history.back();
 m_packet_history.pop_back();
 
+// Decode run-length encoding.
+const std::string expanded_data =
+GDBRemoteCommunication::ExpandRLE(entry.packet.data);
+
 // We've handled the handshake implicitly before. Skip the packet and move
 // on.
 if (entry.packet.data == "+")
   continue;
 
 if (entry.type == GDBRemotePacket::ePacketTypeSend) {
-  if (unexpected(entry.packet.data, packet.GetStringRef())) {
+  if (unexpected(expanded_data, packet.GetStringRef())) {
 LLDB_LOG(log,
  "GDBRemoteCommunicationReplayServer expected packet: '{0}'",
- entry.packet.data);
+ expanded_data);
 LLDB_LOG(log, "GDBRemoteCommunicationReplayServer actual packet: '{0}'",
  packet.GetStringRef());
 #ifndef NDEBUG
 // This behaves like a regular assert, but prints the expected and
 // received packet before aborting.
-printf("Reproducer expected packet: '%s'\n", entry.packet.data.c_str());
+printf("Reproducer expected packet: '%s'\n", expanded_data.c_str());
 printf("Reproducer received packet: '%s'\n",
packet.GetStringRef().data());
 llvm::report_fatal_error("Encountered unexpected packet during replay");
@@ -155,7 +159,7 @@
   }
 
   // Ignore QEnvironment packets as they're handled earlier.
-  if (entry.packet.data.find("QEnvironment") == 1) {
+  if (expanded_data.find("QEnvironment") == 1) {
 assert(m_packet_history.back().type ==
GDBRemotePacket::ePacketTypeRecv);
 m_packet_history.pop_back();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -142,6 +142,9 @@
   static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
 GDBRemoteCommunication &server);
 
+  /// Expand GDB run-length encoding.
+  static std::string ExpandRLE(std::string);
+
 protected:
   std::chrono::seconds m_packet_timeout;
   uint32_t m_echo_number;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -810,31 +810,9 @@
   GDBRemotePacket::ePacketTypeRecv, total_length);
 
   // Copy the packet from m_bytes to packet_str expanding the run-length
-  // encoding in the process. Reserve enough byte for the most common case
-  // (no RLE used)
-  std ::string packet_str;
-  packet_str.reserve(m_bytes.length());
-  for (std::string::const_iterator c = m_bytes.begin() + content_start;
-   c != m_bytes.begin() + content_end; ++c) {
-if (*c == '*') {
-  // '*' indicates RLE. Next character will give us the repeat count
-  // and previous character is what is to be repeated.
-  char char_to_repeat = packet_str.back();
-  // Number of time the previous character is repeated
-  int repeat_count = *++c + 3 - ' ';
-  // We have the char_to_repeat and repeat_count. Now push it in the
-  // packet.
-  for (int i = 0; i < repeat_count; ++i)
-packet_str.push_back(char_to_repeat);
-} else if (*c == 0x7d) {
-   

[Lldb-commits] [PATCH] D76163: [lldb/Reproducers] Decode run-length encoding in GDB replay server.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jasonmolenda.
Herald added a subscriber: abidh.
JDevlieghere updated this revision to Diff 250305.
JDevlieghere added a comment.

Share RLE-decoding logic.


The GDB replay server sanity-checks that every packet it receives matches what 
it expects from the serialized packet log. This mechanism tripped for 
`TestReproducerAttach.py` on Linux, because one of the packets (`jModulesInfo`) 
uses run-length encoding. The replay server was comparing the expanded incoming 
packet with the unexpanded packet in the log. As a result, it claimed to have 
received an unexpected packet, which caused the test to fail.

This patch addresses that issue by expanding the run-length encoding before 
comparing the packets.


https://reviews.llvm.org/D76163

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py

Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -15,7 +15,6 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipIfLinux # Reproducer received unexpected packet.
 @skipIfFreeBSD
 @skipIfNetBSD
 @skipIfWindows
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
@@ -131,22 +131,26 @@
 GDBRemotePacket entry = m_packet_history.back();
 m_packet_history.pop_back();
 
+// Decode run-length encoding.
+const std::string expanded_data =
+GDBRemoteCommunication::ExpandRLE(entry.packet.data);
+
 // We've handled the handshake implicitly before. Skip the packet and move
 // on.
 if (entry.packet.data == "+")
   continue;
 
 if (entry.type == GDBRemotePacket::ePacketTypeSend) {
-  if (unexpected(entry.packet.data, packet.GetStringRef())) {
+  if (unexpected(expanded_data, packet.GetStringRef())) {
 LLDB_LOG(log,
  "GDBRemoteCommunicationReplayServer expected packet: '{0}'",
- entry.packet.data);
+ expanded_data);
 LLDB_LOG(log, "GDBRemoteCommunicationReplayServer actual packet: '{0}'",
  packet.GetStringRef());
 #ifndef NDEBUG
 // This behaves like a regular assert, but prints the expected and
 // received packet before aborting.
-printf("Reproducer expected packet: '%s'\n", entry.packet.data.c_str());
+printf("Reproducer expected packet: '%s'\n", expanded_data.c_str());
 printf("Reproducer received packet: '%s'\n",
packet.GetStringRef().data());
 llvm::report_fatal_error("Encountered unexpected packet during replay");
@@ -155,7 +159,7 @@
   }
 
   // Ignore QEnvironment packets as they're handled earlier.
-  if (entry.packet.data.find("QEnvironment") == 1) {
+  if (expanded_data.find("QEnvironment") == 1) {
 assert(m_packet_history.back().type ==
GDBRemotePacket::ePacketTypeRecv);
 m_packet_history.pop_back();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -142,6 +142,9 @@
   static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
 GDBRemoteCommunication &server);
 
+  /// Expand GDB run-length encoding.
+  static std::string ExpandRLE(std::string);
+
 protected:
   std::chrono::seconds m_packet_timeout;
   uint32_t m_echo_number;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -810,31 +810,9 @@
   GDBRemotePacket::ePacketTypeRecv, total_length);
 
   // Copy the packet from m_bytes to packet_str expanding the run-length
-  // encoding in the process. Reserve enough byte for the most common case
-  // (no RLE used)
-  std ::string packet_str;
-  packet_str.reserve(m_bytes.length());
-  for (std::string::const_iterator c = m_bytes.begin() + content_start;
-  

[Lldb-commits] [PATCH] D76163: [lldb/Reproducers] Decode run-length encoding in GDB replay server.

[Jason Molenda via Phabricator via lldb-commits]

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

lgtm.


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

https://reviews.llvm.org/D76163



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

[Lldb-commits] [PATCH] D76167: [lldb/Utils] Use PYTHON_EXECUTABLE to configurelldb-dotest's shebang

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 250323.
JDevlieghere added a comment.

Context


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

https://reviews.llvm.org/D76167

Files:
  lldb/utils/lldb-dotest/lldb-dotest.in


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
 


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/bindings/interface/SBPlatform.i:197-198
 
+lldb::SBEnvironment
+GetEnvironment();
+

What does it mean to get the environment from a platform? Fetching it from the 
remote platform as what ever binary was used to provide the connection?



Comment at: lldb/include/lldb/API/SBEnvironment.h:26
+
+  int Size();
+

labath wrote:
> s/int/size_t
return size_t or uint32_t.

Possibly rename to GetNumEntries() to be consistent with all of the other APIs 
that have a size and item accessor.



Comment at: lldb/include/lldb/API/SBEnvironment.h:28
+
+  const char *GetEntryAtIndex(int idx);
+

use size_t or uint32_t (which ever one you pick from Size() above



Comment at: lldb/include/lldb/API/SBPlatform.h:157
 
+  SBEnvironment GetEnvironment();
+

I know there isn't much header documentation here already, but it is a good 
time to start with any new functions. We would need to detail what this 
environment would be. Something like: "Return the environment variables 
contained in the remote platform connection process."



Comment at: lldb/source/API/SBEnvironment.cpp:46
+
+int SBEnvironment::Size() {
+  LLDB_RECORD_METHOD_NO_ARGS(int, SBEnvironment, Size);

```
size_t SBEnvironment::GetNumEnrtries()
```



Comment at: lldb/source/API/SBEnvironment.cpp:53
+  LLDB_RECORD_METHOD(const char *, SBEnvironment, GetEntryAtIndex, (int), idx);
+  return LLDB_RECORD_RESULT(m_opaque_up->getEnvp().get()[idx]);
+}

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

No STL in the SB API please, so no std::string. std::string isn't stable on all 
platforms and competing C++ runtimes on different platforms can cause problems. 
If we need string ownership, we can introduce SBString as a class.

Please do bounds check this like Pavel requested as the index isn't guaranteed 
to be valid. If you bounds check first then your code above could be modified 
to return a ConstString().AsCString().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111



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

[Lldb-commits] [PATCH] D76167: [lldb/Utils] Use PYTHON_EXECUTABLE to configurelldb-dotest's shebang

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, labath.
Herald added a subscriber: abidh.
JDevlieghere updated this revision to Diff 250323.
JDevlieghere added a comment.
aprantl accepted this revision.
This revision is now accepted and ready to land.

Context


aprantl added a comment.

Yes, that's in the spirit of this script.


Ideally we'd want all shebangs to be configurable, but that's not a viable 
solution. Given that lldb-dotest is already configured, we might as well make 
sure it uses the correct interpreter.


https://reviews.llvm.org/D76167

Files:
  lldb/utils/lldb-dotest/lldb-dotest.in


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
 


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76167: [lldb/Utils] Use PYTHON_EXECUTABLE to configurelldb-dotest's shebang

[Adrian Prantl via Phabricator via lldb-commits]

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

Yes, that's in the spirit of this script.


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

https://reviews.llvm.org/D76167



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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

My fix in `ConsumeOperator()` is not proper but if everyone feels this is 
correct approach I will create member functions to deal with this cleanly.

Other approaches could be modifying `ExtractTokens()` to detect this case and 
generate two `tok::less`in place of `tok::lessless` but this feels like the 
wrong place to fix this. I tried to understand how clang handles this case 
since but it was not obvious. AFAICT they have to deal with this case too.


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

https://reviews.llvm.org/D76168



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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, labath, jingham.
shafik added a comment.

My fix in `ConsumeOperator()` is not proper but if everyone feels this is 
correct approach I will create member functions to deal with this cleanly.

Other approaches could be modifying `ExtractTokens()` to detect this case and 
generate two `tok::less`in place of `tok::lessless` but this feels like the 
wrong place to fix this. I tried to understand how clang handles this case 
since but it was not obvious. AFAICT they have to deal with this case too.


CPlusPlusNameParser is used in several places on of them is during IR execution 
and setting breakpoints to pull information C++ like the basename, the context 
and arguments.

Currently it does not handle templated `operator<` properly. It used 
`clang::Lexer` which will tokenize `operator<` into:

tok::kw_operator
tok::lessless
tok::raw_identifier

Later on the parser in `ConsumeOperator()` does not handle this case properly 
and we end up failing to parse.


https://reviews.llvm.org/D76168

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp


Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -140,12 +140,20 @@
"std::vector>",
"_M_emplace_back_aux"},
   {"`anonymous namespace'::foo", "`anonymous namespace'", "foo"},
-  {"`operator<'::`2'::B<0>::operator>",
-   "`operator<'::`2'::B<0>",
+  {"`operator<'::`2'::B<0>::operator>", "`operator<'::`2'::B<0>",
"operator>"},
   {"`anonymous namespace'::S::<<::__l2::Foo",
-   "`anonymous namespace'::S::<<::__l2",
-   "Foo"}};
+   "`anonymous namespace'::S::<<::__l2", "Foo"},
+  {"A::operator>", "A", "operator>"},
+  {"operator>", "", "operator>"},
+  {"A::operator<", "A", "operator<"},
+  {"operator<", "", "operator<"},
+  {"A::operator<<", "A", "operator<<"},
+  {"operator<<", "", "operator<<"},
+  // We expect these cases to fail until we turn on C++2a
+  //  {"A::operator<=>", "A", "operator<=>"},
+  //  {"operator<=>", "", "operator<=>" },
+  };
 
   llvm::StringRef context, basename;
   for (const auto &test : test_cases) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -329,6 +329,24 @@
   }
 
   const auto &token = Peek();
+  if (token.getKind() == tok::lessless) {
+if (m_next_token_index + 1 < m_tokens.size()) {
+  clang::Token n_token = m_tokens[m_next_token_index + 1];
+  if (n_token.getKind() != tok::l_paren && n_token.getKind() != tok::less) 
{
+clang::Token tmp_tok;
+
+tmp_tok.setLength(1);
+tmp_tok.setLocation(token.getLocation().getLocWithOffset(1));
+tmp_tok.setKind(tok::less);
+
+m_tokens[m_next_token_index] = tmp_tok;
+
+start_position.Remove();
+return true;
+  }
+}
+  }
+
   switch (token.getKind()) {
   case tok::kw_new:
   case tok::kw_delete:


Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -140,12 +140,20 @@
"std::vector>",
"_M_emplace_back_aux"},
   {"`anonymous namespace'::foo", "`anonymous namespace'", "foo"},
-  {"`operator<'::`2'::B<0>::operator>",
-   "`operator<'::`2'::B<0>",
+  {"`operator<'::`2'::B<0>::operator>", "`operator<'::`2'::B<0>",
"operator>"},
   {"`anonymous namespace'::S::<<::__l2::Foo",
-   "`anonymous namespace'::S::<<::__l2",
-   "Foo"}};
+   "`anonymous namespace'::S::<<::__l2", "Foo"},
+  {"A::operator>", "A", "operator>"},
+  {"operator>", "", "operator>"},
+  {"A::operator<", "A", "operator<"},
+  {"operator<", "", "operator<"},
+  {"A::operator<<", "A", "operator<<"},
+  {"operator<<", "", "operator<<"},
+  // We expect these cases to fail until we turn on C++2a
+  //  {"A::operator<=>", "A", "operator<=>"},
+  //  {"operator<=>", "", "operator<=>" },
+  };
 
   llvm::StringRef context, basename;
   for (const auto &test : test_cases) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -329,6 +329,24 @@
   }
 
   const auto &token = Pee

[Lldb-commits] [PATCH] D75761: [lldb] Fix to get the AST we generate for function templates to be closer to what clang generates and expects

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 250327.
shafik marked 8 inline comments as done.
shafik added a comment.

Addressing comments


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

https://reviews.llvm.org/D75761

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
  lldb/test/API/lang/cpp/template-function/main.cpp

Index: lldb/test/API/lang/cpp/template-function/main.cpp
===
--- lldb/test/API/lang/cpp/template-function/main.cpp
+++ lldb/test/API/lang/cpp/template-function/main.cpp
@@ -3,6 +3,66 @@
 return int(t1);
 }
 
+// Some cases to cover ADL, we hae two cases:
+//
+// - f which will have a overload in the global namespace if unqualified lookup
+// find f(int) and f(T) is found via ADL.
+//
+// - g which does not have an overload in the global namespace.
+namespace A {
+struct C {};
+
+template  int f(T) { return 4; }
+
+template  int g(T) { return 4; }
+} // namespace A
+
+// Meant to overload A::f(T) which may be found via ADL
+int f(int) { return 1; }
+
+// Regular overloaded functions case h(T) and h(double).
+template  int h(T x) { return x; }
+int h(double d) { return 5; }
+
+template  int var(Us... pargs) { return 10; }
+
+// Having the templated overloaded operators in a namespace effects the
+// mangled name generated in the IR e.g. _ZltRK1BS1_ Vs _ZN1AltERKNS_1BES2_
+// One will be in the symbol table but the other won't. This results in a
+// different code path that will result in CPlusPlusNameParser being used.
+// This allows us to cover that code as well.
+namespace A {
+template  bool operator<(const T &, const T &) { return true; }
+
+template  bool operator>(const T &, const T &) { return true; }
+
+template  bool operator<<(const T &, const T &) { return true; }
+
+template  bool operator>>(const T &, const T &) { return true; }
+
+template  bool operator==(const T &, const T &) { return true; }
+
+struct B {};
+} // namespace A
+
+struct D {};
+
+// Make sure we cover more straight forward cases as well.
+bool operator<(const D &, const D &) { return true; }
+bool operator>(const D &, const D &) { return true; }
+bool operator>>(const D &, const D &) { return true; }
+bool operator<<(const D &, const D &) { return true; }
+bool operator==(const D &, const D &) { return true; }
+
 int main() {
-return foo(42);
+  A::B b1;
+  A::B b2;
+  D d1;
+  D d2;
+
+  bool result_b = b1 < b2 && b1 << b2 && b1 == b2 && b1 > b2 && b1 >> b2;
+  bool result_c = d1 < d2 && d1 << d2 && d1 == d2 && d1 > d2 && d1 >> d2;
+
+  return foo(42) + result_b + result_c + f(A::C{}) + g(A::C{}) + h(10) + h(1.) +
+ var(1) + var(1, 2); // break here
 }
Index: lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
===
--- lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
+++ lldb/test/API/lang/cpp/template-function/TestTemplateFunctions.py
@@ -13,14 +13,40 @@
 
 def do_test_template_function(self, add_cast):
 self.build()
-(_, _, thread, _) = lldbutil.run_to_name_breakpoint(self, "main")
-frame = thread.GetSelectedFrame()
-expr = "foo(42)"
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
 if add_cast:
-expr = "(int)" + expr
-expr_result = frame.EvaluateExpression(expr)
-self.assertTrue(expr_result.IsValid())
-self.assertEqual(expr_result.GetValue(), "42")
+  self.expect_expr("(int) foo(42)", result_type="int", result_value="42")
+else:
+  self.expect("expr b1 <=> b2",  error=True, substrs=["warning: :1:4: '<=>' is a single token in C++20; add a space to avoid a change in behavior"])
+
+  self.expect_expr("foo(42)", result_type="int", result_value="42")
+
+  # overload with template case
+  self.expect_expr("h(10)", result_type="int", result_value="10")
+
+  # ADL lookup case
+  self.expect_expr("f(A::C{})", result_type="int", result_value="4")
+
+  # ADL lookup but no overload
+  self.expect_expr("g(A::C{})", result_type="int", result_value="4")
+
+  # variadic function cases
+  self.expect_expr("var(1)", result_type="int", result_value="10")
+  self.expect_expr("var(1,2)", result_type="int", result_value="10")
+
+  # Overloaded templated operator case
+  self.expect_expr("b1 > b2", result_type="bool", result_value="true")
+  self.expect_expr("b1 >> b2", result_type="bool", result_value="true")
+  self.expect_expr("b1 << b2", result_type="bool", result_value="true")
+  self.expect_expr("b1 == b2", result_type="bool", result_value="true")
+
+  # Overloaded operator case
+  self.expect_expr("d1 > d2", result_type="bool", result_valu