[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 443574.
Michael137 added a comment.

- Move helper into new namespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

Files:
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Expression/UserExpression.h
  lldb/source/Expression/Materializer.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/test/API/commands/expression/expr_inside_lambda/Makefile
  lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
  lldb/test/API/commands/expression/expr_inside_lambda/main.cpp

Index: lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
@@ -0,0 +1,99 @@
+#include 
+#include 
+
+namespace {
+int global_var = -5;
+} // namespace
+
+struct Baz {
+  virtual ~Baz() = default;
+
+  virtual int baz_virt() = 0;
+
+  int base_base_var = 12;
+};
+
+struct Bar : public Baz {
+  virtual ~Bar() = default;
+
+  virtual int baz_virt() override {
+base_var = 10;
+return 1;
+  }
+
+  int base_var = 15;
+};
+
+struct Foo : public Bar {
+  int class_var = 9;
+  int shadowed = -137;
+  int *class_ptr;
+
+  virtual ~Foo() = default;
+
+  virtual int baz_virt() override {
+shadowed = -1;
+return 2;
+  }
+
+  void method() {
+int local_var = 137;
+int shadowed;
+class_ptr = &local_var;
+auto lambda = [&shadowed, this, &local_var,
+   local_var_copy = local_var]() mutable {
+  int lambda_local_var = 5;
+  shadowed = 5;
+  class_var = 109;
+  --base_var;
+  --base_base_var;
+  std::puts("break here");
+
+  auto nested_lambda = [this, &lambda_local_var] {
+std::puts("break here");
+lambda_local_var = 0;
+  };
+
+  nested_lambda();
+  --local_var_copy;
+  std::puts("break here");
+
+  struct LocalLambdaClass {
+int lambda_class_local = -12345;
+Foo *outer_ptr;
+
+void inner_method() {
+  auto lambda = [this] {
+std::puts("break here");
+lambda_class_local = -2;
+outer_ptr->class_var *= 2;
+  };
+
+  lambda();
+}
+  };
+
+  LocalLambdaClass l;
+  l.outer_ptr = this;
+  l.inner_method();
+};
+lambda();
+  }
+
+  void non_capturing_method() {
+int local = 5;
+int local2 = 10;
+
+class_var += [=] {
+  std::puts("break here");
+  return local + local2;
+}();
+  }
+};
+
+int main() {
+  Foo f;
+  f.method();
+  f.non_capturing_method();
+  return global_var;
+}
Index: lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
@@ -0,0 +1,123 @@
+""" Test that evaluating expressions from within C++ lambdas works
+Particularly, we test the case of capturing "this" and
+using members of the captured object in expression evaluation
+while we're on a breakpoint inside a lambda.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+
+class ExprInsideLambdaTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def expectExprError(self, expr : str, expected : str):
+frame = self.thread.GetFrameAtIndex(0)
+value = frame.EvaluateExpression(expr)
+errmsg = value.GetError().GetCString()
+self.assertIn(expected, errmsg)
+
+def test_expr_inside_lambda(self):
+"""Test that lldb evaluating expressions inside lambda expressions works correctly."""
+self.build()
+(target, process, self.thread, bkpt) = \
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.cpp"))
+
+# Inside 'Foo::method'
+
+# Check access to captured 'this'
+self.expect_expr("class_var", result_type="int", result_value="109")
+self.expect_expr("this->class_var", result_type="int", result_value="109")
+
+# Check that captured shadowed variables take preference over the
+# correspond

[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Adrian Prantl via Phabricator via lldb-commits]

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

Nice work! I only had superficial comments, maybe let's also wait for @jingham 
to accept the patch.




Comment at: lldb/include/lldb/Expression/UserExpression.h:283
 
+  static lldb::ValueObjectSP
+  GetObjectPointerValueObject(StackFrame *frame, ConstString const 
&object_name,

Nit: The LLVM style wants doxygen comments in the .h file, would be nice to add 
one here.



Comment at: lldb/source/Expression/Materializer.cpp:432
+m_size = g_default_var_byte_size;
+m_alignment = g_default_var_alignment;
   }

FWIW, the refactoring of EntityVariable could have been a separate preparatory 
NFC patch, then the patch that adds the lambda functionality would be shorter. 
It's fine either way, but it's usually easier to review two simple comments 
instead of one complex one :-)



Comment at: lldb/source/Expression/Materializer.cpp:783
 
+class EntityVariable : public EntityVariableBase {
+public:

Maybe add a doxygen comment explaining when this subclass is used as opposed to 
EntityValueObject? 



Comment at: lldb/source/Expression/Materializer.cpp:819
+private:
+  lldb::VariableSP m_variable_sp; ///< Variable that this entity is based on
+  lldb::ValueObjectSP m_value_object_var; ///< ValueObjectVariable created from

Micro-nit: `.` at the end of sentence.



Comment at: lldb/source/Expression/Materializer.cpp:31
 
+static constexpr uint32_t g_default_var_alignment = 8;
+static constexpr uint32_t g_default_var_byte_size = 8;

jingham wrote:
> This seems weird to me.  You didn't do it, you're just replacing hard-coded 
> 8's later in the code.  But this seems like something we should be getting 
> from the target?  You don't need to solve this for this patch since it isn't 
> intrinsic to it, but at least leave a FIXME...
Yeah this is odd, maybe we can clean this up in a follow-up commit.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:837
+  TypeFromUser pointee_type =
+  capturedThis->GetCompilerType().GetPointeeType();
+

Nice!



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1151
+
+  if (!variable_found) {
+if (auto lambda = GetLambdaValueObject(frame)) {

Maybe comment that this is the lambda/this-capture path?



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1700
+  if (ClangExpressionVariable::ParserVars *parser_vars =
+  AddExpressionVariable(context, pt, ut, valobj)) {
+parser_vars->m_llvm_value = nullptr;

Nit:

LLVM style would be
```
ClangExpressionVariable::ParserVars *parser_vars =
  AddExpressionVariable(context, pt, ut, valobj);
if (!parser_vars)
  return;
```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h:570
 
+  bool GetVariableFromValueObject(CompilerType &comp_type,
+  lldb_private::Value &var_location,

Doxygen comment (unless this is is an override function (but it's not marked as 
such))?



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:222
+for (uint32_t i = 0; i < numChildren; ++i) {
+  auto childVal = thisValSP->GetChildAtIndex(i, true);
+  ConstString childName(childVal ? childVal->GetName() : ConstString(""));

at some point we should add a `children()` method that returns a range...



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:306
+
   for (size_t i = 0; i < var_list_sp->GetSize(); i++) {
 lldb::VariableSP var_sp = var_list_sp->GetVariableAtIndex(i);

not your code, but this one has an iterator :-)



Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp:1
+//===-- ClangExpressionUtil.cpp -*- C++ 
-*-===//
+//

the ` -*- C++ -*-` marker only makes sense for `.h` files, where it's ambiguous 
whether it's a C or a C++ header.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.h:132
  /// variable, if it was a symbol
+lldb::ValueObjectSP m_lldb_value_object; ///< ValueObject for this 
variable.
+ ///< Used when only an ivar is

Here a `///` comment on top of the declaration might be more readable. (Maybe 
fix this for the entire class in a follow-up commit)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

with latest changes LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81471

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

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D81471#3637057 , @werat wrote:

> In D81471#3632297 , @labath wrote:
>
>> I guess the condition we really want to express here is "does this 
>> expression refer to a constexpr variable (ideally one without a location)"? 
>> And the problem is that clang does not give us the means to detect that?
>>
>> Is that really the case? Would it maybe be possible to use some c++ magic to 
>> get clang to do that for us. Write something like `if constexpr 
>> (__builtin_constant_p(user_expression)) do_something_rvalue_like(); else 
>> assume_regular_lvalue();` ?
>
> I think you're right here, but I don't know a better way to express this. My 
> clangfu is not good enough for this yet :)
>
> As I understand it, we can't express it purely in the expression (via 
> `__builtin_constant_p` or something), because we need to know the answer 
> before executing the expression (it changes the way we do 
> materialization/dematerialization).

Well, yes, either that, or change how the dematerialization works -- prepare 
for both options and then choose the one that was actually used.

> Could this be something to improve in the future?

Well.. maybe... I don't know. I think this is definitely useful, and I don't 
see much of a downside. The main difference I see is that you won't be able to 
"modify a constant" via expressions like `p const_int`, `p (int&)$0 = 47`, but: 
a) you will still be able to do that within a single expression; and b) such an 
operation is unlikely to produce predictable results in anything but pure C 
(which doesn't really have constants).

So, if noone objects to this, then I think it's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81471

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

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

[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 D129166#3636658 , @dblaikie wrote:

>> The overall logic and the include and library paths make sense to me. The 
>> rpath thingy will not work on windows as it (afaik) has no equivalent 
>> feature (it has the equivalent of (DY)LD_LIBRARY_PATH though).
>
> Any idea what the libc++ tests do on Windows then? (on linux they seem to use 
> rpath to ensure the tests load the just-built libc++)

I believe the libc++ tests don't run at all on windows.


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

https://reviews.llvm.org/D129166

___
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.

I am not fully versed on the patch, but generally speaking, if you're 
reasonably confident that your change fixed the origianl problem, you can 
resubmit the patch (and watch the bot for problems). If you still don't know 
what's the issue, then we may need to find someone with an arm machine.


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] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good. The patch is really trivial so I don't want to make big deal out of 
the test.

However, I would like to continue the discussion started in the patch 
description, as I think it would be very useful if we could figure out a way to 
make this test (and others) cross-platform, so we don't have to have two 
versions of all of them. I'm going to start by asking a couple of questions, 
just so I understand the problem better.

You say that the issue is the lack of symtab in the "msvc" mode. What makes 
this test work then? Do some of the assembler directives (`.def`?) produce some 
sort of debug info entries?

What determines the modes that clang is operating in? Is it the default target 
triple?

What does `-Wl,-debug:symtab` actually produce? Would it make sense to make it 
a part of the `%clang_host` substitution (only for the msvc mode) to make the 
output of %clang_host more uniform? Or maybe achieve equivalence in some other 
way (add `-g1`, or similar, to the cmd line)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129455

___
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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> then we may need to find someone with an arm machine.

I tried this patch on AArch64 and did not get any new failures. You can go 
ahead and reland. If there's still issues I can help fix them.


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] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D129455#3641967 , @labath wrote:

> You say that the issue is the lack of symtab in the "msvc" mode. What makes 
> this test work then?

When invoked via the `%build` python script (lldb/test/Shell/helper/build.py), 
clang is invoked with extra options (`/Z7`) that generate codeview debug info, 
and then later the linker is invoked with extra options (`/DEBUG:FULL` and 
`'/PDB:' + self._pdb_file_name())`) to write that codeview debug info into a 
separate standalone PDB file.

In the MSVC ecosystem, executables/DLLs never have embedded debug info - it's 
always in a separate PDB file. Contrary to the GCC-compatible ecosystem, debug 
info is opt-in here, not opt-out (e.g. if linking with a unix linker, you get 
the symbol table and debug info included automatically unless you pass `-Wl,-s` 
or strip things separately afterwards). In the MSVC ecosystem, the only way to 
have a symbol table (without actual dwarf debug info) is with a PDB file.

In the mingw ecosystem, things behave as on unix - you get any embedded DWARF 
debug info included in the executable by default, and a symbol table - both 
which can be stripped out afterwards.

> Do some of the assembler directives (`.def`?) produce some sort of debug info 
> entries?

Those just set the symbol type to "function" - it could be omitted from this 
test for clarity.

> What determines the modes that clang is operating in? Is it the default 
> target triple?

Exactly.

Additionally, there's a couple more tooling differences that makes things a bit 
more complicated:

In the MSVC ecosystem, you normally would execute cl.exe (the MSVC compiler) or 
clang-cl (the cl.exe compatible clang frontend) - which has got an option 
syntax that is distinct and mostly incompatible from the gcc-compatible normal 
`clang` interface.

Despite this, you can also build code in MSVC mode with the regular 
gcc-compatible clang interface (either by passing e.g. 
`--target=x86_64-windows-msvc` or if such a triple is the deafult target 
triple). You can do most things in this setup too, but some MSVC-isms are 
tricky to achieve. This is what the `%clang_host` lit test macro does. Regular 
compiling, e.g. `%clang_host input.c -o executable -luser32` works fine for 
both MSVC and mingw mode.

In the MSVC ecosystem, you very rarely use the compiler driver to run linking - 
you'd normally execute `link.exe` or `lld-link` directly. If linking via the 
gcc-compatible clang interface, options passed to the linked with `-Wl,` need 
to be link.exe/lld-link options, which a unix-style linker obviously doesn't 
recognize.

> What does `-Wl,-debug:symtab` actually produce?

It produces an executable embedded symbol table, like in mingw mode. This is a 
lld specific option, MS's upstream link.exe doesn't support this option (it 
supports other parameters to `-debug:` but `symtab` is lld's invention). LLDB 
happily picks up and uses that (for mingw built executables it's the default 
case anyway).

> Would it make sense to make it a part of the `%clang_host` substitution (only 
> for the msvc mode) to make the output of %clang_host more uniform? Or maybe 
> achieve equivalence in some other way (add `-g1`, or similar, to the cmd 
> line)?

I think that might be a sensible path forward in general! But as noted, I'd 
rather not hold up this trivial patch with that. (Also I'm a bit more short on 
time at the moment than usual, but I'm trying to get low hanging fruit merged 
before the llvm 15 branch deadline.)

> as I think it would be very useful if we could figure out a way to make this 
> test (and others) cross-platform, so we don't have to have two versions of 
> all of them.

A bunch of tests might be doable crossplatform that way, but any tests that 
involve assembly might be tricky. On i386, arm and aarch64, Windows has got 
mostly the same calling conventions as other OSes, but on x86_64, it's 
radically different - arguments are passed in different registers than on 
x86_64 unix. Additionally, the caller is required to allocate shadow space on 
the stack for all arguments passed in registers (the extra `subq $32, %rsp`) so 
that the callee can dump them there if wanted. And finally, this particular 
testcase generates SEH unwind instructions with the `.seh_*` directives. (I 
didn't test whether the testcase would work without the SEH unwind instructions 
or not - I made it to look mostly like what a regular Windows function would 
look like.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129455

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

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D129455#3642015 , @mstorsjo wrote:

> In D129455#3641967 , @labath wrote:
>
>> What does `-Wl,-debug:symtab` actually produce? Would it make sense to make 
>> it a part of the `%clang_host` substitution (only for the msvc mode) to make 
>> the output of %clang_host more uniform? Or maybe achieve equivalence in some 
>> other way (add `-g1`, or similar, to the cmd line)?
>
> I think that might be a sensible path forward in general!

If we'd want to take that even further, we could consider to have `%clang_host` 
add options for generating DWARF debug info even in MSVC mode, and pass 
`-Wl,-debug:dwarf` to include it embedded in the exe like in mingw mode. (I've 
understood that some people even do this in proper production setups.) It's not 
what you'd normally have in MSVC mode, but could be useful for making testcases 
more portable, for cases where such details don't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129455

___
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

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu added a comment.

In D50304#3641971 , @DavidSpickett 
wrote:

>> then we may need to find someone with an arm machine.
>
> I tried this patch on AArch64 and did not get any new failures. You can go 
> ahead and reland. If there's still issues I can help fix them.

Thank you very much @DavidSpickett.


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] [lldb] 419cc0a - [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via lldb-commits]

Author: Venkata Ramanaiah Nalamothu
Date: 2022-07-11T18:45:37+05:30
New Revision: 419cc0a0b2ab7306dd721c337e7ce6ed31dc7287

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

LOG: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line 
numbers

The requirements for "thread until " are:

a) If any code contributed by  or the nearest subsequent of  is executed before leaving the function, stop
b) If you end up leaving the function w/o triggering (a), then stop

In case of (a), since the  may have multiple entries in the line 
table and the compiler might have scheduled/moved the relevant code across, and 
the lldb does not know the control flow, set breakpoints on all the line table 
entries of best match of  i.e. exact or the nearest subsequent 
line.

Along with the above, currently, CommandObjectThreadUntil is also setting the 
breakpoints on all the subsequent line numbers after the best match and this 
latter part is wrong.

This issue is discussed at 
http://lists.llvm.org/pipermail/lldb-dev/2018-August/013979.html.

In fact, currently `TestStepUntil.py` is not actually testing step until 
scenarios and `test_missing_one` test fails without this patch if tests are 
made to run. Fixed the test as well.

Reviewed By: jingham

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectThread.cpp
lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
lldb/test/API/functionalities/thread/step_until/main.c

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 993523e06736..9211480fa9f8 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -1033,11 +1033,21 @@ class CommandObjectThreadUntil : public 
CommandObjectParsed {
 line_table->FindLineEntryByAddress(fun_end_addr, function_start,
&end_ptr);
 
+// Since not all source lines will contribute code, check if we are
+// setting the breakpoint on the exact line number or the nearest
+// subsequent line number and set breakpoints at all the line table
+// entries of the chosen line number (exact or nearest subsequent).
 for (uint32_t line_number : line_numbers) {
+  LineEntry line_entry;
+  bool exact = false;
   uint32_t start_idx_ptr = index_ptr;
+  start_idx_ptr = sc.comp_unit->FindLineEntry(
+  index_ptr, line_number, nullptr, exact, &line_entry);
+  if (start_idx_ptr != UINT32_MAX)
+line_number = line_entry.line;
+  exact = true;
+  start_idx_ptr = index_ptr;
   while (start_idx_ptr <= end_ptr) {
-LineEntry line_entry;
-const bool exact = false;
 start_idx_ptr = sc.comp_unit->FindLineEntry(
 start_idx_ptr, line_number, nullptr, exact, &line_entry);
 if (start_idx_ptr == UINT32_MAX)

diff  --git a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py 
b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
index 0145b34f31de..ee25d1343735 100644
--- a/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
+++ b/lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
@@ -19,7 +19,7 @@ def setUp(self):
 self.greater_than_two = line_number('main.c', 'Greater than or equal 
to 2.')
 self.back_out_in_main = line_number('main.c', 'Back out in main')
 
-def do_until (self, args, until_lines, expected_linenum):
+def common_setup (self, args):
 self.build()
 exe = self.getBuildArtifact("a.out")
 
@@ -48,7 +48,8 @@ def do_until (self, args, until_lines, expected_linenum):
 thread = threads[0]
 return thread
 
-thread = self.common_setup(None)
+def do_until (self, args, until_lines, expected_linenum):
+thread = self.common_setup(args)
 
 cmd_interp = self.dbg.GetCommandInterpreter()
 ret_obj = lldb.SBCommandReturnObject()
@@ -77,7 +78,7 @@ def test_targetting_two_hitting_second (self):
 self.do_until(None, [self.less_than_two, self.greater_than_two], 
self.less_than_two)
 
 def test_missing_one (self):
-"""Test thread step until - targeting one line and missing it."""
+"""Test thread step until - targeting one line and missing it by 
stepping out to call site"""
 self.do_until(["foo", "bar", "baz"], [self.less_than_two], 
self.back_out_in_main)
 
 

diff  --git a/lldb/test/API/functionalities/thread/step_until/main.c 
b/lldb/test/API/functionalities/thread/step_until/main.c
index e0b4d8ab951e..bb866079cf5f 100644
--- a/l

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

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG419cc0a0b2ab: [lldb] Fix thread step until to not set 
breakpoint(s) on incorrect line numbers (authored by RamNalamothu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

Files:
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
  lldb/test/API/functionalities/thread/step_until/main.c


Index: lldb/test/API/functionalities/thread/step_until/main.c
===
--- lldb/test/API/functionalities/thread/step_until/main.c
+++ lldb/test/API/functionalities/thread/step_until/main.c
@@ -1,20 +1,25 @@
 #include 
 
-void call_me(int argc)
-{
+/* The return statements are purposefully so simple, and
+ * unrelated to the program, just to achieve consistent
+ * debug line tables, across platforms, that are not
+ * dependent on compiler optimzations. */
+int call_me(int argc) {
   printf ("At the start, argc: %d.\n", argc);
 
   if (argc < 2)
-printf("Less than 2.\n");
+return 1; /* Less than 2. */
   else
-printf("Greater than or equal to 2.\n");
+return argc; /* Greater than or equal to 2. */
 }
 
 int
 main(int argc, char **argv)
 {
-  call_me(argc);
-  printf("Back out in main.\n");
+  int res = 0;
+  res = call_me(argc); /* Back out in main. */
+  if (res)
+printf("Result: %d. \n", res);
 
   return 0;
 }
Index: lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
===
--- lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
+++ lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
@@ -19,7 +19,7 @@
 self.greater_than_two = line_number('main.c', 'Greater than or equal 
to 2.')
 self.back_out_in_main = line_number('main.c', 'Back out in main')
 
-def do_until (self, args, until_lines, expected_linenum):
+def common_setup (self, args):
 self.build()
 exe = self.getBuildArtifact("a.out")
 
@@ -48,7 +48,8 @@
 thread = threads[0]
 return thread
 
-thread = self.common_setup(None)
+def do_until (self, args, until_lines, expected_linenum):
+thread = self.common_setup(args)
 
 cmd_interp = self.dbg.GetCommandInterpreter()
 ret_obj = lldb.SBCommandReturnObject()
@@ -77,7 +78,7 @@
 self.do_until(None, [self.less_than_two, self.greater_than_two], 
self.less_than_two)
 
 def test_missing_one (self):
-"""Test thread step until - targeting one line and missing it."""
+"""Test thread step until - targeting one line and missing it by 
stepping out to call site"""
 self.do_until(["foo", "bar", "baz"], [self.less_than_two], 
self.back_out_in_main)
 
 
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1033,11 +1033,21 @@
 line_table->FindLineEntryByAddress(fun_end_addr, function_start,
&end_ptr);
 
+// Since not all source lines will contribute code, check if we are
+// setting the breakpoint on the exact line number or the nearest
+// subsequent line number and set breakpoints at all the line table
+// entries of the chosen line number (exact or nearest subsequent).
 for (uint32_t line_number : line_numbers) {
+  LineEntry line_entry;
+  bool exact = false;
   uint32_t start_idx_ptr = index_ptr;
+  start_idx_ptr = sc.comp_unit->FindLineEntry(
+  index_ptr, line_number, nullptr, exact, &line_entry);
+  if (start_idx_ptr != UINT32_MAX)
+line_number = line_entry.line;
+  exact = true;
+  start_idx_ptr = index_ptr;
   while (start_idx_ptr <= end_ptr) {
-LineEntry line_entry;
-const bool exact = false;
 start_idx_ptr = sc.comp_unit->FindLineEntry(
 start_idx_ptr, line_number, nullptr, exact, &line_entry);
 if (start_idx_ptr == UINT32_MAX)


Index: lldb/test/API/functionalities/thread/step_until/main.c
===
--- lldb/test/API/functionalities/thread/step_until/main.c
+++ lldb/test/API/functionalities/thread/step_until/main.c
@@ -1,20 +1,25 @@
 #include 
 
-void call_me(int argc)
-{
+/* The return statements are purposefully so simple, and
+ * unrelated to the program, just to achieve consistent
+ * debug line tables, across platforms, that are not
+ * dependent on compiler optimzations. */
+int call_me(int argc) {
   printf ("At the start, argc: %d.\n", argc);
 
   if (argc < 2)
-printf("Less than 2.\n");
+return 1; /* Less 

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Well... I'm as puzzled as you are. Whatever the problem is, it seems to be 
limited to debian (kernel). I've filed a bug about that: 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1014754.

One good thing I learned is that this problem seems to be limited to SIGSTOPs, 
so it should be possible to work around it by using some other signal. That's 
probably a good idea anyway, since SIGSTOP has the unpleasant property of 
leaving around a stopped zombie if something goes wrong (that's what killed the 
bot over the weekend).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129012

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

[Lldb-commits] [PATCH] D129487: [lldb][AArch64] Add UnpackTagsFromCoreFileSegment to MemoryTagManager

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This is the first part of support for reading MTE tags from Linux
core files. The format is documented here:
https://www.kernel.org/doc/html/latest/arm64/memory-tagging-extension.html#core-dump-support

This patch adds a method to unpack from the format the core
file uses, which is different to the one chosen for GDB packets.

MemoryTagManagerAArch64MTE is not tied one OS so another OS
might choose a different format in future. However, infrastructure
to handle that would go untested until then so I've chosen not to
attempt to handle that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129487

Files:
  lldb/include/lldb/Target/MemoryTagManager.h
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp

Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
===
--- lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
+++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
@@ -80,6 +80,72 @@
   ASSERT_THAT(expected, testing::ContainerEq(*packed));
 }
 
+TEST(MemoryTagManagerAArch64MTETest, UnpackTagsFromCoreFileSegment) {
+  MemoryTagManagerAArch64MTE manager;
+  // This is our fake segment data where tags are compressed as 2 4 bit tags
+  // per byte.
+  std::vector tags_data;
+  MemoryTagManager::CoreReaderFn reader =
+  [&tags_data](lldb::offset_t offset, size_t length, void *dst) {
+std::memcpy(dst, tags_data.data() + offset, length);
+return length;
+  };
+
+  // Zero length is ok.
+  std::vector tags =
+  manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 0);
+  ASSERT_EQ(tags.size(), (size_t)0);
+
+  // In the simplest case we read 2 tags which are in the same byte.
+  tags_data.push_back(0x21);
+  // The least significant bits are the first tag in memory.
+  std::vector expected{1, 2};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 32);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // If we read just one then it will have to trim off the second one.
+  expected = std::vector{1};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 16);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // If we read the second tag only then the first one must be trimmed.
+  expected = std::vector{2};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 16);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // This trimming logic applies if you read a larger set of tags.
+  tags_data = std::vector{0x21, 0x43, 0x65, 0x87};
+
+  // Trailing tag should be trimmed.
+  expected = std::vector{1, 2, 3};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 0, 48);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // Leading tag should be trimmed.
+  expected = std::vector{2, 3, 4};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 48);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // Leading and trailing trimmmed.
+  expected = std::vector{2, 3, 4, 5};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 0, 16, 64);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // The address given is an offset into the whole file so the address requested
+  // from the reader should be beyond that.
+  tags_data = std::vector{0xFF, 0xFF, 0x21, 0x43, 0x65, 0x87};
+  expected = std::vector{1, 2};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 0, 2, 0, 32);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+
+  // addr is a virtual address that we expect to be >= the tag segment's
+  // starting virtual address. So again an offset must be made from the
+  // difference.
+  expected = std::vector{3, 4};
+  tags = manager.UnpackTagsFromCoreFileSegment(reader, 32, 2, 64, 32);
+  ASSERT_THAT(expected, testing::ContainerEq(tags));
+}
+
 TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) {
   MemoryTagManagerAArch64MTE manager;
 
Index: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
===
--- lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
+++ lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
@@ -44,6 +44,12 @@
   UnpackTagsData(const std::vector &tags,
  size_t granules = 0) const override;
 
+  std::vector
+  UnpackTagsFromCoreFileSegment(CoreReaderFn reader,
+lldb::addr_t tag_segment_virtual_address,
+lldb::addr_t tag_segment_data_address,
+lldb::addr_

[Lldb-commits] [PATCH] D129489: [lldb][AArch64] Add support for memory tags in core files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

This teaches ProcessElfCore to recognise the MTE tag segments.

https://www.kernel.org/doc/html/latest/arm64/memory-tagging-extension.html#core-dump-support

These segments contain all the tags for a matching memory segment
which will have the same size in virtual address terms. In real terms
it's 2 tags per byte so the data in the segment is much smaller.

Since MTE is the only tag type supported I have hardcoded some
things to those values. We could and should support more formats
as they appear but doing so now would leave code untested until that
happens.

A few things to note:

- /proc/pid/smaps is not in the core file, only the details you have in "maps". 
Meaning we mark a region tagged only if it has a tag segment.
- A core file supports memory tagging if it has at least 1 memory tag segment, 
there is no other flag we can check to tell if memory tagging was enabled. 
(unlike a live process that can support memory tagging even if there are 
currently no tagged memory regions)

Tests have been added at the commands level for a core file with
mte and without.

There is a lot of overlap between the "memory tag read" tests here and the unit 
tests for
MemoryTagManagerAArch64MTE::UnpackTagsFromCoreFileSegment, but I think it's
worth keeping to check ProcessElfCore doesn't cause an assert.

Depends on D129487 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129489

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
  
lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
  lldb/test/API/linux/aarch64/mte_core_file/core.mte
  lldb/test/API/linux/aarch64/mte_core_file/core.nomte
  lldb/test/API/linux/aarch64/mte_core_file/main.c
  llvm/include/llvm/BinaryFormat/ELF.h

Index: llvm/include/llvm/BinaryFormat/ELF.h
===
--- llvm/include/llvm/BinaryFormat/ELF.h
+++ llvm/include/llvm/BinaryFormat/ELF.h
@@ -1366,6 +1366,8 @@
   // These all contain stack unwind tables.
   PT_ARM_EXIDX = 0x7001,
   PT_ARM_UNWIND = 0x7001,
+  // MTE memory tag segment type
+  PT_AARCH64_MEMTAG_MTE = 0x7002,
 
   // MIPS program header types.
   PT_MIPS_REGINFO = 0x7000,  // Register usage information.
Index: lldb/test/API/linux/aarch64/mte_core_file/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_core_file/main.c
@@ -0,0 +1,78 @@
+// Program to generate core files to test MTE tag features.
+//
+// This file uses ACLE intrinsics as detailed in:
+// https://developer.arm.com/documentation/101028/0012/10--Memory-tagging-intrinsics?lang=en
+//
+// Compile with:
+//  -march=armv8.5-a+memtag -g main.c -o a.out.mte
+//  -march=armv8.5-a+memtag -g main.c -DNO_MTE -o a.out.nomte
+//
+// /proc/self/coredump_filter was set to 2 when the core files were made.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+#ifdef NO_MTE
+  *(char *)(0) = 0;
+#endif
+
+  if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+return 1;
+  }
+
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  char *mte_buf = mmap(0, page_size, PROT_READ | PROT_WRITE | PROT_MTE,
+   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (!mte_buf)
+return 1;
+
+  printf("mte_buf: %p\n", mte_buf);
+
+  // Allocate some untagged memory before the tagged memory.
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE,
+   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (!buf)
+return 1;
+
+  printf("buf: %p\n", buf);
+
+  // This write means that the memory for buf is included in the corefile.
+  // So we can read from the end of it into mte_buf during the test.
+  *buf = 1;
+
+  // These must be next to each other for the tests to work.
+  // 
+  // mte_buf
+  // buf
+  // 
+  if ((mte_buf - buf) != page_size) {
+return 1;
+  }
+
+  // Set incrementing tags until end of the page.
+  char *tagged_ptr = mte_buf;
+  // This ignores tag bits when subtracting the addresses.
+  while (__arm_mte_ptrdiff(tagged_ptr, mte_buf) < page_size) {
+// Set the allocation tag for this location.
+__arm_mte_set_tag(tagged_ptr);
+// + 16 for 16 byte granules.
+// Earlier we allowed all tag values, so this will give us an
+// incrementing pattern 0-0xF wrapping back to 0.
+  

[Lldb-commits] [PATCH] D129489: [lldb][AArch64] Add support for memory tags in core files

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added reviewers: omjavaid, labath.
DavidSpickett added a comment.
Herald added a subscriber: JDevlieghere.

This change is purely about reading the tags.

There will be another change to get fault reporting working. Right now lldb 
only looks at `signo` so you see a segfault but not what type of segfault.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129489

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

[Lldb-commits] [PATCH] D129490: [lldb/libc++] Simplify the libc++ string formatter

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, mib.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

Precise string layout has changed a lot recently, but a long of these
changes did not have any effect on the usages of its fields -- e.g.
introduction/removal of an anonymous struct or union does not change the
way one can access the field in C++. Our name-based variable lookup
rules (deliberately) copy the C++ semantics, which means these changes
would have been invisible to the them, if only we were using name-based
lookup.

This patch replaces the opaque child index accesses with name-based
lookups, which allows us to greatly simplify the data formatter code.
The formatter continues to support all the string layouts that it
previously supported.

It is unclear why the formatter was not using this approach from the
beginning. I would speculate that the original version was working
around some (now fixed) issue with anonymous members or base classes,
and the subsequent revisions stuck with that approach out of inertia.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129490

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
@@ -547,101 +547,80 @@
 }
 
 /// The field layout in a libc++ string (cap, side, data or data, size, cap).
-enum LibcxxStringLayoutMode {
-  eLibcxxStringLayoutModeCSD = 0,
-  eLibcxxStringLayoutModeDSC = 1,
-  eLibcxxStringLayoutModeInvalid = 0x
-};
+namespace {
+enum class StringLayout { CSD, DSC };
+}
 
 /// Determine the size in bytes of \p valobj (a libc++ std::string object) and
 /// extract its data payload. Return the size + payload pair.
 // TODO: Support big-endian architectures.
 static llvm::Optional>
 ExtractLibcxxStringInfo(ValueObject &valobj) {
-  ValueObjectSP dataval_sp(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
-  if (!dataval_sp)
+  ValueObjectSP valobj_r_sp =
+  valobj.GetChildMemberWithName(ConstString("__r_"), /*can_create=*/true);
+  if (!valobj_r_sp)
 return {};
-  if (!dataval_sp->GetError().Success())
+
+  // __r_ is a compressed_pair of the actual data and the allocator. The data we
+  // want is in the first base class.
+  ValueObjectSP valobj_r_base_sp =
+  valobj_r_sp->GetChildAtIndex(0, /*can_create=*/true);
+  if (!valobj_r_base_sp)
 return {};
 
-  ValueObjectSP layout_decider(
-  dataval_sp->GetChildAtIndexPath(llvm::ArrayRef({0, 0})));
+  ValueObjectSP valobj_rep_sp = valobj_r_base_sp->GetChildMemberWithName(
+  ConstString("__value_"), /*can_create=*/true);
+  if (!valobj_rep_sp)
+return {};
 
-  // this child should exist
-  if (!layout_decider)
+  ValueObjectSP l = valobj_rep_sp->GetChildMemberWithName(ConstString("__l"),
+  /*can_create=*/true);
+  if (!l)
 return {};
 
-  ConstString g_data_name("__data_");
-  ConstString g_size_name("__size_");
+  StringLayout layout = l->GetIndexOfChildWithName(ConstString("__data_")) == 0
+? StringLayout::DSC
+: StringLayout::CSD;
+
   bool short_mode = false; // this means the string is in short-mode and the
// data is stored inline
   bool using_bitmasks = true; // Whether the class uses bitmasks for the mode
   // flag (pre-D123580).
   uint64_t size;
-  LibcxxStringLayoutMode layout = (layout_decider->GetName() == g_data_name)
-  ? eLibcxxStringLayoutModeDSC
-  : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
+  ValueObjectSP short_sp = valobj_rep_sp->GetChildMemberWithName(
+  ConstString("__s"), /*can_create=*/true);
   if (!short_sp)
 return {};
 
-  ValueObjectSP short_fields_sp;
   ValueObjectSP is_long =
   short_sp->GetChildMemberWithName(ConstString("__is_long_"), true);
-  if (is_long) {
-short_fields_sp = short_sp;
-  } else {
-// After D128285, we need to access the `__is_long_` and `__size_` fields
-// from a packed anonymous struct
-short_fields_sp = short_sp->GetChildAtIndex(0, true);
-is_long = short_sp->GetChildMemberWithName(ConstString("__is_long_"), true);
-  }
 
   if (is_long) {
 using_bitmasks = false;
 short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
 if (ValueObjectSP size_member =
-dataval_sp->GetChildAtNamePath({ConstString("__s"), ConstString("__size_")}))
+short_sp->GetChildAtNamePath({ConstString("__size_")}))
   size = size_member->GetValueAsUnsigned(/*fail_value=*/0);
 else
   return {};
-  } else if (layou

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

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

Heh, and I actually thought using `SIGSTOP` was a good idea because it's 
unlikely to have any unexpected side effects ;-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129012

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

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

mgorny added a comment.

Also, big, big thanks for helping out with this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129012

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

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

[David M. Lary via Phabricator via lldb-commits]

dmlary added a comment.

I do not have commit access; could someone merge this patch as time allows?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128069

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

[Lldb-commits] [lldb] 9790406 - Reland "[lldb] [test] Improve stability of llgs vCont-threads tests"

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-07-11T18:05:14+02:00
New Revision: 9790406a9226e112e75eeeac1c326cff9b570264

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

LOG: Reland "[lldb] [test] Improve stability of llgs vCont-threads tests"

Perform a major refactoring of vCont-threads tests in order to attempt
to improve their stability and performance.

Split test_vCont_run_subset_of_threads() into smaller test cases,
and split the whole suite into two files: one for signal-related tests,
the running-subset-of tests.

Eliminate output_match checks entirely, as they are fragile to
fragmentation of output.  Instead, for the initial thread list capture
raise an explicit SIGINT from inside the test program, and for
the remaining output let the test program run until exit, and check all
the captured output afterwards.

For resume tests, capture the LLDB's thread view before and after
starting new threads in order to determine the IDs corresponding
to subthreads rather than relying on program output for that.

Add a mutex for output to guarantee serialization.  A barrier is used
to guarantee that all threads start before SIGINT, and an atomic bool
is used to delay prints from happening until after SIGINT.

Call std::this_thread::yield() to reduce the risk of one of the threads
not being run.

This fixes the test hangs on FreeBSD.  Hopefully, it will also fix all
the flakiness on buildbots.

Thanks to Pavel Labath for figuring out why the original version did not
work on Debian.

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

Added: 
lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py

Modified: 
lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Removed: 
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py



diff  --git 
a/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
new file mode 100644
index 0..2cc60d3d0a5c0
--- /dev/null
+++ b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
@@ -0,0 +1,128 @@
+import re
+
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestPartialResume(gdbremote_testcase.GdbRemoteTestCaseBase):
+THREAD_MATCH_RE = re.compile(r"thread ([0-9a-f]+) running")
+
+def start_vCont_run_subset_of_threads_test(self):
+self.build()
+self.set_inferior_startup_launch()
+
+procs = self.prep_debug_monitor_and_inferior(inferior_args=["3"])
+# grab the main thread id
+self.add_threadinfo_collection_packets()
+main_thread = self.parse_threadinfo_packets(
+self.expect_gdbremote_sequence())
+self.assertEqual(len(main_thread), 1)
+self.reset_test_sequence()
+
+# run until threads start, then grab full thread list
+self.test_sequence.add_log_lines([
+"read packet: $c#63",
+{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
+], True)
+self.add_threadinfo_collection_packets()
+
+all_threads = self.parse_threadinfo_packets(
+self.expect_gdbremote_sequence())
+self.assertEqual(len(all_threads), 4)
+self.assertIn(main_thread[0], all_threads)
+self.reset_test_sequence()
+
+all_subthreads = set(all_threads) - set(main_thread)
+self.assertEqual(len(all_subthreads), 3)
+
+return (main_thread[0], list(all_subthreads))
+
+def continue_and_get_threads_running(self, main_thread, vCont_req):
+self.test_sequence.add_log_lines(
+["read packet: $vCont;c:{:x};{}#00".format(main_thread, vCont_req),
+ "send packet: $W00#00",
+ ], True)
+exp = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+found = set()
+for line in exp["O_content"].decode().splitlines():
+m = self.THREAD_MATCH_RE.match(line)
+if m is not None:
+found.add(int(m.group(1), 16))
+return found
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_vCont_cxcx(self):
+main_thread, all_subthreads_list = (
+self.start_vCont_run_subset_of_threads_test())
+# resume two threads explicitly, stop the third one implicitly
+self.assertEqual(
+self.continue_and_get_threads_running(
+main_thread,
+"c:{:x};c:{:x}".format(*all_subthreads_list[:2])),
+set(all_subthreads_list[:2]))
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_vCont_cxcxt(sel

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9790406a9226: Reland "[lldb] [test] Improve stability 
of llgs vCont-threads tests" (authored by mgorny).

Changed prior to commit:
  https://reviews.llvm.org/D129012?vs=442916&id=443668#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129012

Files:
  lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
  lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Index: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
===
--- lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
+++ lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
@@ -1,31 +1,56 @@
 #include "pseudo_barrier.h"
 #include "thread.h"
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
 pseudo_barrier_t barrier;
+std::mutex print_mutex;
+std::atomic can_work = ATOMIC_VAR_INIT(false);
+thread_local volatile sig_atomic_t can_exit_now = false;
+
+static void sigint_handler(int signo) {}
 
 static void sigusr1_handler(int signo) {
-  char buf[100];
-  std::snprintf(buf, sizeof(buf),
-"received SIGUSR1 on thread id: %" PRIx64 "\n",
-get_thread_id());
-  write(STDOUT_FILENO, buf, strlen(buf));
+  std::lock_guard lock{print_mutex};
+  std::printf("received SIGUSR1 on thread id: %" PRIx64 "\n", get_thread_id());
+  can_exit_now = true;
 }
 
 static void thread_func() {
+  // this ensures that all threads start before we stop
   pseudo_barrier_wait(barrier);
-  for (int i = 0; i < 300; ++i) {
+
+  // wait till the main thread indicates that we can go
+  // (note: using a mutex here causes hang on FreeBSD when another thread
+  // is suspended)
+  while (!can_work.load())
+std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+  // the mutex guarantees that two writes don't get interspersed
+  {
+std::lock_guard lock{print_mutex};
 std::printf("thread %" PRIx64 " running\n", get_thread_id());
+  }
+
+  // give other threads a fair chance to run
+  for (int i = 0; i < 5; ++i) {
+std::this_thread::yield();
 std::this_thread::sleep_for(std::chrono::milliseconds(200));
+if (can_exit_now)
+  return;
   }
+
+  // if we didn't get signaled, terminate the program explicitly.
+  _exit(0);
 }
 
 int main(int argc, char **argv) {
@@ -33,15 +58,19 @@
 
   pseudo_barrier_init(barrier, num + 1);
 
+  signal(SIGINT, sigint_handler);
   signal(SIGUSR1, sigusr1_handler);
 
   std::vector threads;
-  for(int i = 0; i < num; ++i)
+  for (int i = 0; i < num; ++i)
 threads.emplace_back(thread_func);
 
+  // use the barrier to make sure all threads start before we stop
   pseudo_barrier_wait(barrier);
+  std::raise(SIGINT);
 
-  std::puts("@started");
+  // allow the threads to work
+  can_work.store(true);
 
   for (std::thread &thread : threads)
 thread.join();
Index: lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
===
--- lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
+++ lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
@@ -1,23 +1,18 @@
-import json
 import re
-import time
 
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestGdbRemote_vContThreads(gdbremote_testcase.GdbRemoteTestCaseBase):
 
+class TestSignal(gdbremote_testcase.GdbRemoteTestCaseBase):
 def start_threads(self, num):
 procs = self.prep_debug_monitor_and_inferior(inferior_args=[str(num)])
-# start the process and wait for output
 self.test_sequence.add_log_lines([
 "read packet: $c#63",
-{"type": "output_match", "regex": r".*@started\r\n.*"},
+{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
 ], True)
-# then interrupt it
-self.add_interrupt_packets()
 self.add_threadinfo_collection_packets()
 
 context = self.expect_gdbremote_sequence()
@@ -29,22 +24,21 @@
 self.reset_test_sequence()
 return threads
 
+SIGNAL_MATCH_RE = re.compile(r"received SIGUSR1 on thread id: ([0-9a-f]+)")
+
 def send_and_check_signal(self, vCont_data, threads):
 self.test_sequence.add_log_lines([
 "read packet: $vCont;{0}#00".format(vCont_data),
-{"type": "output_match",
- "regex": len(threads) *
-  r".*received SIGUSR1 on thread id: ([0-9a-f]+)\r\n.*",
- "capture": dict((i, "tid{0}".format(i)) for i
- in range(1, len(threads)+1)),
- },
+

[Lldb-commits] [lldb] 9302ff0 - Revert "jGetLoadedDynamicLibrariesInfos can inspect machos not yet loaded"

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-07-11T09:25:14-07:00
New Revision: 9302ff095168875a6a56e79d3401724e7b0069fd

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

LOG: Revert "jGetLoadedDynamicLibrariesInfos can inspect machos not yet loaded"

This reverts commit 77a38f6839980bfac61babb40d83772c51427011 because (I
suspect) it breaks TestAppleSimulatorOSType.py on GreenDragon [1].

[1] https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45191/

Added: 


Modified: 
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm

Removed: 
lldb/test/API/macosx/unregistered-macho/Makefile
lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py
lldb/test/API/macosx/unregistered-macho/main.c



diff  --git a/lldb/test/API/macosx/unregistered-macho/Makefile 
b/lldb/test/API/macosx/unregistered-macho/Makefile
deleted file mode 100644
index 58c3a468659e3..0
--- a/lldb/test/API/macosx/unregistered-macho/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-C_SOURCES = main.c
-
-include Makefile.rules

diff  --git a/lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py 
b/lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py
deleted file mode 100644
index 0c3bb50aec89a..0
--- a/lldb/test/API/macosx/unregistered-macho/TestUnregisteredMacho.py
+++ /dev/null
@@ -1,47 +0,0 @@
-"""Test that debugserver will parse a mach-o in inferior memory even if it's 
not loaded."""
-
-import os
-import re
-import subprocess
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-class TestUnregisteredMacho(TestBase):
-
-# newer debugserver required for jGetLoadedDynamicLibrariesInfos 
-# to support this
-@skipIfOutOfTreeDebugserver  
-@no_debug_info_test
-@skipUnlessDarwin
-def test(self):
-self.build()
-target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
-self, "// break here", lldb.SBFileSpec("main.c"))
-
-frame = thread.GetFrameAtIndex(0)
-macho_buf = frame.GetValueForVariablePath("macho_buf")
-macho_addr = macho_buf.GetValueAsUnsigned()
-invalid_macho_addr = macho_buf.GetValueAsUnsigned() + 4
-gdb_packet = "process plugin packet send 
'jGetLoadedDynamicLibrariesInfos:{\"solib_addresses\":[%d]}]'" % macho_addr
-
-# Send the jGetLoadedDynamicLibrariesInfos packet
-# to debugserver, asking it to parse the mach-o binary
-# at this address and give us the UUID etc, even though
-# dyld doesn't think there is a binary at that address.
-# We won't get a pathname for the binary (from dyld), but
-# we will get to the LC_UUID and include that.
-self.expect (gdb_packet, substrs=['"pathname":""', 
'"uuid":"1B4E28BA-2FA1-11D2-883F-B9A761BDE3FB"'])
-
-no_macho_gdb_packet = "process plugin packet send 
'jGetLoadedDynamicLibrariesInfos:{\"solib_addresses\":[%d]}]'" % 
invalid_macho_addr
-self.expect (no_macho_gdb_packet, substrs=['response: {"images":[]}'])
-
-# Test that we get back the information for the properly
-# formatted Mach-O binary in memory, but do not get an
-# entry for the invalid Mach-O address.
-both_gdb_packet = "process plugin packet send 
'jGetLoadedDynamicLibrariesInfos:{\"solib_addresses\":[%d,%d]}]'" % 
(macho_addr, invalid_macho_addr)
-self.expect (both_gdb_packet, substrs=['"load_address":%d,' % 
macho_addr])
-self.expect (both_gdb_packet, substrs=['"load_address":%d,' % 
invalid_macho_addr], matching=False)
-

diff  --git a/lldb/test/API/macosx/unregistered-macho/main.c 
b/lldb/test/API/macosx/unregistered-macho/main.c
deleted file mode 100644
index 5de4b5f642467..0
--- a/lldb/test/API/macosx/unregistered-macho/main.c
+++ /dev/null
@@ -1,63 +0,0 @@
-#include 
-#include 
-#include 
-#include 
-#include 
-
-int main() {
-  int size_of_load_cmds =
-  sizeof(struct segment_command_64) + sizeof(struct uuid_command);
-  uint8_t *macho_buf =
-  (uint8_t *)malloc(sizeof(struct mach_header_64) + size_of_load_cmds);
-  uint8_t *p = macho_buf;
-  struct mach_header_64 mh;
-  mh.magic = MH_MAGIC_64;
-  mh.cputype = CPU_TYPE_ARM64;
-  mh.cpusubtype = 0;
-  mh.filetype = MH_EXECUTE;
-  mh.ncmds = 2;
-  mh.sizeofcmds = size_of_load_cmds;
-  mh.flags = MH_NOUNDEFS | MH_DYLDLINK | MH_TWOLEVEL | MH_PIE;
-
-  memcpy(p, &mh, sizeof(mh));
-  p += sizeof(mh);
-
-  struct segment_command_64 seg;
-  seg.cmd = LC_SEGMENT_64;
-  seg.cmdsize = sizeof(seg);
-  strcpy(seg.segname, "__TEXT");
-  seg.vmaddr = 0x5000;
-  seg.vmsize = 0x1000;
-  seg.fileoff = 0;
-  seg.filesize = 0;
-  seg.maxprot = 0;
-  seg

[Lldb-commits] [lldb] f921985 - Rebase: [Facebook] Add clang driver options to test debug info and BOLT

[via lldb-commits]

Author: Amir Ayupov
Date: 2022-07-11T09:31:51-07:00
New Revision: f921985a29fc9787b3ed98dbc897146cc3fd91f7

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

LOG: Rebase: [Facebook] Add clang driver options to test debug info and BOLT

Summary:
This is an essential piece of infrastructure for us to be
continuously testing debug info with BOLT. We can't only make changes
to a test repo because we need to change debuginfo tests to call BOLT,
hence, this diff needs to sit in our opensource repo. But when upstreaming
to LLVM, this should be kept BOLT-only outside of LLVM. When upstreaming,
we need to git diff and check all folders that are being modified by our
commits and discard this one (and leave as an internal diff).

To test BOLT in debuginfo tests, configure it with -DLLVM_TEST_BOLT=ON.
Then run check-lldb and check-debuginfo.

Manual rebase conflict history:
https://phabricator.intern.facebook.com/D29205224
https://phabricator.intern.facebook.com/D29564078
https://phabricator.intern.facebook.com/D33289118
https://phabricator.intern.facebook.com/D34957174

Test Plan:
tested locally
Configured with:
-DLLVM_ENABLE_PROJECTS="clang;lld;lldb;compiler-rt;bolt;debuginfo-tests"
-DLLVM_TEST_BOLT=ON
Ran test suite with:
ninja check-debuginfo
ninja check-lldb

Reviewers: #llvm-bolt

Subscribers: ayermolo, phabricatorlinter

Differential Revision: https://phabricator.intern.facebook.com/D35317341

Tasks: T92898286

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Gnu.cpp
cross-project-tests/lit.cfg.py
cross-project-tests/lit.site.cfg.py.in
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in
lldb/test/Shell/helper/toolchain.py
lldb/test/Shell/lit.site.cfg.py.in
llvm/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 8ae9145a271a5..44ea906be543c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4001,6 +4001,10 @@ def pg : Flag<["-"], "pg">, HelpText<"Enable mcount 
instrumentation">, Flags<[CC
   MarshallingInfoFlag>;
 def pipe : Flag<["-", "--"], "pipe">,
   HelpText<"Use pipes between commands, when possible">;
+// Facebook T92898286
+def post_link_optimize : Flag<["--"], "post-link-optimize">,
+  HelpText<"Apply post-link optimizations using BOLT">;
+// End Facebook T92898286
 def prebind__all__twolevel__modules : Flag<["-"], 
"prebind_all_twolevel_modules">;
 def prebind : Flag<["-"], "prebind">;
 def preload : Flag<["-"], "preload">;

diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index f52bb8af5ec96..8377ea44c4d98 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -684,12 +684,41 @@ void tools::gnutools::Linker::ConstructJob(Compilation 
&C, const JobAction &JA,
 }
   }
 
+  // Facebook T92898286
+  if (Args.hasArg(options::OPT_post_link_optimize))
+CmdArgs.push_back("-q");
+  // End Facebook T92898286
+
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
  Exec, CmdArgs, Inputs, Output));
+  // Facebook T92898286
+  if (!Args.hasArg(options::OPT_post_link_optimize) || !Output.isFilename())
+return;
+
+  const char *MvExec = Args.MakeArgString(ToolChain.GetProgramPath("mv"));
+  ArgStringList MoveCmdArgs;
+  MoveCmdArgs.push_back(Output.getFilename());
+  const char *PreBoltBin =
+  Args.MakeArgString(Twine(Output.getFilename()) + ".pre-bolt");
+  MoveCmdArgs.push_back(PreBoltBin);
+  C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
+ MvExec, MoveCmdArgs, None));
+
+  ArgStringList BoltCmdArgs;
+  const char *BoltExec =
+  Args.MakeArgString(ToolChain.GetProgramPath("llvm-bolt"));
+  BoltCmdArgs.push_back(PreBoltBin);
+  BoltCmdArgs.push_back("-reorder-blocks=reverse");
+  BoltCmdArgs.push_back("-update-debug-sections");
+  BoltCmdArgs.push_back("-o");
+  BoltCmdArgs.push_back(Output.getFilename());
+  C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
+ BoltExec, BoltCmdArgs, None));
+  // End Facebook T92898286
 }
 
 void tools::gnutools::Assembler::ConstructJob(Compilation &C,

diff  --git a/cross-project-tests/lit.cfg.py b/cross-project-tests/lit.cfg.py
index 7bda584dc317f..0855af4616995 100644
--- a/cross-project-tests/lit.cfg.py
+++ b/cross-project-tests/lit.cfg.py
@@ -74,7 +74,13 @@ def get_required_attr(config, attr_name):
 # use_cl

[Lldb-commits] [lldb] b444358 - Revert "Rebase: [Facebook] Add clang driver options to test debug info and BOLT"

[via lldb-commits]

Author: spupyrev
Date: 2022-07-11T09:50:46-07:00
New Revision: b444358126aa6354e56bf629f50fdcd607b2a233

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

LOG: Revert "Rebase: [Facebook] Add clang driver options to test debug info and 
BOLT"

This reverts commit f921985a29fc9787b3ed98dbc897146cc3fd91f7.

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Gnu.cpp
cross-project-tests/lit.cfg.py
cross-project-tests/lit.site.cfg.py.in
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in
lldb/test/Shell/helper/toolchain.py
lldb/test/Shell/lit.site.cfg.py.in
llvm/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 44ea906be543c..8ae9145a271a5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4001,10 +4001,6 @@ def pg : Flag<["-"], "pg">, HelpText<"Enable mcount 
instrumentation">, Flags<[CC
   MarshallingInfoFlag>;
 def pipe : Flag<["-", "--"], "pipe">,
   HelpText<"Use pipes between commands, when possible">;
-// Facebook T92898286
-def post_link_optimize : Flag<["--"], "post-link-optimize">,
-  HelpText<"Apply post-link optimizations using BOLT">;
-// End Facebook T92898286
 def prebind__all__twolevel__modules : Flag<["-"], 
"prebind_all_twolevel_modules">;
 def prebind : Flag<["-"], "prebind">;
 def preload : Flag<["-"], "preload">;

diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 8377ea44c4d98..f52bb8af5ec96 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -684,41 +684,12 @@ void tools::gnutools::Linker::ConstructJob(Compilation 
&C, const JobAction &JA,
 }
   }
 
-  // Facebook T92898286
-  if (Args.hasArg(options::OPT_post_link_optimize))
-CmdArgs.push_back("-q");
-  // End Facebook T92898286
-
   Args.AddAllArgs(CmdArgs, options::OPT_T);
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
  Exec, CmdArgs, Inputs, Output));
-  // Facebook T92898286
-  if (!Args.hasArg(options::OPT_post_link_optimize) || !Output.isFilename())
-return;
-
-  const char *MvExec = Args.MakeArgString(ToolChain.GetProgramPath("mv"));
-  ArgStringList MoveCmdArgs;
-  MoveCmdArgs.push_back(Output.getFilename());
-  const char *PreBoltBin =
-  Args.MakeArgString(Twine(Output.getFilename()) + ".pre-bolt");
-  MoveCmdArgs.push_back(PreBoltBin);
-  C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
- MvExec, MoveCmdArgs, None));
-
-  ArgStringList BoltCmdArgs;
-  const char *BoltExec =
-  Args.MakeArgString(ToolChain.GetProgramPath("llvm-bolt"));
-  BoltCmdArgs.push_back(PreBoltBin);
-  BoltCmdArgs.push_back("-reorder-blocks=reverse");
-  BoltCmdArgs.push_back("-update-debug-sections");
-  BoltCmdArgs.push_back("-o");
-  BoltCmdArgs.push_back(Output.getFilename());
-  C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
- BoltExec, BoltCmdArgs, None));
-  // End Facebook T92898286
 }
 
 void tools::gnutools::Assembler::ConstructJob(Compilation &C,

diff  --git a/cross-project-tests/lit.cfg.py b/cross-project-tests/lit.cfg.py
index 0855af4616995..7bda584dc317f 100644
--- a/cross-project-tests/lit.cfg.py
+++ b/cross-project-tests/lit.cfg.py
@@ -74,13 +74,7 @@ def get_required_attr(config, attr_name):
 # use_clang() and use_lld() respectively, so set them to "", if needed.
 if not hasattr(config, 'clang_src_dir'):
 config.clang_src_dir = ""
-# Facebook T92898286
-should_test_bolt = get_required_attr(config, "llvm_test_bolt")
-if should_test_bolt:
-llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects), 
additional_flags=['--post-link-optimize'])
-else:
-llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects))
-# End Facebook T92898286
+llvm_config.use_clang(required=('clang' in config.llvm_enabled_projects))
 
 if not hasattr(config, 'lld_src_dir'):
 config.lld_src_dir = ""
@@ -262,9 +256,3 @@ def get_clang_default_dwarf_version_string(triple):
 # Allow 'REQUIRES: XXX-registered-target' in tests.
 for arch in config.targets_to_build:
 config.available_features.add(arch.lower() + '-registered-target')
-
-# Facebook T92898286
-# Ensure the user's PYTHONPATH is included.
-if 'PYTHONPATH' in os.environ:
-config.environment['PYTHONPATH'] = os.environ['PYTHONPATH']
-# End Facebook T92898286

diff  --git a/cross-project-tests/lit.site.cfg.py.in 
b/cross-project-

[Lldb-commits] [PATCH] D129456: [lldb] Add image dump pcm-info command

[Med Ismail Bennani via Phabricator via lldb-commits]

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

LGTM with a comment




Comment at: lldb/test/API/commands/target/dump-pcm-info/TestDumpPCMInfo.py:37
+
+breakpoint()
+self.expect(

Left over ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129456

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

[Lldb-commits] [PATCH] D129456: [lldb] Add image dump pcm-info command

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a reviewer: Michael137.
kastiglione added inline comments.



Comment at: lldb/test/API/commands/target/dump-pcm-info/TestDumpPCMInfo.py:37
+
+breakpoint()
+self.expect(

mib wrote:
> Left over ?
yep thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129456

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

[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 443707.
Michael137 marked 9 inline comments as done.
Michael137 added a comment.

- Address stylistic/documentation comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

Files:
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Expression/UserExpression.h
  lldb/source/Expression/Materializer.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/test/API/commands/expression/expr_inside_lambda/Makefile
  lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
  lldb/test/API/commands/expression/expr_inside_lambda/main.cpp

Index: lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
@@ -0,0 +1,99 @@
+#include 
+#include 
+
+namespace {
+int global_var = -5;
+} // namespace
+
+struct Baz {
+  virtual ~Baz() = default;
+
+  virtual int baz_virt() = 0;
+
+  int base_base_var = 12;
+};
+
+struct Bar : public Baz {
+  virtual ~Bar() = default;
+
+  virtual int baz_virt() override {
+base_var = 10;
+return 1;
+  }
+
+  int base_var = 15;
+};
+
+struct Foo : public Bar {
+  int class_var = 9;
+  int shadowed = -137;
+  int *class_ptr;
+
+  virtual ~Foo() = default;
+
+  virtual int baz_virt() override {
+shadowed = -1;
+return 2;
+  }
+
+  void method() {
+int local_var = 137;
+int shadowed;
+class_ptr = &local_var;
+auto lambda = [&shadowed, this, &local_var,
+   local_var_copy = local_var]() mutable {
+  int lambda_local_var = 5;
+  shadowed = 5;
+  class_var = 109;
+  --base_var;
+  --base_base_var;
+  std::puts("break here");
+
+  auto nested_lambda = [this, &lambda_local_var] {
+std::puts("break here");
+lambda_local_var = 0;
+  };
+
+  nested_lambda();
+  --local_var_copy;
+  std::puts("break here");
+
+  struct LocalLambdaClass {
+int lambda_class_local = -12345;
+Foo *outer_ptr;
+
+void inner_method() {
+  auto lambda = [this] {
+std::puts("break here");
+lambda_class_local = -2;
+outer_ptr->class_var *= 2;
+  };
+
+  lambda();
+}
+  };
+
+  LocalLambdaClass l;
+  l.outer_ptr = this;
+  l.inner_method();
+};
+lambda();
+  }
+
+  void non_capturing_method() {
+int local = 5;
+int local2 = 10;
+
+class_var += [=] {
+  std::puts("break here");
+  return local + local2;
+}();
+  }
+};
+
+int main() {
+  Foo f;
+  f.method();
+  f.non_capturing_method();
+  return global_var;
+}
Index: lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
@@ -0,0 +1,123 @@
+""" Test that evaluating expressions from within C++ lambdas works
+Particularly, we test the case of capturing "this" and
+using members of the captured object in expression evaluation
+while we're on a breakpoint inside a lambda.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+
+class ExprInsideLambdaTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def expectExprError(self, expr : str, expected : str):
+frame = self.thread.GetFrameAtIndex(0)
+value = frame.EvaluateExpression(expr)
+errmsg = value.GetError().GetCString()
+self.assertIn(expected, errmsg)
+
+def test_expr_inside_lambda(self):
+"""Test that lldb evaluating expressions inside lambda expressions works correctly."""
+self.build()
+(target, process, self.thread, bkpt) = \
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.cpp"))
+
+# Inside 'Foo::method'
+
+# Check access to captured 'this'
+self.expect_expr("class_var", result_type="int", result_value="109")
+self.expect_expr("this->class_var", result_type="int", result_value="109")
+
+# Check that captured shadowed v

[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Expression/Materializer.cpp:432
+m_size = g_default_var_byte_size;
+m_alignment = g_default_var_alignment;
   }

aprantl wrote:
> FWIW, the refactoring of EntityVariable could have been a separate 
> preparatory NFC patch, then the patch that adds the lambda functionality 
> would be shorter. It's fine either way, but it's usually easier to review two 
> simple comments instead of one complex one :-)
True
I added it as a separate commit to this revision. Wasn't sure whether a 
separate patch was preferred over a separate commit in the same Phabricator 
revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

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

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added a comment.

@jingham, thanks for sharing the thoughts.

> Setting the breakpoint search to only check base names will of course make 
> all your file and line breakpoints work, but they will also pick up extra 
> hits. In command line lldb that doesn't look nearly as weird as clicking on 
> one source file window & seeing a breakpoint marker show up in a completely 
> different window. If we didn't care about that problem and expected people to 
> manage these extra locations by hand, then indeed just saying "only set using 
> base-names" is fine. But it still seems weird to me that we would add an lldb 
> setting and code to support that rather than have IDE's just only pass in the 
> base name if that's their strategy.

One thought to mitigate this is, for all the symbol context(s) matched with 
base file name, we could iterate and find if there is any exact match with 
request breakpoint path. We would prefer this exact match symbol context and 
throw away other partial matches. However, if no exact match exists, we have to 
keep all of them to not miss breakpoints because there is no way to know which 
one users want. This should make 90% (my guess) breakpoint cases (with exact 
matching) the same behavior as before.

> I really don't want changing a setting to add or remove locations from an 
> extant breakpoint. That's not how breakpoints work. If I go to the trouble of 
> inputting a command on a location, and the I change a setting and the 
> location goes away, I would rightly be ticked off. So I really think the 
> locations should record whatever search they are going to do when they are 
> created, and stick to it. However, I have no problem with having the 
> breakpoint store "original path/matched path" pairs if that makes it easier 
> to figure out what is going on.

I do not not have much preference on this one. In 99% of the use cases, 
client/IDE/users would set this setting during startup without changing during 
the lifetime of debug sessions.

> The behind your back creation of source maps gives me the willies. 
> Particularly if you have more than one module with debug information, all 
> built originally in the phony locations so they all look like the original 
> directories have the same root, it seems like you could start getting source 
> maps that map everything to everything and after a while this will get hard 
> to reason about. Maybe I'm being pessimistic, however, and if you are excited 
> to try, more power to you.

Ideally, if compiler/linker can emit checksum for each source file into debug 
info we can verify each matched source file to filter noise. I know Microsoft 
toolchain does so but seems like llvm does not?

> But I don't think you can drive this from breakpoints alone. Suppose I set no 
> breakpoints, and the program crashes at 
> /OriginalDirectory/SourceRoot/MyProject/subdir/foo.c, lldb is still going to 
> have to figure out what to show the user, so you're still going to have to 
> come up with the real source file. And you can't use your breakpoint tricks 
> to help you because it's lldb providing the information and at the point 
> where you are doing this it only knows the original location. The IDE is the 
> only agent that knows where to look for other possible locations.

I agree that breakpoint auto source mapping only helps if users/IDE set file 
line breakpoint. And I have spent some thoughts on the non-breakpoint cases. 
Ideally, we want to have a `target.source-paths` setting which users/IDE/CLI 
can tell lldb where to look for source files. IDE can even pop-up a dialog ask 
user to browse the target source file for selected frame without valid source 
files. I know both windbg and visual studio debugger are providing this option. 
I would like to improve this part but currently unverified breakpoint due to 
incorrect/missing source map settings are #1 pain points from our users in Meta.

> It seems like it would be cleaner to have some kind of callback when a binary 
> with debug information gets loaded where the UI could have a look at the CU's 
> in the module that got loaded, and see if it knows about any of those files 
> locally (because it has them in in directory-equivalent places in its 
> project). The UI can then construct a source map from there. That way this 
> would happen predictably and for all consumers, rather than relying on the 
> user's path through breakpoint setting to set the source mapping world back 
> to rights.

We can look into this to further improve the auto source mapping feature. My 
concern is that it requires relative complex interaction from IDE/client which 
increases the barrier for wide adoption.

From high level, I agree that only users/IDE know the true source physical 
locations. Actually, the design of "breakpoint guided auto source map" is using 
this truth information from IDE - breakpoint file path to guide auto source 
mapping. I agree that it is not completely fixing all source map

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG66cdd6548ac5: [lldb] Reduce the stack alignment requirements 
for the Windows x86_64 ABI (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129455

Files:
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
  lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
  lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
  lldb/test/Shell/Unwind/windows-unaligned-x86_64.test


Index: lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
@@ -0,0 +1,26 @@
+# Test unwinding through stack frames that aren't aligned to 16 bytes.
+# (In real world code, this happens when unwinding through
+# KiUserExceptionDispatcher and KiUserCallbackDispatcher in ntdll.dll.)
+
+# REQUIRES: target-x86_64, native, system-windows
+
+# RUN: %build %p/Inputs/windows-unaligned-x86_64.cpp 
%p/Inputs/windows-unaligned-x86_64-asm.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+# Future TODO: If %build could compile the source file in C mode, the symbol
+# name handling would be easier across msvc and mingw build configurations.
+# (In mingw mode, the extern C symbol "func" is printed as "::func" while
+# it's plain "func" in msvc mode. Without the extern C, it's "func(..." in
+# mingw mode, but "void __cdecl func(..." in msvc mode.)
+
+breakpoint set -n func
+# CHECK: Breakpoint 1: where = {{.*}}`{{(::)?}}func
+
+process launch
+# CHECK: stop reason = breakpoint 1.1
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`{{(::)?}}func
+# CHECK: frame #1: {{.*}}`realign_stack
+# CHECK: frame #2: {{.*}}`call_func
+# CHECK: frame #3: {{.*}}`main
Index: lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
@@ -0,0 +1,8 @@
+extern "C" void call_func(void (*ptr)(int a), int a);
+
+extern "C" void func(int arg) { }
+
+int main(int argc, char **argv) {
+  call_func(func, 42);
+  return 0;
+}
Index: lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
@@ -0,0 +1,25 @@
+.globlcall_func
+.def  call_func;   .scl2;  .type   32; .endef
+.seh_proc call_func
+call_func:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+callrealign_stack
+addq$32, %rsp
+ret
+.seh_endproc
+
+.globlrealign_stack
+.def  realign_stack;   .scl2;  .type   32; .endef
+.seh_proc realign_stack
+realign_stack:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+movq%rcx, %rax
+movl%edx, %ecx
+call*%rax
+addq$32, %rsp
+ret
+.seh_endproc
Index: lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
===
--- lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
+++ lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
@@ -40,10 +40,15 @@
 
   bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
 
-  // In Windows_x86_64 ABI, stack will always be maintained 16-byte aligned
+  // In Windows_x86_64 ABI requires that the stack will be maintained 16-byte
+  // aligned.
+  // When ntdll invokes callbacks such as KiUserExceptionDispatcher or
+  // KiUserCallbackDispatcher, those functions won't have a properly 16-byte
+  // aligned stack - but tolerate unwinding through them by relaxing the
+  // requirement to 8 bytes.
   bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
- if (cfa & (16ull - 1ull))
-  return false; // Not 16 byte aligned
+if (cfa & (8ull - 1ull))
+  return false; // Not 8 byte aligned
 if (cfa == 0)
   return false; // Zero is not a valid stack address
 return true;


Index: lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
@@ -0,0 +1,26 @@
+# Test unwinding through stack frames that aren't aligned to 16 bytes.
+# (In real world code, this happens when unwinding through
+# KiUserExceptionDispatcher and KiUserCallbackDispatcher in ntdll.dll.)
+
+# REQUIRES: target-x86_64, native, system-windows
+
+# RUN: %build %p/Inputs/windows-unaligned-x86_64.cpp %p/Inputs/windows-unaligned-x86_64-asm.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+# Future TODO: If %build could compile the source file in C mode, the symbol
+# name handling would

[Lldb-commits] [lldb] 66cdd65 - [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-07-11T23:41:35+03:00
New Revision: 66cdd6548ac51f2039b9f0bc10ec03f387b210c4

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

LOG: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

This fixes https://github.com/llvm/llvm-project/issues/56095.

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

Added: 
lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
lldb/test/Shell/Unwind/windows-unaligned-x86_64.test

Modified: 
lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h

Removed: 




diff  --git a/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h 
b/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
index e74b9126404e1..a9c2ed9c2f141 100644
--- a/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
+++ b/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
@@ -40,10 +40,15 @@ class ABIWindows_x86_64 : public ABIX86_64 {
 
   bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
 
-  // In Windows_x86_64 ABI, stack will always be maintained 16-byte aligned
+  // In Windows_x86_64 ABI requires that the stack will be maintained 16-byte
+  // aligned.
+  // When ntdll invokes callbacks such as KiUserExceptionDispatcher or
+  // KiUserCallbackDispatcher, those functions won't have a properly 16-byte
+  // aligned stack - but tolerate unwinding through them by relaxing the
+  // requirement to 8 bytes.
   bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
- if (cfa & (16ull - 1ull))
-  return false; // Not 16 byte aligned
+if (cfa & (8ull - 1ull))
+  return false; // Not 8 byte aligned
 if (cfa == 0)
   return false; // Zero is not a valid stack address
 return true;

diff  --git a/lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s 
b/lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
new file mode 100644
index 0..c042901683648
--- /dev/null
+++ b/lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
@@ -0,0 +1,25 @@
+.globlcall_func
+.def  call_func;   .scl2;  .type   32; .endef
+.seh_proc call_func
+call_func:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+callrealign_stack
+addq$32, %rsp
+ret
+.seh_endproc
+
+.globlrealign_stack
+.def  realign_stack;   .scl2;  .type   32; .endef
+.seh_proc realign_stack
+realign_stack:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+movq%rcx, %rax
+movl%edx, %ecx
+call*%rax
+addq$32, %rsp
+ret
+.seh_endproc

diff  --git a/lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp 
b/lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
new file mode 100644
index 0..d310ef7d7ed2f
--- /dev/null
+++ b/lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
@@ -0,0 +1,8 @@
+extern "C" void call_func(void (*ptr)(int a), int a);
+
+extern "C" void func(int arg) { }
+
+int main(int argc, char **argv) {
+  call_func(func, 42);
+  return 0;
+}

diff  --git a/lldb/test/Shell/Unwind/windows-unaligned-x86_64.test 
b/lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
new file mode 100644
index 0..94f1c011ebd2a
--- /dev/null
+++ b/lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
@@ -0,0 +1,26 @@
+# Test unwinding through stack frames that aren't aligned to 16 bytes.
+# (In real world code, this happens when unwinding through
+# KiUserExceptionDispatcher and KiUserCallbackDispatcher in ntdll.dll.)
+
+# REQUIRES: target-x86_64, native, system-windows
+
+# RUN: %build %p/Inputs/windows-unaligned-x86_64.cpp 
%p/Inputs/windows-unaligned-x86_64-asm.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+# Future TODO: If %build could compile the source file in C mode, the symbol
+# name handling would be easier across msvc and mingw build configurations.
+# (In mingw mode, the extern C symbol "func" is printed as "::func" while
+# it's plain "func" in msvc mode. Without the extern C, it's "func(..." in
+# mingw mode, but "void __cdecl func(..." in msvc mode.)
+
+breakpoint set -n func
+# CHECK: Breakpoint 1: where = {{.*}}`{{(::)?}}func
+
+process launch
+# CHECK: stop reason = breakpoint 1.1
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`{{(::)?}}func
+# CHECK: frame #1: {{.*}}`realign_stack
+# CHECK: frame #2: {{.*}}`call_func
+# CHECK: frame #3: {{.*}}`main



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

[Lldb-commits] [PATCH] D129490: [lldb/libc++] Simplify the libc++ string formatter

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This is great! Thanks for taking care of this! LGTM!




Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:609
   } else {
-ValueObjectSP size_mode(dataval_sp->GetChildAtIndexPath({1, 0, 0}));
-if (!size_mode)
+if (ValueObjectSP size_mode = short_sp->GetChildMemberWithName(
+ConstString("__size_"), /*can_create=*/true)) {

Nit: In the if statement it's called `size_mode` but it the other branch (line 
`603`) it's called `size_member`. Would be nice to make it more consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129490

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

[Lldb-commits] [PATCH] D129490: [lldb/libc++] Simplify the libc++ string formatter

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129490

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

[Lldb-commits] [PATCH] D129456: [lldb] Add image dump pcm-info command

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

[bikeshedding] `pcm-info` seems a little odd. Do we have other command that end 
with `-info`? `target modules dump` already sounds like you're about to dump 
some kind of "info". What about just `pcm`? [/bikeshedding]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129456

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

[Lldb-commits] [lldb] ce233e7 - [lldb] Use the just-built libc++ for testing the LLDB data formatters

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-07-11T14:49:24-07:00
New Revision: ce233e714665a0499fbd0686226e43130d44ef87

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

LOG: [lldb] Use the just-built libc++ for testing the LLDB data formatters

Make sure we use the libc++ from the build dir. Currently, by passing
-stdlib=libc++, we might pick up the system libc++. This change ensures
that if LLVM_LIBS_DIR is set, we try to use the libc++ from there.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/builder.py
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in
lldb/test/CMakeLists.txt

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 98057066f3f3c..cb82dd4c98817 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -120,6 +120,11 @@ def getModuleCacheSpec(self):
 configuration.clang_module_cache_dir)]
 return []
 
+def getLibCxxArgs(self):
+if configuration.hermetic_libcxx:
+return ["USE_HERMETIC_LIBCPP=1"]
+return []
+
 def _getDebugInfoArgs(self, debug_info):
 if debug_info is None:
 return []
@@ -142,7 +147,7 @@ def getBuildCommand(self, debug_info, architecture=None, 
compiler=None,
 self.getArchCFlags(architecture), self.getArchSpec(architecture),
 self.getCCSpec(compiler), self.getExtraMakeArgs(),
 self.getSDKRootSpec(), self.getModuleCacheSpec(),
-self.getCmdLine(dictionary)]
+self.getLibCxxArgs(), self.getCmdLine(dictionary)]
 command = list(itertools.chain(*command_parts))
 
 return command

diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 6b3da8f255230..c29a44e70638a 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -124,6 +124,9 @@
 # LLDB library directory.
 lldb_libs_dir = None
 
+# Force us to use the just-built libcxx
+hermetic_libcxx = False
+
 # A plugin whose tests will be enabled, like intel-pt.
 enabled_plugins = []
 

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index a9ca741cf3a72..ee59500a4fc7f 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -281,6 +281,11 @@ def parseOptionsAndInitTestdirs():
 logging.warning('No valid FileCheck executable; some tests may 
fail...')
 logging.warning('(Double-check the --llvm-tools-dir argument to 
dotest.py)')
 
+configuration.hermetic_libcxx = args.hermetic_libcxx
+if configuration.hermetic_libcxx and args.lldb_platform_name:
+configuration.hermetic_libcxx = False
+logging.warning('Hermetic libc++ is not supported for remote runs: 
ignoring --hermetic-libcxx')
+
 if args.channels:
 lldbtest_config.channels = args.channels
 

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py 
b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index aac2fecfe6994..768b052885074 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -43,6 +43,8 @@ def create_parser():
 if sys.platform == 'darwin':
 group.add_argument('--apple-sdk', metavar='apple_sdk', 
dest='apple_sdk', default="", help=textwrap.dedent(
 '''Specify the name of the Apple SDK (macosx, macosx.internal, 
iphoneos, iphoneos.internal, or path to SDK) and use the appropriate tools from 
that SDK's toolchain.'''))
+group.add_argument('--hermetic-libcxx', action='store_true', 
help=textwrap.dedent(
+'''Force the just-built libcxx to be used for the libc++ formatter 
tests.'''))
 # FIXME? This won't work for 
diff erent extra flags according to each arch.
 group.add_argument(
 '-E',

diff  --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index abd396d015dff..b553b8940467d 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -158,14 +158,22 @@ def delete_module_cache(path):
 if is_configured('dotest_args_str'):
   dotest_cmd.extend(config.dotest_args_str.split(';'))
 
-# Library path may be needed to locate just-built clang.
+# Library path may be needed to locate just-built clang and libcxx.
 if is_configured('

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce233e714665: [lldb] Use the just-built libc++ for testing 
the LLDB data formatters (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D129166?vs=442994&id=443766#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129166

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/CMakeLists.txt

Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -127,6 +127,7 @@
   # dependency as it's also possible to run the libc++ tests against the libc++
   # installed on the system.
   if (TARGET cxx)
+set(LLDB_HAS_LIBCXX ON)
 add_lldb_test_dependency(cxx)
   endif()
 
@@ -172,6 +173,7 @@
   LLDB_ENABLE_LZMA
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
+  LLDB_HAS_LIBCXX
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 
Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -4,6 +4,7 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
 config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
+config.llvm_include_dir = lit_config.substitute("@LLVM_INCLUDE_DIR@")
 config.llvm_shlib_dir = lit_config.substitute("@SHLIBDIR@")
 config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
@@ -29,6 +30,7 @@
 config.test_arch = '@LLDB_TEST_ARCH@'
 config.test_compiler = lit_config.substitute('@LLDB_TEST_COMPILER@')
 config.dsymutil = lit_config.substitute('@LLDB_TEST_DSYMUTIL@')
+config.has_libcxx = '@LLDB_HAS_LIBCXX@'
 # The API tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api")
 config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api")
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -158,14 +158,22 @@
 if is_configured('dotest_args_str'):
   dotest_cmd.extend(config.dotest_args_str.split(';'))
 
-# Library path may be needed to locate just-built clang.
+# Library path may be needed to locate just-built clang and libcxx.
 if is_configured('llvm_libs_dir'):
   dotest_cmd += ['--env', 'LLVM_LIBS_DIR=' + config.llvm_libs_dir]
 
+# Include path may be needed to locate just-built libcxx.
+if is_configured('llvm_include_dir'):
+  dotest_cmd += ['--env', 'LLVM_INCLUDE_DIR=' + config.llvm_include_dir]
+
 # This path may be needed to locate required llvm tools
 if is_configured('llvm_tools_dir'):
   dotest_cmd += ['--env', 'LLVM_TOOLS_DIR=' + config.llvm_tools_dir]
 
+# If we have a just-built libcxx, prefer it over the system one.
+if is_configured('has_libcxx') and platform.system() != 'Windows':
+  dotest_cmd += ['--hermetic-libcxx']
+
 # Forward ASan-specific environment variables to tests, as a test may load an
 # ASan-ified dylib.
 for env_var in ('ASAN_OPTIONS', 'DYLD_INSERT_LIBRARIES'):
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -43,6 +43,8 @@
 if sys.platform == 'darwin':
 group.add_argument('--apple-sdk', metavar='apple_sdk', dest='apple_sdk', default="", help=textwrap.dedent(
 '''Specify the name of the Apple SDK (macosx, macosx.internal, iphoneos, iphoneos.internal, or path to SDK) and use the appropriate tools from that SDK's toolchain.'''))
+group.add_argument('--hermetic-libcxx', action='store_true', help=textwrap.dedent(
+'''Force the just-built libcxx to be used for the libc++ formatter tests.'''))
 # FIXME? This won't work for different extra flags according to each arch.
 group.add_argument(
 '-E',
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -281,6 +281,11 @@
 logging.warning('No valid FileCheck executable; some tests may fail...')
 logging.warning('(Double-check the --llvm-tools-dir argument to dotest.py)')
 
+configuration.hermetic_libcxx = args.hermetic_libcxx
+if configuration.hermetic_libcxx and args.lldb_platform_name:
+   

[Lldb-commits] [PATCH] D129456: [lldb] Add image dump pcm-info command

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

@JDevlieghere I had it as `dump pcm` at first, but switched it to `pcm-info` 
for two reasons

1. it doesn't dump the whole pcm, the actual ast itself is not dumped
2. to match the clang flag `-module-file-info`

I agree that `pcm` is better, but I was worried it would be misunderstood as 
dumping more than it actually does.

I think -info here is more like -metadata but shorter.

One option is to name it back to `pcm`, and update the help description to be 
more descriptive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129456

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

[Lldb-commits] [PATCH] D129521: Add the ability to run expressions that call fork() or vfork().

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, jingham, yinghuitan.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Before this fix if you tried to call fork for vfork, the expression would stop 
when it received the eStopReasonFork or eStopReasonVFork in the thread plan 
that runs the expression. This is now fixed and I have added a test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129521

Files:
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/test/API/commands/expression/fork/Makefile
  lldb/test/API/commands/expression/fork/TestForkExprs.py
  lldb/test/API/commands/expression/fork/main.cpp

Index: lldb/test/API/commands/expression/fork/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/fork/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+
+pid_t do_fork(char *const argv[], char *const envp[]) {
+  pid_t pid = fork();
+  if (pid == 0)
+execve(argv[0], argv, envp);
+  return pid;
+}
+
+pid_t do_vfork(char *const argv[], char *const envp[]) {
+  pid_t pid = vfork();
+  if (pid == 0)
+execve(argv[0], argv, envp);
+  return pid;
+}
+
+int main(int argc, char *const argv[], char *const envp[]) {
+  // Stop here to evaluate expressions
+  // Optionally call do_fork() when argc is 4 to make sure do_fork() doesn't get
+  // dead stripped
+  if (argc == 4)
+do_fork(argv, envp);
+  // Optionally call do_vfork() when argc is 5 to make sure do_fork() doesn't
+  // get dead stripped
+  if (argc == 5)
+do_vfork(argv, envp);
+  return 0;
+}
Index: lldb/test/API/commands/expression/fork/TestForkExprs.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/fork/TestForkExprs.py
@@ -0,0 +1,41 @@
+"""
+Test some more expression commands.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ExprCommandsForkExpressions(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@skipUnlessPlatform(["linux"])
+def test_more_expr_commands(self):
+"""Test some more expression commands."""
+self.build()
+
+(target, process, self.thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+'Stop here to evaluate expressions',
+ lldb.SBFileSpec("main.cpp"))
+
+self.assertTrue(bkpt.IsValid())
+bkpt.SetEnabled(False)
+options = lldb.SBExpressionOptions()
+options.SetIgnoreBreakpoints(True)
+frame = self.thread.GetFrameAtIndex(0)
+
+value = frame.EvaluateExpression("do_fork(argv, envp)", options)
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+value = frame.EvaluateExpression("do_vfork(argv, envp)", options)
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
Index: lldb/test/API/commands/expression/fork/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/fork/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -267,6 +267,12 @@
   if (stop_reason == eStopReasonBreakpoint && BreakpointsExplainStop())
 return true;
 
+  if (stop_reason == eStopReasonFork || stop_reason == eStopReasonVFork ||
+  stop_reason == eStopReasonVForkDone) {
+LLDB_LOGF(log, "ThreadPlanCallFunction::PlanExplainsStop not done for fork stop reasons.");
+return false;
+  }
+
   // One more quirk here.  If this event was from Halt interrupting the target,
   // then we should not consider ourselves complete.  Return true to
   // acknowledge the stop.
@@ -371,7 +377,7 @@
 
 #ifndef SINGLE_STEP_EXPRESSIONS
   Thread &thread = GetThread();
-  m_subplan_sp = std::make_shared(thread, m_start_addr, 
+  m_subplan_sp = std::make_shared(thread, m_start_addr,
   m_stop_other_threads);
 
   thread.QueueThreadPlan(m_subplan_sp, false);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9097920 - [lldb] Add a test to prefer exact triple matches in platform selection

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-07-11T16:35:51-07:00
New Revision: 9097920ebabb6ab29f94e4051572c42459edcda8

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

LOG: [lldb] Add a test to prefer exact triple matches in platform selection

Add a test that ensures we always prioritize exact triple matches when
creating platforms. This is a regression test for a (now resolved) bug
that that resulted in the remote tvOS platform being selected for a tvOS
simulator binary because the ArchSpecs are compatible.

Added: 


Modified: 
lldb/unittests/Platform/PlatformAppleSimulatorTest.cpp

Removed: 




diff  --git a/lldb/unittests/Platform/PlatformAppleSimulatorTest.cpp 
b/lldb/unittests/Platform/PlatformAppleSimulatorTest.cpp
index 7cb07cbe55d35..f30edfb9541ae 100644
--- a/lldb/unittests/Platform/PlatformAppleSimulatorTest.cpp
+++ b/lldb/unittests/Platform/PlatformAppleSimulatorTest.cpp
@@ -9,6 +9,9 @@
 #include "gtest/gtest.h"
 
 #include "Plugins/Platform/MacOSX/PlatformAppleSimulator.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteiOS.h"
 #include "TestingSupport/SubsystemRAII.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
@@ -18,7 +21,8 @@ using namespace lldb;
 using namespace lldb_private;
 
 class PlatformAppleSimulatorTest : public ::testing::Test {
-  SubsystemRAII
+  SubsystemRAII
   subsystems;
 };
 
@@ -64,4 +68,31 @@ TEST_F(PlatformAppleSimulatorTest, TestHostPlatformToSim) {
   }
 }
 
+TEST_F(PlatformAppleSimulatorTest, TestPlatformSelectionOrder) {
+  static const ArchSpec platform_arch(
+  HostInfo::GetArchitecture(HostInfo::eArchKindDefault));
+
+  const llvm::Triple::OSType sim_platforms[] = {
+  llvm::Triple::IOS,
+  llvm::Triple::TvOS,
+  llvm::Triple::WatchOS,
+  };
+
+  PlatformList list;
+  list.GetOrCreate("remote-ios");
+  list.GetOrCreate("remote-tvos");
+  list.GetOrCreate("remote-watchos");
+
+  for (auto sim : sim_platforms) {
+ArchSpec arch = platform_arch;
+arch.GetTriple().setOS(sim);
+arch.GetTriple().setEnvironment(llvm::Triple::Simulator);
+
+Status error;
+auto platform_sp = list.GetOrCreate(arch, {}, nullptr, error);
+EXPECT_TRUE(platform_sp);
+EXPECT_TRUE(platform_sp->GetName().contains("simulator"));
+  }
+}
+
 #endif



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

[Lldb-commits] [PATCH] D104635: [lldb] Add support for escaping fish arguments

[Jonas Devlieghere 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 rG9bc34636a50f: [lldb] Add support for escaping fish arguments 
(authored by teemperor, committed by JDevlieghere).
Herald added a subscriber: lldb-commits.
Herald added a project: All.

Changed prior to commit:
  https://reviews.llvm.org/D104635?vs=353374&id=443786#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104635

Files:
  lldb/source/Utility/Args.cpp
  lldb/unittests/Utility/ArgsTest.cpp


Index: lldb/unittests/Utility/ArgsTest.cpp
===
--- lldb/unittests/Utility/ArgsTest.cpp
+++ lldb/unittests/Utility/ArgsTest.cpp
@@ -348,6 +348,13 @@
   // Normal characters and globbing expressions that shouldn't be escaped.
   EXPECT_EQ(Args::GetShellSafeArgument(sh, "aA$1*"), "aA$1*");
 
+  // Test escaping fish special characters.
+  FileSpec fish("/bin/fish", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(fish, R"( '"<>()&\|;)"),
+R"(\ \'\"\<\>\(\)\&\\\|\;)");
+  // Normal characters and expressions that shouldn't be escaped.
+  EXPECT_EQ(Args::GetShellSafeArgument(fish, "aA$1*"), "aA$1*");
+
   // Try escaping with an unknown shell.
   FileSpec unknown_shell("/bin/unknown_shell", FileSpec::Style::posix);
   EXPECT_EQ(Args::GetShellSafeArgument(unknown_shell, "a'b"), "a\\'b");
Index: lldb/source/Utility/Args.cpp
===
--- lldb/source/Utility/Args.cpp
+++ lldb/source/Utility/Args.cpp
@@ -385,6 +385,7 @@
   };
 
   static ShellDescriptor g_Shells[] = {{ConstString("bash"), " '\"<>()&;"},
+   {ConstString("fish"), " '\"<>()&\\|;"},
{ConstString("tcsh"), " '\"<>()&;"},
{ConstString("zsh"), " '\"<>()&;\\|"},
{ConstString("sh"), " '\"<>()&;"}};


Index: lldb/unittests/Utility/ArgsTest.cpp
===
--- lldb/unittests/Utility/ArgsTest.cpp
+++ lldb/unittests/Utility/ArgsTest.cpp
@@ -348,6 +348,13 @@
   // Normal characters and globbing expressions that shouldn't be escaped.
   EXPECT_EQ(Args::GetShellSafeArgument(sh, "aA$1*"), "aA$1*");
 
+  // Test escaping fish special characters.
+  FileSpec fish("/bin/fish", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(fish, R"( '"<>()&\|;)"),
+R"(\ \'\"\<\>\(\)\&\\\|\;)");
+  // Normal characters and expressions that shouldn't be escaped.
+  EXPECT_EQ(Args::GetShellSafeArgument(fish, "aA$1*"), "aA$1*");
+
   // Try escaping with an unknown shell.
   FileSpec unknown_shell("/bin/unknown_shell", FileSpec::Style::posix);
   EXPECT_EQ(Args::GetShellSafeArgument(unknown_shell, "a'b"), "a\\'b");
Index: lldb/source/Utility/Args.cpp
===
--- lldb/source/Utility/Args.cpp
+++ lldb/source/Utility/Args.cpp
@@ -385,6 +385,7 @@
   };
 
   static ShellDescriptor g_Shells[] = {{ConstString("bash"), " '\"<>()&;"},
+   {ConstString("fish"), " '\"<>()&\\|;"},
{ConstString("tcsh"), " '\"<>()&;"},
{ConstString("zsh"), " '\"<>()&;\\|"},
{ConstString("sh"), " '\"<>()&;"}};
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9bc3463 - [lldb] Add support for escaping fish arguments

[Jonas Devlieghere via lldb-commits]

Author: Raphael Isemann
Date: 2022-07-11T16:44:46-07:00
New Revision: 9bc34636a50ffd9d417d30af021798b6294a1725

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

LOG: [lldb] Add support for escaping fish arguments

LLDB supports having globbing regexes in the process launch arguments
that will be resolved using the user's shell. This requires that we pass
the launch args to the shell and then read back the expanded arguments
using LLDB's argdumper utility.

As the shell will not just expand the globbing regexes but all special
characters, we need to escape all non-globbing charcters such as $, &,
<, >, etc. as those otherwise are interpreted and removed in the step
where we expand the globbing characters. Also because the special
characters are shell-specific, LLDB needs to maintain a list of all the
characters that need to be escaped for each specific shell.

This patch adds the list of special characters that need to be escaped
for fish. Without this patch on systems where fish is the user's shell
having any of these special characters in your arguments or path to
the binary will cause the process launch to fail. E.g., `lldb -- ./calc
1<2` is failing without this patch. The same happens if the absolute
path to calc is in a directory that contains for example parentheses
or other special characters.

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

Added: 


Modified: 
lldb/source/Utility/Args.cpp
lldb/unittests/Utility/ArgsTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp
index 3978f94226530..daccb91d84360 100644
--- a/lldb/source/Utility/Args.cpp
+++ b/lldb/source/Utility/Args.cpp
@@ -385,6 +385,7 @@ std::string Args::GetShellSafeArgument(const FileSpec 
&shell,
   };
 
   static ShellDescriptor g_Shells[] = {{ConstString("bash"), " '\"<>()&;"},
+   {ConstString("fish"), " '\"<>()&\\|;"},
{ConstString("tcsh"), " '\"<>()&;"},
{ConstString("zsh"), " '\"<>()&;\\|"},
{ConstString("sh"), " '\"<>()&;"}};

diff  --git a/lldb/unittests/Utility/ArgsTest.cpp 
b/lldb/unittests/Utility/ArgsTest.cpp
index 755a110aa555f..bc6a3d2c16e41 100644
--- a/lldb/unittests/Utility/ArgsTest.cpp
+++ b/lldb/unittests/Utility/ArgsTest.cpp
@@ -348,6 +348,13 @@ TEST(ArgsTest, GetShellSafeArgument) {
   // Normal characters and globbing expressions that shouldn't be escaped.
   EXPECT_EQ(Args::GetShellSafeArgument(sh, "aA$1*"), "aA$1*");
 
+  // Test escaping fish special characters.
+  FileSpec fish("/bin/fish", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(fish, R"( '"<>()&\|;)"),
+R"(\ \'\"\<\>\(\)\&\\\|\;)");
+  // Normal characters and expressions that shouldn't be escaped.
+  EXPECT_EQ(Args::GetShellSafeArgument(fish, "aA$1*"), "aA$1*");
+
   // Try escaping with an unknown shell.
   FileSpec unknown_shell("/bin/unknown_shell", FileSpec::Style::posix);
   EXPECT_EQ(Args::GetShellSafeArgument(unknown_shell, "a'b"), "a\\'b");



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

[Lldb-commits] [PATCH] D129528: Modify all register values whose byte size matches the address size to be formatter as eFormatAddressInfo.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, jingham, aadsm, yinghuitan.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This allows users to see similar output to what the "register read" command 
emits in LLDB's command line.

Added a test to verify that the PC has the correct value with contains a 
pointer followed by the module + function name and the source line info. 
Something like:

0x00010a64 a.out`main + 132 at main.cpp:17:11


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129528

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1932,6 +1932,22 @@
/*statics=*/true,
/*in_scope_only=*/true);
   g_vsc.variables.registers = frame.GetRegisters();
+
+  // Change the default format of any pointer sized registers to be the
+  // lldb::eFormatAddressInfo so we show the pointer and resolve what the
+  // pointer resolves to.
+  uint32_t addr_size = frame.GetThread().GetProcess().GetAddressByteSize();
+  const uint32_t num_reg_sets = g_vsc.variables.registers.GetSize();
+  for (uint32_t reg_set_idx=0; reg_set_idxIndex: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1932,6 +1932,22 @@
/*statics=*/true,
/*in_scope_only=*/true);
   g_vsc.variables.registers = frame.GetRegisters();
+
+  // Change the default format of any pointer sized registers to be the
+  // lldb::eFormatAddressInfo so we show the pointer and resolve what the
+  // pointer resolves to.
+  uint32_t addr_size = frame.GetThread().GetProcess().GetAddressByteSize();
+  const uint32_t num_reg_sets = g_vsc.variables.registers.GetSize();
+  for (uint32_t reg_set_idx=0; reg_set_idx___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D129529: [lldb] Make the g_arguments_data constexpr and fix the static assert

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: jingham.
Herald added a project: All.
JDevlieghere requested review of this revision.

This fixes the static assert that's meant to keep the g_arguments_data table in 
sync with the CommandArgumentType enumeration. Indeed, the assert didn't fire 
even though the current code is missing an entry. This patches fixes that as 
well.


https://reviews.llvm.org/D129529

Files:
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/source/Interpreter/CommandObject.cpp


Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -547,7 +547,7 @@
   const ArgumentTableEntry *table = GetArgumentTable();
   for (int i = 0; i < eArgTypeLastArg; ++i)
 if (arg_name == table[i].arg_name)
-  return_type = g_arguments_data[i].arg_type;
+  return_type = GetArgumentTable()[i].arg_type;
 
   return return_type;
 }
@@ -924,14 +924,14 @@
 const lldb::CommandArgumentType arg_type) {
   assert(arg_type < eArgTypeLastArg &&
  "Invalid argument type passed to GetArgumentTypeAsCString");
-  return g_arguments_data[arg_type].arg_name;
+  return GetArgumentTable()[arg_type].arg_name;
 }
 
 const char *CommandObject::GetArgumentDescriptionAsCString(
 const lldb::CommandArgumentType arg_type) {
   assert(arg_type < eArgTypeLastArg &&
  "Invalid argument type passed to GetArgumentDescriptionAsCString");
-  return g_arguments_data[arg_type].help_text;
+  return GetArgumentTable()[arg_type].help_text;
 }
 
 Target &CommandObject::GetDummyTarget() {
@@ -1041,7 +1041,7 @@
   return g_archs_help.GetString();
 }
 
-CommandObject::ArgumentTableEntry CommandObject::g_arguments_data[] = {
+static constexpr CommandObject::ArgumentTableEntry g_arguments_data[] = {
 // clang-format off
 { eArgTypeAddress, "address", CommandCompletions::eNoCompletion, { 
nullptr, false }, "A valid address in the target program's execution space." },
 { eArgTypeAddressOrExpression, "address-expression", 
CommandCompletions::eNoCompletion, { nullptr, false }, "An expression that 
resolves to an address." },
@@ -1134,17 +1134,19 @@
 { eArgTypeSaveCoreStyle, "corefile-style", 
CommandCompletions::eNoCompletion, { nullptr, false }, "The type of corefile 
that lldb will try to create, dependant on this target's capabilities." },
 { eArgTypeLogHandler, "log-handler", CommandCompletions::eNoCompletion, { 
nullptr, false }, "The log handle that will be used to write out log messages." 
},
 { eArgTypeSEDStylePair, "substitution-pair", 
CommandCompletions::eNoCompletion, { nullptr, false }, "A sed-style pattern and 
target pair." },
+{ eArgTypeRecognizerID, "type-recognizer-id", 
CommandCompletions::eNoCompletion, { nullptr, false }, "A type recongizer 
identifier." },
 { eArgTypeConnectURL, "process-connect-url", 
CommandCompletions::eNoCompletion, { nullptr, false }, "A URL-style 
specification for a remote connection." },
 { eArgTypeTargetID, "target-id", CommandCompletions::eNoCompletion, { 
nullptr, false }, "The index ID for an lldb Target." },
 { eArgTypeStopHookID, "stop-hook-id", CommandCompletions::eNoCompletion, { 
nullptr, false }, "The ID you receive when you create a stop-hook." }
 // clang-format on
 };
 
+// CommandArgumentType enumeration
+static_assert(
+(sizeof(g_arguments_data) / sizeof(CommandObject::ArgumentTableEntry)) ==
+eArgTypeLastArg,
+"g_arguments_data out of sync with CommandArgumentType enumeration");
+
 const CommandObject::ArgumentTableEntry *CommandObject::GetArgumentTable() {
-  // If this assertion fires, then the table above is out of date with the
-  // CommandArgumentType enumeration
-  static_assert((sizeof(CommandObject::g_arguments_data) /
- sizeof(CommandObject::ArgumentTableEntry)) == eArgTypeLastArg,
-"");
-  return CommandObject::g_arguments_data;
+  return g_arguments_data;
 }
Index: lldb/include/lldb/Interpreter/CommandObject.h
===
--- lldb/include/lldb/Interpreter/CommandObject.h
+++ lldb/include/lldb/Interpreter/CommandObject.h
@@ -104,9 +104,6 @@
   typedef std::vector
   CommandArgumentEntry; // Used to build individual command argument lists
 
-  static ArgumentTableEntry g_arguments_data
-  [lldb::eArgTypeLastArg]; // Main argument information table
-
   typedef std::map CommandMap;
 
   CommandObject(CommandInterpreter &interpreter, llvm::StringRef name,


Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -547,7 +547,7 @@
   const ArgumentTableEntry *table = GetArgumentTable();
   for (int i = 0; i < eArgTypeLastArg; ++i)
 if (arg_name == table[i].a

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D129307#3643426 , @yinghuitan 
wrote:

> @jingham, thanks for sharing the thoughts.

First off, I am supportive of your project, this is a pain point for some folks 
for sure.  I am in favor of trying to automate "I built my binaries with 
sources in one location but locally they are in another" scenario.

Having breakpoint misses be the point where this gets checked is not a great 
final design, since there are other places where lldb needs to find source 
files.  But if it does the 90% job, it's okay to start there.

OTOH, framing the feature as "you pass me a path, and I'm only going to check 
the last N path components regardless of how many you entered" is just weird.  
I'd rather we not expose that.  That choice, after all, is trivial in the IDE, 
just send the parts of the path you care about.  I think that one is fine to 
leave on the IDE side if you want it explicitly, since it would be trivial for 
an IDE to implement.

I also don't like the original framing as it mixes the problem you are actually 
trying to solve: "match the full paths from IDE project info with those in 
debug info" with the general problem of breakpoint setting.  For instance, 
while I might very well want you to try to match up full paths from a project 
this way, if I type:

  (lldb) break set -f foo/bar.c -l 10

I never want you to set a breakpoint on `baz/bar.c`.  That doesn't make any 
sense.  So I don't think we should make that possible.

What the user cares about is that you are going to auto-deduce out-of-place 
source file mappings for them.  That's the piece that really matters; having 
users have to turn this on by specifying the parameters for finding inexact 
matches seems backwards.  So the patch would make a lot more sense to me if you 
were adding `target.auto-deduce-source-maps`, and doing your work behind that 
setting however you need to.

For that feature, it would be clear that this is only to be done when the input 
path is a full path, since you can't set up a path mapping if the "real file 
location" isn't fully specified.  That will limit the scope for accidental 
matchings and keep it out of all the CLI entered breakpoint setting (except for 
people that enter full paths, presumably to trigger this auto-deduction since 
I've actually never seen somebody do that IRL...)  And you can use whatever 
algorithm to search for potential matching patterns makes sense for the job you 
are actually trying to do see if there's a rigid translation between a source 
tree the IDE knows about, and one from Debug Information.

>> Setting the breakpoint search to only check base names will of course make 
>> all your file and line breakpoints work, but they will also pick up extra 
>> hits. In command line lldb that doesn't look nearly as weird as clicking on 
>> one source file window & seeing a breakpoint marker show up in a completely 
>> different window. If we didn't care about that problem and expected people 
>> to manage these extra locations by hand, then indeed just saying "only set 
>> using base-names" is fine. But it still seems weird to me that we would add 
>> an lldb setting and code to support that rather than have IDE's just only 
>> pass in the base name if that's their strategy.
>
> One thought to mitigate this is, for all the symbol context(s) matched with 
> base file name, we could iterate and find if there is any exact match with 
> request breakpoint path. We would prefer this exact match symbol context and 
> throw away other partial matches. However, if no exact match exists, we have 
> to keep all of them to not miss breakpoints because there is no way to know 
> which one users want. This should make 90% (my guess) breakpoint cases (with 
> exact matching) the same behavior as before.
>
>> I really don't want changing a setting to add or remove locations from an 
>> extant breakpoint. That's not how breakpoints work. If I go to the trouble 
>> of inputting a command on a location, and the I change a setting and the 
>> location goes away, I would rightly be ticked off. So I really think the 
>> locations should record whatever search they are going to do when they are 
>> created, and stick to it. However, I have no problem with having the 
>> breakpoint store "original path/matched path" pairs if that makes it easier 
>> to figure out what is going on.
>
> I do not not have much preference on this one. In 99% of the use cases, 
> client/IDE/users would set this setting during startup without changing 
> during the lifetime of debug sessions.
>
>> The behind your back creation of source maps gives me the willies. 
>> Particularly if you have more than one module with debug information, all 
>> built originally in the phony locations so they all look like the original 
>> directories have the same root, it seems like you could start getting source 
>> maps that map everything to everything and after a while this will ge

[Lldb-commits] [PATCH] D129529: [lldb] Make the g_arguments_data constexpr and fix the static assert

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Ack, sorry I missed adding that one!

Everything else looks fine, but you parsed the enum incorrectly, it's eArgType 
- RecognizerID.  This is in fact a stack-frame recognizer ID.  I suggested an 
appropriate version above.




Comment at: lldb/source/Interpreter/CommandObject.cpp:1137
 { eArgTypeSEDStylePair, "substitution-pair", 
CommandCompletions::eNoCompletion, { nullptr, false }, "A sed-style pattern and 
target pair." },
+{ eArgTypeRecognizerID, "type-recognizer-id", 
CommandCompletions::eNoCompletion, { nullptr, false }, "A type recongizer 
identifier." },
 { eArgTypeConnectURL, "process-connect-url", 
CommandCompletions::eNoCompletion, { nullptr, false }, "A URL-style 
specification for a remote connection." },




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

https://reviews.llvm.org/D129529

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

[Lldb-commits] [PATCH] D129528: Modify all register values whose byte size matches the address size to be formatter as eFormatAddressInfo.

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added inline comments.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1939-1950
+  uint32_t addr_size = frame.GetThread().GetProcess().GetAddressByteSize();
+  const uint32_t num_reg_sets = g_vsc.variables.registers.GetSize();
+  for (uint32_t reg_set_idx=0; reg_set_idxhttps://reviews.llvm.org/D129528/new/

https://reviews.llvm.org/D129528

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

[Lldb-commits] [PATCH] D129456: [lldb] Add image dump pcm-info command

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 443830.
kastiglione added a comment.

added error handling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129456

Files:
  clang/include/clang/Frontend/FrontendActions.h
  clang/lib/Frontend/FrontendActions.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/API/commands/target/dump-pcm-info/Makefile
  lldb/test/API/commands/target/dump-pcm-info/TestDumpPCMInfo.py
  lldb/test/API/commands/target/dump-pcm-info/main.m

Index: lldb/test/API/commands/target/dump-pcm-info/main.m
===
--- /dev/null
+++ lldb/test/API/commands/target/dump-pcm-info/main.m
@@ -0,0 +1 @@
+int main() { return 0; }
Index: lldb/test/API/commands/target/dump-pcm-info/TestDumpPCMInfo.py
===
--- /dev/null
+++ lldb/test/API/commands/target/dump-pcm-info/TestDumpPCMInfo.py
@@ -0,0 +1,40 @@
+"""
+Test 'target modules dump pcm-info'.
+"""
+
+import os
+import shutil
+import glob
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+@no_debug_info_test
+@skipUnlessDarwin
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self, "return", lldb.SBFileSpec("main.m"))
+
+mod_cache = self.getBuildArtifact("private-module-cache")
+if os.path.isdir(mod_cache):
+shutil.rmtree(mod_cache)
+
+self.runCmd(f"settings set symbols.clang-modules-cache-path '{mod_cache}'")
+
+# Cause lldb to generate a Darwin-*.pcm
+self.runCmd("p @import Darwin")
+
+# root//-.pcm
+pcm_paths = glob.glob(os.path.join(mod_cache, '*', 'Darwin-*.pcm'))
+self.assertEqual(len(pcm_paths), 1, "Expected one Darwin pcm")
+pcm_path = pcm_paths[0]
+
+self.expect(
+f"target modules dump pcm-info '{pcm_path}'",
+startstr=f"Information for module file '{pcm_path}'",
+substrs=["Module name: Darwin"])
Index: lldb/test/API/commands/target/dump-pcm-info/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/target/dump-pcm-info/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+USE_PRIVATE_MODULE_CACHE = YES
+include Makefile.rules
+
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -47,12 +47,17 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/Args.h"
+#include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
+#include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatAdapters.h"
@@ -2155,6 +2160,63 @@
   }
 };
 
+class CommandObjectTargetModulesDumpClangPCMInfo : public CommandObjectParsed {
+public:
+  CommandObjectTargetModulesDumpClangPCMInfo(CommandInterpreter &interpreter)
+  : CommandObjectParsed(
+interpreter, "target modules dump pcm-info",
+"Dump information about the given clang module (pcm).") {
+// Take a single file argument.
+CommandArgumentData arg{eArgTypeFilename, eArgRepeatPlain};
+m_arguments.push_back({arg});
+  }
+
+  ~CommandObjectTargetModulesDumpClangPCMInfo() override = default;
+
+protected:
+  bool DoExecute(Args &command, CommandReturnObject &result) override {
+if (command.GetArgumentCount() != 1) {
+  result.AppendErrorWithFormat(
+  "'%s' takes exactly one pcm path argument.\n", m_cmd_name.c_str());
+  return false;
+}
+
+const char *pcm_path = command.GetArgumentAtIndex(0);
+FileSpec file_spec(pcm_path);
+if (!FileSystem::Instance().Exists(FileSpec(pcm_path))) {
+  result.AppendErrorWithFormat("file does not exist");
+  return false;
+}
+
+clang::CompilerInstance compiler;
+
+const char *clang_args[] = {"clang", pcm_path};
+compiler.setInvocation(clang::createInvocation(clang_args));
+
+if (!compiler.hasInvocation()) {
+  result.AppendErrorWithFormat("input is not a pcm file");
+  return false;
+}
+
+auto input_kind = compiler.getFrontendOpts().DashX;
+if (input_kind.getFormat() != clang::InputKind::Format::Precompiled) {
+  result.AppendErrorWithFormat("input is not a pcm file");
+  return false;
+}
+
+clang::DumpM

[Lldb-commits] [PATCH] D129521: Add the ability to run expressions that call fork() or vfork().

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added a comment.

Change looks good to me. I will let @jingham take a second look before 
accepting.

I do wonder if there will be more unexpected signals (like SIGCHILD, SIGPIPE 
etc...) causing expression evaluation to pause? Should we maybe default to not 
stop on signals?




Comment at: lldb/source/Target/ThreadPlanCallFunction.cpp:380
   Thread &thread = GetThread();
-  m_subplan_sp = std::make_shared(thread, 
m_start_addr, 
+  m_subplan_sp = std::make_shared(thread, m_start_addr,
   
m_stop_other_threads);

Undo this accidental change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129521

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