[Lldb-commits] [PATCH] D80793: [lldb][NFC] Make ClangExpressionSourceCode's wrapping logic more consistent

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h:41
+/// Note that this is also used for static member functions of a C++ class.
+Function
+  };

shafik wrote:
> I would actually feel better have a separate enumerator for C++ static member 
> functions and just having it fall-through when used. It would be 
> self-documenting.
If it doesn't complicate the callers (i.e., they always know, or can easily 
find out, whether they are dealing with a static member or a non-member), I 
think that would be great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80793



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

[Lldb-commits] [lldb] cbfae97 - [LLDB] Mark TestCreateDuringInstructionStep as flaky on Linux

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-06-02T14:08:28+05:00
New Revision: cbfae97ca82b11ab2b54562055c49817baa1e26b

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

LOG: [LLDB] Mark TestCreateDuringInstructionStep as flaky on Linux

This patch marks TestCreateDuringInstructionStep.py as flakey for Linux.
This is failing randomly on arm/aarch64. I will monitor buildbot and
skip it if it fails again.

Added: 


Modified: 

lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py

Removed: 




diff  --git 
a/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
 
b/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
index 807f1ae9a2ac..59473a3f924e 100644
--- 
a/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
+++ 
b/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
@@ -18,6 +18,7 @@ class CreateDuringInstructionStepTestCase(TestBase):
 
 @skipUnlessPlatform(['linux'])
 @expectedFailureAndroid('llvm.org/pr24737', archs=['arm'])
+@expectedFlakeyLinux(bugnumber="llvm.org/pr24737")
 def test_step_inst(self):
 self.build(dictionary=self.getBuildFlags())
 exe = self.getBuildArtifact("a.out")



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

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good. Let's just wait a while to see if Fred has any comments. If you 
haven't already, you can use that time to get commit access. :)




Comment at: lldb/source/Utility/UUID.cpp:66-67
   uuid_bytes.clear();
   while (!p.empty()) {
-if (isxdigit(p[0]) && isxdigit(p[1])) {
+if (p.size() >= 2 && isxdigit(p[0]) && isxdigit(p[1])) {
   int hi_nibble = xdigit_to_int(p[0]);

I guess now obsoletes Fred's D80807.

(Btw, I actually liked how Fred's solution rejects strings which end in a 
trailing dash.)


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

https://reviews.llvm.org/D80755



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

Re: [Lldb-commits] [lldb] cbfae97 - [LLDB] Mark TestCreateDuringInstructionStep as flaky on Linux

[Pavel Labath via lldb-commits]

On 02/06/2020 11:10, Muhammad Omair Javaid via lldb-commits wrote:
> 
> Author: Muhammad Omair Javaid
> Date: 2020-06-02T14:08:28+05:00
> New Revision: cbfae97ca82b11ab2b54562055c49817baa1e26b
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/cbfae97ca82b11ab2b54562055c49817baa1e26b
> DIFF: 
> https://github.com/llvm/llvm-project/commit/cbfae97ca82b11ab2b54562055c49817baa1e26b.diff
> 
> LOG: [LLDB] Mark TestCreateDuringInstructionStep as flaky on Linux
> 
> This patch marks TestCreateDuringInstructionStep.py as flakey for Linux.
> This is failing randomly on arm/aarch64. I will monitor buildbot and
> skip it if it fails again.
> 

Hi Omair,

I'd like to note two things:
- I haven't seen this fail on x86 in a very long time. So, whatever the
problem is, it seems limited to arm architectures
- since we've switched to using lit as the test driver, I believe the
"flaky" decorators don't actually do anything (we should really delete
them if that's the case).

So all in all, I believe you're better off just skipping this on
arm+aarch64 until someone can investigate this -- which I am hoping will
not be too hard, given that this fails fairly frequently.

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

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

[Pavel Labath via Phabricator via lldb-commits]

labath marked 5 inline comments as done.
labath added inline comments.



Comment at: 
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8
+  // FROM-FUNC1-NEXT: func1
+  // FROM-FUNC1-SAME: [artificial]
+  // FROM-FUNC1-NEXT: main

dblaikie wrote:
> vsk wrote:
> > labath wrote:
> > > vsk wrote:
> > > > Are these test updates necessary because lldb doesn't print '[opt]' and 
> > > > '[artificial]' next to frame descriptions in a consistent way across 
> > > > platforms? Or is it just that you don't think matching '[opt]' is 
> > > > relevant to the test?
> > > Right, I wanted to mention that as it's not very obvious, but I forgot...
> > > 
> > > The `[opt]` thingy is not printed at all with -ggdb because the attribute 
> > > we get this information from -- DW_AT_APPLE_optimized -- is only emitted 
> > > for -glldb. The optimization flag did not seem very relevant for these 
> > > tests (I mean, technically the compiler could emit call site attributes 
> > > even in non-optimized mode) so instead of forking the expectations I 
> > > chose to simply remove it.
> > Sounds good.
> As an aside, now that lldb understands these attributes - perhaps we should 
> emit them under -glldb as well as -ggdb? (@aprantl might be interested in 
> making that call)
FWIW, I think that would be great as it would reduce the effects of the 
debugger tuning argument, making the compiler output more "portable".

Though, we may want to wait with that until I look at the -1 issue. I believe 
that the way this is implemented now means we will end up pointing to the 
middle of a call instruction in an artificial frame, which would be a slight 
regression. It's not the end of the world, but I believe we can do something 
slightly better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519



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

[Lldb-commits] [lldb] bddd288 - [lldb/DWARF] Add support for pre-standard GNU call site attributes

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-06-02T12:57:51+02:00
New Revision: bddd2888264492a6deb0d447ee6ac042d3fb44e4

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

LOG: [lldb/DWARF] Add support for pre-standard GNU call site attributes

Summary:
The code changes are very straight-forward -- just handle both DW_AT_GNU
and DW_AT_call versions of all tags and attributes. There is just one
small gotcha: in the GNU version, DW_AT_low_pc was used both for the
"return pc" and the "call pc" values, depending on whether the tag was
describing a tail call, while the official scheme uses different
attributes for the two things.

Reviewers: vsk, dblaikie

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp

lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py

lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/test/API/functionalities/tail_call_frames/cross_dso/Makefile
lldb/test/API/functionalities/tail_call_frames/cross_dso/One.mk
lldb/test/API/functionalities/tail_call_frames/cross_dso/Two.mk
lldb/test/API/functionalities/tail_call_frames/cross_object/Makefile

lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/main.cpp

lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp

lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp

lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py

lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp
lldb/test/API/functionalities/tail_call_frames/sbapi_support/Makefile

lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py

lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/main.cpp

lldb/test/API/functionalities/tail_call_frames/thread_step_out_or_return/Makefile

lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Removed: 

lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile

lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile

lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/Makefile

lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/Makefile

lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile

lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/Makefile

lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/Makefile
lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/Makefile



diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index f8fc1db7ec29..94b36e8b18bd 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -396,6 +396,7 @@ static offset_t GetOpcodeDataSize(const DataExtractor &data,
 return offset - data_offset;
   }
 
+  case DW_OP_GNU_entry_value:
   case DW_OP_entry_value: // 0xa3 ULEB128 size + variable-length block
   {
 uint64_t subexpr_len = data.GetULEB128(&offset);
@@ -2522,6 +2523,7 @@ bool DWARFExpression::Evaluate(
   stack.push_back(Scalar(value));
 } break;
 
+case DW_OP_GNU_entry_value:
 case DW_OP_entry_value: {
   if (!Evaluate_DW_OP_entry_value(stack, exe_ctx, reg_ctx, opcodes, offset,
   error_ptr, log)) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 6d22fbe2e2ee..71a01947da26 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3601,7 +3601,8 @@ CollectCallSiteParameters(ModuleSP module, D

[Lldb-commits] [PATCH] D80990: [lldb] Fix sizeof() in Scalar::operator=()

[Albert Miko via Phabricator via lldb-commits]

miko created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In Scalar::operator=(long long) the size of the result was calculated using 
sizeof(long) instead of sizeof(long long). On Windows sizeof(long)==4 but 
sizeof(long long)==8.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80990

Files:
  lldb/source/Utility/Scalar.cpp


Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -331,7 +331,7 @@
 
 Scalar &Scalar::operator=(long long v) {
   m_type = e_slonglong;
-  m_integer = llvm::APInt(sizeof(long) * 8, v, true);
+  m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
   return *this;
 }
 


Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -331,7 +331,7 @@
 
 Scalar &Scalar::operator=(long long v) {
   m_type = e_slonglong;
-  m_integer = llvm::APInt(sizeof(long) * 8, v, true);
+  m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
   return *this;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbddd28882644: [lldb/DWARF] Add support for pre-standard GNU 
call site attributes (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
  
lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
  
lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
  
lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
  
lldb/test/API/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
  lldb/test/API/functionalities/tail_call_frames/cross_dso/Makefile
  lldb/test/API/functionalities/tail_call_frames/cross_dso/One.mk
  lldb/test/API/functionalities/tail_call_frames/cross_dso/Two.mk
  lldb/test/API/functionalities/tail_call_frames/cross_object/Makefile
  lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/Makefile
  
lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
  lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
  
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/Makefile
  
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
  
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp
  
lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
  
lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
  
lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
  
lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/Makefile
  
lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
  
lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp
  lldb/test/API/functionalities/tail_call_frames/sbapi_support/Makefile
  
lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/Makefile
  
lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
  
lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/main.cpp
  
lldb/test/API/functionalities/tail_call_frames/thread_step_out_or_return/Makefile
  lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/Makefile
  
lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
  lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Index: lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
===
--- lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
+++ lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
@@ -2,11 +2,13 @@
 
 void __attribute__((noinline)) sink() {
   x++; //% self.filecheck("bt", "main.cpp", "-implicit-check-not=artificial")
-  // CHECK: frame #0: 0x{{[0-9a-f]+}} a.out`sink() at main.cpp:[[@LINE-1]]:4 [opt]
-  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:14:3 [opt] [artificial]
-  // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2() {{.*}} [opt]
-  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:23:3 [opt] [artificial]
-  // CHECK-NEXT: frame #4: 0x{{[0-9a-f]+}} a.out`main{{.*}} [opt]
+  // CHECK: frame #0: 0x{{[0-9a-f]+}} a.out`sink() at main.cpp:[[@LINE-1]]:4
+  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:16:3
+  // CHECK-SAME: [artificial]
+  // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2()
+  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:25:3
+  // CHECK-SAME: [artificial]
+  // CHECK-NEXT: frame #4: 0x{{[0-9a-f]+}} a.out`main
 }
 
 void __attribute__((noinline)) func3() {
Index: lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
===
--- lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
+++ lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
@@ -1,6 +1,9 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-lldbinline.MakeInlineTest(__file__, globals(),
-[decorators.skipUnlessHasCallSiteInfo,
- decorators.skipIf(dwarf_version=['<', 

[Lldb-commits] [PATCH] D80990: [lldb] Fix sizeof() in Scalar::operator=()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The fix looks obviously correct, but could you please also add a unit test for 
this. I guess this would manifest itself as the inability to roundtrip e.g. 
`std::numeric_limits::max()` to a Scalar and back.

It's not required, but if you're interested, it would be nice to change all of 
these assignment operators to do `return *this = Scalar(v);`. That would remove 
this class of bugs (constructor/operator= inconsistencies) completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80990



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

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: 
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8
+  // FROM-FUNC1-NEXT: func1
+  // FROM-FUNC1-SAME: [artificial]
+  // FROM-FUNC1-NEXT: main

labath wrote:
> dblaikie wrote:
> > vsk wrote:
> > > labath wrote:
> > > > vsk wrote:
> > > > > Are these test updates necessary because lldb doesn't print '[opt]' 
> > > > > and '[artificial]' next to frame descriptions in a consistent way 
> > > > > across platforms? Or is it just that you don't think matching '[opt]' 
> > > > > is relevant to the test?
> > > > Right, I wanted to mention that as it's not very obvious, but I 
> > > > forgot...
> > > > 
> > > > The `[opt]` thingy is not printed at all with -ggdb because the 
> > > > attribute we get this information from -- DW_AT_APPLE_optimized -- is 
> > > > only emitted for -glldb. The optimization flag did not seem very 
> > > > relevant for these tests (I mean, technically the compiler could emit 
> > > > call site attributes even in non-optimized mode) so instead of forking 
> > > > the expectations I chose to simply remove it.
> > > Sounds good.
> > As an aside, now that lldb understands these attributes - perhaps we should 
> > emit them under -glldb as well as -ggdb? (@aprantl might be interested in 
> > making that call)
> FWIW, I think that would be great as it would reduce the effects of the 
> debugger tuning argument, making the compiler output more "portable".
> 
> Though, we may want to wait with that until I look at the -1 issue. I believe 
> that the way this is implemented now means we will end up pointing to the 
> middle of a call instruction in an artificial frame, which would be a slight 
> regression. It's not the end of the world, but I believe we can do something 
> slightly better.
Ok, I take that back. The instruction pointer handling is not terribly 
consistent right now anyway:
```
(lldb) up
frame #1: 0x00401210 a.out`func12(...)
(lldb) register read rip
 rip = 0x00401300  
```

So, I wouldn't worry too much about preserving behavior here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519



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

[Lldb-commits] [PATCH] D80990: [lldb] Fix sizeof() in Scalar::operator=()

[Raphael Isemann via Phabricator via lldb-commits]

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

Could you add a unit test for this? See the`ScalarTest.cpp` for the other 
Scalar tests.
Beside that this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80990



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

[Lldb-commits] [PATCH] D80995: [Scalar] Fix assignment operator for long long.

[Andy Yankovsky via Phabricator via lldb-commits]

werat created this revision.
werat added a reviewer: labath.
werat added a project: LLDB.
werat added a reviewer: teemperor.

Assignment operator `operator=(long long)` currently allocates `sizeof(long)`. 
On some platforms it works as they have `sizeof(long) == sizeof(long long)`, 
but on others (e.g. Windows) it's not the case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80995

Files:
  lldb/source/Utility/Scalar.cpp
  lldb/unittests/Utility/ScalarTest.cpp


Index: lldb/unittests/Utility/ScalarTest.cpp
===
--- lldb/unittests/Utility/ScalarTest.cpp
+++ lldb/unittests/Utility/ScalarTest.cpp
@@ -188,6 +188,16 @@
 ScalarGetValue(std::numeric_limits::max()));
 }
 
+TEST(ScalarTest, LongLongAssigmentOperator) {
+  Scalar ull;
+  ull = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), ull.ULongLong());
+
+  Scalar sll;
+  sll = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), sll.SLongLong());
+}
+
 TEST(ScalarTest, Division) {
   Scalar lhs(5.0);
   Scalar rhs(2.0);
Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -331,7 +331,7 @@
 
 Scalar &Scalar::operator=(long long v) {
   m_type = e_slonglong;
-  m_integer = llvm::APInt(sizeof(long) * 8, v, true);
+  m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
   return *this;
 }
 


Index: lldb/unittests/Utility/ScalarTest.cpp
===
--- lldb/unittests/Utility/ScalarTest.cpp
+++ lldb/unittests/Utility/ScalarTest.cpp
@@ -188,6 +188,16 @@
 ScalarGetValue(std::numeric_limits::max()));
 }
 
+TEST(ScalarTest, LongLongAssigmentOperator) {
+  Scalar ull;
+  ull = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), ull.ULongLong());
+
+  Scalar sll;
+  sll = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), sll.SLongLong());
+}
+
 TEST(ScalarTest, Division) {
   Scalar lhs(5.0);
   Scalar rhs(2.0);
Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -331,7 +331,7 @@
 
 Scalar &Scalar::operator=(long long v) {
   m_type = e_slonglong;
-  m_integer = llvm::APInt(sizeof(long) * 8, v, true);
+  m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
   return *this;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D80995: [Scalar] Fix assignment operator for long long.

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good. Given the timings, I am going to assume you are coordinating with 
@miko.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80995



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

[Lldb-commits] [PATCH] D80990: [lldb] Fix sizeof() in Scalar::operator=()

[Albert Miko via Phabricator via lldb-commits]

miko abandoned this revision.
miko added a comment.

Unittest added in https://reviews.llvm.org/D80995.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80990



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

[Lldb-commits] [PATCH] D80995: [Scalar] Fix assignment operator for long long.

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

In D80995#2068571 , @labath wrote:

> Looks good. Given the timings, I am going to assume you are coordinating with 
> @miko.


Thanks! This is very surprising, but we're not coordinating. I've found out 
about their patch just now. Apparently it was submitted while I was reading the 
documentation about how to submit mine :D

I see @miko has closed their review in favor of this one.

@labath, I don't have commit rights. Can you, please, commit this patch? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80995



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

[Lldb-commits] [lldb] df06f4f - [lldb] Handle a new clang built-in type

[Kadir Cetinkaya via lldb-commits]

Author: Kadir Cetinkaya
Date: 2020-06-02T15:41:33+02:00
New Revision: df06f4ff227bdcbbad01b418199876761f2a1ff0

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

LOG: [lldb] Handle a new clang built-in type

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 9ff8bdb7537f..892df4fd5750 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4830,6 +4830,9 @@ lldb::Encoding 
TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type,
 case clang::BuiltinType::SveFloat32:
 case clang::BuiltinType::SveFloat64:
   break;
+
+case clang::BuiltinType::IncompleteMatrixIdx:
+  break;
 }
 break;
   // All pointer types are represented as unsigned integer encodings. We may



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

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Shu Anzai via Phabricator via lldb-commits]

gedatsu217 created this revision.
gedatsu217 added reviewers: teemperor, JDevlieghere.
gedatsu217 added a project: LLDB.
Herald added a subscriber: lldb-commits.

I implemented autosuggestion if there is one possible suggestion.
I set the keybinds for every character. When a character is typed, 
Editline::TypedCharacter is called. 
Then, autosuggestion part is displayed in gray, and you can actually input by 
typing C-k.
Editline::Autosuggest is a function for finding completion, and it is like 
Editline::TabCommand now, but I will add more features to it.

Testing does not work well in my environment, so I can't confirm that it goes 
well, sorry. I am dealing with it now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81001

Files:
  lldb/include/lldb/Host/Editline.h
  lldb/source/Host/common/Editline.cpp

Index: lldb/source/Host/common/Editline.cpp
===
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Host/Editline.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/CompletionRequest.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBAssert.h"
@@ -114,22 +115,21 @@
   //  - The H_FIRST returns the most recent entry in the history.
   //
   // The naming of the enum entries match the semantic meaning.
-  switch(op) {
-case HistoryOperation::Oldest:
-  return H_LAST;
-case HistoryOperation::Older:
-  return H_NEXT;
-case HistoryOperation::Current:
-  return H_CURR;
-case HistoryOperation::Newer:
-  return H_PREV;
-case HistoryOperation::Newest:
-  return H_FIRST;
+  switch (op) {
+  case HistoryOperation::Oldest:
+return H_LAST;
+  case HistoryOperation::Older:
+return H_NEXT;
+  case HistoryOperation::Current:
+return H_CURR;
+  case HistoryOperation::Newer:
+return H_PREV;
+  case HistoryOperation::Newest:
+return H_FIRST;
   }
   llvm_unreachable("Fully covered switch!");
 }
 
-
 EditLineStringType CombineLines(const std::vector &lines) {
   EditLineStringStreamType combined_stream;
   for (EditLineStringType line : lines) {
@@ -296,8 +296,8 @@
 // to use when loading/saving history
   std::string m_path;   // Path to the history file
 };
-}
-}
+} // namespace line_editor
+} // namespace lldb_private
 
 // Editline private methods
 
@@ -415,9 +415,10 @@
   const char *unfaint = m_color_prompts ? ANSI_UNFAINT : "";
 
   for (int index = firstIndex; index < line_count; index++) {
-fprintf(m_output_file, "%s"
-   "%s"
-   "%s" EditLineStringFormatSpec " ",
+fprintf(m_output_file,
+"%s"
+"%s"
+"%s" EditLineStringFormatSpec " ",
 faint, PromptForIndex(index).c_str(), unfaint,
 m_input_lines[index].c_str());
 if (index < line_count - 1)
@@ -532,9 +533,10 @@
   // (will only be requested if colors are supported)
   if (m_needs_prompt_repaint) {
 MoveCursor(CursorLocation::EditingCursor, CursorLocation::EditingPrompt);
-fprintf(m_output_file, "%s"
-   "%s"
-   "%s",
+fprintf(m_output_file,
+"%s"
+"%s"
+"%s",
 ANSI_FAINT, Prompt(), ANSI_UNFAINT);
 MoveCursor(CursorLocation::EditingPrompt, CursorLocation::EditingCursor);
 m_needs_prompt_repaint = false;
@@ -1040,6 +1042,77 @@
   return CC_REDISPLAY;
 }
 
+std::string Editline::AutoSuggest(std::string typed) {
+  const LineInfo *line_info = el_line(m_editline);
+
+  llvm::StringRef line(line_info->buffer,
+   line_info->lastchar - line_info->buffer);
+  unsigned cursor_index = line_info->cursor - line_info->buffer;
+  CompletionResult result;
+  CompletionRequest request(line, cursor_index, result);
+
+  m_completion_callback(request, m_completion_callback_baton);
+
+  llvm::ArrayRef results = result.GetResults();
+
+  StringList completions;
+  result.GetMatches(completions);
+
+  std::string to_add = "";
+
+  if (results.size() == 1) {
+CompletionResult::Completion completion = results.front();
+switch (completion.GetMode()) {
+case CompletionMode::Normal: {
+  to_add = completion.GetCompletion();
+  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
+  if (request.GetParsedArg().IsQuoted())
+to_add.push_back(request.GetParsedArg().GetQuoteChar());
+  to_add.push_back(' ');
+  break;
+}
+case CompletionMode::Partial: {
+  std::string to_add = completion.GetCompletion();
+  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
+  break;
+}
+case CompletionMode::RewriteLine: {
+  el_deletestr(m_editline, line_info->cursor - line_info->buffer);
+  el_insertstr(m_editline, completion.GetComp

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the patch. It's a very interesting feature, but I'm not sure it is 
actually a good idea. Some tab completions can be very expensive (not to 
mention they can crash lldb), and so running them after every keypress sounds 
problematic to me. Editors usually have very complex logic to implement these 
things efficiently and unobtrusively. It usually involves running the 
completions on a separate thread with various delays to avoid stalling a user 
who types fast and avoid burning the cpu needlessly.

I don't see anything like that in this patch, and I'm not sure the complexity 
that would entail is worth avoiding pressing tab a couple of times. If we 
really do want this feature, I think it should be controlled by a setting, and 
probably the setting should be off by default.

I'm not sure what's your main motivation for this, but they way I could see 
this potentially being useful is if we had a notion of "lightweight" 
completions. These would things that don't do any extraordinary amount of work 
(like parsing gigabytes of debug info) -- just basic completions for lldb 
commands and their arguments.

Regarding the actual patch, I have two quick comments:

- please avoid unrelated formatting changes -- run clang-format only over the 
lines you've changed (`git clang-format HEAD^` or something similar)
- don't work with `stdout` directly -- you should be using the "stdout" notions 
local to the Editline class (e.g.., `m_output_file`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D81010: [lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: vsk, dblaikie.
Herald added subscribers: mgrang, aprantl.
Herald added a project: LLDB.

The way that the support for the GNU dialect of tail call frames was
implemented in D80519  meant that the were 
reporting very bogus PC values
which pointed into the middle of an instruction: the -1 trick is
necessary for the address to resolve to the right function, but we
should still be reporting a more realistic PC value -- I say "realistic"
and not "real", because it's very debatable what should be the correct
PC value for frames like this.

This patch achieves that my moving the -1 from SymbolFileDWARF into the
stack frame computation code. The idea is that SymbolFileDWARF will
merely report whether it has provided an address of the instruction
after the tail call, or the address of the call instruction itself. The
StackFrameList machinery uses this information to set the "behaves like
frame zero" property of the artificial frames (the main thing this flag
does is it controls the -1 subtraction when looking up the function
address).

This required a moderate refactor of the CallEdge class, because it was
implicitly assuming that edges pointing after the call were real calls
and those pointing the the call insn were tail calls. The class now
carries this information explicitly -- it carries three mostly
independent pieces of information:

- an address of interest in the caller
- a bit saying whether this address points to the call insn or after it
- whether this is a tail call


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81010

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Index: lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
===
--- lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
+++ lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
@@ -3,12 +3,22 @@
 void __attribute__((noinline)) sink() {
   x++; //% self.filecheck("bt", "main.cpp", "-implicit-check-not=artificial")
   // CHECK: frame #0: 0x{{[0-9a-f]+}} a.out`sink() at main.cpp:[[@LINE-1]]:4
-  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:16:3
+  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:26:3
   // CHECK-SAME: [artificial]
   // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2()
-  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:25:3
+  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:35:3
   // CHECK-SAME: [artificial]
   // CHECK-NEXT: frame #4: 0x{{[0-9a-f]+}} a.out`main
+  // In the GNU style, the artificial frames will point after the tail call
+  // instruction. In v5 they should point to the instruction itself.
+  //% frame1 = self.thread().GetFrameAtIndex(1)
+  //% func3 = frame1.GetFunction()
+  //% func3_insns = func3.GetInstructions(self.target())
+  //% self.trace("func3:\n%s"%func3_insns)
+  //% last_insn = func3_insns.GetInstructionAtIndex(func3_insns.GetSize()-1)
+  //% addr = last_insn.GetAddress()
+  //% if "GNU" in self.name: addr.OffsetAddress(last_insn.GetByteSize())
+  //% self.assertEquals(frame1.GetPCAddress(), addr)
 }
 
 void __attribute__((noinline)) func3() {
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -239,7 +239,12 @@
 /// A sequence of calls that comprise some portion of a backtrace. Each frame
 /// is represented as a pair of a callee (Function *) and an address within the
 /// callee.
-using CallSequence = std::vector>;
+struct CallDescriptor {
+  Function *func;
+  CallEdge::AddrType address_type;
+  addr_t address;
+};
+using CallSequence = std::vector;
 
 /// Find the unique path through the call graph from \p begin (with return PC
 /// \p return_pc) to \p end. On success this path is stored into \p path, and 
@@ -319,14 +324,14 @@
   }
 
   // Search the calls made from this callee.
-  active_path.emplace_back(&callee, LLDB_INVALID_ADDRESS);
+  active_path.push_back(CallDescriptor{&callee});
   for (const auto &edge : callee.GetTailCallingEdges()) {
 Function *next_callee = edge->GetCallee(images, context);
 if (!next_callee)
   continue;
 
-addr_t tail_call_pc = edge->GetCallInstPC(callee, target);
-active_path.back().second = tail_call_pc;
+std::tie(active_path.back().address_type, active_path.back().address) =
+edge->GetCallerAddress(callee, target);
 
 dfs(*edge, *next_callee);
 if (ambiguous)
@@ -400,16 +405,16 @@
 
   // Push

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

@labath Just to get you into the loop: This is for a GSoC project that is about 
implementing the autosuggestions similar to what fish shell is providing. It's 
not really about actually invoking any completion logic (even though that might 
be an option in the future), but more about the 'history and context' based 
search fish is doing. This patch is just WIP and I asked @gedatsu217 to upload 
it that that I can give some feedback and make sure he's on the right track.

@gedatsu217 I forgot to say that if you upload reviews for early review that 
marking them as WIP and maybe not add the LLDB tag (as this will put all LLDB 
folks in CC). Just adding me/Jonas is fine for those patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the explanation Raphael. This makes more sense now, though I am 
still not very clear on the distinction between "completions" and 
"suggestions". The fish tutorial mentions command history -- that's something 
that's not typically done as a part of tab completion (though maybe it could 
be?). However, the rest -- file paths and command arguments -- that's clearly 
in scope for tab completion too and our tab handlers already do that. So, the 
idea of having "lightweight" completions which would also get run as a part of 
"suggestions" does not seem completely off track. Unless I am completely off 
track, that is...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D81001#2068925 , @labath wrote:

> Thanks for the explanation Raphael. This makes more sense now, though I am 
> still not very clear on the distinction between "completions" and 
> "suggestions". The fish tutorial mentions command history -- that's something 
> that's not typically done as a part of tab completion (though maybe it could 
> be?). However, the rest -- file paths and command arguments -- that's clearly 
> in scope for tab completion too and our tab handlers already do that. So, the 
> idea of having "lightweight" completions which would also get run as a part 
> of "suggestions" does not seem completely off track. Unless I am completely 
> off track, that is...


It's more like a smarter and more user-friendly Ctrl+R search. Having it do 
something similar to tab completion is a stretch goal at the end. But at the 
moment the plan is to do just history based suggestions, then see if history + 
context is possible (i.e., make the suggestions paired to a specific target 
that you only get a suggestion for `b ASTContext.cpp:434` if you debug LLDB but 
not if you debug another random program) and then the lightweight completions 
if there is time left.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [lldb] de04375 - [lldb] Skip tests exercising DW_OP_GNU_entry_value with dsymutil

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-06-02T18:31:15+02:00
New Revision: de04375ac59e6e9290b361b3ffcf4558e688e8a9

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

LOG: [lldb] Skip tests exercising DW_OP_GNU_entry_value with dsymutil

It seems that this opcode needs explicit support in dsymutil. Disable
these tests until that is implemented.

Added: 


Modified: 

lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py

lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py

lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py

lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
 
b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
index ccc737ca24de..4f294f555ec8 100644
--- 
a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
+++ 
b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
@@ -12,6 +12,7 @@
 name="BasicEntryValues_V5",
 build_dict=dict(CXXFLAGS_EXTRAS="-O2 -glldb"))
 
-lldbinline.MakeInlineTest(__file__, globals(), decorators=decorators,
+lldbinline.MakeInlineTest(__file__, globals(),
+decorators=decorators+[skipIf(debug_info="dsym")],
 name="BasicEntryValues_GNU",
 build_dict=dict(CXXFLAGS_EXTRAS="-O2 -ggdb"))

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
index 99a2e762caa4..699263e7150c 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
@@ -1,9 +1,10 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-decorators = [decorators.skipUnlessHasCallSiteInfo,
+decor = [decorators.skipUnlessHasCallSiteInfo,
  decorators.skipIf(dwarf_version=['<', '4'])]
 lldbinline.MakeInlineTest(__file__, globals(), name="DisambiguateCallSite_V5",
-build_dict=dict(CFLAGS_EXTRAS="-O2 -glldb"), decorators=decorators)
+build_dict=dict(CFLAGS_EXTRAS="-O2 -glldb"), decorators=decor)
 lldbinline.MakeInlineTest(__file__, globals(), name="DisambiguateCallSite_GNU",
-build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb"), decorators=decorators)
+build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb"),
+decorators=decor+[decorators.skipIf(debug_info="dsym")])

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
index 815b5852d95c..80ed07992f1e 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
@@ -1,11 +1,12 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-decorators = [decorators.skipUnlessHasCallSiteInfo,
+decor = [decorators.skipUnlessHasCallSiteInfo,
  decorators.skipIf(dwarf_version=['<', '4'])]
 lldbinline.MakeInlineTest(__file__, globals(),
 name="DisambiguatePathsToCommonSink_V5",
-build_dict=dict(CFLAGS_EXTRAS="-O2 -glldb"), decorators=decorators)
+build_dict=dict(CFLAGS_EXTRAS="-O2 -glldb"), decorators=decor)
 lldbinline.MakeInlineTest(__file__, globals(),
 name="DisambiguatePathsToCommonSink_GNU",
-build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb"), decorators=decorators)
+build_dict=dict(CFLAGS_EXTRAS="-O2 -ggdb"),
+decorators=decor+[decorators.skipIf(debug_info="dsym")])

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
index 3fc89201d9fd..9e4422cfa3ae 

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.



Comment at: lldb/source/Utility/UUID.cpp:66-67
   uuid_bytes.clear();
   while (!p.empty()) {
-if (isxdigit(p[0]) && isxdigit(p[1])) {
+if (p.size() >= 2 && isxdigit(p[0]) && isxdigit(p[1])) {
   int hi_nibble = xdigit_to_int(p[0]);

labath wrote:
> I guess now obsoletes Fred's D80807.
> 
> (Btw, I actually liked how Fred's solution rejects strings which end in a 
> trailing dash.)
Yeah, I didn't have a strong opinion before, but given we want to reject a 
buffer that isn't parsed completely, I think it's better to reject a buffer 
ending with a `-`. As the code would test `p.size()` anyway, we might as well 
use `p.size() >= 2 ` as the loop condition.

Otherwise this LGTM. Thanks!


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

https://reviews.llvm.org/D80755



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

[Lldb-commits] [PATCH] D80807: [lldb/Utility] Fix DecodeUUIDBytesFromString not to access past the input buffer

[Frederic Riss via Phabricator via lldb-commits]

friss abandoned this revision.
friss added a comment.

Abandon in favor of D80755 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80807



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

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: 
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8
+  // FROM-FUNC1-NEXT: func1
+  // FROM-FUNC1-SAME: [artificial]
+  // FROM-FUNC1-NEXT: main

labath wrote:
> labath wrote:
> > dblaikie wrote:
> > > vsk wrote:
> > > > labath wrote:
> > > > > vsk wrote:
> > > > > > Are these test updates necessary because lldb doesn't print '[opt]' 
> > > > > > and '[artificial]' next to frame descriptions in a consistent way 
> > > > > > across platforms? Or is it just that you don't think matching 
> > > > > > '[opt]' is relevant to the test?
> > > > > Right, I wanted to mention that as it's not very obvious, but I 
> > > > > forgot...
> > > > > 
> > > > > The `[opt]` thingy is not printed at all with -ggdb because the 
> > > > > attribute we get this information from -- DW_AT_APPLE_optimized -- is 
> > > > > only emitted for -glldb. The optimization flag did not seem very 
> > > > > relevant for these tests (I mean, technically the compiler could emit 
> > > > > call site attributes even in non-optimized mode) so instead of 
> > > > > forking the expectations I chose to simply remove it.
> > > > Sounds good.
> > > As an aside, now that lldb understands these attributes - perhaps we 
> > > should emit them under -glldb as well as -ggdb? (@aprantl might be 
> > > interested in making that call)
> > FWIW, I think that would be great as it would reduce the effects of the 
> > debugger tuning argument, making the compiler output more "portable".
> > 
> > Though, we may want to wait with that until I look at the -1 issue. I 
> > believe that the way this is implemented now means we will end up pointing 
> > to the middle of a call instruction in an artificial frame, which would be 
> > a slight regression. It's not the end of the world, but I believe we can do 
> > something slightly better.
> Ok, I take that back. The instruction pointer handling is not terribly 
> consistent right now anyway:
> ```
> (lldb) up
> frame #1: 0x00401210 a.out`func12(...)
> (lldb) register read rip
>  rip = 0x00401300  
> ```
> 
> So, I wouldn't worry too much about preserving behavior here.
I don't see any concrete benefit to supporting -ggdb on Darwin. Actually, 
changing llvm to emit the GNU opcodes on Darwin seems bad to me, as it could 
force Darwin tools authors to support two sets of call-site related opcodes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519



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

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: 
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8
+  // FROM-FUNC1-NEXT: func1
+  // FROM-FUNC1-SAME: [artificial]
+  // FROM-FUNC1-NEXT: main

vsk wrote:
> labath wrote:
> > labath wrote:
> > > dblaikie wrote:
> > > > vsk wrote:
> > > > > labath wrote:
> > > > > > vsk wrote:
> > > > > > > Are these test updates necessary because lldb doesn't print 
> > > > > > > '[opt]' and '[artificial]' next to frame descriptions in a 
> > > > > > > consistent way across platforms? Or is it just that you don't 
> > > > > > > think matching '[opt]' is relevant to the test?
> > > > > > Right, I wanted to mention that as it's not very obvious, but I 
> > > > > > forgot...
> > > > > > 
> > > > > > The `[opt]` thingy is not printed at all with -ggdb because the 
> > > > > > attribute we get this information from -- DW_AT_APPLE_optimized -- 
> > > > > > is only emitted for -glldb. The optimization flag did not seem very 
> > > > > > relevant for these tests (I mean, technically the compiler could 
> > > > > > emit call site attributes even in non-optimized mode) so instead of 
> > > > > > forking the expectations I chose to simply remove it.
> > > > > Sounds good.
> > > > As an aside, now that lldb understands these attributes - perhaps we 
> > > > should emit them under -glldb as well as -ggdb? (@aprantl might be 
> > > > interested in making that call)
> > > FWIW, I think that would be great as it would reduce the effects of the 
> > > debugger tuning argument, making the compiler output more "portable".
> > > 
> > > Though, we may want to wait with that until I look at the -1 issue. I 
> > > believe that the way this is implemented now means we will end up 
> > > pointing to the middle of a call instruction in an artificial frame, 
> > > which would be a slight regression. It's not the end of the world, but I 
> > > believe we can do something slightly better.
> > Ok, I take that back. The instruction pointer handling is not terribly 
> > consistent right now anyway:
> > ```
> > (lldb) up
> > frame #1: 0x00401210 a.out`func12(...)
> > (lldb) register read rip
> >  rip = 0x00401300  
> > ```
> > 
> > So, I wouldn't worry too much about preserving behavior here.
> I don't see any concrete benefit to supporting -ggdb on Darwin. Actually, 
> changing llvm to emit the GNU opcodes on Darwin seems bad to me, as it could 
> force Darwin tools authors to support two sets of call-site related opcodes.
Not sure what it would mean to "support -ggdb on Darwin" - it's supported in 
that some peolpe might be using gdb on Darwin & compile that way. But I meant 
emitting these when targeting lldb - which someone might be doing on any 
platform & they might still want to use DWARFv4 for whatever reason - or does 
DWARFv4 + -glldb already use the DWARFv5 call site opcodes? If so, then, sure, 
that sounds OK to me & I don't suppose there's much reason to emit the GNU ones 
instead. If DWARFv4 + -glldb doesn't emit any call site info, it seems like an 
improvement to emit the GNU extension (consumers should always be written to 
ignore tags/attributes they don't know - and if so they'll be no worse off than 
if the call site info hadn't been emitted) or the v5 attributes maybe (though I 
don't know that that's quite as valid - I guess a consumer could rightly reject 
tags/attributes that aren't in the extension number space, or in the standard 
number space for the version being parsed)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519



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

[Lldb-commits] [PATCH] D81010: [lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

Thanks, this ended up being a lot cleaner than I expected!




Comment at: lldb/include/lldb/Symbol/Function.h:332
 
-  /// The address of the call instruction. Usually an invalid address, unless
-  /// this is a tail call.
-  lldb::addr_t call_inst_pc;
+  bool caller_address_type : 1, is_tail_call : 1;
 

It seems like there's going to be around sizeof(void *) bytes worth of padding 
here. Maybe it'd be simpler to store these flags as AddrType and bool 
respectively? Alternatively, the struct can be marked 'packed'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81010



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

[Lldb-commits] [PATCH] D81032: [lldb/Test] Don't print 'command invoked'

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
JDevlieghere added a project: LLDB.

The different tools constructing dotest invocations (lit and
lldb-dotest) already print the command invocation so there's no point in
duplicating it in the dotest output.

My motivation for removing it is that it doesn't include the 
the Python interpreter and every time I accidentally copy it
the command fails with an `ImportError`.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D81032

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -49,9 +49,6 @@
 from lldbsuite.test_event.event_builder import EventBuilder
 from ..support import seven
 
-def get_dotest_invocation():
-return ' '.join(sys.argv)
-
 
 def is_exe(fpath):
 """Returns true if fpath is an executable."""
@@ -220,7 +217,6 @@
 parser = dotest_args.create_parser()
 args = parser.parse_args()
 except:
-print(get_dotest_invocation())
 raise
 
 if args.unset_env_varnames:
@@ -243,10 +239,6 @@
 if args.set_inferior_env_vars:
 lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
 
-# Only print the args if being verbose.
-if args.v:
-print(get_dotest_invocation())
-
 if args.h:
 do_help = True
 
@@ -1085,7 +1077,6 @@
 "\nSession logs for test failures/errors/unexpected successes"
 " will go into directory '%s'\n" %
 configuration.sdir_name)
-sys.stderr.write("Command invoked: %s\n" % get_dotest_invocation())
 
 #
 # Invoke the default TextTestRunner to run the test suite


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -49,9 +49,6 @@
 from lldbsuite.test_event.event_builder import EventBuilder
 from ..support import seven
 
-def get_dotest_invocation():
-return ' '.join(sys.argv)
-
 
 def is_exe(fpath):
 """Returns true if fpath is an executable."""
@@ -220,7 +217,6 @@
 parser = dotest_args.create_parser()
 args = parser.parse_args()
 except:
-print(get_dotest_invocation())
 raise
 
 if args.unset_env_varnames:
@@ -243,10 +239,6 @@
 if args.set_inferior_env_vars:
 lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
 
-# Only print the args if being verbose.
-if args.v:
-print(get_dotest_invocation())
-
 if args.h:
 do_help = True
 
@@ -1085,7 +1077,6 @@
 "\nSession logs for test failures/errors/unexpected successes"
 " will go into directory '%s'\n" %
 configuration.sdir_name)
-sys.stderr.write("Command invoked: %s\n" % get_dotest_invocation())
 
 #
 # Invoke the default TextTestRunner to run the test suite
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: 
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8
+  // FROM-FUNC1-NEXT: func1
+  // FROM-FUNC1-SAME: [artificial]
+  // FROM-FUNC1-NEXT: main

dblaikie wrote:
> vsk wrote:
> > labath wrote:
> > > labath wrote:
> > > > dblaikie wrote:
> > > > > vsk wrote:
> > > > > > labath wrote:
> > > > > > > vsk wrote:
> > > > > > > > Are these test updates necessary because lldb doesn't print 
> > > > > > > > '[opt]' and '[artificial]' next to frame descriptions in a 
> > > > > > > > consistent way across platforms? Or is it just that you don't 
> > > > > > > > think matching '[opt]' is relevant to the test?
> > > > > > > Right, I wanted to mention that as it's not very obvious, but I 
> > > > > > > forgot...
> > > > > > > 
> > > > > > > The `[opt]` thingy is not printed at all with -ggdb because the 
> > > > > > > attribute we get this information from -- DW_AT_APPLE_optimized 
> > > > > > > -- is only emitted for -glldb. The optimization flag did not seem 
> > > > > > > very relevant for these tests (I mean, technically the compiler 
> > > > > > > could emit call site attributes even in non-optimized mode) so 
> > > > > > > instead of forking the expectations I chose to simply remove it.
> > > > > > Sounds good.
> > > > > As an aside, now that lldb understands these attributes - perhaps we 
> > > > > should emit them under -glldb as well as -ggdb? (@aprantl might be 
> > > > > interested in making that call)
> > > > FWIW, I think that would be great as it would reduce the effects of the 
> > > > debugger tuning argument, making the compiler output more "portable".
> > > > 
> > > > Though, we may want to wait with that until I look at the -1 issue. I 
> > > > believe that the way this is implemented now means we will end up 
> > > > pointing to the middle of a call instruction in an artificial frame, 
> > > > which would be a slight regression. It's not the end of the world, but 
> > > > I believe we can do something slightly better.
> > > Ok, I take that back. The instruction pointer handling is not terribly 
> > > consistent right now anyway:
> > > ```
> > > (lldb) up
> > > frame #1: 0x00401210 a.out`func12(...)
> > > (lldb) register read rip
> > >  rip = 0x00401300  
> > > ```
> > > 
> > > So, I wouldn't worry too much about preserving behavior here.
> > I don't see any concrete benefit to supporting -ggdb on Darwin. Actually, 
> > changing llvm to emit the GNU opcodes on Darwin seems bad to me, as it 
> > could force Darwin tools authors to support two sets of call-site related 
> > opcodes.
> Not sure what it would mean to "support -ggdb on Darwin" - it's supported in 
> that some peolpe might be using gdb on Darwin & compile that way. But I meant 
> emitting these when targeting lldb - which someone might be doing on any 
> platform & they might still want to use DWARFv4 for whatever reason - or does 
> DWARFv4 + -glldb already use the DWARFv5 call site opcodes? If so, then, 
> sure, that sounds OK to me & I don't suppose there's much reason to emit the 
> GNU ones instead. If DWARFv4 + -glldb doesn't emit any call site info, it 
> seems like an improvement to emit the GNU extension (consumers should always 
> be written to ignore tags/attributes they don't know - and if so they'll be 
> no worse off than if the call site info hadn't been emitted) or the v5 
> attributes maybe (though I don't know that that's quite as valid - I guess a 
> consumer could rightly reject tags/attributes that aren't in the extension 
> number space, or in the standard number space for the version being parsed)
Yep, DWARFv4 + -glldb emits DWARFv5 call site opcodes -- and I should have 
written (as I meant), 'there's no need to change this to behave like -ggdb 
under -gdwarf-4 mode on Darwin'. It should (remain!) possible to use -ggdb on 
Darwin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519



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

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: 
lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8
+  // FROM-FUNC1-NEXT: func1
+  // FROM-FUNC1-SAME: [artificial]
+  // FROM-FUNC1-NEXT: main

vsk wrote:
> dblaikie wrote:
> > vsk wrote:
> > > labath wrote:
> > > > labath wrote:
> > > > > dblaikie wrote:
> > > > > > vsk wrote:
> > > > > > > labath wrote:
> > > > > > > > vsk wrote:
> > > > > > > > > Are these test updates necessary because lldb doesn't print 
> > > > > > > > > '[opt]' and '[artificial]' next to frame descriptions in a 
> > > > > > > > > consistent way across platforms? Or is it just that you don't 
> > > > > > > > > think matching '[opt]' is relevant to the test?
> > > > > > > > Right, I wanted to mention that as it's not very obvious, but I 
> > > > > > > > forgot...
> > > > > > > > 
> > > > > > > > The `[opt]` thingy is not printed at all with -ggdb because the 
> > > > > > > > attribute we get this information from -- DW_AT_APPLE_optimized 
> > > > > > > > -- is only emitted for -glldb. The optimization flag did not 
> > > > > > > > seem very relevant for these tests (I mean, technically the 
> > > > > > > > compiler could emit call site attributes even in non-optimized 
> > > > > > > > mode) so instead of forking the expectations I chose to simply 
> > > > > > > > remove it.
> > > > > > > Sounds good.
> > > > > > As an aside, now that lldb understands these attributes - perhaps 
> > > > > > we should emit them under -glldb as well as -ggdb? (@aprantl might 
> > > > > > be interested in making that call)
> > > > > FWIW, I think that would be great as it would reduce the effects of 
> > > > > the debugger tuning argument, making the compiler output more 
> > > > > "portable".
> > > > > 
> > > > > Though, we may want to wait with that until I look at the -1 issue. I 
> > > > > believe that the way this is implemented now means we will end up 
> > > > > pointing to the middle of a call instruction in an artificial frame, 
> > > > > which would be a slight regression. It's not the end of the world, 
> > > > > but I believe we can do something slightly better.
> > > > Ok, I take that back. The instruction pointer handling is not terribly 
> > > > consistent right now anyway:
> > > > ```
> > > > (lldb) up
> > > > frame #1: 0x00401210 a.out`func12(...)
> > > > (lldb) register read rip
> > > >  rip = 0x00401300  
> > > > ```
> > > > 
> > > > So, I wouldn't worry too much about preserving behavior here.
> > > I don't see any concrete benefit to supporting -ggdb on Darwin. Actually, 
> > > changing llvm to emit the GNU opcodes on Darwin seems bad to me, as it 
> > > could force Darwin tools authors to support two sets of call-site related 
> > > opcodes.
> > Not sure what it would mean to "support -ggdb on Darwin" - it's supported 
> > in that some peolpe might be using gdb on Darwin & compile that way. But I 
> > meant emitting these when targeting lldb - which someone might be doing on 
> > any platform & they might still want to use DWARFv4 for whatever reason - 
> > or does DWARFv4 + -glldb already use the DWARFv5 call site opcodes? If so, 
> > then, sure, that sounds OK to me & I don't suppose there's much reason to 
> > emit the GNU ones instead. If DWARFv4 + -glldb doesn't emit any call site 
> > info, it seems like an improvement to emit the GNU extension (consumers 
> > should always be written to ignore tags/attributes they don't know - and if 
> > so they'll be no worse off than if the call site info hadn't been emitted) 
> > or the v5 attributes maybe (though I don't know that that's quite as valid 
> > - I guess a consumer could rightly reject tags/attributes that aren't in 
> > the extension number space, or in the standard number space for the version 
> > being parsed)
> Yep, DWARFv4 + -glldb emits DWARFv5 call site opcodes -- and I should have 
> written (as I meant), 'there's no need to change this to behave like -ggdb 
> under -gdwarf-4 mode on Darwin'. It should (remain!) possible to use -ggdb on 
> Darwin.
Ah, fair enough - marginally questionable emitting future standard 
tags/attributes in previous versions - but I'm not too fussed so long as it's 
not new forms. As that's the direction chosen, yeah, no point emitting the 
DWARFv4 gdb extension tags/attributes under -glldb.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519



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

[Lldb-commits] [lldb] 5138a91 - [lldb/Test] Don't use the env to pass around configuration variables (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-02T16:11:32-07:00
New Revision: 5138a91ef4f365a3e71eec4cea6b4599dfaabf26

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

LOG: [lldb/Test] Don't use the env to pass around configuration variables (NFC)

Don't use the environment to pass values to the builder that are present
in the dotest configuration module. A subsequent patch will pass the
remaining values through the configuration instead of the environment.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/plugins/builder_base.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index c4e4b615aca8..54e5d4bc2df5 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -462,8 +462,6 @@ def parseOptionsAndInitTestdirs():
 configuration.clang_module_cache_dir = os.path.join(
 configuration.test_build_dir, 'module-cache-clang')
 
-os.environ['CLANG_MODULE_CACHE_DIR'] = configuration.clang_module_cache_dir
-
 if args.lldb_libs_dir:
 configuration.lldb_libs_dir = args.lldb_libs_dir
 
@@ -522,7 +520,6 @@ def setupSysPath():
 os.environ["LLDB_TEST_SRC"] = lldbsuite.lldb_test_root
 
 # Set up the root build directory.
-builddir = configuration.test_build_dir
 if not configuration.test_build_dir:
 raise Exception("test_build_dir is not set")
 os.environ["LLDB_BUILD"] = os.path.abspath(configuration.test_build_dir)
@@ -1096,8 +1093,6 @@ def run_suite():
 print("compiler=%s" % configuration.compiler)
 
 # Iterating over all possible architecture and compiler combinations.
-os.environ["ARCH"] = configuration.arch
-os.environ["CC"] = configuration.compiler
 configString = "arch=%s compiler=%s" % (configuration.arch,
 configuration.compiler)
 

diff  --git a/lldb/packages/Python/lldbsuite/test/plugins/builder_base.py 
b/lldb/packages/Python/lldbsuite/test/plugins/builder_base.py
index 0f30f70546fa..3c19839869c4 100644
--- a/lldb/packages/Python/lldbsuite/test/plugins/builder_base.py
+++ b/lldb/packages/Python/lldbsuite/test/plugins/builder_base.py
@@ -21,17 +21,18 @@
 # Our imports
 import lldbsuite.test.lldbtest as lldbtest
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test import configuration
 from lldbsuite.test_event import build_exception
 
 
 def getArchitecture():
 """Returns the architecture in effect the test suite is running with."""
-return os.environ["ARCH"] if "ARCH" in os.environ else ""
+return configuration.arch if configuration.arch else ""
 
 
 def getCompiler():
 """Returns the compiler in effect the test suite is running with."""
-compiler = os.environ.get("CC", "clang")
+compiler = configuration.compiler if configuration.compiler else "clang"
 compiler = lldbutil.which(compiler)
 return os.path.realpath(compiler)
 
@@ -86,8 +87,8 @@ def getArchSpec(architecture):
 used for the make system.
 """
 arch = architecture if architecture else None
-if not arch and "ARCH" in os.environ:
-arch = os.environ["ARCH"]
+if not arch and configuration.arch:
+arch = configuration.arch
 
 return ("ARCH=" + arch) if arch else ""
 
@@ -98,8 +99,8 @@ def getCCSpec(compiler):
 used for the make system.
 """
 cc = compiler if compiler else None
-if not cc and "CC" in os.environ:
-cc = os.environ["CC"]
+if not cc and configuration.compiler:
+cc = configuration.compiler
 if cc:
 return "CC=\"%s\"" % cc
 else:
@@ -128,9 +129,9 @@ def getModuleCacheSpec():
 Helper function to return the key-value string to specify the clang
 module cache used for the make system.
 """
-if "CLANG_MODULE_CACHE_DIR" in os.environ:
+if configuration.clang_module_cache_dir:
 return "CLANG_MODULE_CACHE_DIR={}".format(
-os.environ["CLANG_MODULE_CACHE_DIR"])
+configuration.clang_module_cache_dir)
 return "";
 
 def getCmdLine(d):



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

[Lldb-commits] [lldb] 393ac21 - [lldb/Test] Pass Make arguments in invocation instead of environment

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-02T16:49:58-07:00
New Revision: 393ac216489773e3676ec7d80c3d993f78a29b6a

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

LOG: [lldb/Test] Pass Make arguments in invocation instead of environment

The Darwin builder is passing some of the make arguments trough the
environment instead of the command line. Update the dsym builder to do
the same as the other variants.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/plugins/builder_darwin.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/plugins/builder_darwin.py 
b/lldb/packages/Python/lldbsuite/test/plugins/builder_darwin.py
index e109f91945eb..e9fc8257dea5 100644
--- a/lldb/packages/Python/lldbsuite/test/plugins/builder_darwin.py
+++ b/lldb/packages/Python/lldbsuite/test/plugins/builder_darwin.py
@@ -16,7 +16,11 @@ def buildDsym(
 ["MAKE_DSYM=YES",
  getArchSpec(architecture),
  getCCSpec(compiler),
- "all", getCmdLine(dictionary)])
+ getDsymutilSpec(),
+ getSDKRootSpec(),
+ getModuleCacheSpec(),
+ "all",
+ getCmdLine(dictionary)])
 
 runBuildCommands(commands, sender=sender)
 



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

[Lldb-commits] [lldb] 4c53d48 - [lldb/Test] Don't use the env to pass around configuration variables (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-02T16:49:58-07:00
New Revision: 4c53d4801cbbb1b573e4ef758f93ead12e1f59a2

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

LOG: [lldb/Test] Don't use the env to pass around configuration variables (NFC)

Don't use the environment to pass values to the builder. Use the
configuration instead.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/packages/Python/lldbsuite/test/plugins/builder_base.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 0439c4e8f1ac..f05152253c75 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -42,8 +42,10 @@
 count = 1
 
 # The 'arch' and 'compiler' can be specified via command line.
-arch = None# Must be initialized after option parsing
-compiler = None# Must be initialized after option parsing
+arch = None
+compiler = None
+dsymutil = None
+sdkroot = None
 
 # The overriden dwarf verison.
 dwarf_version = 0

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 54e5d4bc2df5..80edad811bae 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -275,9 +275,9 @@ def parseOptionsAndInitTestdirs():
 break
 
 if args.dsymutil:
-os.environ['DSYMUTIL'] = args.dsymutil
+configuration.dsymutil = args.dsymutil
 elif platform_system == 'Darwin':
-os.environ['DSYMUTIL'] = seven.get_command_output(
+configuration.dsymutil = seven.get_command_output(
 'xcrun -find -toolchain default dsymutil')
 
 if args.filecheck:
@@ -302,7 +302,7 @@ def parseOptionsAndInitTestdirs():
 
 # Set SDKROOT if we are using an Apple SDK
 if platform_system == 'Darwin' and args.apple_sdk:
-os.environ['SDKROOT'] = seven.get_command_output(
+configuration.sdkroot = seven.get_command_output(
 'xcrun --sdk "%s" --show-sdk-path 2> /dev/null' %
 (args.apple_sdk))
 
@@ -310,10 +310,10 @@ def parseOptionsAndInitTestdirs():
 configuration.arch = args.arch
 if configuration.arch.startswith(
 'arm') and platform_system == 'Darwin' and not args.apple_sdk:
-os.environ['SDKROOT'] = seven.get_command_output(
+configuration.sdkroot = seven.get_command_output(
 'xcrun --sdk iphoneos.internal --show-sdk-path 2> /dev/null')
-if not os.path.exists(os.environ['SDKROOT']):
-os.environ['SDKROOT'] = seven.get_command_output(
+if not os.path.exists(configuration.sdkroot):
+configuration.sdkroot = seven.get_command_output(
 'xcrun --sdk iphoneos --show-sdk-path 2> /dev/null')
 else:
 configuration.arch = platform_machine
@@ -522,7 +522,7 @@ def setupSysPath():
 # Set up the root build directory.
 if not configuration.test_build_dir:
 raise Exception("test_build_dir is not set")
-os.environ["LLDB_BUILD"] = os.path.abspath(configuration.test_build_dir)
+configuration.test_build_dir = 
os.path.abspath(configuration.test_build_dir)
 
 # Set up the LLDB_SRC environment variable, so that the tests can locate
 # the LLDB source code.
@@ -1041,8 +1041,7 @@ def run_suite():
 
 # Set up the working directory.
 # Note that it's not dotest's job to clean this directory.
-build_dir = configuration.test_build_dir
-lldbutil.mkdir_p(build_dir)
+lldbutil.mkdir_p(configuration.test_build_dir)
 
 target_platform = lldb.selected_platform.GetTriple().split('-')[2]
 

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 04ba7ea02d09..0a640d2d5c93 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -668,7 +668,7 @@ def getBuildDirBasename(self):
 
 def getBuildDir(self):
 """Return the full path to the current test."""
-return os.path.join(os.environ["LLDB_BUILD"], self.mydir,
+return os.path.join(configuration.test_build_dir, self.mydir,
 self.getBuildDirBasename())
 
 def getReproducerDir(self):
@@ -682,7 +682,6 @@ def getReproducerDir(self):
 def makeBuildDir(self):
 """Create the test-specific working directory, deleting any previous
 contents."""
-# See also dotest.py which sets up ${LLDB_BUILD}.
 bdir = 

[Lldb-commits] [lldb] 2d2a603 - Remove redundant code (NFC)

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-06-02T16:55:50-07:00
New Revision: 2d2a603d663328e25774982947e3a8a65e098678

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

LOG: Remove redundant code (NFC)

This has no effect on the testsuite and was only needed in an early
prototype from before debugserver was able to report the correct
platform.

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index f2735212dff9..6f3e8b637cf2 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -685,15 +685,8 @@ bool DynamicLoaderDarwin::AddModulesUsingImageInfos(
   // Update the module's platform with the DYLD info.
   ArchSpec dyld_spec = image_infos[idx].GetArchitecture();
   if (dyld_spec.GetTriple().getOS() == llvm::Triple::IOS &&
-  dyld_spec.GetTriple().getEnvironment() == llvm::Triple::MacABI) {
+  dyld_spec.GetTriple().getEnvironment() == llvm::Triple::MacABI)
 image_module_sp->MergeArchitecture(dyld_spec);
-const auto &target_triple = target.GetArchitecture().GetTriple();
-// If dyld reports the process as being loaded as MACCATALYST,
-// force-update the target's architecture to MACCATALYST.
-if (!(target_triple.getOS() == llvm::Triple::IOS &&
-  target_triple.getEnvironment() == llvm::Triple::MacABI))
-  target.SetArchitecture(dyld_spec);
-  }
 }
   }
 



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

[Lldb-commits] [PATCH] D80345: [DwarfExpression] Support entry values for indirect parameters

[Vedant Kumar via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e39379bbbe1: [DwarfExpression] Support entry values for 
indirect parameters (authored by vsk).

Changed prior to commit:
  https://reviews.llvm.org/D80345?vs=265815&id=266337#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80345

Files:
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
  llvm/docs/LangRef.rst
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
  llvm/lib/CodeGen/LiveDebugValues.cpp
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param-with-offset.mir
  llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir

Index: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
===
--- /dev/null
+++ llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir
@@ -0,0 +1,117 @@
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -stop-after=machineverifier -o - %s \
+# RUN:   | FileCheck %s -check-prefix=MIR
+
+# RUN: llc -emit-call-site-info -start-before=livedebugvalues -filetype=obj -o - %s \
+# RUN:   | llvm-dwarfdump - | FileCheck %s -check-prefix=DWARF -implicit-check-not=DW_OP_entry_value
+
+# // Original Source
+# struct fat_ptr {
+#   int *ptr, *low, *high;
+# };
+# extern int baz(int x);
+# int bar(struct fat_ptr f) {
+#   return baz(baz(*f.ptr));
+# }
+
+# MIR:  renamable $w0 = LDRWui killed renamable $x8
+# MIR-NEXT: DBG_VALUE $x0, 0, {{.*}}, !DIExpression(DW_OP_LLVM_entry_value, 1)
+# MIR-NEXT: BL @baz
+# MIR-NEXT: frame-destroy LDPXpost
+# MIR-NEXT: TCRETURNdi @baz
+
+# After w0 is clobbered, we should get an indirect parameter entry value for "f".
+
+# DWARF-LABEL: DW_TAG_formal_parameter
+# DWARF-NEXT: DW_AT_location
+# DWARF-NEXT: [0x, 0x0010): DW_OP_breg0 W0+0
+# DWARF-NEXT: [0x0010, 0x001c): DW_OP_entry_value(DW_OP_reg0 W0))
+# DWARF-NEXT: DW_AT_name("f")
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "arm64-apple-ios10.0.0"
+
+  %struct.fat_ptr = type { i32*, i32*, i32* }
+
+  define i32 @bar(%struct.fat_ptr* nocapture readonly %f) local_unnamed_addr !dbg !13 {
+  entry:
+call void @llvm.dbg.declare(metadata %struct.fat_ptr* %f, metadata !23, metadata !DIExpression()), !dbg !24
+%ptr2 = bitcast %struct.fat_ptr* %f to i32**, !dbg !25
+%0 = load i32*, i32** %ptr2, align 8, !dbg !25
+%1 = load i32, i32* %0, align 4, !dbg !31
+%call = tail call i32 @baz(i32 %1), !dbg !34
+%call1 = tail call i32 @baz(i32 %call), !dbg !35
+ret i32 %call1, !dbg !36
+  }
+
+  declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+  declare !dbg !4 i32 @baz(i32) local_unnamed_addr optsize
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!8, !9, !10, !11}
+  !llvm.ident = !{!12}
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None, sysroot: "/")
+  !1 = !DIFile(filename: "indirect.c", directory: "/tmp/fatptr")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "baz", scope: !1, file: !1, line: 4, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{!7, !7}
+  !7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !8 = !{i32 7, !"Dwarf Version", i32 4}
+  !9 = !{i32 2, !"Debug Info Version", i32 3}
+  !10 = !{i32 1, !"wchar_size", i32 4}
+  !11 = !{i32 7, !"PIC Level", i32 2}
+  !12 = !{!"clang"}
+  !13 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 5, type: !14, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !22)
+  !14 = !DISubroutineType(types: !15)
+  !15 = !{!7, !16}
+  !16 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "fat_ptr", file: !1, line: 1, size: 192, elements: !17)
+  !17 = !{!18, !20, !21}
+  !18 = !DIDerivedType(tag: DW_TAG_member, name: "ptr", scope: !16, file: !1, line: 2, baseType: !19, size: 64)
+  !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+  !20 = !DIDerivedType(tag: DW_TAG_member, name: "low", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 64)
+  !21 = !DIDerivedType(tag: DW_TAG_member, name: "high", scope: !16, file: !1, line: 2, baseType: !19, size: 64, offset: 128)
+  !22 = !{!23}
+  !23 = !DILocalVariable(name: "f", arg: 1, scope: !13, file: !1, line: 5, type: !16)
+  !24 = !DILocation(line: 5, column: 24, scope: !13)
+  !25 = !DILocation(line: 6, column: 23, scope: !13)
+  !31 = !DILocation(lin

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:99
+  if (!overflow)
+overflow |= !llvm::checkedAdd(*signed_sum, SInt(carry_in));
   uint64_t result = unsigned_sum;

aprantl wrote:
> vsk wrote:
> > The docs [1] say 'integer signed_sum = SInt(x) + SInt(y) + UInt(carry_in)', 
> > but I bet checkedAdd doesn't support that, and anyway carry_in is 1-bit so 
> > it doesn't matter.
> > 
> > [1] 
> > https://developer.arm.com/docs/ddi0596/e/shared-pseudocode-functions/shared-functionsinteger-pseudocode#impl-shared.AddWithCarry.3
> Am I misreading this or are we also setting the C(arry) flag inverted 
> according to the documentation?
Oh wow.. nice catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80955



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

[Lldb-commits] [lldb] def72b9 - [lldb/Interpreter] Remove redundant argument (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-06-02T21:23:19-07:00
New Revision: def72b91950d44a68b8613f25fa1a09926171222

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

LOG: [lldb/Interpreter] Remove redundant argument (NFC)

Added: 


Modified: 
lldb/source/Interpreter/CommandReturnObject.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandReturnObject.cpp 
b/lldb/source/Interpreter/CommandReturnObject.cpp
index efeac6d6c90e..a0ede4588291 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -14,12 +14,9 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s,
-  bool add_newline_if_empty) {
+static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
   bool add_newline = false;
-  if (s.empty()) {
-add_newline = add_newline_if_empty;
-  } else {
+  if (!s.empty()) {
 // We already checked for empty above, now make sure there is a newline in
 // the error, and if there isn't one, add one.
 strm.Write(s.c_str(), s.size());
@@ -50,7 +47,7 @@ void CommandReturnObject::AppendErrorWithFormat(const char 
*format, ...) {
   if (!s.empty()) {
 Stream &error_strm = GetErrorStream();
 error_strm.PutCString("error: ");
-DumpStringToStreamWithNewline(error_strm, s, false);
+DumpStringToStreamWithNewline(error_strm, s);
   }
 }
 



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

[Lldb-commits] [PATCH] D80448: [lldb/Test] Add a trace method to replace (commented out) print statements.

[Martin Böhme via Phabricator via lldb-commits]

mboehme added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:510
+with recording(self, self.TraceOn()) as sbuf:
+print(args, kwargs, file=sbuf)
+

labath wrote:
> JDevlieghere wrote:
> > This should be 
> > ```
> > print(*args, kwargs, file=sbuf)
> > ```
> Actually, it should be `print(*args, **kwargs, file=sbuf)`
With both Python 2 and 3, this, is giving me an error:

```
print(*args, **kwargs, file=sbuf)
 ^
SyntaxError: invalid syntax
```

I believe this should be `print(*args, file=sbuf, **kwargs)`. Can you fix this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80448



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