[Lldb-commits] [PATCH] D39283: [lldb-dev] Update LLDB test cases for 'inlineStepping'

[Carlos Alberto Enciso via Phabricator via lldb-commits]

CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D39283#907100, @tberghammer wrote:

> Hi Carlos,
>
> Sorry for not responding to your related e-mails lately.


Hi Tamas,

There is no need for apologies. You have help me a lot in setting LLDB and be 
able to run the test suite. (Windows and Linux).

You are correct; there is a missing line in the referenced test case. It should 
look like this:

  void
  caller_trivial_1 ()
  {
  inline_value += 1;  // At first increment in caller_trivial_1.
  caller_trivial_2(); // In caller_trivial_1.
  inline_value += 1; 
  }

Thanks.,
Carlos


https://reviews.llvm.org/D39283



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

[Lldb-commits] [PATCH] D34774: [lldb] Add a testcase for MainThreadCheckerRuntime plugin

[chenyu via Phabricator via lldb-commits]

SeanChense added a comment.

Hi there, I am here to find help. Is there  a way to get 
`libMainThreadChecker.dylib` output ? Then we can analyze the output to 
generate a report ?


https://reviews.llvm.org/D34774



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

Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

[Greg Clayton via lldb-commits]

> On Oct 25, 2017, at 6:05 PM, Zachary Turner  wrote:
> 
> 
> 
> On Wed, Oct 25, 2017 at 4:59 PM Jim Ingham via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> jingham added a comment.
> 
> Note, BTW, we absolutely need some way to say "this symbol from this 
> library".  But first of all, if we're going to do this you need to be able to 
> mix & match within an expression which you can't do with a flag to expr.   
> Instead you need something like:
> 
> (lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol
> 
> That syntax is ugly, we should try to think of something better.  But the 
> main point is this should only be necessary when lldb can't find a unique 
> symbol.  When we can no intervention should be required.
> 
> +1, this is very useful.  The Microsoft syntax for this is here:
> 
> https://docs.microsoft.com/en-us/visualstudio/debugger/context-operator-cpp 
> 
> 
> Which is pretty nice imo

The main reason for using $ decorated names is because clang will accept them 
as identifiers and ask us about them. So "$$MyOtherDylib$my_other_symbol" is 
just a valid identifier and would result in a find external lexical decl call 
that we can fill in with whatever we want. 

We don't really want to muck with clang by overloading stuff with symbols that 
would hose up clang (like the MSVC examples "{,,EXAMPLE.dll}SomeFunction", 
"EXAMPLE.dll!SomeFunction", and "{,,"a long, long, library name.dll"}g_Var". 
The { } and , characters would hose up the expression parser. So I would vote 
to use the $ decoration as Jim suggested.

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

Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

[Zachary Turner via lldb-commits]

Note that $ is a valid character in a MSVC-mangled symbol name.  So, I
don't think it will work for that reason alone.  FWIW, I also don't like
the {,,} syntax very much, but if you read on there's a simpler ! syntax
that is pretty nice.

libfoo!symbolname


On Thu, Oct 26, 2017 at 9:30 AM Greg Clayton  wrote:

>
> On Oct 25, 2017, at 6:05 PM, Zachary Turner  wrote:
>
>
>
> On Wed, Oct 25, 2017 at 4:59 PM Jim Ingham via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> jingham added a comment.
>>
>> Note, BTW, we absolutely need some way to say "this symbol from this
>> library".  But first of all, if we're going to do this you need to be able
>> to mix & match within an expression which you can't do with a flag to
>> expr.   Instead you need something like:
>>
>> (lldb) expr $$MyDylib$my_symbol + $$MyOtherDylib$my_other_symbol
>>
>> That syntax is ugly, we should try to think of something better.  But the
>> main point is this should only be necessary when lldb can't find a unique
>> symbol.  When we can no intervention should be required.
>>
>
> +1, this is very useful.  The Microsoft syntax for this is here:
>
> https://docs.microsoft.com/en-us/visualstudio/debugger/context-operator-cpp
>
> Which is pretty nice imo
>
>
> The main reason for using $ decorated names is because clang will accept
> them as identifiers and ask us about them. So
> "$$MyOtherDylib$my_other_symbol" is just a valid identifier and would
> result in a find external lexical decl call that we can fill in with
> whatever we want.
>
> We don't really want to muck with clang by overloading stuff with symbols
> that would hose up clang (like the MSVC examples
> "{,,EXAMPLE.dll}SomeFunction", "EXAMPLE.dll!SomeFunction", and "{,,"a long,
> long, library name.dll"}g_Var". The { } and , characters would hose up the
> expression parser. So I would vote to use the $ decoration as Jim suggested.
>
> Greg
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Greg Clayton via Phabricator via lldb-commits]

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

Wow. I didn't realize windows support didn't do any of this. Looks good. Zach 
will want to ok this as he is one of the main windows contributors.


https://reviews.llvm.org/D39314



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

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

can you please try adding a test for this one? :)


https://reviews.llvm.org/D39314



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

[Lldb-commits] [PATCH] D34774: [lldb] Add a testcase for MainThreadCheckerRuntime plugin

[Kuba (Brecka) Mracek via Phabricator via lldb-commits]

kubamracek added a comment.

In https://reviews.llvm.org/D34774#907636, @SeanChense wrote:

> Hi there, I am here to find help. Is there  a way to get 
> `libMainThreadChecker.dylib` output ? Then we can analyze the output to 
> generate a report ?


Hi, what are you trying to do? What info are you missing that the LLDB plugin 
doesn't provide? I don't think parsing the text output (in stderr) from 
libMainThreadChecker.dylib is a good idea, as it's likely to change.


https://reviews.llvm.org/D34774



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

[Lldb-commits] [PATCH] D39335: Allow SysV-i386 ABI on everything other than Apple targets

[Stephane Sezer via Phabricator via lldb-commits]

sas created this revision.
Herald added subscribers: kristof.beyls, aemerson.

This matches other SysV ABIs that are different on Apple and non-Apple targets,
like `ABISysV_arm.cpp` for instance.


https://reviews.llvm.org/D39335

Files:
  source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp


Index: source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) 
{
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }


Index: source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:651-659
+  ArchSpec header_arch;
+  GetArchitecture(header_arch);
+  if (header_arch.GetMachine() == llvm::Triple::arm) {
+if (function_rva & 1) {
+  // For Thumb we need the last bit to be 0 so that the address
+  // points to the right beginning of the symbol.
+  function_rva ^= 1;

Shouldn't this happen for all thumb symbols? Other object file parsers define 
something like:

```
#define THUMB_ADDRESS_BIT_MASK 0xfffeull
```
And do things like:

```
function_rva &=THUMB_ADDRESS_BIT_MASK;
```

Makes it a bit cleaner to read.

It would be better to create a  "bool is_arm = ...;" similar to other symbol 
file plug-ins. Determine that once before all symbols are being parsed. Here it 
is being done for each symbol in the loop. 

Be aware that you must answer the address class correctly later with:

```
AddressClass ObjectFilePECOFF::GetAddressClass(lldb::addr_t file_addr);
```

It must return "eAddressClassCode" for ARM address ranges, and 
"eAddressClassCodeAlternateISA" for Thumb address ranges. Check how 
"ObjectFileMachO::GetAddressClass()" or "ObjectFileELF::GetAddressClass()" does 
this. For mach-o we set a bit in each symbol's flags... I will comment below 
where you could do that. The address class map is used to set breakpoints 
correctly in Thumb or ARM code and must be done accurately.


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Stephane Sezer via Phabricator via lldb-commits]

sas added a comment.

In https://reviews.llvm.org/D39314#908065, @davide wrote:

> can you please try adding a test for this one? :)


To be fully honest, I'm not 100% sure how I'm supposed to add tests for this. I 
looked through the directories containing unit tests and didn't find anything 
specific to DYLD testing.

I'm going to try to sync with @zturner to see if he is still able to run unit 
tests on Windows. We exclusively debug Windows remotes from a macOS or Linux 
host, so I don't have any setup to run local windows tests.


https://reviews.llvm.org/D39314



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Greg Clayton via Phabricator via lldb-commits]

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

Very close. Thanks for making the changes. Just a few nits.




Comment at: utils/clangdiag.py:66
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:

Is there only ever one of these? If so this is good. 



Comment at: utils/clangdiag.py:117
+disable(exe_ctx)
+else:
+getDiagtool(exe_ctx.GetTarget(), args.path[0])

should probably verify that this 'diagtool' with:

```
elif args.subcommands == 'diagtool':
```
and add an else that creates an error:
```
else:
result.SetError("invalid subcommand '%s'" % (args.subcommands))
```


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D19603: Fix entry point lookup for ObjectFilePECOFF

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Does this need some ARM support where we strip bit zero in case the entry point 
is Thumb?


https://reviews.llvm.org/D19603



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

[Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Stephane Sezer via lldb-commits]

Author: sas
Date: Thu Oct 26 10:04:20 2017
New Revision: 316673

URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
Log:
Allow SysV-i386 ABI on everything other than Apple targets

Summary:
This matches other SysV ABIs that are different on Apple and non-Apple targets,
like `ABISysV_arm.cpp` for instance.

Reviewers: clayborg, emaste

Subscribers: aemerson, kristof.beyls, lldb-commits

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

Modified:
lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp

Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
==
--- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
+++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26 
10:04:20 2017
@@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) 
{
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }


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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Stephane Sezer via Phabricator via lldb-commits]

sas requested review of this revision.
sas added a comment.

I saw the `bool is_arm = ` pattern only in `ObjectFileMachO.cpp`. I'm not sure 
about the specifics for Darwin targets, but having an object file with some 
functions in ARM and some functions in Thumb is valid, so I think checking for 
each symbol is the right thing to do.

As for the `GetAddressClass` function, I have another diff that does exactly 
what you are talking about. I'll upload it when this one goes in.


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D39314#908115, @sas wrote:

> In https://reviews.llvm.org/D39314#908065, @davide wrote:
>
> > can you please try adding a test for this one? :)
>
>
> To be fully honest, I'm not 100% sure how I'm supposed to add tests for this. 
> I looked through the directories containing unit tests and didn't find 
> anything specific to DYLD testing.
>
> I'm going to try to sync with @zturner to see if he is still able to run unit 
> tests on Windows. We exclusively debug Windows remotes from a macOS or Linux 
> host, so I don't have any setup to run local windows tests.


I understand. I'm not picking on you, of course, and I appreciate you trying to 
do that.
From what I can tell, lack of testing can cause a lot of problems in the future 
[i.e. I could just remove part of your functions, and all the tests would pass 
anyway].
This is not ideal, from somebody who's trying to get more involved in lldb with 
a LLVM background :)
I think every change committed to the codebase should have a test associated, 
unless it's NFC.
Sorry if this sounds like captain obvious, but in case we can't test something, 
we might consider slowing down and revisiting the testing infrastructure that's 
there and improve it.
There's of course a tension between adding features and reducing technical 
debt, but keep checking fixes for new code without tests associated is a 
slippery road. Happy to discuss this further (and i know @zturner has ideas on 
how to fix this)


https://reviews.llvm.org/D39314



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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Greg Clayton via Phabricator via lldb-commits]

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

ok, just move the ARM detection out of the loop and use a 
THUMB_ADDRESS_BIT_MASK for ARM symbols and this is good to go.


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D39335: Allow SysV-i386 ABI on everything other than Apple targets

[Stephane Sezer via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316673: Allow SysV-i386 ABI on everything other than Apple 
targets (authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D39335

Files:
  lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp


Index: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) 
{
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }


Index: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Testing would include a test that dynamically loads a shared library and sets a 
breakpoint it in. We have these tests, but I am guessing they are disabled on 
windows probably because they might use "dlopen(...)" which is probably not 
available on windows? I know we have attach tests as well. Many tests are 
disabled on windows, but probably shouldn't be. So I would start looking for 
those tests. The code will need to be window-ized with #ifdefs and such, but it 
shouldn't be hard. The shared libraries tests will dynamically open a shared 
library and then verify we can hit a breakpoint in that shared library (a hint 
that the DYLD worked). So these tests don't need to be windows specific.


https://reviews.llvm.org/D39314



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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added a comment.

This change should be unit-testable, no?


https://reviews.llvm.org/D39315



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

Re: [Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Davide Italiano via lldb-commits]

On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits
 wrote:
> Author: sas
> Date: Thu Oct 26 10:04:20 2017
> New Revision: 316673
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
> Log:
> Allow SysV-i386 ABI on everything other than Apple targets
>
> Summary:
> This matches other SysV ABIs that are different on Apple and non-Apple 
> targets,
> like `ABISysV_arm.cpp` for instance.
>
> Reviewers: clayborg, emaste
>
> Subscribers: aemerson, kristof.beyls, lldb-commits
>
> Differential Revision: https://reviews.llvm.org/D39335
>
> Modified:
> lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
>
> Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
> ==
> --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
> +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26 
> 10:04:20 2017
> @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
>  ABISP
>  ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
> &arch) {
>static ABISP g_abi_sp;
> -  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
> -  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
> -if (!g_abi_sp)
> -  g_abi_sp.reset(new ABISysV_i386(process_sp));
> -return g_abi_sp;
> +  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
> +if (arch.GetTriple().getArch() == llvm::Triple::x86) {
> +  if (!g_abi_sp)
> +g_abi_sp.reset(new ABISysV_i386(process_sp));
> +  return g_abi_sp;
> +}
>}
>return ABISP();
>  }
>

This seems to change a fairly fundamental function for lldb-i386.
I think we should have an unit-test for this. Sorry for being
pedantic, I promise I'll stop after this one.

Thanks,

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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Stephane Sezer via Phabricator via lldb-commits]

sas updated this revision to Diff 120447.
sas added a comment.

Address comments by @clayborg.


https://reviews.llvm.org/D39315

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -37,6 +37,7 @@
 #define OPT_HEADER_MAGIC_PE32 0x010b
 #define OPT_HEADER_MAGIC_PE32_PLUS 0x020b
 
+#define THUMB_ADDRESS_BIT_MASK 0xfffeull
 using namespace lldb;
 using namespace lldb_private;
 
@@ -633,6 +634,10 @@
 
 std::string symbol_name;
 
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+bool is_arm = header_arch.GetMachine() == llvm::Triple::arm;
+
 // Read each export table entry
 for (size_t i = 0; i < export_table.number_of_names; ++i) {
   uint32_t name_ordinal =
@@ -648,6 +653,12 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  if (is_arm && function_rva & 1) {
+// For Thumb we need the last bit to be 0 so that the address
+// points to the right beginning of the symbol.
+function_rva &= THUMB_ADDRESS_BIT_MASK;
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -37,6 +37,7 @@
 #define OPT_HEADER_MAGIC_PE32 0x010b
 #define OPT_HEADER_MAGIC_PE32_PLUS 0x020b
 
+#define THUMB_ADDRESS_BIT_MASK 0xfffeull
 using namespace lldb;
 using namespace lldb_private;
 
@@ -633,6 +634,10 @@
 
 std::string symbol_name;
 
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+bool is_arm = header_arch.GetMachine() == llvm::Triple::arm;
+
 // Read each export table entry
 for (size_t i = 0; i < export_table.number_of_names; ++i) {
   uint32_t name_ordinal =
@@ -648,6 +653,12 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  if (is_arm && function_rva & 1) {
+// For Thumb we need the last bit to be 0 so that the address
+// points to the right beginning of the symbol.
+function_rva &= THUMB_ADDRESS_BIT_MASK;
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Thanks for the feedback (addressed below).

btw, where should this module go?

Since it's intended for clang, and clang based tool, developers, I put it in 
`clang/utils`, but Zackery suggested `lldb/examples/python` might be a better 
place.  Please let me know if anyone has a preference.




Comment at: utils/clangdiag.py:66
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:

clayborg wrote:
> Is there only ever one of these? If so this is good. 
Currently, DiagnosticsEngine::Report only has two signatures, and both contain 
an `unsigned DiagID` parameter, so this is just a sanity check to make sure we 
are in the right place.




Comment at: utils/clangdiag.py:117
+disable(exe_ctx)
+else:
+getDiagtool(exe_ctx.GetTarget(), args.path[0])

clayborg wrote:
> should probably verify that this 'diagtool' with:
> 
> ```
> elif args.subcommands == 'diagtool':
> ```
> and add an else that creates an error:
> ```
> else:
> result.SetError("invalid subcommand '%s'" % (args.subcommands))
> ```
This is already handled by `argparser`, which will raise an exception if the 
first parameter isn't one of (enable, disable, daigtool).  That's the benefit 
of using `subparsers`.  So, by the time you get to this point, it must be 
"diagtool".

However, I think I should probably verify the path past actually exists.  Plus, 
I think I could add a better exception to alert the user how to fix the problem 
if diagtool couldn't be found in the callback.  Suggestions welcome.


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D19603: Fix entry point lookup for ObjectFilePECOFF

[Stephane Sezer via Phabricator via lldb-commits]

sas added a comment.

In https://reviews.llvm.org/D19603#908130, @clayborg wrote:

> Does this need some ARM support where we strip bit zero in case the entry 
> point is Thumb?


Good question. Somehow we never had any issue with this but I don't remember 
explicitly checking for difference with thumb entry points either.

That being said, Winphone expects thumb-only for user-space code, so I'd assume 
`m_coff_header_opt.entry` does **not** include the thumb bit, but the kernel 
still jumps to user code in thumb mode.


https://reviews.llvm.org/D19603



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Do I understand correctly that this will insert breakpoints on *all* clang 
diagnostics?  That's not necessarily bad, but I was under the impression 
originally that it would let you pick the diagnostics you wanted to insert 
breakpoints on.

Also, What is the workflow for using the "clangdiag diagtool" subcommand?  
Would you have to do two steps, `clangdiag enable` and then `clangdiag 
diagtool`?  If so, maybe it could just be `clangdiag enable --diagtool=`


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D39337: Implement a simple GetAddressClass for PECOFF

[Stephane Sezer via Phabricator via lldb-commits]

sas created this revision.

This function apparently is called only by addresses that are valid.
We mainly need to use the ISA class whenever is needed.

Depends on https://reviews.llvm.org/D39315.

Change by Walter Erquinigo 


https://reviews.llvm.org/D39337

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -102,8 +102,7 @@
 
   uint32_t GetAddressByteSize() const override;
 
-  //virtual lldb_private::AddressClass
-  //GetAddressClass (lldb::addr_t file_addr);
+  lldb::AddressClass GetAddressClass(lldb::addr_t file_addr) override;
 
   lldb_private::Symtab *GetSymtab() override;
 
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -678,6 +678,23 @@
   return false;
 }
 
+lldb::AddressClass ObjectFilePECOFF::GetAddressClass(lldb::addr_t file_addr) {
+  auto address_class = ObjectFile::GetAddressClass(file_addr);
+  // Some addresses (e.g. from trampolines) are marked as eAddressClassUnknown.
+  if (address_class == eAddressClassCode ||
+  address_class == eAddressClassUnknown) {
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+if (header_arch.GetMachine() == llvm::Triple::arm) {
+  // The only PECOFF/ARM target we support, Windows Phone, requires all
+  // user code to be Thumb, so we can always return
+  // eAddressClassCodeAlternateISA.
+  return eAddressClassCodeAlternateISA;
+}
+  }
+  return address_class;
+}
+
 void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_ap.get()) {
 m_sections_ap.reset(new SectionList());


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -102,8 +102,7 @@
 
   uint32_t GetAddressByteSize() const override;
 
-  //virtual lldb_private::AddressClass
-  //GetAddressClass (lldb::addr_t file_addr);
+  lldb::AddressClass GetAddressClass(lldb::addr_t file_addr) override;
 
   lldb_private::Symtab *GetSymtab() override;
 
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -678,6 +678,23 @@
   return false;
 }
 
+lldb::AddressClass ObjectFilePECOFF::GetAddressClass(lldb::addr_t file_addr) {
+  auto address_class = ObjectFile::GetAddressClass(file_addr);
+  // Some addresses (e.g. from trampolines) are marked as eAddressClassUnknown.
+  if (address_class == eAddressClassCode ||
+  address_class == eAddressClassUnknown) {
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+if (header_arch.GetMachine() == llvm::Triple::arm) {
+  // The only PECOFF/ARM target we support, Windows Phone, requires all
+  // user code to be Thumb, so we can always return
+  // eAddressClassCodeAlternateISA.
+  return eAddressClassCodeAlternateISA;
+}
+  }
+  return address_class;
+}
+
 void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_ap.get()) {
 m_sections_ap.reset(new SectionList());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Stephane Sezer via Phabricator via lldb-commits]

sas added a comment.

@clayborg: the follow-up to this change is in https://reviews.llvm.org/D39337, 
where we implement `GetAddressClass()`.


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added inline comments.



Comment at: utils/clangdiag.py:117
+disable(exe_ctx)
+else:
+getDiagtool(exe_ctx.GetTarget(), args.path[0])

hintonda wrote:
> clayborg wrote:
> > should probably verify that this 'diagtool' with:
> > 
> > ```
> > elif args.subcommands == 'diagtool':
> > ```
> > and add an else that creates an error:
> > ```
> > else:
> > result.SetError("invalid subcommand '%s'" % (args.subcommands))
> > ```
> This is already handled by `argparser`, which will raise an exception if the 
> first parameter isn't one of (enable, disable, daigtool).  That's the benefit 
> of using `subparsers`.  So, by the time you get to this point, it must be 
> "diagtool".
> 
> However, I think I should probably verify the path past actually exists.  
> Plus, I think I could add a better exception to alert the user how to fix the 
> problem if diagtool couldn't be found in the callback.  Suggestions welcome.
nice, I didn't realized that argparser would protect against that. Checking the 
file exists would be a good idea.


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

A test would be something like this:

  // dll.cpp
  BOOL WINAPI DllMain(HINSTANCE h, DWORD reason, void* reserved) {
return TRUE;
  }
  
  int __declspec(dllexport) DllFunc(int n) {
int x = n * n;  
return x;  // set breakpoint here
  }



  // main.cpp
  int __declspec(dllimport) DllFunc(int n);
  
  int main(int argc, char ** argv) {
int x = DllFunc(4);
int y = DllFunc(8);   // set breakpoint here
int z = DllFunc(16);
return x + y + z;
  }

Run `main.exe` in the debugger, when you're stopped at the breakpoint get the 
value of `x` and ensure that it's 4.
**Step out** and get the value of `x` and ensure that it's 8.
**Step in** and ensure the value of `n` is 8, then **run** and ensure the value 
of `x` is 16.
Delete the breakpoint in the dll, then **run** and ensure the program 
terminates with a return value of 56


https://reviews.llvm.org/D39314



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

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Ahh, you might also run the exact same test again but where you've loaded this 
inside of main with `LoadLibrary` instead of relying on early binding by the 
loader.


https://reviews.llvm.org/D39314



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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Stephane Sezer via Phabricator via lldb-commits]

sas added a comment.

Same thing here, I have no idea how to go about testing something specific like 
this. Given that this is Windows Phone, it's even worse than simply Windows 
Desktop because the only way to test is by doing remote debugging. I suppose 
checking in binaries to the tree so we can analyze the symbols and ensure 
correctness is the only way to do it?


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D39315#908246, @sas wrote:

> Same thing here, I have no idea how to go about testing something specific 
> like this. Given that this is Windows Phone, it's even worse than simply 
> Windows Desktop because the only way to test is by doing remote debugging. I 
> suppose checking in binaries to the tree so we can analyze the symbols and 
> ensure correctness is the only way to do it?


@zturner is probably the most expert in this area.


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D39315#908246, @sas wrote:

> Same thing here, I have no idea how to go about testing something specific 
> like this. Given that this is Windows Phone, it's even worse than simply 
> Windows Desktop because the only way to test is by doing remote debugging. I 
> suppose checking in binaries to the tree so we can analyze the symbols and 
> ensure correctness is the only way to do it?


Do you guys not have access to Windows Desktops?  It sounds like we need to get 
remote debugging of Windows targets working.  I don't think I'm gonna be the 
one to do that in the near future, but if it's going to be a frequent thing 
that you're improving windows support for remote targets like Windows Phone, I 
imagine you're going to want the code to be tested, so hopefully you can 
convince someone to give you cycles to make remote debugging work properly, 
otherwise you're shipping untested code :-/


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Stephane Sezer via Phabricator via lldb-commits]

sas added a comment.

In https://reviews.llvm.org/D39315#908251, @zturner wrote:

> In https://reviews.llvm.org/D39315#908246, @sas wrote:
>
> > Same thing here, I have no idea how to go about testing something specific 
> > like this. Given that this is Windows Phone, it's even worse than simply 
> > Windows Desktop because the only way to test is by doing remote debugging. 
> > I suppose checking in binaries to the tree so we can analyze the symbols 
> > and ensure correctness is the only way to do it?
>
>
> Do you guys not have access to Windows Desktops?  It sounds like we need to 
> get remote debugging of Windows targets working.  I don't think I'm gonna be 
> the one to do that in the near future, but if it's going to be a frequent 
> thing that you're improving windows support for remote targets like Windows 
> Phone, I imagine you're going to want the code to be tested, so hopefully you 
> can convince someone to give you cycles to make remote debugging work 
> properly, otherwise you're shipping untested code :-/


Well as far as I can tell, remote debugging for Windows **does** work, assuming 
you use ds2 on the remote, and that you have a few of our local patches (which 
I'm trying to send upstream currently). Remote Windows debugging is the 
**only** way we debug on Windows for now and the testing we have is just a 
combination of people using our tools internally as well as some end-to-end 
tests we run internally with full debug session setup + breakpoints + execution 
control, etc, with a remote session.

Given that this works for us, I'm gonna have a hard time justifying assigning 
people to bringing up windows remote debugging in lldb without ds2, and that 
leaves me in a state where testing stuff **in** lldb is hard/impossible for now.


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

[Gustavo Serra Scalet via Phabricator via lldb-commits]

gut added a comment.

In https://reviews.llvm.org/D38897#903581, @clayborg wrote:

> Looks fine. Thanks for doing the changes.


Hi, it's been already 4 days since this patch was accepted but not merged. Did 
something happen internally or we just need to wait a little longer?


https://reviews.llvm.org/D38897



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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

So when you're using ds2 as the remote, are you still using the normal lldb 
test suite?  `dotest.py`?  If so, then we could have a test decorator that says 
`@expectedFailure(not(debugserver=ds2))` or something.  Then, even though 
you're the only one that can run it, at least YOU are sure it works.  Some 
coverage is better than nothing.  Is something like this possible?


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120459.
hintonda added a comment.

- Enhance diagtool option to check, reset, print out current value.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,137 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for operating on clang diagnostic breakpoints
+
+Syntax: clangdiag enable
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = None
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if getDiagtool.diagtool is None:
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = exe_ctx.GetTarget()
+# Remove all diag breakpoints.
+bkpts = lldb.SBBreakpointList(target)
+target.FindBreakpointsByName("clang::Diagnostic", bkpts)
+for i in range(bkpts.GetSize()):
+target.BreakpointDelete(bkpts.GetBreakpointAtIndex(i).GetID())
+
+return
+
+def the_diag_command(debugger, command, exe_ctx, result, dict):
+# Use the Shell Lexer to properly parse up command options just like a
+# shell would
+command_args = shlex.split(command)
+parser = create_diag_options()
+try:
+args = parser.parse_args(command_args)
+except:
+return
+
+if args.subcommands == 'enable':
+enable(exe_ctx, args)
+elif args.subcommands == 'disable':
+disable(exe_ctx)
+else:
+print(args)
+diagtool = getDiagtool(exe_ctx.GetTarget(), args.path)
+print('diagtool = %s' % diagtool)
+
+return
+
+def __lldb_init_module(debugger, dict):
+# This initializer is being run from LLDB in the embedded command interpreter
+# Make the options so we can generate the help text for the new LLDB
+# command line command prior to registering it with LLDB below
+parser = create_diag_options()
+the_diag_command.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLD

Re: [Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

[Zachary Turner via lldb-commits]

Nothing happened internally.  Usually people submit their own patches after
they're accepted.  If neither of you have commit access though, then you'll
need to let us know so that we can submit on your behalf.

On Thu, Oct 26, 2017 at 11:30 AM Gustavo Serra Scalet via Phabricator <
revi...@reviews.llvm.org> wrote:

> gut added a comment.
>
> In https://reviews.llvm.org/D38897#903581, @clayborg wrote:
>
> > Looks fine. Thanks for doing the changes.
>
>
> Hi, it's been already 4 days since this patch was accepted but not merged.
> Did something happen internally or we just need to wait a little longer?
>
>
> https://reviews.llvm.org/D38897
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

[Gustavo Serra Scalet via lldb-commits]

Hi,

None of us have commit access. Please submit for us.

Thanks once again

> -Original Message-
> From: Zachary Turner [mailto:ztur...@google.com]
> Sent: quinta-feira, 26 de outubro de 2017 16:34
> To: reviews+d38897+public+f7e86848ad3ba...@reviews.llvm.org
> Cc: Ana Julia Pereira Caetano ;
> clayb...@gmail.com; Leonardo Bianconi
> ; Gustavo Serra Scalet
> ; nemanja.i@gmail.com; Alexandre
> Yukio Yamashita ; lldb-
> comm...@lists.llvm.org
> Subject: Re: [PATCH] D38897: Add specific ppc64le hardware watchpoint
> handler
> 
> Nothing happened internally.  Usually people submit their own patches
> after they're accepted.  If neither of you have commit access though,
> then you'll need to let us know so that we can submit on your behalf.
> 
> On Thu, Oct 26, 2017 at 11:30 AM Gustavo Serra Scalet via Phabricator
> mailto:revi...@reviews.llvm.org> > wrote:
> 
> 
>   gut added a comment.
> 
>   In https://reviews.llvm.org/D38897#903581, @clayborg wrote:
> 
>   > Looks fine. Thanks for doing the changes.
> 
> 
>   Hi, it's been already 4 days since this patch was accepted but not
> merged. Did something happen internally or we just need to wait a little
> longer?
> 
> 
>   https://reviews.llvm.org/D38897
> 
> 
> 
> 

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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The test suite can be run remotely if ds2 supports the "lldb-server platform" 
packets. I'll be happy to help you get this going Stephane. Ping me internally 
if interested.


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks good.


https://reviews.llvm.org/D36347



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

In https://reviews.llvm.org/D36347#908186, @zturner wrote:

> Do I understand correctly that this will insert breakpoints on *all* clang 
> diagnostics?  That's not necessarily bad, but I was under the impression 
> originally that it would let you pick the diagnostics you wanted to insert 
> breakpoints on.


`clangdiag enable` sets a breakpoint in `DiagnosticsEngine::Report` and adds a 
callback.  The callback passed the value of DiagID to diagtool to get the enum 
spelling of the DiagID, e.g., `err_undeclared_var_use`, then calls 
`BreakpointCreateBySourceRegex` with that string and and empty `FileSpec` to 
set breakpoints in every location that string is seen -- specifically, we use 
"diag::err_undeclared_var_use` for this case.  Now, that might add a few more 
breakpoints than actually needed, but not many, and only rarely in places that 
were actually covered by this particular run.

Also, since we add the breakpoint after it was seen -- 
`DiagnosticsEngine::Report` is called later, and sometimes much later, you'll 
need to run the debugger again to actually hit all the breakpoints.

> Also, What is the workflow for using the "clangdiag diagtool" subcommand?  
> Would you have to do two steps, `clangdiag enable` and then `clangdiag 
> diagtool`?  If so, maybe it could just be `clangdiag enable --diagtool=`

I put `command script import 
/Users/dhinton/projects/llvm_project/llvm/tools/clang/utils/clangdiag.py` in my 
`~/.lldbinit` file, so here's an example of how I use it:

  $ lldb bin/clang-6.0
  The "clangdiag" command has been installed, type "help clangdiag" or 
"clangdiag --help" for detailed help.
  (lldb) target create "bin/clang-6.0"
  Current executable set to 'bin/clang-6.0' (x86_64).
  
  (lldb) clangdiag diagtool
  diagtool = /Users/dhinton/projects/llvm_project/build/Debug/bin/diagtool
  
  (lldb) clangdiag diagtool /bad/path/xx
  clangdiag: /bad/path/xx not found.
  diagtool = /Users/dhinton/projects/llvm_project/build/Debug/bin/diagtool
  
  (lldb) clangdiag diagtool 
/Users/dhinton/projects/monorepo/build/Debug/bin/diagtool
  diagtool = /Users/dhinton/projects/monorepo/build/Debug/bin/diagtool
  
  (lldb) clangdiag diagtool reset
  diagtool = /Users/dhinton/projects/llvm_project/build/Debug/bin/diagtool
  
  (lldb) clangdiag enable
  (lldb) br l
  Current breakpoints:
  1: name = 'DiagnosticsEngine::Report', locations = 33
  Breakpoint commands (Python):
return clangdiag.setDiagBreakpoint(frame, bp_loc, internal_dict)
  
Names:
  clang::Diagnostic
  
1.1: where = clang-6.0`clang::DiagnosticsEngine::Report(unsigned int) + 38 
at Diagnostic.h:1215, address = clang-6.0[0x000100022cd6], unresolved, hit 
count = 0
1.2: where = 
clang-6.0`clang::DiagnosticsEngine::Report(clang::SourceLocation, unsigned int) 
+ 46 at Diagnostic.h:1207, address = clang-6.0[0x00010002c6ce], unresolved, 
hit count = 0
  <...snip...>
  
  (lldb) clangdiag disable
  (lldb) br l
  No breakpoints currently set.
  
  (lldb) clangdiag enable
  (lldb) br l
  Current breakpoints:
  2: name = 'DiagnosticsEngine::Report', locations = 33
  Breakpoint commands (Python):
return clangdiag.setDiagBreakpoint(frame, bp_loc, internal_dict)
  
Names:
  clang::Diagnostic
  
2.1: where = clang-6.0`clang::DiagnosticsEngine::Report(unsigned int) + 38 
at Diagnostic.h:1215, address = clang-6.0[0x000100022cd6], unresolved, hit 
count = 0
2.2: where = 
clang-6.0`clang::DiagnosticsEngine::Report(clang::SourceLocation, unsigned int) 
+ 46 at Diagnostic.h:1207, address = clang-6.0[0x00010002c6ce], unresolved, 
hit count = 0
  <...snip...>
  
  (lldb) run <...>
   might hit one of the new breakpoints if they are seen more than once
  (lldb) run 
   should hit all the breakpoints for which diagnostics were produced
  
  (lldb) br l
  Current breakpoints:
  2: name = 'DiagnosticsEngine::Report', locations = 33, resolved = 33, hit 
count = 5
  Breakpoint commands (Python):
return clangdiag.setDiagBreakpoint(frame, bp_loc, internal_dict)
  
Names:
  clang::Diagnostic
2.1: where = clang-6.0`clang::DiagnosticsEngine::Report(unsigned int) + 38 
at Diagnostic.h:1215, address = 0x000100022cd6, resolved, hit count = 0 

   
2.2: where = 
clang-6.0`clang::DiagnosticsEngine::Report(clang::SourceLocation, unsigned int) 
+ 46 at Diagnostic.h:1207, address = 0x00010002c6ce, resolved, hit count = 0
  <...snip...>
  
  3: source regex = "err_unknown_typename", exact_match = 0, locations = 6, 
resolved = 6, hit count = 2 

  Names:

   

[Lldb-commits] [lldb] r316688 - Fix TestMinidump for r316673

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Oct 26 12:08:34 2017
New Revision: 316688

URL: http://llvm.org/viewvc/llvm-project?rev=316688&view=rev
Log:
Fix TestMinidump for r316673

The test was asserting that we can only find one frame in the minidump.
Now that we have the default unwind plan from the ABI plugin, we are
able to find 5 more frames using the frame pointer chaining. Correct the
expectation in the test.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py?rev=316688&r1=316687&r2=316688&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump/TestMiniDump.py
 Thu Oct 26 12:08:34 2017
@@ -49,14 +49,14 @@ class MiniDumpTestCase(TestBase):
 self.process = self.target.LoadCore("fizzbuzz_no_heap.dmp")
 self.assertEqual(self.process.GetNumThreads(), 1)
 thread = self.process.GetThreadAtIndex(0)
-# The crash is in main, so there should be one frame on the stack.
-self.assertEqual(thread.GetNumFrames(), 1)
-frame = thread.GetFrameAtIndex(0)
-self.assertTrue(frame.IsValid())
-pc = frame.GetPC()
-eip = frame.FindRegister("pc")
-self.assertTrue(eip.IsValid())
-self.assertEqual(pc, eip.GetValueAsUnsigned())
+
+pc_list = [ 0x00164d14, 0x00167c79, 0x00167e6d, 0x7510336a, 
0x77759882, 0x77759855]
+
+self.assertEqual(thread.GetNumFrames(), len(pc_list))
+for i in range(len(pc_list)):
+frame = thread.GetFrameAtIndex(i)
+self.assertTrue(frame.IsValid())
+self.assertEqual(frame.GetPC(), pc_list[i])
 
 @skipUnlessWindows # Minidump saving works only on windows
 def test_deeper_stack_in_mini_dump(self):


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

Re: [Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Stephane Sezer via lldb-commits]

This changes nothing except for 32bit Windows, which used to not be able to
benefit from ABISysV_i386 but can use it now. The ABI support is tested
independently from Windows itself.

On Thu, Oct 26, 2017 at 10:25 AM Davide Italiano 
wrote:

> On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits
>  wrote:
> > Author: sas
> > Date: Thu Oct 26 10:04:20 2017
> > New Revision: 316673
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
> > Log:
> > Allow SysV-i386 ABI on everything other than Apple targets
> >
> > Summary:
> > This matches other SysV ABIs that are different on Apple and non-Apple
> targets,
> > like `ABISysV_arm.cpp` for instance.
> >
> > Reviewers: clayborg, emaste
> >
> > Subscribers: aemerson, kristof.beyls, lldb-commits
> >
> > Differential Revision: https://reviews.llvm.org/D39335
> >
> > Modified:
> > lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
> >
> > Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
> >
> ==
> > --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
> > +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26
> 10:04:20 2017
> > @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
> >  ABISP
> >  ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec
> &arch) {
> >static ABISP g_abi_sp;
> > -  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
> > -  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD()))
> {
> > -if (!g_abi_sp)
> > -  g_abi_sp.reset(new ABISysV_i386(process_sp));
> > -return g_abi_sp;
> > +  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
> > +if (arch.GetTriple().getArch() == llvm::Triple::x86) {
> > +  if (!g_abi_sp)
> > +g_abi_sp.reset(new ABISysV_i386(process_sp));
> > +  return g_abi_sp;
> > +}
> >}
> >return ABISP();
> >  }
> >
>
> This seems to change a fairly fundamental function for lldb-i386.
> I think we should have an unit-test for this. Sorry for being
> pedantic, I promise I'll stop after this one.
>
> Thanks,
>
> --
> Davide
>
-- 
-- 
Stephane Sezer
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Davide Italiano via lldb-commits]

On Thu, Oct 26, 2017 at 12:39 PM, Stephane Sezer  wrote:
> This changes nothing except for 32bit Windows, which used to not be able to
> benefit from ABISysV_i386 but can use it now. The ABI support is tested
> independently from Windows itself.
>

Sure, makes sense. I guess we should still be able to unittest this
change as we do for the stuff in lib/Support?

Thanks!

--
Davide

> On Thu, Oct 26, 2017 at 10:25 AM Davide Italiano 
> wrote:
>>
>> On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits
>>  wrote:
>> > Author: sas
>> > Date: Thu Oct 26 10:04:20 2017
>> > New Revision: 316673
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
>> > Log:
>> > Allow SysV-i386 ABI on everything other than Apple targets
>> >
>> > Summary:
>> > This matches other SysV ABIs that are different on Apple and non-Apple
>> > targets,
>> > like `ABISysV_arm.cpp` for instance.
>> >
>> > Reviewers: clayborg, emaste
>> >
>> > Subscribers: aemerson, kristof.beyls, lldb-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D39335
>> >
>> > Modified:
>> > lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
>> >
>> > Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
>> >
>> > ==
>> > --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
>> > +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26
>> > 10:04:20 2017
>> > @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
>> >  ABISP
>> >  ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec
>> > &arch) {
>> >static ABISP g_abi_sp;
>> > -  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
>> > -  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD()))
>> > {
>> > -if (!g_abi_sp)
>> > -  g_abi_sp.reset(new ABISysV_i386(process_sp));
>> > -return g_abi_sp;
>> > +  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
>> > +if (arch.GetTriple().getArch() == llvm::Triple::x86) {
>> > +  if (!g_abi_sp)
>> > +g_abi_sp.reset(new ABISysV_i386(process_sp));
>> > +  return g_abi_sp;
>> > +}
>> >}
>> >return ABISP();
>> >  }
>> >
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Stephane Sezer via Phabricator via lldb-commits]

sas added a subscriber: fjricci.
sas added a comment.

In https://reviews.llvm.org/D39315#908284, @zturner wrote:

> So when you're using ds2 as the remote, are you still using the normal lldb 
> test suite?  `dotest.py`?  If so, then we could have a test decorator that 
> says `@expectedFailure(not(debugserver=ds2))` or something.  Then, even 
> though you're the only one that can run it, at least YOU are sure it works.  
> Some coverage is better than nothing.  Is something like this possible?


So there are two ways we test our stuff.

First part is centered around ds2 testing, and you can see it here: 
https://travis-ci.org/facebook/ds2/builds/292216534. We build ds2 for a bunch 
of different targets, and for some of them, we run `dotest.py` with 
`LLDB_DEBUGSERVER_PATH=/path/to/ds2`. This gives us a lot of coverage for 
non-Windows targets.

For Windows targets, we can only build ds2 (see 
https://ci.appveyor.com/project/a20012251/ds2/branch/master) and we are unable 
to run tests because we need a binary of lldb for Windows, but the builds 
available on http://releases.llvm.org/ were historically broken, and when 
@fjricci tried to fix them it didn't lead anywhere IIRC.

The second way we have to test our debugging tools is to simply run our 
debugging scripts with a known target, internally. That doesn't use 
`dotest.py`. It simply launches lldb and ds2, tries to set breakpoints, control 
the execution of the process, etc. The changes I am currently sending upstream 
work with that type of internal testing, but that's of no use for you because 
you can't run it yourself, nor can you see the results of our runs. :/


https://reviews.llvm.org/D39315



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

[Lldb-commits] [PATCH] D35556: Add data formatter for libc++'s forward_list

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp:5
+{
+  std::forward_list empty{}, one_elt{47}, five_elts{1,22,333,,5};
+  return 0; // break here

jingham wrote:
> Some compilers (including clang at other times) will omit debug info for 
> variables that it doesn't see getting used.  I usually try to use the 
> variables I'm going to print (call size on them, pass an element to printf, 
> whatever...) to keep this from happening.
> 
> OTOH, it's nice if compilers don't do that, so maybe relying on their not 
> doing that in our tests is a useful forcing function???
Hmm... I'd say we can leave it like this. I'd be surprised if the compiler can 
figure out that these declarations are a no-op without very aggressive inlining 
(which it just won't do at -O0).



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:141
 
-namespace lldb_private {
-namespace formatters {
-class LibcxxStdListSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+class ForwardListFrontEnd: public AbstractListFrontEnd {
+public:

jingham wrote:
> I wonder about the decision to move these classes out of 
> lldb_private::formatters.  They don't need to be in a globally visible 
> namespace, but all the others are.  They also all have a consistent naming 
> convention, which this one doesn't have.  This doesn't seem like the sort of 
> thing we should change piecemeal, that will just lead to confusion.
> 
> Was there some other reason for doing this that I'm missing?
There's no hard reason, but I was trying to make these names more concise

I've put them in the anonymous namespace, as llvm coding style encourages that 
for file-local entities 
. I'd be happy 
to move the other formatters into the anonymous namespace as well, as I have a 
couple of other formatters in the pipeline, which already are in the anon 
namespace.

As for the names, if I followed the existing style, this would read something 
like
```
class LibCxxStdForwardListSyntheticFrontEnd: public 
LibCxxAbstractListSyntheticFrontEnd
```
which would be formatted horribly, as it does not fit a single line. As these 
are file-local entities, I think we can be a bit less verbose.

I had not renamed the std::list class because of the way I wrote this change 
(make a copy of the std::list formatter, and then factor out the common code), 
but you are right that they should follow the same convention. I propose to 
shorten the name of the other class as well.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:153
+  std::map m_iterators;
+};
+

jingham wrote:
> Why are these three ivars not in the AbstractListFrontEnd? They appear in 
> both derived classes and seem to be used in the same way.
Good question. I started with two completely distinct implementations and then 
worked towards merging them. I guess did not finish the job.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:182
+  m_head = nullptr;
+  return false;
 }

jingham wrote:
> The AbstractFrontEndList has the m_fast_runner and m_slow_runner but they are 
> only reset in the Update for the LibcxxStdListSyntheticFrontEnd, and not 
> here.  Is that on purpose?
It doesn't matter since they are re-initialized in HasLoop(), but it does look 
weird. Fixed it.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:251-260
+  ListIterator current(m_head);
+  if (idx > 0) {
+auto cached_iterator = m_iterators.find(idx - 1);
+if (cached_iterator != m_iterators.end()) {
+  current = cached_iterator->second;
+  actual_advance = 1;
+}

jingham wrote:
> This use of the iterators also seems like it should be shared.
Indeed it can.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:299-301
+  /*m_node_address = backend_addr->GetValueAsUnsigned(0);
+  if (!m_node_address || m_node_address == LLDB_INVALID_ADDRESS)
+return false;*/

jingham wrote:
> ?
Oops.


https://reviews.llvm.org/D35556



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

[Lldb-commits] [PATCH] D35556: Add data formatter for libc++'s forward_list

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 120491.
labath marked 4 inline comments as done.
labath added a comment.

Address review comments.


https://reviews.llvm.org/D35556

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/main.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxList.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxList.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxList.cpp
@@ -114,58 +114,91 @@
   ListEntry m_entry;
 };
 
-} // end anonymous namespace
-
-namespace lldb_private {
-namespace formatters {
-class LibcxxStdListSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+class AbstractListFrontEnd : public SyntheticChildrenFrontEnd {
 public:
-  LibcxxStdListSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return ExtractIndexFromString(name.GetCString());
+  }
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
 
-  ~LibcxxStdListSyntheticFrontEnd() override = default;
+protected:
+  AbstractListFrontEnd(ValueObject &valobj)
+  : SyntheticChildrenFrontEnd(valobj) {}
 
-  size_t CalculateNumChildren() override;
+  size_t m_count;
+  ValueObject *m_head;
 
-  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override;
+  static constexpr bool g_use_loop_detect = true;
+  size_t m_loop_detected; // The number of elements that have had loop detection
+  // run over them.
+  ListEntry m_slow_runner; // Used for loop detection
+  ListEntry m_fast_runner; // Used for loop detection
+
+  size_t m_list_capping_size;
+  CompilerType m_element_type;
+  std::map m_iterators;
+
+  bool HasLoop(size_t count);
+  ValueObjectSP GetItem(size_t idx);
+};
 
+class ForwardListFrontEnd : public AbstractListFrontEnd {
+public:
+  ForwardListFrontEnd(ValueObject &valobj);
+
+  size_t CalculateNumChildren() override;
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
   bool Update() override;
+};
 
-  bool MightHaveChildren() override;
+class ListFrontEnd : public AbstractListFrontEnd {
+public:
+  ListFrontEnd(lldb::ValueObjectSP valobj_sp);
 
-  size_t GetIndexOfChildWithName(const ConstString &name) override;
+  ~ListFrontEnd() override = default;
 
-private:
-  bool HasLoop(size_t count);
+  size_t CalculateNumChildren() override;
 
-  size_t m_list_capping_size;
-  static const bool g_use_loop_detect = true;
+  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override;
 
-  size_t m_loop_detected; // The number of elements that have had loop detection
-  // run over them.
-  ListEntry m_slow_runner; // Used for loop detection
-  ListEntry m_fast_runner; // Used for loop detection
+  bool Update() override;
 
+private:
   lldb::addr_t m_node_address;
-  ValueObject *m_head;
   ValueObject *m_tail;
-  CompilerType m_element_type;
-  size_t m_count;
-  std::map m_iterators;
 };
-} // namespace formatters
-} // namespace lldb_private
-
-lldb_private::formatters::LibcxxStdListSyntheticFrontEnd::
-LibcxxStdListSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-: SyntheticChildrenFrontEnd(*valobj_sp), m_list_capping_size(0),
-  m_loop_detected(0), m_node_address(), m_head(nullptr), m_tail(nullptr),
-  m_element_type(), m_count(UINT32_MAX), m_iterators() {
-  if (valobj_sp)
-Update();
+
+} // end anonymous namespace
+
+bool AbstractListFrontEnd::Update() {
+  m_loop_detected = 0;
+  m_count = UINT32_MAX;
+  m_head = nullptr;
+  m_list_capping_size = 0;
+  m_slow_runner.SetEntry(nullptr);
+  m_fast_runner.SetEntry(nullptr);
+  m_iterators.clear();
+
+  if (m_backend.GetTargetSP())
+m_list_capping_size =
+m_backend.GetTargetSP()->GetMaximumNumberOfChildrenToDisplay();
+  if (m_list_capping_size == 0)
+m_list_capping_size = 255;
+
+  CompilerType list_type = m_backend.GetCompilerType();
+  if (list_type.IsReferenceType())
+list_type = list_type.GetNonReferenceType();
+
+  if (list_type.GetNumTemplateArguments() == 0)
+return false;
+  TemplateArgumentKind kind;
+  m_element_type = list_type.GetTemplateArgument(0, kind);
+
+  return false;
 }
 
-bool lldb_private::formatters::LibcxxStdListSyntheticFrontEnd::HasLoop(
-size_t count) {
+bool AbstractListFrontEnd::HasLoop(size_t count) {
   if (!g_use_loop_detect)
 return false;
   // don't bother checking for a loop if we won't actually need to jump nodes
@@ -201,8 +234,96 @@
   return m_slow_runner == m_fast_runner;
 }
 
-size_t lld

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120492.
hintonda added a comment.

- Remove debugging print statement, and enhance help message.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,161 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Make it always available by adding this to your ~.lldbinit file:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = None
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if getDiagtool.diagtool is None:
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = exe_ctx.GetTarget()
+# Remove all diag breakpoints.
+bkpts = lldb.SBBreakpointList(target)
+target.Fi

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda added a comment.

Is there a way to associate a particular diagtool variable to an exe_ctx?


https://reviews.llvm.org/D36347



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

Re: [Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Zachary Turner via lldb-commits]

Seems fine, it would be nice if the workflow could be improved a little bit
so that all you have to do is say `clangdiag break
—error=“-Wcovered-switch”` or something . I think that gives the most
intuitive usage for people, even it’s a bit harder to implement.

I also think user shouldn’t really have to concern themselves with
diagtool, it should all just be magic. I get why it’s easier to do this
way, but from the users perspective, having the commands map as closely as
possible to the thing the person wants to do and hiding implementation
details is a big win from a usability standpoint.

We can iterate on it later though
On Thu, Oct 26, 2017 at 2:38 PM Don Hinton via Phabricator <
revi...@reviews.llvm.org> wrote:

> hintonda updated this revision to Diff 120492.
> hintonda added a comment.
>
> - Remove debugging print statement, and enhance help message.
>
>
> https://reviews.llvm.org/D36347
>
> Files:
>   utils/clangdiag.py
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Each lldb.SBValue has accessors for the stuff in an execution context:

``

  lldb::SBTarget GetTarget();
  lldb::SBProcess GetProcess();
  lldb::SBThread GetThread();
  lldb::SBFrame GetFrame();

  You could keep a global map of process ID to diagtool if you want?
  
  What are you thinking of using this for?


https://reviews.llvm.org/D36347



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

Re: [Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Jason Molenda via lldb-commits]

> On Oct 26, 2017, at 10:24 AM, Davide Italiano via lldb-commits 
>  wrote:
> 
> On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits
>  wrote:
>> Author: sas
>> Date: Thu Oct 26 10:04:20 2017
>> New Revision: 316673
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
>> Log:
>> Allow SysV-i386 ABI on everything other than Apple targets
>> 
>> Summary:
>> This matches other SysV ABIs that are different on Apple and non-Apple 
>> targets,
>> like `ABISysV_arm.cpp` for instance.
>> 
>> Reviewers: clayborg, emaste
>> 
>> Subscribers: aemerson, kristof.beyls, lldb-commits
>> 
>> Differential Revision: https://reviews.llvm.org/D39335
>> 
>> Modified:
>>lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
>> 
>> Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
>> ==
>> --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
>> +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26 
>> 10:04:20 2017
>> @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
>> ABISP
>> ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
>> &arch) {
>>   static ABISP g_abi_sp;
>> -  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
>> -  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
>> -if (!g_abi_sp)
>> -  g_abi_sp.reset(new ABISysV_i386(process_sp));
>> -return g_abi_sp;
>> +  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
>> +if (arch.GetTriple().getArch() == llvm::Triple::x86) {
>> +  if (!g_abi_sp)
>> +g_abi_sp.reset(new ABISysV_i386(process_sp));
>> +  return g_abi_sp;
>> +}
>>   }
>>   return ABISP();
>> }
>> 
> 
> This seems to change a fairly fundamental function for lldb-i386.
> I think we should have an unit-test for this. Sorry for being
> pedantic, I promise I'll stop after this one.


It's a good suggestion, and not something we test today.  Right now there are 
two i386 ABIs that lldb supports:  Darwin and SysV.  This patch implements that 
correctly -- but the obvious problem is if a third i386 ABI is added in the 
future.  Now it's a race to see whether SysV-i386 or CrazyOtherABI-i386 gets 
CreateInstance'd, depending on the order they're registered or something.  And 
I'm not sure how you write a test today that would test a new target that uses 
CrazyOtherABI-i386 is getting the correct plugin activated.



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

Re: [Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via lldb-commits]

On Thu, Oct 26, 2017 at 3:00 PM, Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg added a comment.
>
> Each lldb.SBValue has accessors for the stuff in an execution context:
>
> ``
>
>   lldb::SBTarget GetTarget();
>   lldb::SBProcess GetProcess();
>   lldb::SBThread GetThread();
>   lldb::SBFrame GetFrame();
>
>   You could keep a global map of process ID to diagtool if you want?
>
>   What are you thinking of using this for?
>

Part of the rational for using exe_ctx was to allow debugging multiple
targets as the same time, but if these use different versions of clang, the
diagtool map won't match.

I'll try moving it from the function __dict__ to the exe_ctx and see if
that works.

thanks...


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

Re: [Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Davide Italiano via lldb-commits]

On Thu, Oct 26, 2017 at 3:04 PM, Jason Molenda  wrote:
>
>
>> On Oct 26, 2017, at 10:24 AM, Davide Italiano via lldb-commits 
>>  wrote:
>>
>> On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits
>>  wrote:
>>> Author: sas
>>> Date: Thu Oct 26 10:04:20 2017
>>> New Revision: 316673
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
>>> Log:
>>> Allow SysV-i386 ABI on everything other than Apple targets
>>>
>>> Summary:
>>> This matches other SysV ABIs that are different on Apple and non-Apple 
>>> targets,
>>> like `ABISysV_arm.cpp` for instance.
>>>
>>> Reviewers: clayborg, emaste
>>>
>>> Subscribers: aemerson, kristof.beyls, lldb-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D39335
>>>
>>> Modified:
>>>lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
>>>
>>> Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
>>> ==
>>> --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
>>> +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26 
>>> 10:04:20 2017
>>> @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
>>> ABISP
>>> ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
>>> &arch) {
>>>   static ABISP g_abi_sp;
>>> -  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
>>> -  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
>>> -if (!g_abi_sp)
>>> -  g_abi_sp.reset(new ABISysV_i386(process_sp));
>>> -return g_abi_sp;
>>> +  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
>>> +if (arch.GetTriple().getArch() == llvm::Triple::x86) {
>>> +  if (!g_abi_sp)
>>> +g_abi_sp.reset(new ABISysV_i386(process_sp));
>>> +  return g_abi_sp;
>>> +}
>>>   }
>>>   return ABISP();
>>> }
>>>
>>
>> This seems to change a fairly fundamental function for lldb-i386.
>> I think we should have an unit-test for this. Sorry for being
>> pedantic, I promise I'll stop after this one.
>
>
> It's a good suggestion, and not something we test today.  Right now there are 
> two i386 ABIs that lldb supports:  Darwin and SysV.  This patch implements 
> that correctly -- but the obvious problem is if a third i386 ABI is added in 
> the future.  Now it's a race to see whether SysV-i386 or CrazyOtherABI-i386 
> gets CreateInstance'd, depending on the order they're registered or 
> something.  And I'm not sure how you write a test today that would test a new 
> target that uses CrazyOtherABI-i386 is getting the correct plugin activated.
>
>

Thanks for your reply, Jason. I'm not sure how to test this either,
but I'll take a look.
In theory, (or at lesat what I have in mind :)) you should be able to
have a unit test that just allocates an object and calls
createInstance() directly [if possible], then checks that the result
is of the right type? [ABISysV_i386 vs ABIDarwin_i386 or something
like that?]
That won't of course take care of the race, but the test will break in
case somebody deletes code from the function (and/or allocates an
object with the wrong ABI).
I think it's not testing this feature entirely (and I think testing
the lack of races might be hard, but at least should give us some
coverage [if nothing, to discriminate dead code VS non-dead code].
To be fair, I haven't looked into how hard this is to get working, but
I might. CC:ing Zachary, maybe he has some ideas.

Thanks!

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

Re: [Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via lldb-commits]

On Thu, Oct 26, 2017 at 2:48 PM, Zachary Turner  wrote:

> Seems fine, it would be nice if the workflow could be improved a little
> bit so that all you have to do is say `clangdiag break
> —error=“-Wcovered-switch”` or something . I think that gives the most
> intuitive usage for people, even it’s a bit harder to implement.
>

The idea was to break on actual diagnostics emitted, but if you want to
break on diagnostic usage, i.e., when it was checked but not emitted, I
suppose that's possible as well.  diagtool doesn't produce a mapping for
this, but it could be added -- assuming tablegen produced enough info in
the .inc files to support it.  I added the feature I'm using here a few
months ago, which was an extension to what Alex added earlier.


>
> I also think user shouldn’t really have to concern themselves with
> diagtool, it should all just be magic. I get why it’s easier to do this
> way, but from the users perspective, having the commands map as closely as
> possible to the thing the person wants to do and hiding implementation
> details is a big win from a usability standpoint.
>

For the normal use case, i.e., clang/llvm developers that build both
together, it will just work by magic, i.e., you just run enable/disable.
The only problem is when you build out-of-tree.  If you can suggest a way
to find the correct location by examining the executable, I'd be happy to
add it.


>
> We can iterate on it later though
>

I'm happy to keep hacking on it -- got plenty of time on my hands right
now...  And I get to learn more about lldb...


> On Thu, Oct 26, 2017 at 2:38 PM Don Hinton via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> hintonda updated this revision to Diff 120492.
>> hintonda added a comment.
>>
>> - Remove debugging print statement, and enhance help message.
>>
>>
>> https://reviews.llvm.org/D36347
>>
>> Files:
>>   utils/clangdiag.py
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Jason Molenda via lldb-commits]

> On Oct 26, 2017, at 3:12 PM, Davide Italiano  wrote:
> 
> On Thu, Oct 26, 2017 at 3:04 PM, Jason Molenda  wrote:
>> 
>> 
>>> On Oct 26, 2017, at 10:24 AM, Davide Italiano via lldb-commits 
>>>  wrote:
>>> 
>>> On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits
>>>  wrote:
 Author: sas
 Date: Thu Oct 26 10:04:20 2017
 New Revision: 316673
 
 URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
 Log:
 Allow SysV-i386 ABI on everything other than Apple targets
 
 Summary:
 This matches other SysV ABIs that are different on Apple and non-Apple 
 targets,
 like `ABISysV_arm.cpp` for instance.
 
 Reviewers: clayborg, emaste
 
 Subscribers: aemerson, kristof.beyls, lldb-commits
 
 Differential Revision: https://reviews.llvm.org/D39335
 
 Modified:
   lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
 
 Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
 ==
 --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
 +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26 
 10:04:20 2017
 @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
 &arch) {
  static ABISP g_abi_sp;
 -  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
 -  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
 -if (!g_abi_sp)
 -  g_abi_sp.reset(new ABISysV_i386(process_sp));
 -return g_abi_sp;
 +  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
 +if (arch.GetTriple().getArch() == llvm::Triple::x86) {
 +  if (!g_abi_sp)
 +g_abi_sp.reset(new ABISysV_i386(process_sp));
 +  return g_abi_sp;
 +}
  }
  return ABISP();
 }
 
>>> 
>>> This seems to change a fairly fundamental function for lldb-i386.
>>> I think we should have an unit-test for this. Sorry for being
>>> pedantic, I promise I'll stop after this one.
>> 
>> 
>> It's a good suggestion, and not something we test today.  Right now there 
>> are two i386 ABIs that lldb supports:  Darwin and SysV.  This patch 
>> implements that correctly -- but the obvious problem is if a third i386 ABI 
>> is added in the future.  Now it's a race to see whether SysV-i386 or 
>> CrazyOtherABI-i386 gets CreateInstance'd, depending on the order they're 
>> registered or something.  And I'm not sure how you write a test today that 
>> would test a new target that uses CrazyOtherABI-i386 is getting the correct 
>> plugin activated.
>> 
>> 
> 
> Thanks for your reply, Jason. I'm not sure how to test this either,
> but I'll take a look.
> In theory, (or at lesat what I have in mind :)) you should be able to
> have a unit test that just allocates an object and calls
> createInstance() directly [if possible], then checks that the result
> is of the right type? [ABISysV_i386 vs ABIDarwin_i386 or something
> like that?]
> That won't of course take care of the race, but the test will break in
> case somebody deletes code from the function (and/or allocates an
> object with the wrong ABI).
> I think it's not testing this feature entirely (and I think testing
> the lack of races might be hard, but at least should give us some
> coverage [if nothing, to discriminate dead code VS non-dead code].
> To be fair, I haven't looked into how hard this is to get working, but
> I might. CC:ing Zachary, maybe he has some ideas.



Yep agree, this should be easy to do in a unit test.  I think it could be as 
simple as

ArchSpec arch("apple-i386-macosx");
ABISP m_abi_sp = ABI::FindPlugin(ProcessSP(), archspec);

if (m_abi_sp.get() == nullptr || m_abi_sp->GetPluginName() != 
ConstString("abi.macosx-i386"))
  fail;


I don't think any ABI actually needs a live Process object for their 
CreateInstance() methods today.  If it becomes necessary in the future, then 
that would need to be done for real in the unit test.


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

Re: [Lldb-commits] [lldb] r316673 - Allow SysV-i386 ABI on everything other than Apple targets

[Jason Molenda via lldb-commits]

> On Oct 26, 2017, at 3:21 PM, Jason Molenda  wrote:
> 
> 
> 
>> On Oct 26, 2017, at 3:12 PM, Davide Italiano  wrote:
>> 
>> On Thu, Oct 26, 2017 at 3:04 PM, Jason Molenda  wrote:
>>> 
>>> 
 On Oct 26, 2017, at 10:24 AM, Davide Italiano via lldb-commits 
  wrote:
 
 On Thu, Oct 26, 2017 at 10:04 AM, Stephane Sezer via lldb-commits
  wrote:
> Author: sas
> Date: Thu Oct 26 10:04:20 2017
> New Revision: 316673
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=316673&view=rev
> Log:
> Allow SysV-i386 ABI on everything other than Apple targets
> 
> Summary:
> This matches other SysV ABIs that are different on Apple and non-Apple 
> targets,
> like `ABISysV_arm.cpp` for instance.
> 
> Reviewers: clayborg, emaste
> 
> Subscribers: aemerson, kristof.beyls, lldb-commits
> 
> Differential Revision: https://reviews.llvm.org/D39335
> 
> Modified:
>  lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
> 
> Modified: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp?rev=316673&r1=316672&r2=316673&view=diff
> ==
> --- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp (original)
> +++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp Thu Oct 26 
> 10:04:20 2017
> @@ -205,11 +205,12 @@ ABISysV_i386::GetRegisterInfoArray(uint3
> ABISP
> ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
> &arch) {
> static ABISP g_abi_sp;
> -  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
> -  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
> -if (!g_abi_sp)
> -  g_abi_sp.reset(new ABISysV_i386(process_sp));
> -return g_abi_sp;
> +  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
> +if (arch.GetTriple().getArch() == llvm::Triple::x86) {
> +  if (!g_abi_sp)
> +g_abi_sp.reset(new ABISysV_i386(process_sp));
> +  return g_abi_sp;
> +}
> }
> return ABISP();
> }
> 
 
 This seems to change a fairly fundamental function for lldb-i386.
 I think we should have an unit-test for this. Sorry for being
 pedantic, I promise I'll stop after this one.
>>> 
>>> 
>>> It's a good suggestion, and not something we test today.  Right now there 
>>> are two i386 ABIs that lldb supports:  Darwin and SysV.  This patch 
>>> implements that correctly -- but the obvious problem is if a third i386 ABI 
>>> is added in the future.  Now it's a race to see whether SysV-i386 or 
>>> CrazyOtherABI-i386 gets CreateInstance'd, depending on the order they're 
>>> registered or something.  And I'm not sure how you write a test today that 
>>> would test a new target that uses CrazyOtherABI-i386 is getting the correct 
>>> plugin activated.
>>> 
>>> 
>> 
>> Thanks for your reply, Jason. I'm not sure how to test this either,
>> but I'll take a look.
>> In theory, (or at lesat what I have in mind :)) you should be able to
>> have a unit test that just allocates an object and calls
>> createInstance() directly [if possible], then checks that the result
>> is of the right type? [ABISysV_i386 vs ABIDarwin_i386 or something
>> like that?]
>> That won't of course take care of the race, but the test will break in
>> case somebody deletes code from the function (and/or allocates an
>> object with the wrong ABI).
>> I think it's not testing this feature entirely (and I think testing
>> the lack of races might be hard, but at least should give us some
>> coverage [if nothing, to discriminate dead code VS non-dead code].
>> To be fair, I haven't looked into how hard this is to get working, but
>> I might. CC:ing Zachary, maybe he has some ideas.
> 
> 
> 
> Yep agree, this should be easy to do in a unit test.  I think it could be as 
> simple as
> 
> ArchSpec arch("apple-i386-macosx");
> ABISP m_abi_sp = ABI::FindPlugin(ProcessSP(), archspec);
> 
> if (m_abi_sp.get() == nullptr || m_abi_sp->GetPluginName() != 
> ConstString("abi.macosx-i386"))
>  fail;
> 
> 
> I don't think any ABI actually needs a live Process object for their 
> CreateInstance() methods today.  If it becomes necessary in the future, then 
> that would need to be done for real in the unit test.
> 


(ugh, I got the order of the triple wrong above.  i386-apple-macosx of course.)



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

[Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

Has this gone in?  I'm wondering because I starting playing with the monorepo, 
ran cmake with -DLLDB_TEST_COMPILER=$PWD/bin/clang, and today's test failure 
seems to be trying to build the test program with the system compiler (gcc) 
rather than my copy of clang.  But it looks like you're deprecating 
LLDB_TEST_COMPILER?


https://reviews.llvm.org/D39215



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

Re: [Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

[Zachary Turner via lldb-commits]

I think you now need to use `-DLLDB_TEST_C_COMPILER` and
`-DLLDB_TEST_CXX_COMPILER`

On Thu, Oct 26, 2017 at 4:40 PM Paul Robinson via Phabricator <
revi...@reviews.llvm.org> wrote:

> probinson added a comment.
>
> Has this gone in?  I'm wondering because I starting playing with the
> monorepo, ran cmake with -DLLDB_TEST_COMPILER=$PWD/bin/clang, and today's
> test failure seems to be trying to build the test program with the system
> compiler (gcc) rather than my copy of clang.  But it looks like you're
> deprecating LLDB_TEST_COMPILER?
>
>
> https://reviews.llvm.org/D39215
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

[Pavel Labath via lldb-commits]

I haven't put this in yet. I was still waiting for a reaction from
Zachary about the two cmake vars, as he felt pretty strongly about
unifying to one. (I didn't want to do that as I would have to write
code for transforming /usr/bin/arm-linux-gnu-gcc-4.7 (and likes) into
/usr/bin/arm-linux-gnu-g++-4.7, which would probably end up very
ugly).

On 26 October 2017 at 16:50, Zachary Turner  wrote:
> I think you now need to use `-DLLDB_TEST_C_COMPILER` and
> `-DLLDB_TEST_CXX_COMPILER`
>
> On Thu, Oct 26, 2017 at 4:40 PM Paul Robinson via Phabricator
>  wrote:
>>
>> probinson added a comment.
>>
>> Has this gone in?  I'm wondering because I starting playing with the
>> monorepo, ran cmake with -DLLDB_TEST_COMPILER=$PWD/bin/clang, and today's
>> test failure seems to be trying to build the test program with the system
>> compiler (gcc) rather than my copy of clang.  But it looks like you're
>> deprecating LLDB_TEST_COMPILER?
>>
>>
>> https://reviews.llvm.org/D39215
>>
>>
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

[Zachary Turner via lldb-commits]

Ahh, sorry.  While I feel strongly about 1 long term, going from 4 -> 2 is
already a win, so I see no reason to block this over that.  We can then
work on going from 2 -> 1 later.

On Thu, Oct 26, 2017 at 4:57 PM Pavel Labath  wrote:

> I haven't put this in yet. I was still waiting for a reaction from
> Zachary about the two cmake vars, as he felt pretty strongly about
> unifying to one. (I didn't want to do that as I would have to write
> code for transforming /usr/bin/arm-linux-gnu-gcc-4.7 (and likes) into
> /usr/bin/arm-linux-gnu-g++-4.7, which would probably end up very
> ugly).
>
> On 26 October 2017 at 16:50, Zachary Turner  wrote:
> > I think you now need to use `-DLLDB_TEST_C_COMPILER` and
> > `-DLLDB_TEST_CXX_COMPILER`
> >
> > On Thu, Oct 26, 2017 at 4:40 PM Paul Robinson via Phabricator
> >  wrote:
> >>
> >> probinson added a comment.
> >>
> >> Has this gone in?  I'm wondering because I starting playing with the
> >> monorepo, ran cmake with -DLLDB_TEST_COMPILER=$PWD/bin/clang, and
> today's
> >> test failure seems to be trying to build the test program with the
> system
> >> compiler (gcc) rather than my copy of clang.  But it looks like you're
> >> deprecating LLDB_TEST_COMPILER?
> >>
> >>
> >> https://reviews.llvm.org/D39215
> >>
> >>
> >>
> >
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120518.
hintonda added a comment.

- Maintain process id map for diagtool.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,163 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Make it always available by adding this to your ~.lldbinit file:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+id = target.GetProcess().GetProcessID()
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = {}
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool[id] = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if not id in getDiagtool.diagtool or not getDiagtool.diagtool[id]:
+getDiagtool.diagtool[id] = None
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool[id]
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+ print('clangdiag: id is None')
+ return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+# Just make sure we can find diagtool since it gets used in callbacks.
+diagtool = getDiagtool(target)
+bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
+bp.SetScriptCallbackFunction('clangdiag.setDiagBreakpoint')
+bp.AddName("clang::Diagnostic")
+
+return
+
+def disable(exe_ctx):
+target = e

Re: [Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Zachary Turner via lldb-commits]

On Thu, Oct 26, 2017 at 3:18 PM Don Hinton  wrote:

> On Thu, Oct 26, 2017 at 2:48 PM, Zachary Turner 
> wrote:
>
>> Seems fine, it would be nice if the workflow could be improved a little
>> bit so that all you have to do is say `clangdiag break
>> —error=“-Wcovered-switch”` or something . I think that gives the most
>> intuitive usage for people, even it’s a bit harder to implement.
>>
>
> The idea was to break on actual diagnostics emitted, but if you want to
> break on diagnostic usage, i.e., when it was checked but not emitted, I
> suppose that's possible as well.  diagtool doesn't produce a mapping for
> this, but it could be added -- assuming tablegen produced enough info in
> the .inc files to support it.  I added the feature I'm using here a few
> months ago, which was an extension to what Alex added earlier.
>

That was my idea too.  But still, wouldn't it be possible to say `clangdiag
break --error="-Wcovered-switch"` and then have it break only when the
-Wcovered-switch diagnostic is *emitted*?

The reason I keep using this syntax though is because clang developers
always think in terms of the warning names.  If you want to find out why a
warning is being emitted amidst a spew of other warnings and errors, you
really want to be able to specify the name of the warning.

Don't get me wrong though, I do think this is a nice feature, I'm just
thinking of ways to make it more compelling by looking at it from the clang
developer's perspective and how they will most likely want to use it.

Also, I still think it should go in lldb, not in clang.  That's kind of
exactly what the lldb/examples folder is for.

That said, lgtm, but I'm still interested to see if the workflow can be
streamlined down the line.  Perhaps after it gets checked in you can make a
PSA on cfe-dev@ and mention that you want people to try it out and offer
feedback ;-)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r316727 - Add Arm Architecture plugin to the xcode project file.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Thu Oct 26 19:21:55 2017
New Revision: 316727

URL: http://llvm.org/viewvc/llvm-project?rev=316727&view=rev
Log:
Add Arm Architecture plugin to the xcode project file.

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=316727&r1=316726&r2=316727&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Thu Oct 26 19:21:55 2017
@@ -962,6 +962,8 @@
AF2907BF1D3F082400E10654 /* DynamicLoaderMacOS.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = AF2907BD1D3F082400E10654 /* 
DynamicLoaderMacOS.cpp */; };
AF2BA6EC1A707E3400C5248A /* UriParser.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = 33064C991A5C7A330033D415 /* UriParser.cpp */; };
AF2BCA6C18C7EFDE005B4526 /* JITLoaderGDB.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF2BCA6918C7EFDE005B4526 /* JITLoaderGDB.cpp */; 
};
+   AF2E02A31FA2CEAF00A86C34 /* ArchitectureArm.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF2E02A11FA2CEAF00A86C34 /* ArchitectureArm.cpp 
*/; };
+   AF2E02A41FA2CEAF00A86C34 /* ArchitectureArm.h in Headers */ = 
{isa = PBXBuildFile; fileRef = AF2E02A21FA2CEAF00A86C34 /* ArchitectureArm.h 
*/; };
AF33B4BE1C1FA441001B28D9 /* NetBSDSignals.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF33B4BC1C1FA441001B28D9 /* NetBSDSignals.cpp 
*/; };
AF33B4BF1C1FA441001B28D9 /* NetBSDSignals.h in Headers */ = 
{isa = PBXBuildFile; fileRef = AF33B4BD1C1FA441001B28D9 /* NetBSDSignals.h */; 
};
AF37E10A17C861F20061E18E /* ProcessRunLock.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF37E10917C861F20061E18E /* ProcessRunLock.cpp 
*/; };
@@ -3032,6 +3034,8 @@
AF2907BE1D3F082400E10654 /* DynamicLoaderMacOS.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
DynamicLoaderMacOS.h; sourceTree = ""; };
AF2BCA6918C7EFDE005B4526 /* JITLoaderGDB.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = JITLoaderGDB.cpp; sourceTree = ""; };
AF2BCA6A18C7EFDE005B4526 /* JITLoaderGDB.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
JITLoaderGDB.h; sourceTree = ""; };
+   AF2E02A11FA2CEAF00A86C34 /* ArchitectureArm.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = ArchitectureArm.cpp; sourceTree = ""; };
+   AF2E02A21FA2CEAF00A86C34 /* ArchitectureArm.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
ArchitectureArm.h; sourceTree = ""; };
AF33B4BC1C1FA441001B28D9 /* NetBSDSignals.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = NetBSDSignals.cpp; path = Utility/NetBSDSignals.cpp; sourceTree = 
""; };
AF33B4BD1C1FA441001B28D9 /* NetBSDSignals.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
NetBSDSignals.h; path = Utility/NetBSDSignals.h; sourceTree = ""; };
AF37E10917C861F20061E18E /* ProcessRunLock.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = ProcessRunLock.cpp; sourceTree = ""; };
@@ -3695,6 +3699,7 @@
260C897110F57C5600BB2B04 /* Plugins */ = {
isa = PBXGroup;
children = (
+   AF2E029F1FA2CE8A00A86C34 /* Architecture */,
8CF02ADD19DCBEC200B14BE0 /* 
InstrumentationRuntime */,
8C2D6A58197A1FB9006989C9 /* MemoryHistory */,
26DB3E051379E7AD0080DC73 /* ABI */,
@@ -6548,6 +6553,23 @@
path = GDB;
sourceTree = "";
};
+   AF2E029F1FA2CE8A00A86C34 /* Architecture */ = {
+   isa = PBXGroup;
+   children = (
+   AF2E02A01FA2CE9900A86C34 /* Arm */,
+   );
+   path = Architecture;
+   sourceTree = "";
+   };
+   AF2E02A01FA2CE9900A86C34 /* Arm */ = {
+   isa = PBXGroup;
+   children = (
+   AF2E02A11FA2CEAF00A86C34 /* ArchitectureArm.cpp 
*/,
+   AF2E02A21FA2CEAF00A86C34 /* ArchitectureArm.h 
*/,
+   );
+   path = Arm;
+   sourceTree = "";
+   };
AF6335DF

[Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316728: Default to using in-tree clang for building test 
executables (authored by labath).

Repository:
  rL LLVM

https://reviews.llvm.org/D39215

Files:
  lldb/trunk/CMakeLists.txt
  lldb/trunk/lit/CMakeLists.txt
  lldb/trunk/lit/lit.site.cfg.in
  lldb/trunk/test/CMakeLists.txt

Index: lldb/trunk/lit/lit.site.cfg.in
===
--- lldb/trunk/lit/lit.site.cfg.in
+++ lldb/trunk/lit/lit.site.cfg.in
@@ -10,22 +10,8 @@
 config.lldb_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
-config.cc = "@CMAKE_C_COMPILER@"
-config.cxx = "@CMAKE_CXX_COMPILER@"
-
-test_c_compiler = "@LLDB_TEST_C_COMPILER@"
-test_cxx_compiler = "@LLDB_TEST_CXX_COMPILER@"
-test_clang = "@LLDB_TEST_CLANG@".lower()
-test_clang = test_clang == "on" or test_clang == "true" or test_clang == "1"
-
-if len(test_c_compiler) > 0:
-  config.cc = test_c_compiler
-if len(test_c_compiler) > 0:
-  config.cxx = test_cxx_compiler
-
-if test_clang:
-  config.cc = 'clang'
-  config.cxx = 'clang++'
+config.cc = "@LLDB_TEST_C_COMPILER@"
+config.cxx = "@LLDB_TEST_CXX_COMPILER@"
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.
Index: lldb/trunk/lit/CMakeLists.txt
===
--- lldb/trunk/lit/CMakeLists.txt
+++ lldb/trunk/lit/CMakeLists.txt
@@ -11,10 +11,6 @@
   set(ENABLE_SHARED 0)
 endif(BUILD_SHARED_LIBS)
 
-option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off)
-set(LLDB_TEST_C_COMPILER "" CACHE STRING "C compiler to use when testing LLDB")
-set(LLDB_TEST_CXX_COMPILER "" CACHE STRING "C++ compiler to use when testing LLDB")
-
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg)
@@ -41,10 +37,7 @@
   list(APPEND LLDB_TEST_DEPS debugserver)
 endif()
 
-if(LLDB_TEST_CLANG)
-  if(LLDB_TEST_C_COMPILER OR LLDB_TEST_CXX_COMPILER)
-message(SEND_ERROR "Cannot override LLDB_TEST__COMPILER and set LLDB_TEST_CLANG.")
-  endif()
+if(TARGET clang)
   list(APPEND LLDB_TEST_DEPS clang)
 endif()
 
Index: lldb/trunk/test/CMakeLists.txt
===
--- lldb/trunk/test/CMakeLists.txt
+++ lldb/trunk/test/CMakeLists.txt
@@ -27,10 +27,6 @@
   list(APPEND LLDB_TEST_DEPS lldb-mi)
 endif()
 
-if ("${LLDB_TEST_COMPILER}" STREQUAL "")
-string(REGEX REPLACE ".*ccache\ +" "" LLDB_TEST_COMPILER ${CMAKE_C_COMPILER} ${CMAKE_C_COMPILER_ARG1})
-endif()
-
 # The default architecture with which to compile test executables is the default LLVM target
 # architecture, which itself defaults to the host architecture.
 string(TOLOWER "${LLVM_TARGET_ARCH}" LLDB_DEFAULT_TEST_ARCH)
@@ -43,10 +39,6 @@
 	${LLDB_DEFAULT_TEST_ARCH}
 	CACHE STRING "Specify the architecture to run LLDB tests as (x86|x64).  Determines whether tests are compiled with -m32 or -m64")
 
-if(LLDB_TEST_CLANG)
-  set(LLDB_TEST_COMPILER $)
-endif()
-
 # Users can override LLDB_TEST_USER_ARGS to specify arbitrary arguments to pass to the script
 set(LLDB_TEST_USER_ARGS
   ""
@@ -60,7 +52,7 @@
   -S nm
   -u CXXFLAGS
   -u CFLAGS
-  -C ${LLDB_TEST_COMPILER}
+  -C ${LLDB_TEST_C_COMPILER}
   )
 
 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
Index: lldb/trunk/CMakeLists.txt
===
--- lldb/trunk/CMakeLists.txt
+++ lldb/trunk/CMakeLists.txt
@@ -62,6 +62,22 @@
 option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests."
   ${LLVM_INCLUDE_TESTS})
 if(LLDB_INCLUDE_TESTS)
+  if (TARGET clang)
+set(LLDB_DEFAULT_TEST_C_COMPILER "${LLVM_BINARY_DIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
+set(LLDB_DEFAULT_TEST_CXX_COMPILER "${LLVM_BINARY_DIR}/bin/clang++${CMAKE_EXECUTABLE_SUFFIX}")
+  else()
+set(LLDB_DEFAULT_TEST_C_COMPILER "")
+set(LLDB_DEFAULT_TEST_CXX_COMPILER "")
+  endif()
+
+  set(LLDB_TEST_C_COMPILER "${LLDB_DEFAULT_TEST_C_COMPILER}" CACHE PATH "C Compiler to use for building LLDB test inferiors")
+  set(LLDB_TEST_CXX_COMPILER "${LLDB_DEFAULT_TEST_CXX_COMPILER}" CACHE PATH "C++ Compiler to use for building LLDB test inferiors")
+
+  if (("${LLDB_TEST_C_COMPILER}" STREQUAL "") OR
+  ("${LLDB_TEST_CXX_COMPILER}" STREQUAL ""))
+message(FATAL_ERROR "LLDB test compilers not specified.  Tests will not run")
+  endif()
+
   add_subdirectory(test)
   add_subdirectory(unittests)
   add_subdirectory(lit)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r316728 - Default to using in-tree clang for building test executables

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Oct 26 19:24:04 2017
New Revision: 316728

URL: http://llvm.org/viewvc/llvm-project?rev=316728&view=rev
Log:
Default to using in-tree clang for building test executables

Summary:
Using the in-tree clang should be the default test configuration as that
is the one compiler that we can be sure everyone has (better
reproducibility of test results). Also, it should hopefully reduce the
impact of pr35040.

This also reduces the number of settings which control the compiler
used. LLDB_TEST_C_COMPILER is used for C files and
LLDB_TEST_CXX_COMPILER for C++ files. Both of the settings default to
the in-tree clang.

Reviewers: zturner

Subscribers: mgorny, davide, lldb-commits

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

Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/lit/CMakeLists.txt
lldb/trunk/lit/lit.site.cfg.in
lldb/trunk/test/CMakeLists.txt

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=316728&r1=316727&r2=316728&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Thu Oct 26 19:24:04 2017
@@ -62,6 +62,22 @@ add_subdirectory(tools)
 option(LLDB_INCLUDE_TESTS "Generate build targets for the LLDB unit tests."
   ${LLVM_INCLUDE_TESTS})
 if(LLDB_INCLUDE_TESTS)
+  if (TARGET clang)
+set(LLDB_DEFAULT_TEST_C_COMPILER 
"${LLVM_BINARY_DIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
+set(LLDB_DEFAULT_TEST_CXX_COMPILER 
"${LLVM_BINARY_DIR}/bin/clang++${CMAKE_EXECUTABLE_SUFFIX}")
+  else()
+set(LLDB_DEFAULT_TEST_C_COMPILER "")
+set(LLDB_DEFAULT_TEST_CXX_COMPILER "")
+  endif()
+
+  set(LLDB_TEST_C_COMPILER "${LLDB_DEFAULT_TEST_C_COMPILER}" CACHE PATH "C 
Compiler to use for building LLDB test inferiors")
+  set(LLDB_TEST_CXX_COMPILER "${LLDB_DEFAULT_TEST_CXX_COMPILER}" CACHE PATH 
"C++ Compiler to use for building LLDB test inferiors")
+
+  if (("${LLDB_TEST_C_COMPILER}" STREQUAL "") OR
+  ("${LLDB_TEST_CXX_COMPILER}" STREQUAL ""))
+message(FATAL_ERROR "LLDB test compilers not specified.  Tests will not 
run")
+  endif()
+
   add_subdirectory(test)
   add_subdirectory(unittests)
   add_subdirectory(lit)

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=316728&r1=316727&r2=316728&view=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Thu Oct 26 19:24:04 2017
@@ -11,10 +11,6 @@ else()
   set(ENABLE_SHARED 0)
 endif(BUILD_SHARED_LIBS)
 
-option(LLDB_TEST_CLANG "Use in-tree clang when testing lldb" Off)
-set(LLDB_TEST_C_COMPILER "" CACHE STRING "C compiler to use when testing LLDB")
-set(LLDB_TEST_CXX_COMPILER "" CACHE STRING "C++ compiler to use when testing 
LLDB")
-
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg)
@@ -41,10 +37,7 @@ if(APPLE)
   list(APPEND LLDB_TEST_DEPS debugserver)
 endif()
 
-if(LLDB_TEST_CLANG)
-  if(LLDB_TEST_C_COMPILER OR LLDB_TEST_CXX_COMPILER)
-message(SEND_ERROR "Cannot override LLDB_TEST__COMPILER and set 
LLDB_TEST_CLANG.")
-  endif()
+if(TARGET clang)
   list(APPEND LLDB_TEST_DEPS clang)
 endif()
 

Modified: lldb/trunk/lit/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.site.cfg.in?rev=316728&r1=316727&r2=316728&view=diff
==
--- lldb/trunk/lit/lit.site.cfg.in (original)
+++ lldb/trunk/lit/lit.site.cfg.in Thu Oct 26 19:24:04 2017
@@ -10,22 +10,8 @@ config.lldb_libs_dir = "@LLVM_LIBRARY_OU
 config.lldb_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
-config.cc = "@CMAKE_C_COMPILER@"
-config.cxx = "@CMAKE_CXX_COMPILER@"
-
-test_c_compiler = "@LLDB_TEST_C_COMPILER@"
-test_cxx_compiler = "@LLDB_TEST_CXX_COMPILER@"
-test_clang = "@LLDB_TEST_CLANG@".lower()
-test_clang = test_clang == "on" or test_clang == "true" or test_clang == "1"
-
-if len(test_c_compiler) > 0:
-  config.cc = test_c_compiler
-if len(test_c_compiler) > 0:
-  config.cxx = test_cxx_compiler
-
-if test_clang:
-  config.cc = 'clang'
-  config.cxx = 'clang++'
+config.cc = "@LLDB_TEST_C_COMPILER@"
+config.cxx = "@LLDB_TEST_CXX_COMPILER@"
 
 # Support substitution of the tools and libs dirs with user parameters. This is
 # used when we can't determine the tool dir at configuration time.

Modified: lldb/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/CMakeLists.txt?rev=316728&r1=316727&r2=316728&view=diff
==
--- lldb/trunk/test/CMakeLists.txt (original)
+++ lldb/trunk/test/CMakeLists.txt Thu Oct 26 19:24:04 2017
@@ -27,10 +27,6 @@ if(TARGET lldb-mi)
   lis

[Lldb-commits] [PATCH] D34774: [lldb] Add a testcase for MainThreadCheckerRuntime plugin

[chenyu via Phabricator via lldb-commits]

SeanChense added a comment.

In https://reviews.llvm.org/D34774#908084, @kubamracek wrote:

> In https://reviews.llvm.org/D34774#907636, @SeanChense wrote:
>
> > Hi there, I am here to find help. Is there  a way to get 
> > `libMainThreadChecker.dylib` output ? Then we can analyze the output to 
> > generate a report ?
>
>
> Hi, what are you trying to do? What info are you missing that the LLDB plugin 
> doesn't provide? I don't think parsing the text output (in stderr) from 
> libMainThreadChecker.dylib is a good idea, as it's likely to change.


I am an iOS developer. We are using XCTest to run automated testing twice per 
day and to generate a report. In Xcode 9, I notice that 
`libMainThreadChecker.dylib` which documented in 
https://developer.apple.com/documentation/code_diagnostics/main_thread_checker. 
It seems that there is no API for `libMainThreadChecker.dylib` we can use to 
collect warning to append in report. So I wanna analyze the log to extract 
`libMainThreadChecker.dylib` output. Since you are saying parsing the text 
output is a bad idea, is there a better way to do that ?


https://reviews.llvm.org/D34774



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

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via Phabricator via lldb-commits]

hintonda updated this revision to Diff 120538.
hintonda added a comment.

- Add ability to add breakpoints matching -W warnings.


https://reviews.llvm.org/D36347

Files:
  utils/clangdiag.py

Index: utils/clangdiag.py
===
--- /dev/null
+++ utils/clangdiag.py
@@ -0,0 +1,175 @@
+#!/usr/bin/python
+
+#--
+# Be sure to add the python path that points to the LLDB shared library.
+#
+# # To use this in the embedded python interpreter using "lldb" just
+# import it with the full path using the "command script import"
+# command
+#   (lldb) command script import /path/to/clandiag.py
+#--
+
+import lldb
+import argparse
+import commands
+import shlex
+import os
+import re
+import subprocess
+
+class MyParser(argparse.ArgumentParser):
+def format_help(self):
+return ''' Commands for managing clang diagnostic breakpoints
+
+Syntax: clangdiag enable []
+clangdiag disable
+clangdiag diagtool [|reset]
+
+The following subcommands are supported:
+
+  enable   -- Enable clang diagnostic breakpoints.
+  disable  -- Disable all clang diagnostic breakpoints.
+  diagtool -- Return, set, or reset diagtool path.
+
+This command sets breakpoints in clang, and clang based tools, that
+emit diagnostics.  When a diagnostic is emitted, and clangdiag is
+enabled, it will use the appropriate diagtool application to determine
+the name of the DiagID, and set breakpoints in all locations that
+'diag::name' appears in the source.  Since the new breakpoints are set
+after they are encountered, users will need to launch the executable a
+second time in order to hit the new breakpoints.
+
+For in-tree builds, the diagtool application, used to map DiagID's to
+names, is found automatically in the same directory as the target
+executable.  However, out-or-tree builds must use the 'diagtool'
+subcommand to set the appropriate path for diagtool in the clang debug
+bin directory.  Since this mapping is created at build-time, it's
+important for users to use the same version that was generated when
+clang was compiled, or else the id's won't match.
+
+Notes:
+
+- If  is passed, just enable the DiagID for that warning.
+- Rerunning enable clears existing breakpoints.
+- diagtool is used in breakpoint callbacks, so it can be changed
+  without the need to rerun enable.
+- Make it always available by adding this to your ~.lldbinit file:
+  "command script import /path/to/clangdiag.py"
+
+'''
+
+def create_diag_options():
+parser = MyParser(prog='clangdiag')
+subparsers = parser.add_subparsers(
+title='subcommands',
+dest='subcommands',
+metavar='')
+disable_parser = subparsers.add_parser('disable')
+enable_parser = subparsers.add_parser('enable')
+enable_parser.add_argument('id', nargs='?')
+diagtool_parser = subparsers.add_parser('diagtool')
+diagtool_parser.add_argument('path', nargs='?')
+return parser
+
+def getDiagtool(target, diagtool = None):
+id = target.GetProcess().GetProcessID()
+if 'diagtool' not in getDiagtool.__dict__:
+getDiagtool.diagtool = {}
+if diagtool:
+if diagtool == 'reset':
+getDiagtool.diagtool[id] = None
+elif os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: %s not found.' % diagtool)
+if not id in getDiagtool.diagtool or not getDiagtool.diagtool[id]:
+getDiagtool.diagtool[id] = None
+exe = target.GetExecutable()
+if not exe.Exists():
+print('clangdiag: Target (%s) not set.' % exe.GetFilename())
+else:
+diagtool = os.path.join(exe.GetDirectory(), 'diagtool')
+if os.path.exists(diagtool):
+getDiagtool.diagtool[id] = diagtool
+else:
+print('clangdiag: diagtool not found along side %s' % exe)
+
+return getDiagtool.diagtool[id]
+
+def setDiagBreakpoint(frame, bp_loc, dict):
+id = frame.FindVariable("DiagID").GetValue()
+if id is None:
+print('clangdiag: id is None')
+return False
+
+# Don't need to test this time, since we did that in enable.
+target = frame.GetThread().GetProcess().GetTarget()
+diagtool = getDiagtool(target)
+name = subprocess.check_output([diagtool, "find-diagnostic-id", id]).rstrip();
+bp = target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec())
+bp.AddName("clang::Diagnostic")
+
+return False
+
+def enable(exe_ctx, args):
+# Always disable existing breakpoints
+disable(exe_ctx)
+
+target = exe_ctx.GetTarget()
+
+if args.id:
+diagtool = getDiagtool(target)
+list = subprocess.check_output([diagtool, "list-warnings"]).rstrip();
+for line in list.splitlines(True):
+m = re.

Re: [Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Don Hinton via lldb-commits]

On Thu, Oct 26, 2017 at 5:44 PM, Zachary Turner  wrote:

>
>
> On Thu, Oct 26, 2017 at 3:18 PM Don Hinton  wrote:
>
>> On Thu, Oct 26, 2017 at 2:48 PM, Zachary Turner 
>> wrote:
>>
>>> Seems fine, it would be nice if the workflow could be improved a little
>>> bit so that all you have to do is say `clangdiag break
>>> —error=“-Wcovered-switch”` or something . I think that gives the most
>>> intuitive usage for people, even it’s a bit harder to implement.
>>>
>>
>> The idea was to break on actual diagnostics emitted, but if you want to
>> break on diagnostic usage, i.e., when it was checked but not emitted, I
>> suppose that's possible as well.  diagtool doesn't produce a mapping for
>> this, but it could be added -- assuming tablegen produced enough info in
>> the .inc files to support it.  I added the feature I'm using here a few
>> months ago, which was an extension to what Alex added earlier.
>>
>
> That was my idea too.  But still, wouldn't it be possible to say
> `clangdiag break --error="-Wcovered-switch"` and then have it break only
> when the -Wcovered-switch diagnostic is *emitted*?
>

Please give it a try, e.g., here are a few I tried:

clangdiag enable covered-switch-default
clangdiag enable c++11-compat

You can't pass the "-W" part since argparse thinks it's an option (can
probably fix that if it's a problem), and you must provide the entire
name.  You can get the available names from diagtool, e.g.:

diagtool list-warnings

Please let me know what you think, and thanks for suggesting it.



>
> The reason I keep using this syntax though is because clang developers
> always think in terms of the warning names.  If you want to find out why a
> warning is being emitted amidst a spew of other warnings and errors, you
> really want to be able to specify the name of the warning.
>
> Don't get me wrong though, I do think this is a nice feature, I'm just
> thinking of ways to make it more compelling by looking at it from the clang
> developer's perspective and how they will most likely want to use it.
>
> Also, I still think it should go in lldb, not in clang.  That's kind of
> exactly what the lldb/examples folder is for.
>
> That said, lgtm, but I'm still interested to see if the workflow can be
> streamlined down the line.  Perhaps after it gets checked in you can make a
> PSA on cfe-dev@ and mention that you want people to try it out and offer
> feedback ;-)
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Oct 26 21:53:24 2017
New Revision: 316740

URL: http://llvm.org/viewvc/llvm-project?rev=316740&view=rev
Log:
Fix a use-after-free in lldb-server

UriParser::Parse is returning a StringRef pointing the the parsed
string, but we were calling it with a temporary string. Change this to a
local variable to make sure the string persists as long as we need it.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=316740&r1=316739&r2=316740&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 Thu Oct 26 21:53:24 2017
@@ -128,8 +128,9 @@ Status GDBRemoteCommunicationServerPlatf
   llvm::StringRef platform_ip;
   int platform_port;
   llvm::StringRef platform_path;
-  bool ok = UriParser::Parse(GetConnection()->GetURI(), platform_scheme,
- platform_ip, platform_port, platform_path);
+  std::string platform_uri = GetConnection()->GetURI();
+  bool ok = UriParser::Parse(platform_uri, platform_scheme, platform_ip,
+ platform_port, platform_path);
   UNUSED_IF_ASSERT_DISABLED(ok);
   assert(ok);
 


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

Re: [Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

[Zachary Turner via lldb-commits]

looks good!  Feel free to commit whenever, I'd definitely recommend posting
a PSA on cfe-dev@ (after you commit) so that people know about it.  You
might also get some useful ideas for improvements that way too.

On Thu, Oct 26, 2017 at 9:52 PM Don Hinton  wrote:

> On Thu, Oct 26, 2017 at 5:44 PM, Zachary Turner 
> wrote:
>
>>
>>
>> On Thu, Oct 26, 2017 at 3:18 PM Don Hinton  wrote:
>>
>>> On Thu, Oct 26, 2017 at 2:48 PM, Zachary Turner 
>>> wrote:
>>>
 Seems fine, it would be nice if the workflow could be improved a little
 bit so that all you have to do is say `clangdiag break
 —error=“-Wcovered-switch”` or something . I think that gives the most
 intuitive usage for people, even it’s a bit harder to implement.

>>>
>>> The idea was to break on actual diagnostics emitted, but if you want to
>>> break on diagnostic usage, i.e., when it was checked but not emitted, I
>>> suppose that's possible as well.  diagtool doesn't produce a mapping for
>>> this, but it could be added -- assuming tablegen produced enough info in
>>> the .inc files to support it.  I added the feature I'm using here a few
>>> months ago, which was an extension to what Alex added earlier.
>>>
>>
>> That was my idea too.  But still, wouldn't it be possible to say
>> `clangdiag break --error="-Wcovered-switch"` and then have it break only
>> when the -Wcovered-switch diagnostic is *emitted*?
>>
>
> Please give it a try, e.g., here are a few I tried:
>
> clangdiag enable covered-switch-default
> clangdiag enable c++11-compat
>
> You can't pass the "-W" part since argparse thinks it's an option (can
> probably fix that if it's a problem), and you must provide the entire
> name.  You can get the available names from diagtool, e.g.:
>
> diagtool list-warnings
>
> Please let me know what you think, and thanks for suggesting it.
>
>
>
>>
>> The reason I keep using this syntax though is because clang developers
>> always think in terms of the warning names.  If you want to find out why a
>> warning is being emitted amidst a spew of other warnings and errors, you
>> really want to be able to specify the name of the warning.
>>
>> Don't get me wrong though, I do think this is a nice feature, I'm just
>> thinking of ways to make it more compelling by looking at it from the clang
>> developer's perspective and how they will most likely want to use it.
>>
>> Also, I still think it should go in lldb, not in clang.  That's kind of
>> exactly what the lldb/examples folder is for.
>>
>> That said, lgtm, but I'm still interested to see if the workflow can be
>> streamlined down the line.  Perhaps after it gets checked in you can make a
>> PSA on cfe-dev@ and mention that you want people to try it out and offer
>> feedback ;-)
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits