[Lldb-commits] [PATCH] D134133: WIP: [lldb][COFF] Enhance symtab loading of symbol and export tables

2022-09-19 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added a comment.

So previously, LLDB essentially used the COFF symbol table for executables, but 
only the list of exported symbols for DLLs, ignoring (or, reading and then 
overwriting) the symbol table for any DLL with exports? Then this certainly 
does look like a good direction.

Overall, I think this does look nice, but it's certainly hard to rewrite and 
wrap one's head around. When you've settled what you want to achieve, can you 
try to split it up into a short series of patches, so that the initial rewrite 
patch doesn't add a ton of extra functionality, but only covers the rewrite and 
appending instead of wiping symbols?




Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1558
   }
+  strm.IndentLess();
 }

Looks like this is a stray change unrelated to the rest (although it does seem 
correct).



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:787
+  Symbol *symbols = symtab.Extend(num_syms);
+  uint32_t i = 0;
+  for (const auto &sym_ref : m_binary->symbols()) {

If we mean to append to the symbol table here, shouldn't we start with `i = 
symtab.size()` or similar? Otherwise we'd start overwriting the existing 
symbols from the start of the table?



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:819
+  if (exported->GetType() != lldb::eSymbolTypeReExported &&
+  exported->GetAddressRef() == symbols[i].GetAddressRef()) {
+symbols[i].SetID(exported->GetID());

What about the case when a symbol is exported with a different name than the 
local symbol? (This is doable with def files e.g. as `ExportedName = LocalName` 
iirc.) Is it worth to have a map of address -> exported symbol, to use instead 
of the raw name?



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:917
+  // actually be data, but this is relatively rare and we cannot tell
+  // from just the export table.
   symbols[i].SetType(lldb::eSymbolTypeCode);

If it's relevant, we can look up what section the exported address is in, and 
check if the section is executable or not.



Comment at: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp:84
 
+  if (symtab->HasSymbolWithType(eSymbolTypeReExported, Symtab::eDebugAny,
+Symtab::eVisibilityAny)) {

For ease of review (when removing the WIP status), I think it'd be easier to 
wrap the head around, if new features like these were deferred to a separate 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134133

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


[Lldb-commits] [PATCH] D134133: WIP: [lldb][COFF] Enhance symtab loading of symbol and export tables

2022-09-19 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun added a comment.

Thanks for the review. Yes I shall split the changes into smaller pieces to aid 
review.




Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1558
   }
+  strm.IndentLess();
 }

mstorsjo wrote:
> Looks like this is a stray change unrelated to the rest (although it does 
> seem correct).
Oh right, this is relevant for https://reviews.llvm.org/D131705#inline-1292343. 
I'll make sure not to mix in this change for the final patch.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:787
+  Symbol *symbols = symtab.Extend(num_syms);
+  uint32_t i = 0;
+  for (const auto &sym_ref : m_binary->symbols()) {

mstorsjo wrote:
> If we mean to append to the symbol table here, shouldn't we start with `i = 
> symtab.size()` or similar? Otherwise we'd start overwriting the existing 
> symbols from the start of the table?
I made `symtab.Extend` return a pointer to the first item of the appended 
range, so counting from 0 does not overwrite existing symbols.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:819
+  if (exported->GetType() != lldb::eSymbolTypeReExported &&
+  exported->GetAddressRef() == symbols[i].GetAddressRef()) {
+symbols[i].SetID(exported->GetID());

mstorsjo wrote:
> What about the case when a symbol is exported with a different name than the 
> local symbol? (This is doable with def files e.g. as `ExportedName = 
> LocalName` iirc.) Is it worth to have a map of address -> exported symbol, to 
> use instead of the raw name?
Indeed it is a good idea to match symbols with different export names to 
synchronize the symbol types. I probably should use a `vector>` sorted by address for lookup instead.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:917
+  // actually be data, but this is relatively rare and we cannot tell
+  // from just the export table.
   symbols[i].SetType(lldb::eSymbolTypeCode);

mstorsjo wrote:
> If it's relevant, we can look up what section the exported address is in, and 
> check if the section is executable or not.
Right, this is something we can do too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134133

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


[Lldb-commits] [PATCH] D134133: WIP: [lldb][COFF] Enhance symtab loading of symbol and export tables

2022-09-19 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:787
+  Symbol *symbols = symtab.Extend(num_syms);
+  uint32_t i = 0;
+  for (const auto &sym_ref : m_binary->symbols()) {

alvinhochun wrote:
> mstorsjo wrote:
> > If we mean to append to the symbol table here, shouldn't we start with `i = 
> > symtab.size()` or similar? Otherwise we'd start overwriting the existing 
> > symbols from the start of the table?
> I made `symtab.Extend` return a pointer to the first item of the appended 
> range, so counting from 0 does not overwrite existing symbols.
Ah, I see - ok, good then, otherwise I was wondering how this was working at 
all :-)



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:819
+  if (exported->GetType() != lldb::eSymbolTypeReExported &&
+  exported->GetAddressRef() == symbols[i].GetAddressRef()) {
+symbols[i].SetID(exported->GetID());

alvinhochun wrote:
> mstorsjo wrote:
> > What about the case when a symbol is exported with a different name than 
> > the local symbol? (This is doable with def files e.g. as `ExportedName = 
> > LocalName` iirc.) Is it worth to have a map of address -> exported symbol, 
> > to use instead of the raw name?
> Indeed it is a good idea to match symbols with different export names to 
> synchronize the symbol types. I probably should use a `vector export_name>>` sorted by address for lookup instead.
Is it even relevant to keep the name in the vector in that case? As you'd only 
be looking up on address anyway, and based on that you can get the symbol name 
from the pointed-at symbol anyway, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134133

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


[Lldb-commits] [PATCH] D134133: WIP: [lldb][COFF] Enhance symtab loading of symbol and export tables

2022-09-19 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:819
+  if (exported->GetType() != lldb::eSymbolTypeReExported &&
+  exported->GetAddressRef() == symbols[i].GetAddressRef()) {
+symbols[i].SetID(exported->GetID());

mstorsjo wrote:
> alvinhochun wrote:
> > mstorsjo wrote:
> > > What about the case when a symbol is exported with a different name than 
> > > the local symbol? (This is doable with def files e.g. as `ExportedName = 
> > > LocalName` iirc.) Is it worth to have a map of address -> exported 
> > > symbol, to use instead of the raw name?
> > Indeed it is a good idea to match symbols with different export names to 
> > synchronize the symbol types. I probably should use a `vector > export_name>>` sorted by address for lookup instead.
> Is it even relevant to keep the name in the vector in that case? As you'd 
> only be looking up on address anyway, and based on that you can get the 
> symbol name from the pointed-at symbol anyway, right?
Ah, I guess I meant to say the symbol index there...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134133

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


[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

2022-09-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:681
+auto &ts = llvm::cast(*ct.GetTypeSystem());
+ts.GetMetadata(&tag)->SetIsForcefullyCompleted();
+  }

zequanwu wrote:
> rnk wrote:
> > Is this what we do for DWARF? The same kind of situation seems like it can 
> > arise, where clang requires a type to be complete, but the information is 
> > missing, and we still try to proceed with evaluation.
> I think it's here: 
> https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L221-L250,
>  relevant: https://reviews.llvm.org/D85968.
The trick is to do the forced completion only when the type is used within 
contexts where the c++ rules require it to be complete. something like `class 
A; A* a;` is perfectly legal c++. `class A; class B : A {};` is not. You can't 
do this from within the completion callback. In DWARF code, we check for this 
when we're parsing the enclosing entity (so, when we're parsing `B`, we'd 
check, and if needed, "forcefully complete" the class `A`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

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


[Lldb-commits] [PATCH] D134111: [lldb] Add newline in output of `target modules lookup`

2022-09-19 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/test/Shell/Commands/command-target-modules-lookup.test:13
+# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc(double)
+# CHECK-NOT:  ignoreThisFunction

This might be better off as an --implicit-check-not argument to FileCheck. As 
it stands now, it will only check that ignoreThisFunction does not appear at 
the very end of the output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134111

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


[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

2022-09-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I think the main reason that the RegisterInfo struct is such as it is, is 
because it wants to be trivially constructible (or whatever the c++ term for 
that is). I'm pretty sure that adding a std::string member will make it stop 
being that, and I don't think we suddenly want to run global constructors for 
all of the global RegisterInfo instances we have (and we have a lot of them).

If you wanted to preserve the existing patterns, the right way to store a 
string/array member inside a static struct would be to copy what is being done 
in the value_regs/invalidate_regs field, i.e., store the value itself in 
another static object, and then store the pointer to that object into 
RegisterInfo. For dynamic RegisterInfos (like those received from the 
lldb-server), you'd use DynamicRegisterInfo and DynamicRegisterInfo::Register, 
which have logic to persist/create storage for these values (that logic would 
have to be extended to handle the new fields as well).

That said I would *love* is someone changed the RegisterInfo structs into 
something saner, but I think that will need to be more elaborate than simply 
stuffing a std::vector member into it. I can give you my idea of how this could 
work, if you're interested in trying this out.




Comment at: lldb/include/lldb/Core/EmulateInstruction.h:196
+  struct RegisterPlusOffsetStruct {
 RegisterInfo reg;  // base register
 int64_t signed_offset; // signed offset added to base register

I actually think that the simplest solution here would be to store the 
RegisterInfos as pointers. Copying them around doesn't make sense, particularly 
if their size is about to grow.



Comment at: lldb/include/lldb/Core/EmulateInstruction.h:342-345
+  info.~ContextUnion();
+  info.info_type = eInfoTypeRegisterRegisterOperands;
+  new (&info.RegisterRegisterOperands.operand1) RegisterInfo(op1_reg);
+  new (&info.RegisterRegisterOperands.operand2) RegisterInfo(op2_reg);

I am not a C++ expert, but I have a strong suspicion that this is not correct. 
You're destroyed the whole object, but then are only recreating its individual 
submembers. I get that these are the only members which have non-trivial 
constructors, but I don't think that's how c++ works. I can confirm this with 
some c++ experts if you want, but I am pretty sure that you need to recreate 
the ContextUnion object as a whole here to avoid straying into UB territory.



Comment at: lldb/include/lldb/lldb-private-types.h:67-77
+  RegisterInfo() { kinds.fill(LLDB_INVALID_REGNUM); }
+
+  RegisterInfo(const char *name, const char *alt_name, uint32_t byte_size,
+   uint32_t byte_offset, lldb::Encoding encoding,
+   lldb::Format format,
+   std::array kinds,
+   uint32_t *value_regs, uint32_t *invalidate_regs)

Is this class still trivially constructible (not requiring dynamic 
initialization)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134041

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


[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

2022-09-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D133906#3792666 , @JDevlieghere 
wrote:

> In D133906#3792230 , @labath wrote:
>
>>   // no .def file
>>   #define LLDB_FORWARD_CLASS(cls) \
>> namespace lldb_private { class cls; } \
>> namespace lldb {  using cls##SP = std::shared_ptr; } \
>> ...
>>   
>>   LLDB_FORWARD_CLASS(Foo)
>>   LLDB_FORWARD_CLASS(Bar)
>>   ...
>
> Works for me, but I don't see how that would help with go-to definition. 
> Xcode still won't show you the macro expansion so there's nothing to click 
> through, which was Jim's complaint.

Right, I see. I misunderstood the problem somehow. That said, I can imagine 
Xcode offering a (clickable) tooltip showin the macro expansion in this case. I 
know of an IDE that does that, and it's pretty cool.

In D133906#3793505 , @clayborg wrote:

> Wouldn't this also slow down compilation a bit? Each file that #include 
> "lldb-forward.h" will not do a large amount of preprocessor stuff for each 
> and every file that is compiled that includes this?

Technically yes, but I very much doubt the difference will be measurable. The 
slowness of compiling c++ comes from instantiating templates and other fancy 
stuff. The preprocessor step is ridiculously fast.


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

https://reviews.llvm.org/D133906

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


[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting with reading/writing memory in a Scripted Process (WIP)

2022-09-19 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/include/lldb/API/SBError.h:95
 private:
-  std::unique_ptr m_opaque_up;
+  std::shared_ptr m_opaque_sp;
 

This is technically an ABI break (changes `sizeof(SBError)`). I don't care, but 
someone might.



Comment at: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp:27
 template  const char *GetPythonValueFormatString(T t);
+// FIXME: This should probably be a PyObject * instead of PythonObject
+template <> const char *GetPythonValueFormatString(python::PythonObject*) { 
return "O"; }

Yes, it should.



Comment at: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h:35
 namespace lldb_private {
+  namespace python {
+  typedef struct swig_type_info swig_type_info;

we don't indent namespaces



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:186
+  StatusSP status_sp = std::make_shared(error);
+  PythonObject* sb_error = new PythonObject(ToSWIGWrapper(status_sp));
+  

mib wrote:
> @labath In order to pass down the `Status&` to python, I create a `StatusSP` 
> (to avoid leaking the `lldb_private::Status` type to python), and turn that 
> into a `python::PyObject` by calling `ToSWIGWrapper`. This is why I moved its 
> declaration to `SWIGPythonBridge.h`.
> 
> Finally, the `python::PyObject` is wrapped into another `PythonObject*` so it 
> can be passed to `GetPythonValueFormatString` and `PyObject_CallMethod` in 
> `ScriptedPythonInterface::Dispatch`
> 
> I tried to follow the logic explained in 7f09ab0, however, when 
> `ToSWIGWrapper` is called at runtime, it crashes in Python:
> 
> ```
> * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS 
> (code=1, address=0x10)
> frame #0: 0x0001056f3c3c Python`PyTuple_New + 212
> frame #1: 0x000135524720 
> liblldb.16.0.0git.dylib`SWIG_Python_NewShadowInstance(data=0x611d6340,
>  swig_this=0x000105ec5d70) at LLDBWrapPython.cpp:2284:28
>   * frame #2: 0x000135515a00 
> liblldb.16.0.0git.dylib`SWIG_Python_NewPointerObj(self=0x, 
> ptr=0x61dc4040, type=0x00014448c140, flags=1) at 
> LLDBWrapPython.cpp:2395:22
> frame #3: 0x0001355157f4 
> liblldb.16.0.0git.dylib`lldb_private::python::ToSWIGHelper(obj=0x61dc4040,
>  info=0x00014448c140) at python-swigsafecast.swig:5:29
> frame #4: 0x000135515e14 
> liblldb.16.0.0git.dylib`lldb_private::python::ToSWIGWrapper(status_sp=std::__1::shared_ptr::element_type
>  @ 0x60ac9a18 strong=3 weak=1) at python-swigsafecast.swig:37:10
> frame #5: 0x0001367383c4 
> liblldb.16.0.0git.dylib`lldb_private::ScriptedProcessPythonInterface::ReadMemoryAtAddress(this=0x611cd2c0,
>  address=8034160640, size=512, error=0x00016b35c650) at 
> ScriptedProcessPythonInterface.cpp:161:45
> ```
> 
> Am I doing something wrong or maybe I'm missing something ?
> 
> 
Well.. I'm not sure if this is the cause of the crash, but one problem I see is 
the passing of `PythonObject*` to the Dispatch function, which forwards it 
unmodified to the `PyObject_CallMethod` function. Python clearly does not know 
anything about our lldb_private::python::PythonObject wrappers. You'll either 
need to pass a PyObject here, or teach Dispatch how to unwrap a PythonObject.

I also don't think that it was necessary to move all of this code around. I 
don't see why would Status be any different from say `StructuredDataImpl`, 
which is already passed as a plain reference (no smart pointers). Using an 
lldb-private object inside `python-swigsafecast.swig` is still ok, as that code 
is still part of liblldb. We already include [[ 
https://github.com/llvm/llvm-project/blob/main/lldb/bindings/python/python.swig#L121
 | some ]] internal headers from the swig files, but I think that in this case 
a forward declaration should be enough as well (you may need to add a 
`SBError(const Status &)` constructor). The opposite (moving this code to 
"regular" files) is not ideal as well, as you've needed to add a bunch of SB 
forward declarations into `SWIGPythonBridge`, and split the implementation of 
ToSWIGWrapper into two files.

I also don't think the shared_ptr transition is helping with anything (and it 
is an ABI break). It might be necessary if the problem was that the SBError 
object was being copied, and you couldn't get a hold of the copy which the user 
code has modified, but in that case you'd also have to modify the objects copy 
constructors to do a shallow (instead of a deep) copy (which in turn could 
break other code which was relying on this).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134033

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lis

[Lldb-commits] [PATCH] D133973: [test] Fix LLDB tests with just-built libcxx when using a target directory.

2022-09-19 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM




Comment at: lldb/test/API/lit.cfg.py:178-179
+  dotest_cmd += ['--libcxx-include-dir', config.libcxx_include_dir]
+  if is_configured('libcxx_include_target_dir'):
+dotest_cmd += ['--libcxx-include-target-dir', 
config.libcxx_include_target_dir]
+  dotest_cmd += ['--libcxx-library-dir', config.libcxx_libs_dir]

Any reason you put this before adding `config.libcxx_libs_dir` below? Same 
question for `lldb-dotest`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133973

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


[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

2022-09-19 Thread Adrian Vogelsgesang via Phabricator via lldb-commits
avogelsgesang updated this revision to Diff 461232.
avogelsgesang marked an inline comment as done.
avogelsgesang added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132815

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -68,7 +68,7 @@
 result_children=[
 ValueCheck(name="resume", summary = test_generator_func_ptr_re),
 ValueCheck(name="destroy", summary = test_generator_func_ptr_re),
-ValueCheck(name="promise", value="-1")
+ValueCheck(name="promise", dereference=ValueCheck(value="-1"))
 ])
 
 # Run until after the `co_yield`
Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
@@ -44,7 +44,9 @@
   size_t GetIndexOfChildWithName(ConstString name) override;
 
 private:
-  lldb::ValueObjectSP m_frame_ptr_sp;
+  lldb::ValueObjectSP m_resume_ptr_sp;
+  lldb::ValueObjectSP m_destroy_ptr_sp;
+  lldb::ValueObjectSP m_promise_ptr_sp;
 };
 
 SyntheticChildrenFrontEnd *
Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -16,34 +16,35 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
-static ValueObjectSP GetCoroFramePtrFromHandle(ValueObject &valobj) {
-  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+static lldb::addr_t GetCoroFramePtrFromHandle(ValueObjectSP valobj_sp) {
   if (!valobj_sp)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
 
   // We expect a single pointer in the `coroutine_handle` class.
   // We don't care about its name.
   if (valobj_sp->GetNumChildren() != 1)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
   ValueObjectSP ptr_sp(valobj_sp->GetChildAtIndex(0, true));
   if (!ptr_sp)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
   if (!ptr_sp->GetCompilerType().IsPointerType())
-return nullptr;
-
-  return ptr_sp;
-}
-
-static Function *ExtractFunction(ValueObjectSP &frame_ptr_sp, int offset) {
-  lldb::TargetSP target_sp = frame_ptr_sp->GetTargetSP();
-  lldb::ProcessSP process_sp = frame_ptr_sp->GetProcessSP();
-  auto ptr_size = process_sp->GetAddressByteSize();
+return LLDB_INVALID_ADDRESS;
 
   AddressType addr_type;
-  lldb::addr_t frame_ptr_addr = frame_ptr_sp->GetPointerValue(&addr_type);
+  lldb::addr_t frame_ptr_addr = ptr_sp->GetPointerValue(&addr_type);
   if (!frame_ptr_addr || frame_ptr_addr == LLDB_INVALID_ADDRESS)
-return nullptr;
+return LLDB_INVALID_ADDRESS;
   lldbassert(addr_type == AddressType::eAddressTypeLoad);
+  if (addr_type != AddressType::eAddressTypeLoad)
+return LLDB_INVALID_ADDRESS;
+
+  return frame_ptr_addr;
+}
+
+static Function *ExtractFunction(lldb::TargetSP target_sp,
+ lldb::addr_t frame_ptr_addr, int offset) {
+  lldb::ProcessSP process_sp = target_sp->GetProcessSP();
+  auto ptr_size = process_sp->GetAddressByteSize();
 
   Status error;
   auto func_ptr_addr = frame_ptr_addr + offset * ptr_size;
@@ -59,12 +60,14 @@
   return func_address.CalculateSymbolContextFunction();
 }
 
-static Function *ExtractResumeFunction(ValueObjectSP &frame_ptr_sp) {
-  return ExtractFunction(frame_ptr_sp, 0);
+static Function *ExtractResumeFunction(lldb::TargetSP target_sp,
+   lldb::addr_t frame_ptr_addr) {
+  return ExtractFunction(target_sp, frame_ptr_addr, 0);
 }
 
-static Function *ExtractDestroyFunction(ValueObjectSP &frame_ptr_sp) {
-  return ExtractFunction(frame_ptr_sp, 1);
+static Function *ExtractDestroyFunction(lldb::TargetSP target_sp,
+lldb::addr_t frame_ptr_addr) {
+  return ExtractFunction(target_sp, frame_ptr_addr, 1);
 }
 
 static bool IsNoopResumeDestroy(Function *f) {
@@ -125,43 +128,26 @@
   return promise_type->GetForwardCompilerType();
 }
 
-static CompilerType GetCoroutineFrameType(TypeSystemClang &ast_ct

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

2022-09-19 Thread Adrian Vogelsgesang via Phabricator via lldb-commits
avogelsgesang added a comment.

> wondering if we couldn't fix this by creating the (non-pointer) object using 
> the CreateValueObjectFromAddress function, as above, but then actually use 
> valobj->AddressOf as the synthetic child

yes, that worked quite nicely


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132815

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


[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

2022-09-19 Thread Adrian Vogelsgesang via Phabricator via lldb-commits
avogelsgesang marked 4 inline comments as done.
avogelsgesang added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:19
 
-static ValueObjectSP GetCoroFramePtrFromHandle(ValueObject &valobj) {
-  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+static uint64_t GetCoroFramePtrFromHandle(ValueObjectSP valobj_sp) {
   if (!valobj_sp)

hawkinsw wrote:
> Do we want to return a `lldb::addr_t`? I know that they are likely the same, 
> but do we want it for consistency?
> 
> I see (below) several instances where we are passing around "addresses" and 
> they are plain `uint64_t` so I am obviously being unhelpful. Sorry!
changed to `lldb::addr_t` in this complete file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132815

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


[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

2022-09-19 Thread Adrian Vogelsgesang via Phabricator via lldb-commits
avogelsgesang updated this revision to Diff 461235.
avogelsgesang marked an inline comment as done.
avogelsgesang added a comment.

rename `IsNoopResumeDestroy` -> `IsNoopCoroFunction`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132735

Files:
  lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
@@ -45,6 +45,7 @@
   std::coroutine_handle<> type_erased_hdl = gen.hdl;
   std::coroutine_handle incorrectly_typed_hdl =
   std::coroutine_handle::from_address(gen.hdl.address());
+  std::coroutine_handle<> noop_hdl = std::noop_coroutine();
   gen.hdl.resume();// Break at initial_suspend
   gen.hdl.resume();// Break after co_yield
   empty_function_so_we_can_set_a_breakpoint(); // Break at final_suspend
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -38,6 +38,13 @@
 ValueCheck(name="current_value", value = "-1"),
 ])
 ])
+# We recognize and pretty-print `std::noop_coroutine`. We don't display
+# any children as those are irrelevant for the noop coroutine.
+# clang version < 16 did not yet write debug info for the noop coroutines.
+if not (is_clang and self.expectedCompilerVersion(["<", "16"])):
+self.expect_expr("noop_hdl",
+result_summary="noop_coroutine",
+result_children=[])
 if is_clang:
 # For a type-erased `coroutine_handle<>`, we can still devirtualize
 # the promise call and display the correctly typed promise.
Index: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -34,7 +34,7 @@
   return ptr_sp;
 }
 
-static Function *ExtractDestroyFunction(ValueObjectSP &frame_ptr_sp) {
+static Function *ExtractFunction(ValueObjectSP &frame_ptr_sp, int offset) {
   lldb::TargetSP target_sp = frame_ptr_sp->GetTargetSP();
   lldb::ProcessSP process_sp = frame_ptr_sp->GetProcessSP();
   auto ptr_size = process_sp->GetAddressByteSize();
@@ -46,24 +46,64 @@
   lldbassert(addr_type == AddressType::eAddressTypeLoad);
 
   Status error;
-  // The destroy pointer is the 2nd pointer inside the compiler-generated
-  // `pair`.
-  auto destroy_func_ptr_addr = frame_ptr_addr + ptr_size;
-  lldb::addr_t destroy_func_addr =
-  process_sp->ReadPointerFromMemory(destroy_func_ptr_addr, error);
+  auto func_ptr_addr = frame_ptr_addr + offset * ptr_size;
+  lldb::addr_t func_addr =
+  process_sp->ReadPointerFromMemory(func_ptr_addr, error);
   if (error.Fail())
 return nullptr;
 
-  Address destroy_func_address;
-  if (!target_sp->ResolveLoadAddress(destroy_func_addr, destroy_func_address))
+  Address func_address;
+  if (!target_sp->ResolveLoadAddress(func_addr, func_address))
 return nullptr;
 
-  Function *destroy_func =
-  destroy_func_address.CalculateSymbolContextFunction();
-  if (!destroy_func)
-return nullptr;
+  return func_address.CalculateSymbolContextFunction();
+}
 
-  return destroy_func;
+static Function *ExtractResumeFunction(ValueObjectSP &frame_ptr_sp) {
+  return ExtractFunction(frame_ptr_sp, 0);
+}
+
+static Function *ExtractDestroyFunction(ValueObjectSP &frame_ptr_sp) {
+  return ExtractFunction(frame_ptr_sp, 1);
+}
+
+static bool IsNoopCoroFunction(Function *f) {
+  if (!f)
+return false;
+
+  // clang's `__builtin_coro_noop` gets lowered to
+  // `_NoopCoro_ResumeDestroy`. This is used by libc++
+  // on clang.
+  auto mangledName = f->GetMangled().GetMangledName();
+  if (mangledName == "__NoopCoro_ResumeDestroy")
+return true;
+
+  // libc++ uses the following name as a fallback on
+  // compilers without `__builtin_coro_noop`.
+  auto name = f->GetNameNoArguments();
+  static RegularExpression libcxxRegex(
+  "^std::coroutine_handle::"

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

2022-09-19 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG974958749827: [lldb] Reset breakpoint hit count before new 
runs (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

Files:
  lldb/include/lldb/Breakpoint/Breakpoint.h
  lldb/include/lldb/Breakpoint/BreakpointList.h
  lldb/include/lldb/Breakpoint/BreakpointLocation.h
  lldb/include/lldb/Breakpoint/BreakpointLocationList.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointList.cpp
  lldb/source/Breakpoint/BreakpointLocationList.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile
  
lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py
  lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp

Index: lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp
@@ -0,0 +1,6 @@
+#include 
+
+int main(int argc, char const *argv[]) {
+  printf("Set a breakpoint here.\n");
+  return 0;
+}
Index: lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py
@@ -0,0 +1,58 @@
+"""
+Test breakpoint hit count is reset when target runs.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class HitcountResetUponRun(TestBase):
+BREAKPOINT_TEXT = 'Set a breakpoint here'
+
+def check_stopped_at_breakpoint_and_hit_once(self, thread, breakpoint):
+frame0 = thread.GetFrameAtIndex(0)
+location1 = breakpoint.FindLocationByAddress(frame0.GetPC())
+self.assertTrue(location1)
+self.assertEqual(location1.GetHitCount(), 1)
+self.assertEqual(breakpoint.GetHitCount(), 1)
+
+def test_hitcount_reset_upon_run(self):
+self.build()
+
+exe = self.getBuildArtifact("a.out")
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+breakpoint = target.BreakpointCreateBySourceRegex(
+self.BREAKPOINT_TEXT, lldb.SBFileSpec('main.cpp'))
+self.assertTrue(
+breakpoint.IsValid() and breakpoint.GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(process, PROCESS_IS_VALID)
+
+from lldbsuite.test.lldbutil import get_stopped_thread
+
+# Verify 1st breakpoint location is hit.
+thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint")
+self.check_stopped_at_breakpoint_and_hit_once(thread, breakpoint)
+
+# Relaunch
+process.Kill()
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Verify the hit counts are still one.
+thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint")
+self.check_stopped_at_breakpoint_and_hit_once(thread, breakpoint)
Index: lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -313,8 +313,9 @@
 substrs=['stopped',
  

[Lldb-commits] [lldb] 9749587 - [lldb] Reset breakpoint hit count before new runs

2022-09-19 Thread Felipe de Azevedo Piovezan via lldb-commits

Author: Felipe de Azevedo Piovezan
Date: 2022-09-19T12:56:12-04:00
New Revision: 974958749827bc4fb88b67feab4bcd7536f70456

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

LOG: [lldb] Reset breakpoint hit count before new runs

A common debugging pattern is to set a breakpoint that only stops after
a number of hits is recorded. The current implementation never resets
the hit count of breakpoints; as such, if a user re-`run`s their
program, the debugger will never stop on such a breakpoint again.

This behavior is arguably undesirable, as it renders such breakpoints
ineffective on all but the first run. This commit changes the
implementation of the `Will{Launch, Attach}` methods so that they reset
the _target's_ breakpoint hitcounts.

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

Added: 
lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile

lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py
lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp

Modified: 
lldb/include/lldb/Breakpoint/Breakpoint.h
lldb/include/lldb/Breakpoint/BreakpointList.h
lldb/include/lldb/Breakpoint/BreakpointLocation.h
lldb/include/lldb/Breakpoint/BreakpointLocationList.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Target.h
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Breakpoint/BreakpointList.cpp
lldb/source/Breakpoint/BreakpointLocationList.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp

lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 39c6d0b7f3a77..701d0823bd282 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -330,6 +330,9 @@ class Breakpoint : public 
std::enable_shared_from_this,
   /// The current hit count for all locations.
   uint32_t GetHitCount() const;
 
+  /// Resets the current hit count for all locations.
+  void ResetHitCount();
+
   /// If \a one_shot is \b true, breakpoint will be deleted on first hit.
   void SetOneShot(bool one_shot);
 

diff  --git a/lldb/include/lldb/Breakpoint/BreakpointList.h 
b/lldb/include/lldb/Breakpoint/BreakpointList.h
index 346972ec3a1fc..a7399d385f6f0 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -138,6 +138,9 @@ class BreakpointList {
 
   void ClearAllBreakpointSites();
 
+  /// Resets the hit count of all breakpoints.
+  void ResetHitCounts();
+
   /// Sets the passed in Locker to hold the Breakpoint List mutex.
   ///
   /// \param[in] lock

diff  --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index a6d1232162fce..2a4f9fc01bf32 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -87,6 +87,9 @@ class BreakpointLocation
   /// Return the current Hit Count.
   uint32_t GetHitCount() const { return m_hit_counter.GetValue(); }
 
+  /// Resets the current Hit Count.
+  void ResetHitCount() { m_hit_counter.Reset(); }
+
   /// Return the current Ignore Count.
   ///
   /// \return

diff  --git a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
index 4b36c919ee3c5..f76a8fcfdd7e7 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
@@ -126,6 +126,9 @@ class BreakpointLocationList {
   /// Hit count of all locations in this list.
   uint32_t GetHitCount() const;
 
+  /// Resets the hit count of all locations in this list.
+  void ResetHitCount();
+
   /// Enquires of the breakpoint location in this list with ID \a breakID
   /// whether we should stop.
   ///

diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 4f5ae45a431ec..6975eb8029de0 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -882,13 +882,28 @@ class Process : public 
std::enable_shared_from_this,
   // Plug-in Process Control Overrides
   //==
 
+  /// Called before attaching to a proce

[Lldb-commits] [PATCH] D134196: [lldb][COFF] Rewrite ParseSymtab to list both export and symbol tables

2022-09-19 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun created this revision.
alvinhochun added reviewers: labath, DavidSpickett, mstorsjo.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

This reimplements `ObjectFilePECOFF::ParseSymtab` to replace the manual
data extraction with what `COFFObjectFile` already provides. Also use
`SymTab::AddSymbol` instead of resizing the SymTab then assigning each
elements afterwards.

Previously, ParseSymTab loads symbols from both the COFF symbol table
and the export table, but if there are any entries in the export table,
it overwrites all the symbols already loaded from the COFF symbol table.
Due to the change to use AddSymbols, this no longer happens, and so the
SymTab now contains all symbols from both tables as expected.

The export symbols are now ordered by ordinal, instead of by the name
table order.

In its current state, it is possible for symbols in the COFF symbol
table to be duplicated by those in the export table. This behaviour will
be modified in a separate change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134196

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/test/Shell/ObjectFile/PECOFF/symbols-export-table.yaml
  llvm/include/llvm/Object/COFF.h

Index: llvm/include/llvm/Object/COFF.h
===
--- llvm/include/llvm/Object/COFF.h
+++ llvm/include/llvm/Object/COFF.h
@@ -914,6 +914,10 @@
 
   uint32_t getStringTableSize() const { return StringTableSize; }
 
+  const export_directory_table_entry *getExportTable() const {
+return ExportDirectory;
+  }
+
   const coff_load_configuration32 *getLoadConfig32() const {
 assert(!is64());
 return reinterpret_cast(LoadConfig);
Index: lldb/test/Shell/ObjectFile/PECOFF/symbols-export-table.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/symbols-export-table.yaml
@@ -0,0 +1,103 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test symbols %t | FileCheck %s
+
+# Checks that the symtab contains both symbols from the export table and the
+# COFF symbol table.
+
+# CHECK:  DSX Type File Address/Value {{.*}} SizeFlags   Name
+# CHECK-NEXT: ---
+# CHECK-NEXT: D   Code 0x0001800010100x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportfunc
+# CHECK-NEXT: Code 0x0001800010000x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} entry
+# CHECK-NEXT: Code 0x0001800010100x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportfunc
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4096
+  ImageBase:   6442450944
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_GUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+RelativeVirtualAddress: 8224
+Size:87
+  ImportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:
+RelativeVirtualAddress: 0
+Size:0
+  CertificateTable:
+RelativeVirtualAddress: 0
+Size:0
+  BaseRelocationTable:
+RelativeVirtualAddress: 0
+Size:0
+  Debug:
+RelativeVirtualAddress: 0
+Size:0
+  Architecture:
+RelativeVirtualAddress: 0
+Size:0
+  GlobalPtr:
+RelativeVirtualAddress: 0
+Size:0
+  TlsTable:
+RelativeVirtualAddress: 0
+Size:0
+  LoadConfigTable:
+RelativeVirtualAddress: 0
+Size:0
+  BoundImport:
+RelativeVirtualAddress: 0
+Size:0
+  IAT:
+RelativeVirtualAddress: 0
+Size:0
+  DelayImportDescriptor:
+RelativeVirtualAddress: 0
+Size:0
+  ClrRuntimeHeader:
+RelativeVirtualAddress: 0
+Size:0
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE, IMAGE_FILE_DLL ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 17
+SectionData: C32E0F1F8400C3
+  - Name:.rdata
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 119
+SectionData: 

[Lldb-commits] [PATCH] D134133: WIP: [lldb][COFF] Enhance symtab loading of symbol and export tables

2022-09-19 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun added a comment.

Separated the first part with some new changes here: 
https://reviews.llvm.org/D134196


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134133

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


[Lldb-commits] [PATCH] D131705: Don't create sections for SHN_ABS symbols in ELF files.

2022-09-19 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1550
+} else {
+  strm.IndentMore();
+  strm.Indent("Value: ");

alvinhochun wrote:
> This `IndentMore` added is missing a matching `IndentLess` call.
Yes, a fix is needed. Feel free to post a patch, or I will try to get to this 
soon.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1551-1556
+  strm.Indent("Value: ");
+  strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetRawValue());
+  if (symbol->GetByteSizeIsValid()) {
+strm.Indent("Size: ");
+strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetByteSize());
+  }

alvinhochun wrote:
> May I ask, is it expected for this to print only the value and size but not 
> the name of the symbol?
> 
> Also, in some COFF binaries linked by LLD there are a bunch of absolute 
> symbols (e.g. `__guard_flags`) and when they hit this code path all they 
> print is `Value: 0x`.
> 
> llvm-readobj shows the symbol as:
> 
> ```
>   Symbol {
> Name: __guard_flags
> Value: 2147550464
> Section: IMAGE_SYM_ABSOLUTE (-1)
> BaseType: Null (0x0)
> ComplexType: Null (0x0)
> StorageClass: External (0x2)
> AuxSymbolCount: 0
>   }
> ```
> Though neither values are correct. I expect the value to be `0x10500`. I 
> haven't looked into this yet. Any idea what might be going on?
We should be printing the name. I guess in the "symbol->ValueIsAddress()" case 
it would print the name from the symbolication of the address. Should be an 
easy fix, feel free to submit a patch, or I can try to get to it when i have 
some time.

I would look at the lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
and check the "ObjectFilePECOFF::ParseSymtab(Symtab &symtab)" method. It must 
not be creating the symbols correctly from the symbol table. Any fixes for the 
PECOFF object file plug-in would be appreciated if you find any issues or 
things we can do better. Back in the day I had a hard time getting any symbol 
table the show up in the PECOFF file, but that was a long time ago and I wasn't 
super familiar with Windows binaries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131705

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


[Lldb-commits] [lldb] dc9e6c5 - Add auto deduce source map setting

2022-09-19 Thread Jeffrey Tan via lldb-commits

Author: Jeffrey Tan
Date: 2022-09-19T13:40:22-07:00
New Revision: dc9e6c52f3d805ab454bcf2a4502037d15fb2b8c

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

LOG: Add auto deduce source map setting

This patch adds a new "target.auto-source-map-relative" setting.

If enabled, this setting may auto deduce a source map entry based on 
requested
breakpoint path and the original path stored in debug info for resolved
breakpoint.

As an example, if debug info contains "./a/b/c/main.cpp", user sets a source
breakpoint at "/root/repo/x/y/z/a/b/c/main.cpp". The breakpoint will resolve
correctly now with Greg's patch https://reviews.llvm.org/D130401. However, 
the
resolved breakpoint will use "./a/b/c/main.cpp" to locate source file during
stop event which would fail most of the time.

With the new "target.auto-source-map-relative" setting enabled, a auto 
deduced
source map entry "." => "/root/repo/x/y/z" will be added. This new mapping 
will
help lldb to map resolved breakpoint path "./a/b/c/main.cpp" back to
"/root/repo/x/y/z/a/b/c/main.cpp" and locate it on disk.

If an existing source map entry is used the patch also concatenates the auto
deduced entry with any stripped reverse mapping prefix (see example below).

As a second example, debug info contains "./a/b/c/main.cpp" and user sets
breakpoint at "/root/repo/x/y/z/a/b/c/main.cpp". Let's say there is an 
existing
source map entry "." => "/root/repo"; this mapping would strip the prefix 
out of
"/root/repo/x/y/z/a/b/c/main.cpp" and use "x/y/z/a/b/c/main.cpp" to resolve
breakpoint. "target.auto-source-map-relative" setting would auto deduce a 
new
potential mapping of "." => "x/y/z", then it detects that there is a 
stripped
prefix from reverse mapping and concatenates it as the new mapping:
 "." => "/root/repo/x/y/z" which would correct map "./a/b/c/main.cpp" path 
to
new path in disk.

This patches depends on https://reviews.llvm.org/D130401 to use new added
SBDebugger::GetSetting() API for testing.

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

Added: 


Modified: 
lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
lldb/include/lldb/Target/PathMappingList.h
lldb/include/lldb/Target/Target.h
lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
lldb/source/Target/PathMappingList.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
lldb/unittests/Target/PathMappingListTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h 
b/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
index e73632aad6a85..bc1fdff92b8db 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
@@ -21,9 +21,10 @@ namespace lldb_private {
 
 class BreakpointResolverFileLine : public BreakpointResolver {
 public:
-  BreakpointResolverFileLine(const lldb::BreakpointSP &bkpt,
- lldb::addr_t offset, bool skip_prologue,
- const SourceLocationSpec &location_spec);
+  BreakpointResolverFileLine(
+  const lldb::BreakpointSP &bkpt, lldb::addr_t offset, bool skip_prologue,
+  const SourceLocationSpec &location_spec,
+  llvm::Optional removed_prefix_opt = llvm::None);
 
   static BreakpointResolver *
   CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
@@ -57,10 +58,14 @@ class BreakpointResolverFileLine : public 
BreakpointResolver {
 
 protected:
   void FilterContexts(SymbolContextList &sc_list);
+  void DeduceSourceMapping(SymbolContextList &sc_list);
 
   friend class Breakpoint;
   SourceLocationSpec m_location_spec;
   bool m_skip_prologue;
+  // Any previously removed file path prefix by reverse source mapping.
+  // This is used to auto deduce source map if needed.
+  llvm::Optional m_removed_prefix_opt;
 
 private:
   BreakpointResolverFileLine(const BreakpointResolverFileLine &) = delete;

diff  --git a/lldb/include/lldb/Target/PathMappingList.h 
b/lldb/include/lldb/Target/PathMappingList.h
index badbe3e0f8c63..413809ba3f5d2 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -37,6 +37,9 @@ class PathMappingList {
 
   void Append(const PathMappingList &rhs, bool notify);
 
+  void AppendUnique(llvm::StringRef path, llvm::StringRef replacement,
+bool notify);
+
   void Clear(bool notify);
 
   // By default, dump all pairs.
@@ -88,7 +91,22 @@ class PathMappingList {
  bool only_if_exists = false) co

[Lldb-commits] [PATCH] D133042: Add auto deduce source map setting

2022-09-19 Thread jeffrey tan via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc9e6c52f3d8: Add auto deduce source map setting (authored 
by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133042

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Target/PathMappingList.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/unittests/Target/PathMappingListTest.cpp

Index: lldb/unittests/Target/PathMappingListTest.cpp
===
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -40,7 +40,7 @@
 map.RemapPath(ConstString(match.original.GetPath()), actual_remapped));
 EXPECT_EQ(FileSpec(actual_remapped.GetStringRef()), match.remapped);
 FileSpec unmapped_spec;
-EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec));
+EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec).hasValue());
 std::string unmapped_path = unmapped_spec.GetPath();
 EXPECT_EQ(unmapped_path, orig_normalized);
   }
Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -8,6 +8,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import json
 import os
 import side_effect
 
@@ -97,6 +98,8 @@
 "/x/main.cpp",
 "./x/y/a/d/c/main.cpp",
 ]
+# Reset source map.
+self.runCmd("settings clear target.source-map")
 for path in invalid_paths:
 bkpt = target.BreakpointCreateByLocation(path, 2)
 self.assertTrue(bkpt.GetNumLocations() == 0,
@@ -429,3 +432,68 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
+
+
+def get_source_map_json(self):
+stream = lldb.SBStream()
+self.dbg.GetSetting("target.source-map").GetAsJSON(stream)
+return json.loads(stream.GetData())
+
+def verify_source_map_entry_pair(self, entry, original, replacement):
+self.assertEquals(entry[0], original,
+"source map entry 'original' does not match")
+self.assertEquals(entry[1], replacement,
+"source map entry 'replacement' does not match")
+
+@skipIf(oslist=["windows"])
+@no_debug_info_test
+def test_breakpoints_auto_source_map_relative(self):
+"""
+Test that with target.auto-source-map-relative settings.
+
+The "relative.yaml" contains a line table that is:
+
+Line table for a/b/c/main.cpp in `a.out
+0x00013f94: a/b/c/main.cpp:1
+0x00013fb0: a/b/c/main.cpp:2:3
+0x00013fb8: a/b/c/main.cpp:2:3
+"""
+src_dir = self.getSourceDir()
+yaml_path = os.path.join(src_dir, "relative.yaml")
+yaml_base, ext = os.path.splitext(yaml_path)
+obj_path = self.getBuildArtifact("a.out")
+self.yaml2obj(yaml_path, obj_path)
+
+# Create a target with the object file we just created from YAML
+target = self.dbg.CreateTarget(obj_path)
+# We now have debug information with line table paths that start are
+# "./a/b/c/main.cpp".
+
+source_map_json = self.get_source_map_json()
+self.assertEquals(len(source_map_json), 0, "source map should be empty initially")
+
+# Verify auto deduced source map when file path in debug info
+# is a suffix of request breakpoint file path
+path = "/x/y/a/b/c/main.cpp"
+bp = target.BreakpointCreateByLocation(path, 2)
+self.assertTrue(bp.GetNumLocations() > 0,
+'Couldn\'t resolve breakpoint using full path "%s" in executate "%s" with '
+'debug info that has relative path with matching suffix' % (path, self.getBuildArtifact("a.out")))
+
+source_map_json = self.get_source_map_json()
+self.assertEquals(len(source_map_json), 1, "source map should not be empty")
+self.verify_source_map_entry_pair(source_map_json[0], ".", "/x/y")
+
+# Reset source map.
+self.runCmd("settings clear target.source-map")
+
+# Verify source map will not auto deduced when file path of request breakpoint
+# equals the file path in debug info.
+p

[Lldb-commits] [PATCH] D134244: [NFCI] Clean up enum FormatCategoryItem.

2022-09-19 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
jgorbe created this revision.
jgorbe added reviewers: labath, rupprecht, jingham.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.



- Merge pairs like `eFormatCategoryItemSummary` and 
`eFormatCategoryItemRegexSummary` into a single value. See explanation below.

- Rename `eFormatCategoryItemValue` to `eFormatCategoryItemFormat`. This makes 
the enum match the names used elsewhere for formatter kinds (format, summary, 
filter, synth).

- Delete unused values `eFormatCategoryItemValidator` and 
`eFormatCategoryItemRegexValidator`.

This enum is only used to reuse some code in CommandObjectType.cpp.  For
example, instead of having separate implementations for `type summary
delete`, `type format delete`, and so on, there's a single generic
implementation that takes an enum value, and then the specific commands
derive from it and set the right flags for the specific kind of
formatter.

Even though the enum distinguishes between regular and regex matches for
every kind of formatter, this distinction is never used: enum values are
always specified in pairs like
`eFormatCategoryItemSummary | eFormatCategoryItemRegexSummary`.

This causes some ugly code duplication in TypeCategory.cpp. In order to
handle every flag combination some code appears 8 times:

{format, summary, synth, filter} x {exact, regex}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134244

Files:
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/DataFormatters/DataVisualization.cpp
  lldb/source/DataFormatters/TypeCategory.cpp

Index: lldb/source/DataFormatters/TypeCategory.cpp
===
--- lldb/source/DataFormatters/TypeCategory.cpp
+++ lldb/source/DataFormatters/TypeCategory.cpp
@@ -142,53 +142,49 @@
 }
 
 void TypeCategoryImpl::Clear(FormatCategoryItems items) {
-  if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue)
+  if (items & eFormatCategoryItemFormat) {
 GetTypeFormatsContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexValue) == eFormatCategoryItemRegexValue)
 GetRegexTypeFormatsContainer()->Clear();
+  }
 
-  if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary)
+  if (items & eFormatCategoryItemSummary) {
 GetTypeSummariesContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexSummary) ==
-  eFormatCategoryItemRegexSummary)
 GetRegexTypeSummariesContainer()->Clear();
+  }
 
-  if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter)
+  if (items & eFormatCategoryItemFilter) {
 GetTypeFiltersContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexFilter) ==
-  eFormatCategoryItemRegexFilter)
 GetRegexTypeFiltersContainer()->Clear();
+  }
 
-  if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
+  if (items & eFormatCategoryItemSynth) {
 GetTypeSyntheticsContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
 GetRegexTypeSyntheticsContainer()->Clear();
+  }
 }
 
 bool TypeCategoryImpl::Delete(ConstString name, FormatCategoryItems items) {
   bool success = false;
 
-  if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue)
+  if (items & eFormatCategoryItemFormat) {
 success = GetTypeFormatsContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexValue) == eFormatCategoryItemRegexValue)
 success = GetRegexTypeFormatsContainer()->Delete(name) || success;
+  }
 
-  if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary)
+  if (items & eFormatCategoryItemSummary) {
 success = GetTypeSummariesContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexSummary) ==
-  eFormatCategoryItemRegexSummary)
 success = GetRegexTypeSummariesContainer()->Delete(name) || success;
+  }
 
-  if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter)
+  if (items & eFormatCategoryItemFilter) {
 success = GetTypeFiltersContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexFilter) ==
-  eFormatCategoryItemRegexFilter)
 success = GetRegexTypeFiltersContainer()->Delete(name) || success;
+  }
 
-  if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
+  if (items & eFormatCategoryItemSynth) {
 success = GetTypeSyntheticsContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
 success = GetRegexTypeSyntheticsContainer()->Delete(name) || success;
+  }
 
   return success;
 }
@@ -196,27 +192,25 @@
 uint32_t TypeCategoryImpl::GetCount(FormatCategoryItems items) {
   uint32_t count = 0;
 
-  if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue)
+  if (items & eFormatCategoryItemFormat) {
 count += GetTypeFormatsContainer()->GetCount();
- 

[Lldb-commits] [PATCH] D134245: [lldb] Actually support more than 32 logging categories

2022-09-19 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg.
Herald added a project: All.
JDevlieghere requested review of this revision.

In January, Greg put up a patch (D117382 ) to 
support more than 32 log categories, among other things. That led to a bunch of 
nice cleanups, but categories are still limited to 32 because different places 
still using `uint32_t`. This patch fixes the remaining issues and enables 
adding a 32nd log category.


https://reviews.llvm.org/D134245

Files:
  lldb/include/lldb/Utility/LLDBLog.h
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp

Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -60,13 +60,14 @@
   });
 }
 
-uint32_t Log::GetFlags(llvm::raw_ostream &stream, const ChannelMap::value_type &entry,
- llvm::ArrayRef categories) {
+Log::MaskType Log::GetFlags(llvm::raw_ostream &stream,
+const ChannelMap::value_type &entry,
+llvm::ArrayRef categories) {
   bool list_categories = false;
-  uint32_t flags = 0;
+  Log::MaskType flags = 0;
   for (const char *category : categories) {
 if (llvm::StringRef("all").equals_insensitive(category)) {
-  flags |= UINT32_MAX;
+  flags |= std::numeric_limits::max();
   continue;
 }
 if (llvm::StringRef("default").equals_insensitive(category)) {
@@ -91,7 +92,7 @@
 }
 
 void Log::Enable(const std::shared_ptr &handler_sp,
- uint32_t options, uint32_t flags) {
+ uint32_t options, Log::MaskType flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_or(flags, std::memory_order_relaxed);
@@ -102,7 +103,7 @@
   }
 }
 
-void Log::Disable(uint32_t flags) {
+void Log::Disable(Log::MaskType flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_and(~flags, std::memory_order_relaxed);
@@ -126,7 +127,7 @@
   return m_options.load(std::memory_order_relaxed);
 }
 
-const Flags Log::GetMask() const {
+Log::MaskType Log::GetMask() const {
   return m_mask.load(std::memory_order_relaxed);
 }
 
@@ -203,7 +204,7 @@
 void Log::Unregister(llvm::StringRef name) {
   auto iter = g_channel_map->find(name);
   assert(iter != g_channel_map->end());
-  iter->second.Disable(UINT32_MAX);
+  iter->second.Disable(std::numeric_limits::max());
   g_channel_map->erase(iter);
 }
 
@@ -216,7 +217,7 @@
 error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
 return false;
   }
-  uint32_t flags = categories.empty()
+  MaskType flags = categories.empty()
? iter->second.m_channel.default_flags
: GetFlags(error_stream, *iter, categories);
   iter->second.Enable(log_handler_sp, log_options, flags);
@@ -231,8 +232,8 @@
 error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
 return false;
   }
-  uint32_t flags = categories.empty()
-   ? UINT32_MAX
+  MaskType flags = categories.empty()
+   ? std::numeric_limits::max()
: GetFlags(error_stream, *iter, categories);
   iter->second.Disable(flags);
   return true;
@@ -267,7 +268,7 @@
 
 void Log::DisableAllLogChannels() {
   for (auto &entry : *g_channel_map)
-entry.second.Disable(UINT32_MAX);
+entry.second.Disable(std::numeric_limits::max());
 }
 
 void Log::ForEachChannelCategory(
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -166,7 +166,7 @@
 // output will be discarded.
 Log *GetLog(MaskType mask) {
   Log *log = log_ptr.load(std::memory_order_relaxed);
-  if (log && log->GetMask().AnySet(mask))
+  if (log && ((log->GetMask() & mask) != 0))
 return log;
   return nullptr;
 }
@@ -243,7 +243,7 @@
 
   const Flags GetOptions() const;
 
-  const Flags GetMask() const;
+  MaskType GetMask() const;
 
   bool GetVerbose() const;
 
@@ -276,9 +276,9 @@
   }
 
   void Enable(const std::shared_ptr &handler_sp, uint32_t options,
-  uint32_t flags);
+  MaskType flags);
 
-  void Disable(uint32_t flags);
+  void Disable(MaskType flags);
 
   bool Dump(llvm::raw_ostream &stream);
 
@@ -291,8 +291,9 @@
 
   static void ListCategories(llvm::raw_ostream &stream,
  const ChannelMap::value_type &entry);
-  static uint32_t GetFlags(llvm::raw_ostream &stream, const ChannelMap::value_type &entry,
-   llvm::ArrayRef categories);
+  static Log::MaskType GetFlags(llvm::raw_ostream &stream,
+const ChannelMap::value_type &entry,
+llvm::ArrayRef categories);
 
   Log(const Log &) = delet

[Lldb-commits] [lldb] d3a536f - [lldb] Appease the MSCV compiler

2022-09-19 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2022-09-19T17:37:27-07:00
New Revision: d3a536fa933bba5ffad48d7e5ab7de6f358550e7

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

LOG: [lldb] Appease the MSCV compiler

Fix error C2027: use of undefined type 'llvm::MemoryBuffer'.

Added: 


Modified: 
lldb/include/lldb/Core/DataFileCache.h
lldb/source/Core/DataFileCache.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/DataFileCache.h 
b/lldb/include/lldb/Core/DataFileCache.h
index 300c4842d9d2..b6cc3cd8f1cc 100644
--- a/lldb/include/lldb/Core/DataFileCache.h
+++ b/lldb/include/lldb/Core/DataFileCache.h
@@ -16,6 +16,8 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/Caching.h"
+#include "llvm/Support/MemoryBuffer.h"
+
 #include 
 
 namespace lldb_private {

diff  --git a/lldb/source/Core/DataFileCache.cpp 
b/lldb/source/Core/DataFileCache.cpp
index a678cd91efb8..36efccd296a3 100644
--- a/lldb/source/Core/DataFileCache.cpp
+++ b/lldb/source/Core/DataFileCache.cpp
@@ -15,7 +15,6 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "llvm/Support/CachePruning.h"
-#include "llvm/Support/MemoryBuffer.h"
 
 using namespace lldb_private;
 



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


[Lldb-commits] [PATCH] D134252: Track .dwo/.dwp loading errors and notify user when viewing variables.

2022-09-19 Thread Greg Clayton via Phabricator via lldb-commits
clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, aadsm, yinghuitan.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When debugging using Fission (-gsplit-dwarf), we can sometimes have issues 
loading .dwo files if they are missing or if the path was relative and we were 
unable to locate the file. We can also skip loading due to DWO ID mismatch or 
if a .dwp file doesn't contain a matching .dwo file. Also .dwo files could be 
updated due to a recompile and if the user debugs an executable that was linked 
against the old .dwo file, it could fail to load the information.

This patch adds a m_dwo_error to DWARFUnit that can be get/set and will cause 
"frame variable" to show errors when there are .dwo/.dwp issues informing the 
user about the error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134252

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/commands/frame/var/TestFrameVar.py

Index: lldb/test/API/commands/frame/var/TestFrameVar.py
===
--- lldb/test/API/commands/frame/var/TestFrameVar.py
+++ lldb/test/API/commands/frame/var/TestFrameVar.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import os
+import shutil
 import time
 
 class TestFrameVar(TestBase):
@@ -182,3 +183,61 @@
 'no variable information is available in debug info for this compile unit'
 ]
 self.check_frame_variable_errors(thread, error_strings)
+
+@skipUnlessPlatform(["linux", "freebsd"])
+@add_test_categories(["dwo"])
+def test_fission_missing_dwo(self):
+'''
+Test that if we build a binary with "-gsplit-dwarf" that we can
+set a file and line breakpoint successfully, and get an error
+letting us know we were unable to load the .dwo file.
+'''
+self.build(dictionary={'CFLAGS_EXTRAS': '-gsplit-dwarf'})
+exe = self.getBuildArtifact("a.out")
+main_dwo = self.getBuildArtifact("main.dwo")
+
+# self.assertTrue(False, exe)
+self.assertTrue(os.path.exists(main_dwo), 'Make sure "%s" file exists' % (main_dwo))
+# Delete the main.dwo file that contains the debug info so we force an
+# error when we run to main and try to get variables.
+os.unlink(main_dwo)
+
+# We have to set a named breakpoint because we don't have any debug info
+# because we deleted the main.o file since the mod times don't match
+# and debug info won't be loaded
+(target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(self, 'main')
+error_strings = [
+'unable to locate .dwo debug file "',
+'main.dwo" for skeleton DIE 0x'
+]
+self.check_frame_variable_errors(thread, error_strings)
+
+@skipUnlessPlatform(["linux", "freebsd"])
+@add_test_categories(["dwo"])
+def test_fission_invalid_dwo_objectfile(self):
+'''
+Test that if we build a binary with "-gsplit-dwarf" that we can
+set a file and line breakpoint successfully, and get an error
+letting us know we were unable to load the .dwo file because it
+existed, but it wasn't a valid object file.
+'''
+self.build(dictionary={'CFLAGS_EXTRAS': '-gsplit-dwarf'})
+exe = self.getBuildArtifact("a.out")
+main_dwo = self.getBuildArtifact("main.dwo")
+
+# self.assertTrue(False, exe)
+self.assertTrue(os.path.exists(main_dwo), 'Make sure "%s" file exists' % (main_dwo))
+# Overwrite the main.dwo with the main.c source file so that the .dwo
+# file exists, but it isn't a valid object file as there is an error
+# for this case.
+shutil.copyfile(self.getSourcePath('main.c'), main_dwo)
+
+# We have to set a named breakpoint because we don't have any debug info
+# because we deleted the main.o file since the mod times don't match
+# and debug info won't be loaded
+(target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(self, 'main')
+error_strings = [
+'unable to load object file for .dwo debug file "'
+'main.dwo" for unit DIE 0x',
+]
+self.check_frame_variable_errors(thread, error_strings)
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1732,13 +1732,17 @@
 return nullptr;
 
   DWARFCompileUnit *dwarf_cu = llvm::dyn_cast(&unit);
-  // Only compile units can be split int

[Lldb-commits] [lldb] 3b4db10 - [lldb] Actually support more than 32 logging categories

2022-09-19 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2022-09-19T22:31:20-07:00
New Revision: 3b4db10f3492dfbab705ca4b2dd19d32fee075b9

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

LOG: [lldb] Actually support more than 32 logging categories

In January, Greg put up a patch (D117382) to support, among other
things, more than 32 log categories. That led to a bunch of nice
cleanups, but categories remained constrained because different parts of
the code were still using uint32_t. This patch fixes the remaining
issues and makes it possible to add a 32nd log category.

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

Added: 


Modified: 
lldb/include/lldb/Utility/LLDBLog.h
lldb/include/lldb/Utility/Log.h
lldb/source/Utility/Log.cpp
lldb/unittests/Utility/LogTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/LLDBLog.h 
b/lldb/include/lldb/Utility/LLDBLog.h
index 63dbb63f6f56..a440a72df9ea 100644
--- a/lldb/include/lldb/Utility/LLDBLog.h
+++ b/lldb/include/lldb/Utility/LLDBLog.h
@@ -48,7 +48,7 @@ enum class LLDBLog : Log::MaskType {
   Unwind = Log::ChannelFlag<29>,
   Watchpoints = Log::ChannelFlag<30>,
   OnDemand = Log::ChannelFlag<31>,
-  LLVM_MARK_AS_BITMASK_ENUM(Watchpoints),
+  LLVM_MARK_AS_BITMASK_ENUM(OnDemand),
 };
 
 LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();

diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 304dc7a0368f..596bec21b6f1 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -166,7 +166,7 @@ class Log final {
 // output will be discarded.
 Log *GetLog(MaskType mask) {
   Log *log = log_ptr.load(std::memory_order_relaxed);
-  if (log && log->GetMask().AnySet(mask))
+  if (log && ((log->GetMask() & mask) != 0))
 return log;
   return nullptr;
 }
@@ -243,7 +243,7 @@ class Log final {
 
   const Flags GetOptions() const;
 
-  const Flags GetMask() const;
+  MaskType GetMask() const;
 
   bool GetVerbose() const;
 
@@ -276,9 +276,9 @@ class Log final {
   }
 
   void Enable(const std::shared_ptr &handler_sp, uint32_t options,
-  uint32_t flags);
+  MaskType flags);
 
-  void Disable(uint32_t flags);
+  void Disable(MaskType flags);
 
   bool Dump(llvm::raw_ostream &stream);
 
@@ -291,8 +291,9 @@ class Log final {
 
   static void ListCategories(llvm::raw_ostream &stream,
  const ChannelMap::value_type &entry);
-  static uint32_t GetFlags(llvm::raw_ostream &stream, const 
ChannelMap::value_type &entry,
-   llvm::ArrayRef categories);
+  static Log::MaskType GetFlags(llvm::raw_ostream &stream,
+const ChannelMap::value_type &entry,
+llvm::ArrayRef categories);
 
   Log(const Log &) = delete;
   void operator=(const Log &) = delete;

diff  --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index 67edb15ba684..045e0f2cb68a 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -60,13 +60,14 @@ void Log::ListCategories(llvm::raw_ostream &stream,
   });
 }
 
-uint32_t Log::GetFlags(llvm::raw_ostream &stream, const ChannelMap::value_type 
&entry,
- llvm::ArrayRef categories) {
+Log::MaskType Log::GetFlags(llvm::raw_ostream &stream,
+const ChannelMap::value_type &entry,
+llvm::ArrayRef categories) {
   bool list_categories = false;
-  uint32_t flags = 0;
+  Log::MaskType flags = 0;
   for (const char *category : categories) {
 if (llvm::StringRef("all").equals_insensitive(category)) {
-  flags |= UINT32_MAX;
+  flags |= std::numeric_limits::max();
   continue;
 }
 if (llvm::StringRef("default").equals_insensitive(category)) {
@@ -91,7 +92,7 @@ uint32_t Log::GetFlags(llvm::raw_ostream &stream, const 
ChannelMap::value_type &
 }
 
 void Log::Enable(const std::shared_ptr &handler_sp,
- uint32_t options, uint32_t flags) {
+ uint32_t options, Log::MaskType flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_or(flags, std::memory_order_relaxed);
@@ -102,7 +103,7 @@ void Log::Enable(const std::shared_ptr 
&handler_sp,
   }
 }
 
-void Log::Disable(uint32_t flags) {
+void Log::Disable(Log::MaskType flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_and(~flags, std::memory_order_relaxed);
@@ -126,7 +127,7 @@ const Flags Log::GetOptions() const {
   return m_options.load(std::memory_order_relaxed);
 }
 
-const Flags Log::GetMask() const {
+Log::MaskType Log::GetMask() const {
   return m_mask.load(std::memory_order_relaxed);
 }
 
@@ -203,7 +204,7 @@ void Log::Register(llvm::Stri

[Lldb-commits] [PATCH] D134245: [lldb] Actually support more than 32 logging categories

2022-09-19 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3b4db10f3492: [lldb] Actually support more than 32 logging 
categories (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D134245?vs=461420&id=461466#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134245

Files:
  lldb/include/lldb/Utility/LLDBLog.h
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -363,8 +363,8 @@
 
   // Try fetching the log mask on one thread. Concurrently, try disabling the
   // log channel.
-  uint32_t mask;
-  std::thread log_thread([this, &mask] { mask = getLog()->GetMask().Get(); });
+  uint64_t mask;
+  std::thread log_thread([this, &mask] { mask = getLog()->GetMask(); });
   EXPECT_TRUE(DisableChannel("chan", {}, err));
   log_thread.join();
 
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -60,13 +60,14 @@
   });
 }
 
-uint32_t Log::GetFlags(llvm::raw_ostream &stream, const ChannelMap::value_type &entry,
- llvm::ArrayRef categories) {
+Log::MaskType Log::GetFlags(llvm::raw_ostream &stream,
+const ChannelMap::value_type &entry,
+llvm::ArrayRef categories) {
   bool list_categories = false;
-  uint32_t flags = 0;
+  Log::MaskType flags = 0;
   for (const char *category : categories) {
 if (llvm::StringRef("all").equals_insensitive(category)) {
-  flags |= UINT32_MAX;
+  flags |= std::numeric_limits::max();
   continue;
 }
 if (llvm::StringRef("default").equals_insensitive(category)) {
@@ -91,7 +92,7 @@
 }
 
 void Log::Enable(const std::shared_ptr &handler_sp,
- uint32_t options, uint32_t flags) {
+ uint32_t options, Log::MaskType flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_or(flags, std::memory_order_relaxed);
@@ -102,7 +103,7 @@
   }
 }
 
-void Log::Disable(uint32_t flags) {
+void Log::Disable(Log::MaskType flags) {
   llvm::sys::ScopedWriter lock(m_mutex);
 
   MaskType mask = m_mask.fetch_and(~flags, std::memory_order_relaxed);
@@ -126,7 +127,7 @@
   return m_options.load(std::memory_order_relaxed);
 }
 
-const Flags Log::GetMask() const {
+Log::MaskType Log::GetMask() const {
   return m_mask.load(std::memory_order_relaxed);
 }
 
@@ -203,7 +204,7 @@
 void Log::Unregister(llvm::StringRef name) {
   auto iter = g_channel_map->find(name);
   assert(iter != g_channel_map->end());
-  iter->second.Disable(UINT32_MAX);
+  iter->second.Disable(std::numeric_limits::max());
   g_channel_map->erase(iter);
 }
 
@@ -216,7 +217,7 @@
 error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
 return false;
   }
-  uint32_t flags = categories.empty()
+  MaskType flags = categories.empty()
? iter->second.m_channel.default_flags
: GetFlags(error_stream, *iter, categories);
   iter->second.Enable(log_handler_sp, log_options, flags);
@@ -231,8 +232,8 @@
 error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
 return false;
   }
-  uint32_t flags = categories.empty()
-   ? UINT32_MAX
+  MaskType flags = categories.empty()
+   ? std::numeric_limits::max()
: GetFlags(error_stream, *iter, categories);
   iter->second.Disable(flags);
   return true;
@@ -267,7 +268,7 @@
 
 void Log::DisableAllLogChannels() {
   for (auto &entry : *g_channel_map)
-entry.second.Disable(UINT32_MAX);
+entry.second.Disable(std::numeric_limits::max());
 }
 
 void Log::ForEachChannelCategory(
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -166,7 +166,7 @@
 // output will be discarded.
 Log *GetLog(MaskType mask) {
   Log *log = log_ptr.load(std::memory_order_relaxed);
-  if (log && log->GetMask().AnySet(mask))
+  if (log && ((log->GetMask() & mask) != 0))
 return log;
   return nullptr;
 }
@@ -243,7 +243,7 @@
 
   const Flags GetOptions() const;
 
-  const Flags GetMask() const;
+  MaskType GetMask() const;
 
   bool GetVerbose() const;
 
@@ -276,9 +276,9 @@
   }
 
   void Enable(const std::shared_ptr &handler_sp, uint32_t options,
-  uint32_t flags);
+  MaskType flags);
 
-  void Disable(uint32_t flags);
+  void Disable(MaskType flags);
 
   bool Dump(llvm::raw_ostream &stream);
 
@@

[Lldb-commits] [PATCH] D134252: Track .dwo/.dwp loading errors and notify user when viewing variables.

2022-09-19 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan added a comment.

Do you plan to detect missing dwp file from 
`SymbolFileDWARF::GetDwpSymbolFile()` as well?




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1755-1756
 cu_die.GetAttributeValueAsString(dwarf_cu, DW_AT_comp_dir, nullptr);
 if (!comp_dir)
   return nullptr;
 

We should detect and emit this error as well, like `can't locate  
relative to working dir or DW_AT_comp_dir`



Comment at: lldb/test/API/commands/frame/var/TestFrameVar.py:199
+
+# self.assertTrue(False, exe)
+self.assertTrue(os.path.exists(main_dwo), 'Make sure "%s" file exists' 
% (main_dwo))

Delete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134252

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