Re: [Lldb-commits] [lldb] 92b475f - [lldb] silence -Wsometimes-uninitialized warnings

2021-09-28 Thread Krasimir Georgiev via lldb-commits 
Thank you David. I just initialized these variables as suggested by
the warnings, without considering this case.
Michał, could you please double check this case?



On Mon, Sep 27, 2021 at 9:09 PM David Blaikie  wrote:

> Maybe this isn't the right fix - msbit and lsbit probably shouldn't be
> printed if to_integer returns false in the diagnostic on line 195. If that
> diagnostic didn't use the variables, then I don't think this'd warn?
>
> On Mon, Sep 27, 2021 at 12:38 AM Krasimir Georgiev via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>>
>> Author: Krasimir Georgiev
>> Date: 2021-09-27T09:35:58+02:00
>> New Revision: 92b475f0b079e125c588b121dc50116ea5d6d9f2
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/92b475f0b079e125c588b121dc50116ea5d6d9f2
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/92b475f0b079e125c588b121dc50116ea5d6d9f2.diff
>>
>> LOG: [lldb] silence -Wsometimes-uninitialized warnings
>>
>> No functional changes intended.
>>
>> Silence warnings from
>>
>> https://github.com/llvm/llvm-project/commit/3a6ba3675177cb5e47dee325f300aced4cd864ed
>> .
>>
>> Added:
>>
>>
>> Modified:
>> lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
>> b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
>> index a3a9d963c2213..f6a2cdf309815 100644
>> --- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
>> +++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
>> @@ -141,8 +141,8 @@ DynamicRegisterInfo::SetRegisterInfo(const
>> StructuredData::Dictionary &dict,
>>std::string reg_name_str = matches[1].str();
>>std::string msbit_str = matches[2].str();
>>std::string lsbit_str = matches[3].str();
>> -  uint32_t msbit;
>> -  uint32_t lsbit;
>> +  uint32_t msbit = 0;
>> +  uint32_t lsbit = 0;
>>if (llvm::to_integer(msbit_str, msbit) &&
>>llvm::to_integer(lsbit_str, lsbit)) {
>>  if (msbit > lsbit) {
>>
>>
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] 92b475f - [lldb] silence -Wsometimes-uninitialized warnings

2021-09-28 Thread Michał Górny via lldb-commits 
On Tue, 2021-09-28 at 09:37 +0200, Krasimir Georgiev wrote:
> Thank you David. I just initialized these variables as suggested by
> the warnings, without considering this case.
> Michał, could you please double check this case?

Hmm, you're probably right.  The diagnostic should probably print
msbit_str and lsbit_str instead.  I'm sorry for missing that.

That said, I think the function really needs refactoring to avoid that
whole if/else hell.  I'll make a patch for that.

Still, thanks for fixing the immediate problem.

> On Mon, Sep 27, 2021 at 9:09 PM David Blaikie  wrote:
> 
> > Maybe this isn't the right fix - msbit and lsbit probably shouldn't be
> > printed if to_integer returns false in the diagnostic on line 195. If that
> > diagnostic didn't use the variables, then I don't think this'd warn?
> > 
> > On Mon, Sep 27, 2021 at 12:38 AM Krasimir Georgiev via lldb-commits <
> > lldb-commits@lists.llvm.org> wrote:
> > 
> > > 
> > > Author: Krasimir Georgiev
> > > Date: 2021-09-27T09:35:58+02:00
> > > New Revision: 92b475f0b079e125c588b121dc50116ea5d6d9f2
> > > 
> > > URL:
> > > https://github.com/llvm/llvm-project/commit/92b475f0b079e125c588b121dc50116ea5d6d9f2
> > > DIFF:
> > > https://github.com/llvm/llvm-project/commit/92b475f0b079e125c588b121dc50116ea5d6d9f2.diff
> > > 
> > > LOG: [lldb] silence -Wsometimes-uninitialized warnings
> > > 
> > > No functional changes intended.
> > > 
> > > Silence warnings from
> > > 
> > > https://github.com/llvm/llvm-project/commit/3a6ba3675177cb5e47dee325f300aced4cd864ed
> > > .
> > > 
> > > Added:
> > > 
> > > 
> > > Modified:
> > > lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> > > 
> > > Removed:
> > > 
> > > 
> > > 
> > > 
> > > 
> > > diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> > > b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> > > index a3a9d963c2213..f6a2cdf309815 100644
> > > --- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> > > +++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> > > @@ -141,8 +141,8 @@ DynamicRegisterInfo::SetRegisterInfo(const
> > > StructuredData::Dictionary &dict,
> > >std::string reg_name_str = matches[1].str();
> > >std::string msbit_str = matches[2].str();
> > >std::string lsbit_str = matches[3].str();
> > > -  uint32_t msbit;
> > > -  uint32_t lsbit;
> > > +  uint32_t msbit = 0;
> > > +  uint32_t lsbit = 0;
> > >if (llvm::to_integer(msbit_str, msbit) &&
> > >llvm::to_integer(lsbit_str, lsbit)) {
> > >  if (msbit > lsbit) {
> > > 
> > > 
> > > 
> > > ___
> > > lldb-commits mailing list
> > > lldb-commits@lists.llvm.org
> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> > > 
> > 

-- 
Best regards,
Michał Górny


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

[Lldb-commits] [PATCH] D110619: [lldb] [DynamicRegisterInfo] Refactor SetRegisterInfo()

2021-09-28 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, teemperor, krasimir, 
dblaikie.
mgorny requested review of this revision.

Move the "slice" and "composite" handling into separate methods to avoid
if/else hell.  Use more LLVM types whenever possible.  Replace printf()s
with llvm::Error combined with LLDB logging.


https://reviews.llvm.org/D110619

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h

Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -86,6 +86,13 @@
   typedef std::vector dwarf_opcode;
   typedef std::map dynamic_reg_size_map;
 
+  llvm::Expected ByteOffsetFromSlice(uint32_t index,
+   llvm::StringRef slice_str,
+   lldb::ByteOrder byte_order);
+  llvm::Expected ByteOffsetFromComposite(
+  uint32_t index, lldb_private::StructuredData::Array &composite_reg_list,
+  lldb::ByteOrder byte_order);
+
   void MoveFrom(DynamicRegisterInfo &&info);
 
   void ConfigureOffsets();
@@ -102,4 +109,5 @@
   bool m_finalized = false;
   bool m_is_reconfigurable = false;
 };
+
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_DYNAMICREGISTERINFO_H
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -12,6 +12,7 @@
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/StringExtractor.h"
 #include "lldb/Utility/StructuredData.h"
@@ -56,9 +57,120 @@
   info.Clear();
 }
 
+llvm::Expected DynamicRegisterInfo::ByteOffsetFromSlice(
+uint32_t index, llvm::StringRef slice_str, lldb::ByteOrder byte_order) {
+  // Slices use the following format:
+  //  REGNAME[MSBIT:LSBIT]
+  // REGNAME - name of the register to grab a slice of
+  // MSBIT - the most significant bit at which the current register value
+  // starts at
+  // LSBIT - the least significant bit at which the current register value
+  // ends at
+  static llvm::Regex g_bitfield_regex(
+  "([A-Za-z_][A-Za-z0-9_]*)\\[([0-9]+):([0-9]+)\\]");
+  llvm::SmallVector matches;
+  if (!g_bitfield_regex.match(slice_str, &matches))
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to match against register bitfield regex (slice: %s)",
+slice_str.str().c_str());
+
+  llvm::StringRef reg_name_str = matches[1];
+  llvm::StringRef msbit_str = matches[2];
+  llvm::StringRef lsbit_str = matches[3];
+  uint32_t msbit;
+  uint32_t lsbit;
+  if (!llvm::to_integer(msbit_str, msbit) ||
+  !llvm::to_integer(lsbit_str, lsbit))
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(), "msbit (%s) or lsbit (%s) are invalid",
+msbit_str.str().c_str(), lsbit_str.str().c_str());
+
+  if (msbit <= lsbit)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "msbit (%u) must be greater than lsbit (%u)",
+   msbit, lsbit);
+
+  const uint32_t msbyte = msbit / 8;
+  const uint32_t lsbyte = lsbit / 8;
+
+  const RegisterInfo *containing_reg_info = GetRegisterInfo(reg_name_str);
+  if (!containing_reg_info)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid concrete register \"%s\"",
+   reg_name_str.str().c_str());
+
+  const uint32_t max_bit = containing_reg_info->byte_size * 8;
+
+  if (msbit > max_bit)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"msbit (%u) must be less than the bitsize of the register \"%s\" (%u)",
+msbit, reg_name_str.str().c_str(), max_bit);
+  if (lsbit > max_bit)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"lsbit (%u) must be less than the bitsize of the register \"%s\" (%u)",
+lsbit, reg_name_str.str().c_str(), max_bit);
+
+  m_invalidate_regs_map[containing_reg_info->kinds[eRegisterKindLLDB]]
+  .push_back(index);
+  m_value_regs_map[index].push_back(
+  containing_reg_info->kinds[eRegisterKindLLDB]);
+  m_invalidate_regs_map[index].push_back(
+  containing_reg_info->kinds[eRegisterKindLLDB]);
+
+  if (byte_order == eByteOrderLittle)
+return containing_reg_info->byte_offset + lsbyte;
+  if (byte_order == eByteOrderBig)
+return containing_reg_info->byte_offset + msbyte;
+  llvm_unreachable("Inva

[Lldb-commits] [PATCH] D110619: [lldb] [DynamicRegisterInfo] Refactor SetRegisterInfo()

2021-09-28 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:244
+  ? ByteOffsetFromSlice(i, slice_str, byte_order)
+  : reg_info_dict->GetValueForKeyAsArray("composite",
+ composite_reg_list)

clang-format seems to indent this weirdly.


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

https://reviews.llvm.org/D110619

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

[Lldb-commits] [PATCH] D110569: [lldb] [Process/FreeBSD] Rework arm64 register access

2021-09-28 Thread Andrew Turner via Phabricator via lldb-commits 
andrew added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:73
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_GETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==

mgorny wrote:
> These helpers don't seem very helpful. Isn't it better to use `switch` on the 
> value returned by `GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg)`?
Using a switch will be messy when adding pointer authentication or MTE 
registers as we need to call `RegisterInfoPOSIX_arm64::IsPAuthReg` [1] to check 
if a given register is a pointer authentication register, and similar for MTE. 
This is because they are dynamically added as needed.

[1] 
https://github.com/llvm/llvm-project/blob/76e47d4/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp#L392-L396



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:133
+  if (IsGPR(reg)) {
+error = ReadGPR();
+if (error.Fail())

mgorny wrote:
> To be honest, it seems counterproductive to restore the duplication that I've 
> removed before. Is there any real advantage to having two methods here, and 
> calling them separately from inside the `if` instead of calling 
> `ReadRegisterSet(set)` before the `if`?
Because of the dynamic nature of the register sets we may not have a usable 
value to pass into `ReadRegisterSet` and any logic to decide which register set 
is being read in `ReadRegisterSet` would need to be repeated here and below 
when writing a register.

Calling the function `ReadRegisterSet` will also be confusing when I add 
`PT_GETREGSET` and `PT_SETREGSET` [1] to FreeBSD. This will be used to access 
the pointer authentication address masks on FreeBSD (and future registers, e.g. 
MTE & SVE).

See [2] for a WIP patch with how I plan to extend this for code & data address 
masks needed by pointer authentication.

[1] https://reviews.freebsd.org/D19831
[2] 
https://github.com/zxombie/llvm-project/commit/b801eca#diff-3d6a219398418eb59bf8104bda44a111f47d29081e076b4ce69336882e482cc2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110569

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

[Lldb-commits] [PATCH] D110619: [lldb] [DynamicRegisterInfo] Refactor SetRegisterInfo()

2021-09-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

wow




Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:244
+  ? ByteOffsetFromSlice(i, slice_str, byte_order)
+  : reg_info_dict->GetValueForKeyAsArray("composite",
+ composite_reg_list)

mgorny wrote:
> clang-format seems to indent this weirdly.
I can't say I blame it. :)

Maybe make this a separate function too, so you can use ifs and returns ?



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:252-253
+
+  LLDB_LOG_ERROR(log, byte_offset.takeError(),
+ "error while parsing register {1}: {0}", reg_info.name);
+  if (byte_offset)

I think this would look better in the else branch below.


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

https://reviews.llvm.org/D110619

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

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

2021-09-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

The patch seems reasonable to me, and is an improvement over the status quo.

The thing I'm left wondering is the lambda in 
`ParseVariablesInFunctionContext`. It covers like 95% of the enclosing 
function. Could we just make a separate method for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110570

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

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

2021-09-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I haven't looked at the actual code yet, so I could be off, but here are some 
thoughts.

In D110571#3025527 , @jarin wrote:

> Hi, could you take a look at this change?
>
> Some discussion points:
>
> - In the ParseVariablesInFunctionContext method, we are using a lambda for 
> the recursive parser. We could also use a function-local class or inner class 
> of SymbolFileDWARF. Would any of these be preferable?

Yeah, what's the deal with that? Why wouldn't a regular function be sufficient? 
You can just pass things in arguments instead of closures or classes..

> - The variables created by ParseVariableDIE on abstract formal parameters are 
> fairly strange, especially if a function gets inlined into two different 
> functions. If that happens, then the parsed variable will refer to a symbol 
> context that does not contain the variable DIE and a block can contain a 
> variable that is not in the DIE of tree of the block. Is that a big problem? 
> (Quick testing of this situation did not reveal any strange stack traces or 
> `frame var` anomalies.) Unfortunately, there is no good way to provide the 
> correct block and the correct function because LLDB does not parse functions 
> and blocks for the abstract functions (i.e., for the DW_TAG_subroutines that 
> are referenced by DW_AT_abstract_origin of concrete functions).

Judging by your description, I take it you parse these variables only once, 
regardless of how many functions they are inlined in. Could we fix that my 
creating a fresh variable object for each inlined instance? Then it could maybe 
be correctly made to point to the actual block and function it is inlined 
into(?)

> - The provided test only tests the case of an inlined function where some 
> parameters are unused/omitted. Would it make sense to also provide tests for 
> other interesting cases or would that be too much bloat? The particularly 
> interesting cases are:
>   - Inlined function with all its parameters unused/omitted,
>   - Inlined function that is called from different top-level functions.
>   - Test correctness of the stack trace in the cases above.
> - We could supply a test written in C, but it needs -O1 and is fairly 
> sensitive to the meaning of -O1 (e.g., clang started inlining and omitting 
> unsued inlined parameters only recently, so changes to -O1 could make a C 
> test easily meaningless). Any concerns here?
> - The provided test is a bit verbose, mostly because we wanted to mostly 
> preserve the structure of the C compiler output. We could still cut the size 
> of the test down by removing the main function in favour of _start and by 
> removing all the file/line info. Would any of that make sense?

I think you could get quite far by just testing the output of the "image 
lookup" command. That should give you list variables that are in scope for any 
particular address, and a bunch of details about each var, including the 
expression used to compute its value (not the value itself, obviously). The 
main advantage is that you wouldn't need a fully functional program, as you 
wouldn't be running anything. That would remove a lot of bloat, and also allow 
the test to run on non-x86-pc-linux hosts. Then, maybe it wouldn't be too messy 
to add the additional test cases you mention.

You can look at (e.g.) DW_AT_loclists_base.s for an example of a test case with 
image lookup and local variables.

After that, we could think about adding a c++ test case. Although tests with 
optimized code are tricky, it is often possible (with judicious use of 
noinline, always_inline and optnone attributes) to constrain the optimizer in a 
way that it has no choice but to do exactly what we want.




Comment at: 
lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test:32-34
+# CHECK: (int) unused2 = <
+# CHECK: (int) partial = <
+# CHECK: (int) unused3 = <

Including the actual message would make it clearer what is going on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571

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

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

2021-09-28 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

(It would also make it easier to understand the code if you could paste some 
dwarfdump output which illustrates the kind of debug info we're dealing  with.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571

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

[Lldb-commits] [lldb] 156cb4c - [lldb] Remove non-stop mode code

2021-09-28 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2021-09-28T14:13:50+02:00
New Revision: 156cb4cc64bec2b72aad9848f8181b338ff19ebc

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

LOG: [lldb] Remove non-stop mode code

We added some support for this mode back in 2015, but the feature was
never productionized. It is completely untested, and there are known
major structural lldb issues that need to be resolved before this
feature can really be supported.

It also complicates making further changes to stop reply packet
handling, which is what I am about to do.

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

Added: 


Modified: 
lldb/include/lldb/Target/Target.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td

Removed: 




diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index fc9dd97515aa9..4ddaf3fe2fca6 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -206,10 +206,6 @@ class TargetProperties : public Properties {
 
   void SetUserSpecifiedTrapHandlerNames(const Args &args);
 
-  bool GetNonStopModeEnabled() const;
-
-  void SetNonStopModeEnabled(bool b);
-
   bool GetDisplayRuntimeSupportValues() const;
 
   void SetDisplayRuntimeSupportValues(bool b);

diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 7247601b292b0..1534959989d3e 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -292,16 +292,10 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
 // Check if we are in Non-Stop mode
 TargetSP target_sp =
 execution_context ? execution_context->GetTargetSP() : TargetSP();
-if (target_sp && target_sp->GetNonStopModeEnabled()) {
-  // NonStopMode runs all threads by definition, so when it is on we don't
-  // need to check the process setting for runs all threads.
-  m_run_mode = eOnlyThisThread;
-} else {
-  ProcessSP process_sp =
-  execution_context ? execution_context->GetProcessSP() : ProcessSP();
-  if (process_sp && process_sp->GetSteppingRunsAllThreads())
-m_run_mode = eAllThreads;
-}
+ProcessSP process_sp =
+execution_context ? execution_context->GetProcessSP() : ProcessSP();
+if (process_sp && process_sp->GetSteppingRunsAllThreads())
+  m_run_mode = eAllThreads;
 
 m_avoid_regexp.clear();
 m_step_in_target.clear();

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index a4c71e864a767..803e5842cd7de 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -249,32 +249,6 @@ GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(
   return packet_result;
 }
 
-bool GDBRemoteClientBase::SendvContPacket(
-llvm::StringRef payload, std::chrono::seconds interrupt_timeout,
-StringExtractorGDBRemote &response) {
-  Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
-  LLDB_LOGF(log, "GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
-
-  // we want to lock down packet sending while we continue
-  Lock lock(*this, interrupt_timeout);
-
-  LLDB_LOGF(log,
-"GDBRemoteCommunicationClient::%s () sending vCont packet: %.*s",
-__FUNCTION__, int(payload.size()), payload.data());
-
-  if (SendPacketNoLock(payload) != PacketResult::Success)
-return false;
-
-  OnRunPacketSent(true);
-
-  // wait for the response to the vCont
-  if (ReadPacket(response, llvm::None, false) == PacketResult::Success) {
-if (response.IsOKResponse())
-  return true;
-  }
-
-  return false;
-}
 bool GDBRemoteClientBase::ShouldStop(const UnixSignals &signals,
  StringExtractorGDBRemote &response) {
   std::lock_guard lock(m_mutex);

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
index 518b81318b6cc..43a5313eae6ad 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -59,10 +59,6 @@ class GDB

[Lldb-commits] [lldb] 9413ead - [lldb/test] Add ability to specify environment when spawning processes

2021-09-28 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2021-09-28T14:13:50+02:00
New Revision: 9413ead7bcbaf5d8876ee484256df65c2846c5c9

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

LOG: [lldb/test] Add ability to specify environment when spawning processes

We only had that ability for regular debugger launches. This meant that
it was not possible to use the normal dlopen patterns in attach tests.
This fixes that.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
lldb/test/API/functionalities/load_after_attach/main.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index a899b1b154fe2..db350aee4a182 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -360,7 +360,7 @@ def pid(self):
 """Returns process PID if has been launched already."""
 
 @abc.abstractmethod
-def launch(self, executable, args):
+def launch(self, executable, args, extra_env):
 """Launches new process with given executable and args."""
 
 @abc.abstractmethod
@@ -379,13 +379,19 @@ def __init__(self, trace_on):
 def pid(self):
 return self._proc.pid
 
-def launch(self, executable, args):
+def launch(self, executable, args, extra_env):
+env=None
+if extra_env:
+env = dict(os.environ)
+env.update([kv.split("=", 1) for kv in extra_env])
+
 self._proc = Popen(
 [executable] + args,
 stdout=open(
 os.devnull) if not self._trace_on else None,
 stdin=PIPE,
-preexec_fn=lldbplatformutil.enable_attach)
+preexec_fn=lldbplatformutil.enable_attach,
+env=env)
 
 def terminate(self):
 if self._proc.poll() is None:
@@ -424,7 +430,7 @@ def __init__(self, install_remote):
 def pid(self):
 return self._pid
 
-def launch(self, executable, args):
+def launch(self, executable, args, extra_env):
 if self._install_remote:
 src_path = executable
 dst_path = lldbutil.join_remote_paths(
@@ -450,6 +456,9 @@ def launch(self, executable, args):
 launch_info.AddSuppressFileAction(1, False, True)
 launch_info.AddSuppressFileAction(2, False, True)
 
+if extra_env:
+launch_info.SetEnvironmentEntries(extra_env, True)
+
 err = lldb.remote_platform.Launch(launch_info)
 if err.Fail():
 raise Exception(
@@ -952,13 +961,13 @@ def cleanupSubprocesses(self):
 del p
 del self.subprocesses[:]
 
-def spawnSubprocess(self, executable, args=[], install_remote=True):
+def spawnSubprocess(self, executable, args=[], extra_env=None, 
install_remote=True):
 """ Creates a subprocess.Popen object with the specified executable 
and arguments,
 saves it in self.subprocesses, and returns the object.
 """
 proc = _RemoteProcess(
 install_remote) if lldb.remote_platform else 
_LocalProcess(self.TraceOn())
-proc.launch(executable, args)
+proc.launch(executable, args, extra_env=extra_env)
 self.subprocesses.append(proc)
 return proc
 

diff  --git 
a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py 
b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
index 0e9b3c40ff2b3..3b261e632d4c8 100644
--- a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
+++ b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
@@ -6,6 +6,7 @@
 class TestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfRemote
 def test_load_after_attach(self):
@@ -17,18 +18,19 @@ def test_load_after_attach(self):
 exe = self.getBuildArtifact("a.out")
 lib = self.getBuildArtifact(lib_name)
 
+target = self.dbg.CreateTarget(exe)
+environment = self.registerSharedLibrariesWithTarget(target, ["lib_b"])
+
 # Spawn a new process.
 # use realpath to workaround llvm.org/pr48376
 # Pass path to solib for dlopen to properly locate the library.
-popen = self.spawnSubprocess(os.path.realpath(exe), args = 
[os.path.realpath(lib)])
-pid = popen.pid
+popen = self.spawnSubprocess(os.path.realpath(exe), 
extra_env=environment)
 
 # Attach to the spawned process.
-self.runCmd("process attach -p " + str(pid))
-
-target = self.dbg.GetSelectedTarget()
-process = target.GetProcess()
-self.assertTrue(process, PROCESS_IS_VALID)
+

[Lldb-commits] [PATCH] D110553: [lldb] Remove non-stop mode code

2021-09-28 Thread Pavel Labath via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG156cb4cc64be: [lldb] Remove non-stop mode code (authored by 
labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110553

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -163,9 +163,6 @@
   def DisplayRecognizedArguments: Property<"display-recognized-arguments", "Boolean">,
 DefaultFalse,
 Desc<"Show recognized arguments in variable listings by default.">;
-  def NonStopModeEnabled: Property<"non-stop-mode", "Boolean">,
-DefaultFalse,
-Desc<"Disable lock-step debugging, instead control threads independently.">;
   def RequireHardwareBreakpoints: Property<"require-hardware-breakpoint", "Boolean">,
 DefaultFalse,
 Desc<"Require all breakpoints to be hardware breakpoints.">;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -4288,16 +4288,6 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
 }
 
-bool TargetProperties::GetNonStopModeEnabled() const {
-  const uint32_t idx = ePropertyNonStopModeEnabled;
-  return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, false);
-}
-
-void TargetProperties::SetNonStopModeEnabled(bool b) {
-  const uint32_t idx = ePropertyNonStopModeEnabled;
-  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
-}
-
 const ProcessLaunchInfo &TargetProperties::GetProcessLaunchInfo() const {
   return m_launch_info;
 }
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -269,11 +269,10 @@
   GDBRemoteCommunicationClient m_gdb_comm;
   GDBRemoteCommunicationReplayServer m_gdb_replay_server;
   std::atomic m_debugserver_pid;
-  std::vector m_stop_packet_stack; // The stop packet
- // stack replaces
- // the last stop
- // packet variable
+
+  llvm::Optional m_last_stop_packet;
   std::recursive_mutex m_last_stop_packet_mutex;
+
   GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
   Broadcaster m_async_broadcaster;
   lldb::ListenerSP m_async_listener_sp;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -603,10 +603,6 @@
 if (m_gdb_comm.GetStopReply(response)) {
   SetLastStopPacket(response);
 
-  // '?' Packets must be handled differently in non-stop mode
-  if (GetTarget().GetNonStopModeEnabled())
-HandleStopReplySequence();
-
   Target &target = GetTarget();
   if (!target.GetArchitecture().IsValid()) {
 if (m_gdb_comm.GetProcessArchitecture().IsValid()) {
@@ -846,9 +842,6 @@
   StringExtractorGDBRemote response;
   if (m_gdb_comm.GetStopReply(response)) {
 SetLastStopPacket(response);
-// '?' Packets must be handled differently in non-stop mode
-if (GetTarget().GetNonStopModeEnabled())
-  HandleStopReplySequence();
 
 const ArchSpec &process_arch = m_gdb_comm.GetProcessArchitecture();
 
@@ -919,11 +912,6 @@
 return error;
   }
 
-  // Start the communications read thread so all incoming data can be parsed
-  // into packets and queued as they arrive.
-  if (GetTarget().GetNonStopModeEnabled())
-m_gdb_comm.StartReadThread();
-
   // We always seem to be able to open a connection to a local port so we need
   // to make sure we can then send data to it. If we can't then we aren't
   // actually connected to anything, so try and do the handshake with the
@@ -935,10 +923,6 @@
 return error;
   }
 
-  // Send $QNonStop:1 packet on startup if required
-  if (GetTarget().GetNonStopModeEnabled())
-GetTarget().SetNonStopModeEnabled(m

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

2021-09-28 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin updated this revision to Diff 375535.
jarin added a comment.

Separated ParseVariablesInFunctionContext's lambda into a method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110570

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -380,11 +380,19 @@
 const DWARFDIE &die,
 const lldb::addr_t func_low_pc);
 
-  size_t ParseVariables(const lldb_private::SymbolContext &sc,
-const DWARFDIE &orig_die,
-const lldb::addr_t func_low_pc, bool parse_siblings,
-bool parse_children,
-lldb_private::VariableList *cc_variable_list = nullptr);
+  void
+  ParseAndAppendGlobalVariable(const lldb_private::SymbolContext &sc,
+   const DWARFDIE &die,
+   lldb_private::VariableList &cc_variable_list);
+
+  size_t ParseVariablesInFunctionContext(const lldb_private::SymbolContext &sc,
+ const DWARFDIE &die,
+ const lldb::addr_t func_low_pc);
+
+  size_t ParseVariablesInFunctionContextRecursive(
+  const lldb_private::SymbolContext &sc, const DWARFDIE &die,
+  const lldb::addr_t func_low_pc,
+  lldb_private::VariableList &variable_list);
 
   bool ClassOrStructIsVirtual(const DWARFDIE &die);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2135,7 +2135,7 @@
   }
 }
 
-ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+ParseAndAppendGlobalVariable(sc, die, variables);
 while (pruned_idx < variables.GetSize()) {
   VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
   if (name_is_mangled ||
@@ -2188,7 +2188,7 @@
   return true;
 sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+ParseAndAppendGlobalVariable(sc, die, variables);
 
 return variables.GetSize() - original_size < max_matches;
   });
@@ -3049,8 +3049,8 @@
   /*check_hi_lo_pc=*/true))
 func_lo_pc = ranges.GetMinRangeBase(0);
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
-const size_t num_variables = ParseVariables(
-sc, function_die.GetFirstChild(), func_lo_pc, true, true);
+const size_t num_variables =
+ParseVariablesInFunctionContext(sc, function_die, func_lo_pc);
 
 // Let all blocks know they have parse all their variables
 sc.function->GetBlock(false).SetDidParseVariables(true, true);
@@ -3481,117 +3481,135 @@
   return DWARFDIE();
 }
 
-size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc,
-   const DWARFDIE &orig_die,
-   const lldb::addr_t func_low_pc,
-   bool parse_siblings, bool parse_children,
-   VariableList *cc_variable_list) {
-  if (!orig_die)
-return 0;
+void SymbolFileDWARF::ParseAndAppendGlobalVariable(
+const SymbolContext &sc, const DWARFDIE &die,
+VariableList &cc_variable_list) {
+  if (!die)
+return;
 
+  dw_tag_t tag = die.Tag();
+  if (tag != DW_TAG_variable && tag != DW_TAG_constant)
+return;
+
+  // Check to see if we have already parsed this variable or constant?
+  VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
+  if (var_sp) {
+cc_variable_list.AddVariableIfUnique(var_sp);
+return;
+  }
+
+  // We haven't already parsed it, lets do that now.
   VariableListSP variable_list_sp;
+  DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die);
+  dw_tag_t parent_tag = sc_parent_die.Tag();
+  switch (parent_tag) {
+  case DW_TAG_compile_unit:
+  case DW_TAG_partial_unit:
+if (sc.comp_unit != nullptr) {
+  variable_list_sp = sc.comp_unit->GetVariableList(false);
+} else {
+  GetObjectFile()->GetModule()->ReportError(
+  "parent 0x%8.8" PRIx64 " %s with no valid compile unit in "
+  "symbol context for 0x%8.8" PRIx64 " %s.\n",
+  sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(), die.GetID(),
+  die.GetTagAsCString());
+  return;
+}
+break;
 
+  default:
+GetObjectFile()->GetModule()->ReportError(
+"didn't find appropriate par

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

2021-09-28 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin updated this revision to Diff 375538.
jarin added a comment.

Added the `/*can_create=*/` comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110570

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -380,11 +380,19 @@
 const DWARFDIE &die,
 const lldb::addr_t func_low_pc);
 
-  size_t ParseVariables(const lldb_private::SymbolContext &sc,
-const DWARFDIE &orig_die,
-const lldb::addr_t func_low_pc, bool parse_siblings,
-bool parse_children,
-lldb_private::VariableList *cc_variable_list = nullptr);
+  void
+  ParseAndAppendGlobalVariable(const lldb_private::SymbolContext &sc,
+   const DWARFDIE &die,
+   lldb_private::VariableList &cc_variable_list);
+
+  size_t ParseVariablesInFunctionContext(const lldb_private::SymbolContext &sc,
+ const DWARFDIE &die,
+ const lldb::addr_t func_low_pc);
+
+  size_t ParseVariablesInFunctionContextRecursive(
+  const lldb_private::SymbolContext &sc, const DWARFDIE &die,
+  const lldb::addr_t func_low_pc,
+  lldb_private::VariableList &variable_list);
 
   bool ClassOrStructIsVirtual(const DWARFDIE &die);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2135,7 +2135,7 @@
   }
 }
 
-ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+ParseAndAppendGlobalVariable(sc, die, variables);
 while (pruned_idx < variables.GetSize()) {
   VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
   if (name_is_mangled ||
@@ -2188,7 +2188,7 @@
   return true;
 sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+ParseAndAppendGlobalVariable(sc, die, variables);
 
 return variables.GetSize() - original_size < max_matches;
   });
@@ -3049,8 +3049,8 @@
   /*check_hi_lo_pc=*/true))
 func_lo_pc = ranges.GetMinRangeBase(0);
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
-const size_t num_variables = ParseVariables(
-sc, function_die.GetFirstChild(), func_lo_pc, true, true);
+const size_t num_variables =
+ParseVariablesInFunctionContext(sc, function_die, func_lo_pc);
 
 // Let all blocks know they have parse all their variables
 sc.function->GetBlock(false).SetDidParseVariables(true, true);
@@ -3481,117 +3481,137 @@
   return DWARFDIE();
 }
 
-size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc,
-   const DWARFDIE &orig_die,
-   const lldb::addr_t func_low_pc,
-   bool parse_siblings, bool parse_children,
-   VariableList *cc_variable_list) {
-  if (!orig_die)
-return 0;
+void SymbolFileDWARF::ParseAndAppendGlobalVariable(
+const SymbolContext &sc, const DWARFDIE &die,
+VariableList &cc_variable_list) {
+  if (!die)
+return;
 
+  dw_tag_t tag = die.Tag();
+  if (tag != DW_TAG_variable && tag != DW_TAG_constant)
+return;
+
+  // Check to see if we have already parsed this variable or constant?
+  VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
+  if (var_sp) {
+cc_variable_list.AddVariableIfUnique(var_sp);
+return;
+  }
+
+  // We haven't already parsed it, lets do that now.
   VariableListSP variable_list_sp;
+  DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die);
+  dw_tag_t parent_tag = sc_parent_die.Tag();
+  switch (parent_tag) {
+  case DW_TAG_compile_unit:
+  case DW_TAG_partial_unit:
+if (sc.comp_unit != nullptr) {
+  variable_list_sp = sc.comp_unit->GetVariableList(false);
+} else {
+  GetObjectFile()->GetModule()->ReportError(
+  "parent 0x%8.8" PRIx64 " %s with no valid compile unit in "
+  "symbol context for 0x%8.8" PRIx64 " %s.\n",
+  sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(), die.GetID(),
+  die.GetTagAsCString());
+  return;
+}
+break;
 
+  default:
+GetObjectFile()->GetModule()->ReportError(
+"didn't find appropriate parent DIE for variable list fo

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

2021-09-28 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin added a comment.

Thank you for the review! Does the separate method look more reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110570

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

[Lldb-commits] [lldb] 7866dbb - [lldb/test] Remove a check from TestLoadAfterAttach

2021-09-28 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2021-09-28T14:47:46+02:00
New Revision: 7866dbb261240f71c79e70d2ed52351846f795cd

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

LOG: [lldb/test] Remove a check from TestLoadAfterAttach

The two module retrieval methods (qXfer:libraries-svr4 and manual list
traversal) differ in how the handle the
manually-added-but-not-yet-loaded modules. The svr4 path will remove it,
while the manual one will keep in the list.

It's likely the two paths need ought to be synchronized, but right now,
this distinction is not relevant for the test.

Added: 


Modified: 
lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py 
b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
index 3b261e632d4c..de691e6fdda4 100644
--- a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
+++ b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
@@ -39,13 +39,6 @@ def test_load_after_attach(self):
 stopped_threads = lldbutil.continue_to_breakpoint(self.process(), 
breakpoint1)
 self.assertEqual(len(stopped_threads), 1)
 
-# Check that image list does not contain liblib_b before dlopen.
-self.match(
-"image list",
-patterns = [lib_name],
-matching = False,
-msg = lib_name + " should not have been in image list")
-
 # Change a variable to escape the loop
 self.runCmd("expression main_thread_continue = 1")
 



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

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

2021-09-28 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin added a comment.

Just a few replies below; I am hoping to put the relevant code changes together 
tomorrow.

In D110571#3027173 , @labath wrote:

> I haven't looked at the actual code yet, so I could be off, but here are some 
> thoughts.
>
> In D110571#3025527 , @jarin wrote:
>
>> Hi, could you take a look at this change?
>>
>> Some discussion points:
>>
>> - In the ParseVariablesInFunctionContext method, we are using a lambda for 
>> the recursive parser. We could also use a function-local class or inner 
>> class of SymbolFileDWARF. Would any of these be preferable?
>
> Yeah, what's the deal with that? Why wouldn't a regular function be 
> sufficient? You can just pass things in arguments instead of closures or 
> classes..

Right, I worked on a codebase where they used local classes for such things and 
in lldb I have seen lambdas. I actually do not have a strong preference, will 
rewrite this to use plain methods.

>> - The variables created by ParseVariableDIE on abstract formal parameters 
>> are fairly strange, especially if a function gets inlined into two different 
>> functions. If that happens, then the parsed variable will refer to a symbol 
>> context that does not contain the variable DIE and a block can contain a 
>> variable that is not in the DIE of tree of the block. Is that a big problem? 
>> (Quick testing of this situation did not reveal any strange stack traces or 
>> `frame var` anomalies.) Unfortunately, there is no good way to provide the 
>> correct block and the correct function because LLDB does not parse functions 
>> and blocks for the abstract functions (i.e., for the DW_TAG_subroutines that 
>> are referenced by DW_AT_abstract_origin of concrete functions).
>
> Judging by your description, I take it you parse these variables only once, 
> regardless of how many functions they are inlined in. Could we fix that my 
> creating a fresh variable object for each inlined instance? Then it could 
> maybe be correctly made to point to the actual block and function it is 
> inlined into(?)

Yes, they are parsed only once. This is because there is a DIE->Variable map 
(see SymbolFileDWARF::GetDIEToVariable) that makes sure no DIE gets parsed 
twice. Are you suggesting to index the map with a pair ? That would indeed create healthier structure (even though I 
could not spot any problems even with my current somewhat flawed approach).

>> - The provided test only tests the case of an inlined function where some 
>> parameters are unused/omitted. Would it make sense to also provide tests for 
>> other interesting cases or would that be too much bloat? The particularly 
>> interesting cases are:
>>   - Inlined function with all its parameters unused/omitted,
>>   - Inlined function that is called from different top-level functions.
>>   - Test correctness of the stack trace in the cases above.
>> - We could supply a test written in C, but it needs -O1 and is fairly 
>> sensitive to the meaning of -O1 (e.g., clang started inlining and omitting 
>> unsued inlined parameters only recently, so changes to -O1 could make a C 
>> test easily meaningless). Any concerns here?
>> - The provided test is a bit verbose, mostly because we wanted to mostly 
>> preserve the structure of the C compiler output. We could still cut the size 
>> of the test down by removing the main function in favour of _start and by 
>> removing all the file/line info. Would any of that make sense?
>
> I think you could get quite far by just testing the output of the "image 
> lookup" command. That should give you list variables that are in scope for 
> any particular address, and a bunch of details about each var, including the 
> expression used to compute its value (not the value itself, obviously). The 
> main advantage is that you wouldn't need a fully functional program, as you 
> wouldn't be running anything. That would remove a lot of bloat, and also 
> allow the test to run on non-x86-pc-linux hosts. Then, maybe it wouldn't be 
> too messy to add the additional test cases you mention.
>
> You can look at (e.g.) DW_AT_loclists_base.s for an example of a test case 
> with image lookup and local variables.

Makes sense, I really like that approach. Let me try to get that working.

> After that, we could think about adding a c++ test case. Although tests with 
> optimized code are tricky, it is often possible (with judicious use of 
> noinline, always_inline and optnone attributes) to constrain the optimizer in 
> a way that it has no choice but to do exactly what we want.

I have actually use the attributes when experimenting with the patch - if you 
think this is useful, I can certainly provide those tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571

___
lldb-commits mailing list
lldb-com

[Lldb-commits] [PATCH] D110619: [lldb] [DynamicRegisterInfo] Refactor SetRegisterInfo()

2021-09-28 Thread Michał Górny via Phabricator via lldb-commits 
mgorny marked 2 inline comments as done.
mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:244
+  ? ByteOffsetFromSlice(i, slice_str, byte_order)
+  : reg_info_dict->GetValueForKeyAsArray("composite",
+ composite_reg_list)

labath wrote:
> mgorny wrote:
> > clang-format seems to indent this weirdly.
> I can't say I blame it. :)
> 
> Maybe make this a separate function too, so you can use ifs and returns ?
I didn't like this idea at first, then realized I can move the `offset` getter 
there too, and this makes everything even nicer.


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

https://reviews.llvm.org/D110619

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

[Lldb-commits] [lldb] 86cd236 - [lldb] [DynamicRegisterInfo] Refactor SetRegisterInfo()

2021-09-28 Thread Michał Górny via lldb-commits 

Author: Michał Górny
Date: 2021-09-28T16:47:58+02:00
New Revision: 86cd2369b6cd7eb17374fb31bccac7895fe34658

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

LOG: [lldb] [DynamicRegisterInfo] Refactor SetRegisterInfo()

Move the "slice" and "composite" handling into separate methods to avoid
if/else hell.  Use more LLVM types whenever possible.  Replace printf()s
with llvm::Error combined with LLDB logging.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index 6535df20de11d..a5f4b8c7d7f3e 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -12,6 +12,7 @@
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/StringExtractor.h"
 #include "lldb/Utility/StructuredData.h"
@@ -56,9 +57,144 @@ void DynamicRegisterInfo::MoveFrom(DynamicRegisterInfo 
&&info) {
   info.Clear();
 }
 
+llvm::Expected DynamicRegisterInfo::ByteOffsetFromSlice(
+uint32_t index, llvm::StringRef slice_str, lldb::ByteOrder byte_order) {
+  // Slices use the following format:
+  //  REGNAME[MSBIT:LSBIT]
+  // REGNAME - name of the register to grab a slice of
+  // MSBIT - the most significant bit at which the current register value
+  // starts at
+  // LSBIT - the least significant bit at which the current register value
+  // ends at
+  static llvm::Regex g_bitfield_regex(
+  "([A-Za-z_][A-Za-z0-9_]*)\\[([0-9]+):([0-9]+)\\]");
+  llvm::SmallVector matches;
+  if (!g_bitfield_regex.match(slice_str, &matches))
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to match against register bitfield regex (slice: %s)",
+slice_str.str().c_str());
+
+  llvm::StringRef reg_name_str = matches[1];
+  llvm::StringRef msbit_str = matches[2];
+  llvm::StringRef lsbit_str = matches[3];
+  uint32_t msbit;
+  uint32_t lsbit;
+  if (!llvm::to_integer(msbit_str, msbit) ||
+  !llvm::to_integer(lsbit_str, lsbit))
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(), "msbit (%s) or lsbit (%s) are invalid",
+msbit_str.str().c_str(), lsbit_str.str().c_str());
+
+  if (msbit <= lsbit)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "msbit (%u) must be greater than lsbit 
(%u)",
+   msbit, lsbit);
+
+  const uint32_t msbyte = msbit / 8;
+  const uint32_t lsbyte = lsbit / 8;
+
+  const RegisterInfo *containing_reg_info = GetRegisterInfo(reg_name_str);
+  if (!containing_reg_info)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid concrete register \"%s\"",
+   reg_name_str.str().c_str());
+
+  const uint32_t max_bit = containing_reg_info->byte_size * 8;
+
+  if (msbit > max_bit)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"msbit (%u) must be less than the bitsize of the register \"%s\" (%u)",
+msbit, reg_name_str.str().c_str(), max_bit);
+  if (lsbit > max_bit)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"lsbit (%u) must be less than the bitsize of the register \"%s\" (%u)",
+lsbit, reg_name_str.str().c_str(), max_bit);
+
+  m_invalidate_regs_map[containing_reg_info->kinds[eRegisterKindLLDB]]
+  .push_back(index);
+  m_value_regs_map[index].push_back(
+  containing_reg_info->kinds[eRegisterKindLLDB]);
+  m_invalidate_regs_map[index].push_back(
+  containing_reg_info->kinds[eRegisterKindLLDB]);
+
+  if (byte_order == eByteOrderLittle)
+return containing_reg_info->byte_offset + lsbyte;
+  if (byte_order == eByteOrderBig)
+return containing_reg_info->byte_offset + msbyte;
+  llvm_unreachable("Invalid byte order");
+}
+
+llvm::Expected DynamicRegisterInfo::ByteOffsetFromComposite(
+uint32_t index, StructuredData::Array &composite_reg_list,
+lldb::ByteOrder byte_order) {
+  const size_t num_composite_regs = composite_reg_list.GetSize();
+  if (num_composite_regs == 0)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "\"composite\" list is empty");
+
+  uint32_t composite_offset = UINT32_MAX;
+  for (uint32_t composite_idx = 0; c

[Lldb-commits] [PATCH] D110619: [lldb] [DynamicRegisterInfo] Refactor SetRegisterInfo()

2021-09-28 Thread Michał Górny via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rG86cd2369b6cd: [lldb] [DynamicRegisterInfo] Refactor 
SetRegisterInfo() (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110619?vs=375514&id=375577#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110619

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h

Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -86,6 +86,16 @@
   typedef std::vector dwarf_opcode;
   typedef std::map dynamic_reg_size_map;
 
+  llvm::Expected ByteOffsetFromSlice(uint32_t index,
+   llvm::StringRef slice_str,
+   lldb::ByteOrder byte_order);
+  llvm::Expected ByteOffsetFromComposite(
+  uint32_t index, lldb_private::StructuredData::Array &composite_reg_list,
+  lldb::ByteOrder byte_order);
+  llvm::Expected ByteOffsetFromRegInfoDict(
+  uint32_t index, lldb_private::StructuredData::Dictionary ®_info_dict,
+  lldb::ByteOrder byte_order);
+
   void MoveFrom(DynamicRegisterInfo &&info);
 
   void ConfigureOffsets();
@@ -102,4 +112,5 @@
   bool m_finalized = false;
   bool m_is_reconfigurable = false;
 };
+
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_DYNAMICREGISTERINFO_H
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -12,6 +12,7 @@
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/StringExtractor.h"
 #include "lldb/Utility/StructuredData.h"
@@ -56,9 +57,144 @@
   info.Clear();
 }
 
+llvm::Expected DynamicRegisterInfo::ByteOffsetFromSlice(
+uint32_t index, llvm::StringRef slice_str, lldb::ByteOrder byte_order) {
+  // Slices use the following format:
+  //  REGNAME[MSBIT:LSBIT]
+  // REGNAME - name of the register to grab a slice of
+  // MSBIT - the most significant bit at which the current register value
+  // starts at
+  // LSBIT - the least significant bit at which the current register value
+  // ends at
+  static llvm::Regex g_bitfield_regex(
+  "([A-Za-z_][A-Za-z0-9_]*)\\[([0-9]+):([0-9]+)\\]");
+  llvm::SmallVector matches;
+  if (!g_bitfield_regex.match(slice_str, &matches))
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to match against register bitfield regex (slice: %s)",
+slice_str.str().c_str());
+
+  llvm::StringRef reg_name_str = matches[1];
+  llvm::StringRef msbit_str = matches[2];
+  llvm::StringRef lsbit_str = matches[3];
+  uint32_t msbit;
+  uint32_t lsbit;
+  if (!llvm::to_integer(msbit_str, msbit) ||
+  !llvm::to_integer(lsbit_str, lsbit))
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(), "msbit (%s) or lsbit (%s) are invalid",
+msbit_str.str().c_str(), lsbit_str.str().c_str());
+
+  if (msbit <= lsbit)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "msbit (%u) must be greater than lsbit (%u)",
+   msbit, lsbit);
+
+  const uint32_t msbyte = msbit / 8;
+  const uint32_t lsbyte = lsbit / 8;
+
+  const RegisterInfo *containing_reg_info = GetRegisterInfo(reg_name_str);
+  if (!containing_reg_info)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid concrete register \"%s\"",
+   reg_name_str.str().c_str());
+
+  const uint32_t max_bit = containing_reg_info->byte_size * 8;
+
+  if (msbit > max_bit)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"msbit (%u) must be less than the bitsize of the register \"%s\" (%u)",
+msbit, reg_name_str.str().c_str(), max_bit);
+  if (lsbit > max_bit)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"lsbit (%u) must be less than the bitsize of the register \"%s\" (%u)",
+lsbit, reg_name_str.str().c_str(), max_bit);
+
+  m_invalidate_regs_map[containing_reg_info->kinds[eRegisterKindLLDB]]
+  .push_back(index);
+  m_value_regs_map[index].push_back(
+  containing_reg_info->kinds[eRegisterKindLLDB]);
+  m_invalidate_regs_map[index].push_back(
+  cont

[Lldb-commits] [lldb] 993ada0 - [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

2021-09-28 Thread Ed Maste via lldb-commits 

Author: Andrew Turner
Date: 2021-09-28T10:51:06-04:00
New Revision: 993ada05f5a05615ec16da4a69bd368529a7e5d1

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

LOG: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

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

Added: 


Modified: 
lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Removed: 




diff  --git a/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp 
b/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
index 2d2c183a0c55c..e541a34e6e22a 100644
--- a/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ b/lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@ TEST(RegisterContextFreeBSDTest, arm) {
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);



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

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

2021-09-28 Thread Ed Maste via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG993ada05f5a0: [lldb] [unittests] Fix building the FreeBSD 
arm64 Register Context test (authored by andrew, committed by emaste).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110545

Files:
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

2021-09-28 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110578

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

[Lldb-commits] [PATCH] D110601: Add -relative-to-command-file to "command source"

2021-09-28 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D110601#3026363 , @jingham wrote:

> The one thing that makes me sad about this is that "command script import" 
> chose "-c" as the short letter for this option, but "-c" was already taken 
> for "command source", so I used -C for "command source".

Maybe it's not too late to change the short letter for `command script import`, 
or we could add `C` as an alias?




Comment at: lldb/source/Commands/CommandObjectCommands.cpp:156
+  // doesn't alter paths that are already relative.
+  // Question, should we make it an error to pass -C and an absolute path?
+  cmd_file.MakeAbsolute(source_dir);

Seems easy enough to detect, so why not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110601

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

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

2021-09-28 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.

Do you have commit access or should someone land this for you?




Comment at: lldb/include/lldb/Core/Mangled.h:48
+eManglingSchemeRustV0,
+eManglingSchemeD
   };

Nit which can be fixed when landing (so no need to update this revision): Could 
we add a trailing comma here just to keep the (future) git blame a bit cleaner 
once we get patches that add E, F, G, etc. support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110578

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

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

2021-09-28 Thread Luís Ferreira via Phabricator via lldb-commits 
ljmf00 added a comment.

In D110578#3027692 , @teemperor wrote:

> Do you have commit access or should someone land this for you?

I don't have commit access, although this is a stacked revision, so there are 
some dependent patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110578

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

[Lldb-commits] [PATCH] D110578: [lldb] Add support for D programming language

2021-09-28 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

In D110578#3027707 , @ljmf00 wrote:

> In D110578#3027692 , @teemperor 
> wrote:
>
>> Do you have commit access or should someone land this for you?
>
> I don't have commit access, although this is a stacked revision, so there are 
> some dependent patches.

Alright, feel free to ping then if this is ready to land (or maybe ping the 
person for the dependent patch to land this too). Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110578

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

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

2021-09-28 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM with a followup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110115

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

[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

2021-09-28 Thread Joerg Sonnenberger via Phabricator via lldb-commits 
joerg added a comment.

Why are all the changes from separator character to separator string necessary 
or desirable?


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

https://reviews.llvm.org/D110535

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

[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

2021-09-28 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

In D110535#3028450 , @joerg wrote:

> Why are all the changes from separator character to separator string 
> necessary or desirable?

Because otherwise we'd have to resort to hacks to extend the separator's 
lifetime.


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

https://reviews.llvm.org/D110535

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