[Lldb-commits] [lldb] r284674 - Added a decorator for the macOS version and switched over testcases that used platform.release

[Sean Callanan via lldb-commits]

Author: spyffe
Date: Wed Oct 19 19:03:39 2016
New Revision: 284674

URL: http://llvm.org/viewvc/llvm-project?rev=284674&view=rev
Log:
Added a decorator for the macOS version and switched over testcases that used 
platform.release

Modified:
lldb/trunk/packages/Python/lldbsuite/test/decorators.py
lldb/trunk/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules/TestObjCModules.py

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/decorators.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/decorators.py?rev=284674&r1=284673&r2=284674&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/decorators.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/decorators.py Wed Oct 19 19:03:39 
2016
@@ -157,6 +157,7 @@ def _decorateTest(mode,
   archs=None, triple=None,
   debug_info=None,
   swig_version=None, py_version=None,
+  macos_version=None,
   remote=None):
 def fn(self):
 skip_for_os = _match_decorator_property(
@@ -187,6 +188,11 @@ def _decorateTest(mode,
 skip_for_py_version = (
 py_version is None) or _check_expected_version(
 py_version[0], py_version[1], sys.version_info)
+skip_for_macos_version = (macos_version is None) or (
+_check_expected_version(
+macos_version[0],
+macos_version[1],
+platform.mac_ver()[0]))
 
 # For the test to be skipped, all specified (e.g. not None) parameters 
must be True.
 # An unspecified parameter means "any", so those are marked skip by 
default.  And we skip
@@ -199,6 +205,7 @@ def _decorateTest(mode,
   (triple, skip_for_triple, "target triple"),
   (swig_version, skip_for_swig_version, "swig version"),
   (py_version, skip_for_py_version, "python version"),
+  (macos_version, skip_for_macos_version, "macOS version"),
   (remote, skip_for_remote, "platform locality 
(remote/local)")]
 reasons = []
 final_skip_result = True
@@ -242,6 +249,7 @@ def expectedFailureAll(bugnumber=None,
archs=None, triple=None,
debug_info=None,
swig_version=None, py_version=None,
+   macos_version=None,
remote=None):
 return _decorateTest(DecorateMode.Xfail,
  bugnumber=bugnumber,
@@ -250,6 +258,7 @@ def expectedFailureAll(bugnumber=None,
  archs=archs, triple=triple,
  debug_info=debug_info,
  swig_version=swig_version, py_version=py_version,
+ macos_version=None,
  remote=remote)
 
 
@@ -265,6 +274,7 @@ def skipIf(bugnumber=None,
archs=None, triple=None,
debug_info=None,
swig_version=None, py_version=None,
+   macos_version=None,
remote=None):
 return _decorateTest(DecorateMode.Skip,
  bugnumber=bugnumber,
@@ -273,6 +283,7 @@ def skipIf(bugnumber=None,
  archs=archs, triple=triple,
  debug_info=debug_info,
  swig_version=swig_version, py_version=py_version,
+ macos_version=macos_version,
  remote=remote)
 
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py?rev=284674&r1=284673&r2=284674&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py 
Wed Oct 19 19:03:39 2016
@@ -25,10 +25,8 @@ class CModulesTestCase(TestBase):
 @expectedFailureAll(
 oslist=["windows"],
 bugnumber="llvm.org/pr24489: Name lookup not working correctly on 
Windows")
+@skipIf(macos_version=["<", "10.12"])
 def test_expr(self):
-if platform.system() == "Darwin" and platform.release() < 
StrictVersion('12.0.0'):
-self.skipTest()
-
 self.build

Re: [Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

Actually ignore that last comment , the behavior is the same as before, but
linux will still behave differently than other platforms
On Wed, Oct 19, 2016 at 3:17 PM Zachary Turner  wrote:

> zturner added a comment.
>
> Incidentally, this patch actually makes all platforms behave consistently
> when the `Write` overload with offset is used with `O_APPEND`, so there's
> probably some value in having that consistency.
>
>
> https://reviews.llvm.org/D25783
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r284666 - Add lldb register definitions for w0-w28, s0-s31, and d0-d31 to

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Wed Oct 19 18:38:38 2016
New Revision: 284666

URL: http://llvm.org/viewvc/llvm-project?rev=284666&view=rev
Log:
Add lldb register definitions for w0-w28, s0-s31, and d0-d31 to
RegisterInfos_arm64.h.  These register definitions include the
offset into the register context, which will vary depending on the
endianness of the arm64 target system (e.g. s8 is at offset 0 in
v8 on little-endian, it is at offset 12 on big-endian) and I've
only added the little-endian definitions to the table.  If we want
to add a big-endian arm64 target, we'll need a separate table which
uses the big-endian offsets for these registers.  I changed the
name of the register table from g_register_infos_arm64 to
g_register_infos_arm64_le to make it explicit that this is the
little-endian version of that table, and updated users of the table
to use the new name.

I added support for the "w", "s", and "d" registers to
RegisterContextDarwin_arm64 but it was more an example than anything
useful -- this plugin is only used when working with core files and
darwin core files do not (today) include the floating point register
context, so it only added the support for the "w" pseudo registers.
When we're connected to a real arm64 device, we use the ProcessGDBRemote
code.

Modified:
lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextFreeBSD_arm64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLinux_arm64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_arm64.h

Modified: 
lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp?rev=284666&r1=284665&r2=284666&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp 
Wed Oct 19 18:38:38 2016
@@ -25,6 +25,8 @@
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/Scalar.h"
 #include "lldb/Host/Endian.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Thread.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Compiler.h"
 
@@ -101,7 +103,7 @@ static uint32_t g_fpu_regnums[] = {
 static uint32_t g_exc_regnums[] = {exc_far, exc_esr, exc_exception};
 
 static size_t k_num_register_infos =
-llvm::array_lengthof(g_register_infos_arm64);
+llvm::array_lengthof(g_register_infos_arm64_le);
 
 RegisterContextDarwin_arm64::RegisterContextDarwin_arm64(
 Thread &thread, uint32_t concrete_frame_idx)
@@ -129,7 +131,7 @@ const RegisterInfo *
 RegisterContextDarwin_arm64::GetRegisterInfoAtIndex(size_t reg) {
   assert(k_num_register_infos == k_num_registers);
   if (reg < k_num_registers)
-return &g_register_infos_arm64[reg];
+return &g_register_infos_arm64_le[reg];
   return NULL;
 }
 
@@ -138,7 +140,7 @@ size_t RegisterContextDarwin_arm64::GetR
 }
 
 const RegisterInfo *RegisterContextDarwin_arm64::GetRegisterInfos() {
-  return g_register_infos_arm64;
+  return g_register_infos_arm64_le;
 }
 
 // Number of registers in each register set
@@ -352,6 +354,46 @@ bool RegisterContextDarwin_arm64::ReadRe
 value.SetUInt64(gpr.x[reg - gpr_x0]);
 break;
 
+  case gpr_w0:
+  case gpr_w1:
+  case gpr_w2:
+  case gpr_w3:
+  case gpr_w4:
+  case gpr_w5:
+  case gpr_w6:
+  case gpr_w7:
+  case gpr_w8:
+  case gpr_w9:
+  case gpr_w10:
+  case gpr_w11:
+  case gpr_w12:
+  case gpr_w13:
+  case gpr_w14:
+  case gpr_w15:
+  case gpr_w16:
+  case gpr_w17:
+  case gpr_w18:
+  case gpr_w19:
+  case gpr_w20:
+  case gpr_w21:
+  case gpr_w22:
+  case gpr_w23:
+  case gpr_w24:
+  case gpr_w25:
+  case gpr_w26:
+  case gpr_w27:
+  case gpr_w28: {
+ProcessSP process_sp(m_thread.GetProcess());
+if (process_sp.get()) {
+  DataExtractor regdata(&gpr.x[reg - gpr_w0], 8, 
process_sp->GetByteOrder(),
+process_sp->GetAddressByteSize());
+  offset_t offset = 0;
+  uint64_t retval = regdata.GetMaxU64(&offset, 8);
+  uint32_t retval_lower32 = static_cast(retval & 0x);
+  value.SetUInt32(retval_lower32);
+}
+  } break;
+
   case fpu_v0:
   case fpu_v1:
   case fpu_v2:
@@ -388,6 +430,88 @@ bool RegisterContextDarwin_arm64::ReadRe
endian::InlHostByteOrder());
 break;
 
+  case fpu_s0:
+  case fpu_s1:
+  case fpu_s2:
+  case fpu_s3:
+  case fpu_s4:
+  case fpu_s5:
+  case fpu_s6:
+  case fpu_s7:
+  case fpu_s8:
+  case fpu_s9:
+  case fpu_s10:
+  case fpu_s11:
+  case fpu_s12:
+  case fpu_s13:
+  case fpu_s14:
+  case fpu_s15:
+  case fpu_s16:
+  case fpu_s17:
+  case fpu_s18:
+  case fpu_s19:
+  case fpu_s20:
+  case fpu_s21:
+  case fpu_s22:
+  case fpu_s23:
+  case fpu_s24:
+  case fpu_s25:
+  case fpu_s26:
+  case fpu_s27:
+  case fpu_s28:
+

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner added a comment.

Incidentally, this patch actually makes all platforms behave consistently when 
the `Write` overload with offset is used with `O_APPEND`, so there's probably 
some value in having that consistency.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D25783#574860, @labath wrote:

> > This can happen with any number of bytes and at any time.  `write`, `read`, 
> > and all other related functions will return a non-negative value indicating 
> > the number of bytes successfully read/written, which will be less than the 
> > number requested.
>
> Except if fd refers to a pipe, in which case writes of up to PIPE_MAX will be 
> atomic.
>
> Also, I just noticed another problem. What if the fd does not refer to an 
> actual file, but a non-seekable file system object (named pipe, domain 
> socket, ...). Will the lseek work on that? I have no idea. But, I think 
> you're implementing a broken API to save a couple of lines of code.


In that case the user should be using `Host/Pipe` or a more suitable class, 
this class is already unsuitable just on the grounds that it exposes a method 
(the offset version) that requires a seekable device.  I'd even be fine 
asserting in the constructor if the device is not seekable.

> (btw, you may want to know that pwrite() on O_APPEND descriptors basically 
> ignores the offset argument, and always does an append).

Only on Linux, but this is apparently non-conformant behavior :(

https://linux.die.net/man/2/pwrite

> POSIX requires that opening a file with the O_APPEND flag should have no 
> affect on the location at which pwrite() writes data. However, on Linux, if a 
> file is opened with O_APPEND, pwrite() appends data to the end of the file, 
> regardless of the value of offset.

IDK, I'm the Windows guy so this isn't really my call, but I think part of the 
reason why the test suite is so flaky and bugs are so hard to track down 
sometimes is because we don't make assumptions.  If we find some code that is 
clearly broken without a given set of assumptions, I think making it break 
"even more" without those assumptions is not only fine, but even desirable so 
that problems become easier to track down.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25792: Don't set a software stepping breakpoint at 0 on arm.

[Pavel Labath via lldb-commits]

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

I envisioned a bit different solution to this problem. The issue does not 
happen only when the inferior jumps to a literal 0x0 address, but for any 
address that is not mapped into memory (so we cannot set a breakpoint there). 
The simplest way to figure out whether we are able to set a breakpoint is to 
actually try setting it, and check for the error. (plus, I don't think your 
code   handles the thumb case)

And yes, the .gitignore changes should not be a part of this change. :)


https://reviews.llvm.org/D25792



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Pavel Labath via lldb-commits]

labath added a comment.

> This can happen with any number of bytes and at any time.  `write`, `read`, 
> and all other related functions will return a non-negative value indicating 
> the number of bytes successfully read/written, which will be less than the 
> number requested.

Except if fd refers to a pipe, in which case writes of up to PIPE_MAX will be 
atomic.

Also, I just noticed another problem. What if the fd does not refer to an 
actual file, but a non-seekable file system object (named pipe, domain socket, 
...). Will the lseek work on that? I have no idea. But, I think you're 
implementing a broken API to save a couple of lines of code.

(btw, you may want to know that pwrite() on O_APPEND descriptors basically 
ignores the offset argument, and always does an append).


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25792: Don't set a software stepping breakpoint at 0 on arm.

[Jim Ingham via lldb-commits]

jingham added a comment.

Not commenting on the substance of the change (though it seems sensible to me.) 
 The inclusion of the .gitignore was probably accidental, but in any case, try 
not to gang unrelated changes together like this.  Thanks!


https://reviews.llvm.org/D25792



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

[Lldb-commits] [PATCH] D25792: Don't set a software stepping breakpoint at 0 on arm.

[Jason Majors via lldb-commits]

jmajors created this revision.
jmajors added a reviewer: labath.
jmajors added a subscriber: lldb-commits.
Herald added subscribers: srhines, danalbert, tberghammer, rengolin, aemerson.

Added a test for the next_pc being zero before setting a software breakpoint
in arm32.
Reenabled the crash during step test for android/linux.


https://reviews.llvm.org/D25792

Files:
  .gitignore
  
packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
  source/Plugins/Process/Linux/NativeProcessLinux.cpp


Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1347,7 +1347,7 @@
 if (next_flags & 0x20) {
   // Thumb mode
   error = SetSoftwareBreakpoint(next_pc, 2);
-} else {
+} else if (next_pc) {
   // Arm mode
   error = SetSoftwareBreakpoint(next_pc, 4);
 }
Index: 
packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
===
--- 
packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
+++ 
packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
@@ -21,11 +21,6 @@
 self.breakpoint = line_number('main.cpp', '// Set breakpoint here')
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
-@expectedFailureAndroid("llvm.org/pr24497", archs=['arm', 'aarch64'])
-@expectedFailureAll(
-oslist=["linux"],
-archs=["arm"],
-bugnumber="llvm.org/pr24497")
 # IO error due to breakpoint at invalid address
 @expectedFailureAll(triple=re.compile('^mips'))
 def test_step_inst_with(self):
Index: .gitignore
===
--- .gitignore
+++ .gitignore
@@ -21,7 +21,7 @@
 *.sln
 *.suo
 # vim swap files
-.*.swp
+.*.sw?
 .sw?
 # OS X specific files.
 .DS_store
@@ -39,7 +39,12 @@
 __pycache__/
 *.lock
 *.so
+a.out
+*.o
+*.d
+*.dwo
 
+.ycm_extra_conf.py
 clang-module-cache
 
 # Skip ctags-style tags files


Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1347,7 +1347,7 @@
 if (next_flags & 0x20) {
   // Thumb mode
   error = SetSoftwareBreakpoint(next_pc, 2);
-} else {
+} else if (next_pc) {
   // Arm mode
   error = SetSoftwareBreakpoint(next_pc, 4);
 }
Index: packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
===
--- packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
+++ packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
@@ -21,11 +21,6 @@
 self.breakpoint = line_number('main.cpp', '// Set breakpoint here')
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
-@expectedFailureAndroid("llvm.org/pr24497", archs=['arm', 'aarch64'])
-@expectedFailureAll(
-oslist=["linux"],
-archs=["arm"],
-bugnumber="llvm.org/pr24497")
 # IO error due to breakpoint at invalid address
 @expectedFailureAll(triple=re.compile('^mips'))
 def test_step_inst_with(self):
Index: .gitignore
===
--- .gitignore
+++ .gitignore
@@ -21,7 +21,7 @@
 *.sln
 *.suo
 # vim swap files
-.*.swp
+.*.sw?
 .sw?
 # OS X specific files.
 .DS_store
@@ -39,7 +39,12 @@
 __pycache__/
 *.lock
 *.so
+a.out
+*.o
+*.d
+*.dwo
 
+.ycm_extra_conf.py
 clang-module-cache
 
 # Skip ctags-style tags files
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Greg Clayton via lldb-commits]

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

Ok, the lseek calls make sense then.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner added inline comments.



Comment at: source/Host/common/File.cpp:405
 
-if (bytes_read == 0) {
-  if (::feof(m_stream))
-error.SetErrorString("feof");
-  else if (::ferror(m_stream))
-error.SetErrorString("ferror");
-  num_bytes = 0;
-} else
-  num_bytes = bytes_read;
-  } else {
-num_bytes = 0;
-error.SetErrorString("invalid file handle");
-  }
-  return error;
+  off_t offset = lseek(fd, 0, SEEK_CUR);
+  return Read(buf, num_bytes, offset);

clayborg wrote:
> Why are we calling lseek when we are passing the offset into the read below?
> 
> Shouldn't this just be:
> ```
> off_t offset = 0;
> ```
This `lseek` is to get the current file pointer.  If someone calls `Write()` 
with no offset, they expect this to mean "write at the current file position".  
In order to do that with `pwrite()`, you need to know what the current file 
position actually is.  If we just set `offset=0`, it will write at the 
beginning of the file, which is probably not the intention.

LMK if I've misunderstood.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner added a comment.

Also you are right that I misspoke about the append case.  But still, I just 
think that if writing to the same file from multiple processes is something we 
care about, we should support it "for real" instead of just pretending to.  
That means some kind of cross-process synchronization such as a shared mutex.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Greg Clayton via lldb-commits]

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

Just a few questions on why we are calling lseek in the read and write 
functions. See inlined comments.




Comment at: source/Host/common/File.cpp:405
 
-if (bytes_read == 0) {
-  if (::feof(m_stream))
-error.SetErrorString("feof");
-  else if (::ferror(m_stream))
-error.SetErrorString("ferror");
-  num_bytes = 0;
-} else
-  num_bytes = bytes_read;
-  } else {
-num_bytes = 0;
-error.SetErrorString("invalid file handle");
-  }
-  return error;
+  off_t offset = lseek(fd, 0, SEEK_CUR);
+  return Read(buf, num_bytes, offset);

Why are we calling lseek when we are passing the offset into the read below?

Shouldn't this just be:
```
off_t offset = 0;
```



Comment at: source/Host/common/File.cpp:416
 
-  ssize_t bytes_written = -1;
-  if (DescriptorIsValid()) {
-do {
-  bytes_written = ::write(m_descriptor, buf, num_bytes);
-} while (bytes_written < 0 && errno == EINTR);
-
-if (bytes_written == -1) {
-  error.SetErrorToErrno();
-  num_bytes = 0;
-} else
-  num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
-bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);
-
-if (bytes_written == 0) {
-  if (::feof(m_stream))
-error.SetErrorString("feof");
-  else if (::ferror(m_stream))
-error.SetErrorString("ferror");
-  num_bytes = 0;
-} else
-  num_bytes = bytes_written;
-
-  } else {
-num_bytes = 0;
-error.SetErrorString("invalid file handle");
+  off_t offset = lseek(fd, 0, SEEK_CUR);
+  if (m_options & File::eOpenOptionAppend) {

Why are we calling lseek here? We specify the offset to the Write below and 
that function should do the right thing with the offset. 

Shouldn't this just be:
```
off_t offset = 0;
```


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D25783#574699, @labath wrote:

> In https://reviews.llvm.org/D25783#574684, @zturner wrote:
>
> > There are many other problems with this code if we want to deal with 
> > atomicity.  For example, the whole point of this patch was to handle short 
> > reads and writes.  Well, if you have a short read or a write, then reading 
> > and writing a subsequent chunk is not atomic.
>
>
> I am willing abandon the atomicity if someone tries to write more than 2GB of 
> data - I think he has bigger problems than that.


Writing more than 2GB of data is not the only reason to get a short read or 
write.  You can be interrupted by a signal after some of the data has been 
written but not all.  This can happen with any number of bytes and at any time. 
 `write`, `read`, and all other related functions will return a non-negative 
value indicating the number of bytes successfully read/written, which will be 
less than the number requested.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Pavel Labath via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D25783#574684, @zturner wrote:

> There are many other problems with this code if we want to deal with 
> atomicity.  For example, the whole point of this patch was to handle short 
> reads and writes.  Well, if you have a short read or a write, then reading 
> and writing a subsequent chunk is not atomic.


I am willing abandon the atomicity if someone tries to write more than 2GB of 
data - I think he has bigger problems than that.

> That being said, I don't think it's hugely important here.  The worst that 
> will happen is two log messages will be printed out of order.  I don't think 
> anything will be corrupt or interwoven.  For example, if two threads come in 
> at the same time and both compute the same file size, they will both attempt 
> to write at the same offset.  One will win, the other will insert right 
> before the message.  So it's possible the two could wind up reversed, but 
> that's about it.

I am not sure what you mean by "insert right before the message". The second 
thread will just overwrite the thing that the first one wrote (plus you'll end 
up with a dangling fragment at the end if the second message was shorter.

> Only way to deal with multi-threading correctly is to put all this in a mutex.

Actually, a mutex cannot handle this case if you have the multiple file 
descriptors (possibly in multiple processes) referring to the same file. That 
is the reason why append mode exists - to let the kernel do the arbitration.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner added a comment.

Also, for the record, if you specify the threadsafe logging option, it already 
does put this in a mutex, so there should be no issue.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner added a comment.

In https://reviews.llvm.org/D25783#574679, @labath wrote:

> I am not sure the "append" thing is actually a "fix". I consider it more like 
> a feature of the append mode. It's also nice that it guarantees atomicity of 
> writes even if two processes are writing to the same file (very useful for 
> logging, although I'm not sure if that goes through these functions). I think 
> we should keep the original behavior of append mode, as that is the behavior 
> anyone familiar with standard file APIs will expect.


There are many other problems with this code if we want to deal with atomicity. 
 For example, the whole point of this patch was to handle short reads and 
writes.  Well, if you have a short read or a write, then reading and writing a 
subsequent chunk is not atomic.

That being said, I don't think it's hugely important here.  The worst that will 
happen is two log messages will be printed out of order.  I don't think 
anything will be corrupt or interwoven.  For example, if two threads come in at 
the same time and both compute the same file size, they will both attempt to 
write at the same offset.  One will win, the other will insert right before the 
message.  So it's possible the two could wind up reversed, but that's about it.

Only way to deal with multi-threading correctly is to put all this in a mutex.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Pavel Labath via lldb-commits]

labath added a comment.

I am not sure the "append" thing is actually a "fix". I consider it more like a 
feature of the append mode. It's also nice that it guarantees atomicity of 
writes even if two processes are writing to the same file (very useful for 
logging, although I'm not sure if that goes through these functions). I think 
we should keep the original behavior of append mode, as that is the behavior 
anyone familiar with standard file APIs will expect.


https://reviews.llvm.org/D25783



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

[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Zachary Turner via lldb-commits]

zturner created this revision.
zturner added reviewers: clayborg, beanz, EugeneBi.
zturner added a subscriber: lldb-commits.

The original motivation for this came from https://reviews.llvm.org/D25712, in 
which Eugene pointed out that `File::Read()` does not correctly handle short 
reads.  However, I felt the fix was incomplete because it left the bug in other 
functions, and the code in general could have used some cleanup since there was 
a ton of duplication, which may be what led to this bug showing up in the first 
place.

Changes in this patch are:

1. Have the normal `Read()` and `Write()` functions delegate to the versions 
that read and write with offset.
2. Supply thread-safe versions of the Windows codepaths, which were previously 
incorrect in a multi-threaded environment.
3. Delete a bunch of dead functions that are not used anywhere in LLDB.
4. Remove the apple specific path due to `MAX_READ_SIZE` and `MAX_WRITE_SIZE` 
and merge with the standard non-Windows path.

The only real tricky part about this patch was that when you open a file in 
append mode, the old version of `Write()` with no offset would write at the 
end, whereas `pwrite()` always writes at the offset you specify.  So to fix 
this I made the version of `Write()` with no offset explicitly compute the 
offset of the end of the file and pass that to the offset-version of `Write()`.


https://reviews.llvm.org/D25783

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

Index: source/Host/common/File.cpp
===
--- source/Host/common/File.cpp
+++ source/Host/common/File.cpp
@@ -367,58 +367,6 @@
   return result;
 }
 
-off_t File::SeekFromCurrent(off_t offset, Error *error_ptr) {
-  off_t result = -1;
-  if (DescriptorIsValid()) {
-result = ::lseek(m_descriptor, offset, SEEK_CUR);
-
-if (error_ptr) {
-  if (result == -1)
-error_ptr->SetErrorToErrno();
-  else
-error_ptr->Clear();
-}
-  } else if (StreamIsValid()) {
-result = ::fseek(m_stream, offset, SEEK_CUR);
-
-if (error_ptr) {
-  if (result == -1)
-error_ptr->SetErrorToErrno();
-  else
-error_ptr->Clear();
-}
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
-  }
-  return result;
-}
-
-off_t File::SeekFromEnd(off_t offset, Error *error_ptr) {
-  off_t result = -1;
-  if (DescriptorIsValid()) {
-result = ::lseek(m_descriptor, offset, SEEK_END);
-
-if (error_ptr) {
-  if (result == -1)
-error_ptr->SetErrorToErrno();
-  else
-error_ptr->Clear();
-}
-  } else if (StreamIsValid()) {
-result = ::fseek(m_stream, offset, SEEK_END);
-
-if (error_ptr) {
-  if (result == -1)
-error_ptr->SetErrorToErrno();
-  else
-error_ptr->Clear();
-}
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
-  }
-  return result;
-}
-
 Error File::Flush() {
   Error error;
   if (StreamIsValid()) {
@@ -435,218 +383,103 @@
   return error;
 }
 
-Error File::Sync() {
-  Error error;
-  if (DescriptorIsValid()) {
-#ifdef _WIN32
-int err = FlushFileBuffers((HANDLE)_get_osfhandle(m_descriptor));
-if (err == 0)
-  error.SetErrorToGenericError();
-#else
-int err = 0;
-do {
-  err = ::fsync(m_descriptor);
-} while (err == -1 && errno == EINTR);
-
-if (err == -1)
-  error.SetErrorToErrno();
-#endif
-  } else {
-error.SetErrorString("invalid file handle");
-  }
-  return error;
-}
-
 #if defined(__APPLE__)
 // Darwin kernels only can read/write <= INT_MAX bytes
-#define MAX_READ_SIZE INT_MAX
-#define MAX_WRITE_SIZE INT_MAX
+constexpr size_t MAX_READ_SIZE = INT_MAX;
+constexpr size_t MAX_WRITE_SIZE = INT_MAX;
+#elif defined(LLVM_ON_WIN32)
+constexpr size_t MAX_READ_SIZE = ULONG_MAX;
+constexpr size_t MAX_WRITE_SIZE = ULONG_MAX;
+#else
+constexpr size_t MAX_READ_SIZE = SIZE_MAX;
+constexpr size_t MAX_WRITE_SIZE = SIZE_MAX;
 #endif
 
 Error File::Read(void *buf, size_t &num_bytes) {
-  Error error;
-
-#if defined(MAX_READ_SIZE)
-  if (num_bytes > MAX_READ_SIZE) {
-uint8_t *p = (uint8_t *)buf;
-size_t bytes_left = num_bytes;
-// Init the num_bytes read to zero
+  int fd = GetDescriptor();
+  if (fd == kInvalidDescriptor) {
 num_bytes = 0;
-
-while (bytes_left > 0) {
-  size_t curr_num_bytes;
-  if (bytes_left > MAX_READ_SIZE)
-curr_num_bytes = MAX_READ_SIZE;
-  else
-curr_num_bytes = bytes_left;
-
-  error = Read(p + num_bytes, curr_num_bytes);
-
-  // Update how many bytes were read
-  num_bytes += curr_num_bytes;
-  if (bytes_left < curr_num_bytes)
-bytes_left = 0;
-  else
-bytes_left -= curr_num_bytes;
-
-  if (error.Fail())
-break;
-}
-return error;
+return Error("invalid file handle");
   }
-#endif
-
-  ssize_t bytes_read = -1;
-  if (DescriptorIsValid()) {
-do {
-  bytes_read = ::read(m_d

[Lldb-commits] [PATCH] D25726: Improve the libstdc++ smart pointer formatters

[Enrico Granata via lldb-commits]

granata.enrico added a comment.

I see you already got a bunch of feedback on specific items. The overall idea 
looks good to me. I'll try to delve a little deeper in the code ASAP (I was out 
for a couple days and have some backlog...), but should be good to go assuming 
you address the feedback you already got + make sure tests work!


https://reviews.llvm.org/D25726



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

[Lldb-commits] [PATCH] D25734: Add data formatter for libstdc++ unique_ptr

[Enrico Granata via lldb-commits]

granata.enrico added a comment.

I see you already got a bunch of feedback on specific items. The overall idea 
looks good to me. I'll try to delve a little deeper in the code ASAP (I was out 
for a couple days and have some backlog...), but should be good to go assuming 
you address the feedback you already got + make sure tests work!


https://reviews.llvm.org/D25734



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

Re: [Lldb-commits] [PATCH] D25734: Add data formatter for libstdc++ unique_ptr

[Jim Ingham via lldb-commits]

> On Oct 19, 2016, at 10:38 AM, Jim Ingham via lldb-commits 
>  wrote:
> 
> 
>> On Oct 19, 2016, at 6:35 AM, Pavel Labath via lldb-commits 
>>  wrote:
>> 
>> 
>> 
>> Comment at: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp:110
>> +  if (name == ConstString("ptr") || name == ConstString("pointer")) return 
>> 2;
>> +  return UINT32_MAX;
>> +}
>> 
>> ~0 ?
>> 
> 
> Everywhere else in lldb where we use the maximum of the size returned to mean 
> some error condition, we use UINT32_MAX.  I actually think that's clearer, 
> and looks similar to places where we have other illegal value defines, 
> whereas this just looks like some odd computation.  I don't think this is a 
> good change, and certainly not done in just one place.

To be pedantic:

UINT32_MAX -> _MAX...

Jim

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

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

Re: [Lldb-commits] [PATCH] D25734: Add data formatter for libstdc++ unique_ptr

[Jim Ingham via lldb-commits]

> On Oct 19, 2016, at 6:35 AM, Pavel Labath via lldb-commits 
>  wrote:
> 
> 
> 
> Comment at: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp:110
> +  if (name == ConstString("ptr") || name == ConstString("pointer")) return 2;
> +  return UINT32_MAX;
> +}
> 
> ~0 ?
> 

Everywhere else in lldb where we use the maximum of the size returned to mean 
some error condition, we use UINT32_MAX.  I actually think that's clearer, and 
looks similar to places where we have other illegal value defines, whereas this 
just looks like some odd computation.  I don't think this is a good change, and 
certainly not done in just one place.

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

Re: [Lldb-commits] [PATCH] D25057: Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints

[Jim Ingham via lldb-commits]

> On Oct 4, 2016, at 9:28 AM, Pavel Labath via lldb-commits 
>  wrote:
> 
> Also, apparently lldb has a bug, where it mishandles the case where you do a 
> (user-level) single step over an instruction triggering a watchpoint. That 
> would be pretty important to fix as well.

Do you have a reference for this?  In a trivial case, it works correctly on 
MacOS so far as I can tell:

(lldb) si
Process 99358 stopped
* thread #1: tid = 0x148d67e, function: main , stop reason = instruction step 
into
frame #0: 0x00010f5e changeit`main at changeit.c:9
   6
   7  printf("my_var is: %d.\n", my_var);
   8
-> 9  my_var = 20;
   10   
   11 printf("my_var is: %d.\n", my_var);
   12   
changeit`main:
->  0x10f5e <+46>: movl   $0x14, -0x8(%rbp)
0x10f65 <+53>: movl   -0x8(%rbp), %esi
0x10f68 <+56>: movl   %eax, -0xc(%rbp)
0x10f6b <+59>: movb   $0x0, %al
(lldb) si

Watchpoint 1 hit:
old value: 10
new value: 20
Process 99358 stopped
* thread #1: tid = 0x148d67e, function: main , stop reason = watchpoint 1
frame #0: 0x00010f65 changeit`main at changeit.c:11
   8
   9  my_var = 20;
   10   
-> 11 printf("my_var is: %d.\n", my_var);
   12   
   13 return 0;
   14   }
changeit`main:
->  0x10f65 <+53>: movl   -0x8(%rbp), %esi
0x10f68 <+56>: movl   %eax, -0xc(%rbp)
0x10f6b <+59>: movb   $0x0, %al
0x10f6d <+61>: callq  0x10f80   ; symbol stub for: 
printf

Jim

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

[Lldb-commits] [PATCH] D25756: FreeBSD ARM support for software single step.

[Pavel Labath via lldb-commits]

labath added a reviewer: emaste.
labath added a comment.

No, I'm saying someone *should*. :P

Ed looked into that at some point but, I don't think get got too far with it. 
Adding @emaste, who should probably review this.


Repository:
  rL LLVM

https://reviews.llvm.org/D25756



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

[Lldb-commits] [PATCH] D25756: FreeBSD ARM support for software single step.

[Dmitry Mikulin via lldb-commits]

dmikulin added a comment.

Are you saying someone is working on switching FreeBSD to lldb-server 
debugging? Can I get my hands on this patch?


Repository:
  rL LLVM

https://reviews.llvm.org/D25756



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

Re: [Lldb-commits] [PATCH] D25681: [PseudoTerminal] Fix PseudoTerminal MSVC release crash

[Zachary Turner via lldb-commits]

It will return true in this case. Should it be returning false to indicate
failure?
On Tue, Oct 18, 2016 at 11:26 PM Carlo Kok  wrote:

> fwiw this looks good to me, a much better fix than I had. but it'z
> zturner who objected to my approach.
>
> On 2016-10-17 17:51, Rudy Pons via lldb-commits wrote:
> > Ilod created this revision.
> > Ilod added reviewers: zturner, carlokok.
> > Ilod added a subscriber: lldb-commits.
> >
> > Since r278177, the Posix functions in
> PseudoTerminal::OpenFirstAvailableMaster on windows are now inlined
> LLVM_UNREACHABLE instead of return 0.
> > This causes __assume(0) to be inlined in OpenFirstAvailableMaster,
> allowing the optimizer to change code because this function should never be
> executed. In particular, on Visual 2015 Update 3 Win32 Release builds, the
> optimizer skips the if (error_str) test, causing a crash if error_str is
> nullptr.
> > The added #if !defined(LLDB_DISABLE_POSIX) restore the previous
> behaviour (which was doing nothing and returning true, as every function
> was returning 0, and prevent crashes.
> > The crash was 100% when launching a test x86 executable (built with
> clang, linked with lld-link) in lldb.
> > I don't know if there is another fix in progress (not calling the
> function on Win32?), but it seems to be called from several places, so it
> may be simpler to fix it in PseudoTerminal.
> >
> >
> > https://reviews.llvm.org/D25681
> >
> > Files:
> >   source/Utility/PseudoTerminal.cpp
> >
> >
> > Index: source/Utility/PseudoTerminal.cpp
> > ===
> > --- source/Utility/PseudoTerminal.cpp
> > +++ source/Utility/PseudoTerminal.cpp
> > @@ -88,6 +88,7 @@
> >if (error_str)
> >  error_str[0] = '\0';
> >
> > +#if !defined(LLDB_DISABLE_POSIX)
> >// Open the master side of a pseudo terminal
> >m_master_fd = ::posix_openpt(oflag);
> >if (m_master_fd < 0) {
> > @@ -111,6 +112,7 @@
> >  CloseMasterFileDescriptor();
> >  return false;
> >}
> > +#endif
> >
> >return true;
> >  }
> >
> >
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >
>
> --
> Carlo Kok
> RemObjects Software
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r284601 - Simplify GetGlobalProperties functions of Thread/Process/Target

[Pavel Labath via lldb-commits]

I'll put that on my list. :)

On 19 October 2016 at 16:36, Zachary Turner  wrote:
> This was using call_once because msvc didn't support function local static
> thread safe initialization. But it does now in msvc 2015.
>
> Can you fix HostInfoBase `g_flags` while you're at it?
>
> On Wed, Oct 19, 2016 at 8:21 AM Pavel Labath via lldb-commits
>  wrote:
>>
>> Author: labath
>> Date: Wed Oct 19 10:12:45 2016
>> New Revision: 284601
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=284601&view=rev
>> Log:
>> Simplify GetGlobalProperties functions of Thread/Process/Target
>>
>> Summary:
>> "Initialization of function-local statics is guaranteed to occur only once
>> even when called from
>> multiple threads, and may be more efficient than the equivalent code using
>> std::call_once."
>> 
>>
>> I'd add that it's also more readable.
>>
>> Reviewers: clayborg, zturner
>>
>> Subscribers: lldb-commits
>>
>> Differential Revision: http://reviews.llvm.org/D17710
>>
>> Modified:
>> lldb/trunk/source/Target/Process.cpp
>> lldb/trunk/source/Target/Target.cpp
>> lldb/trunk/source/Target/Thread.cpp
>>
>> Modified: lldb/trunk/source/Target/Process.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=284601&r1=284600&r2=284601&view=diff
>>
>> ==
>> --- lldb/trunk/source/Target/Process.cpp (original)
>> +++ lldb/trunk/source/Target/Process.cpp Wed Oct 19 10:12:45 2016
>> @@ -812,11 +812,8 @@ Process::~Process() {
>>  const ProcessPropertiesSP &Process::GetGlobalProperties() {
>>// NOTE: intentional leak so we don't crash if global destructor chain
>> gets
>>// called as other threads still use the result of this function
>> -  static ProcessPropertiesSP *g_settings_sp_ptr = nullptr;
>> -  static std::once_flag g_once_flag;
>> -  std::call_once(g_once_flag, []() {
>> -g_settings_sp_ptr = new ProcessPropertiesSP(new
>> ProcessProperties(nullptr));
>> -  });
>> +  static ProcessPropertiesSP *g_settings_sp_ptr =
>> +  new ProcessPropertiesSP(new ProcessProperties(nullptr));
>>return *g_settings_sp_ptr;
>>  }
>>
>>
>> Modified: lldb/trunk/source/Target/Target.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=284601&r1=284600&r2=284601&view=diff
>>
>> ==
>> --- lldb/trunk/source/Target/Target.cpp (original)
>> +++ lldb/trunk/source/Target/Target.cpp Wed Oct 19 10:12:45 2016
>> @@ -2643,11 +2643,8 @@ void Target::RunStopHooks() {
>>  const TargetPropertiesSP &Target::GetGlobalProperties() {
>>// NOTE: intentional leak so we don't crash if global destructor chain
>> gets
>>// called as other threads still use the result of this function
>> -  static TargetPropertiesSP *g_settings_sp_ptr = nullptr;
>> -  static std::once_flag g_once_flag;
>> -  std::call_once(g_once_flag, []() {
>> -g_settings_sp_ptr = new TargetPropertiesSP(new
>> TargetProperties(nullptr));
>> -  });
>> +  static TargetPropertiesSP *g_settings_sp_ptr =
>> +  new TargetPropertiesSP(new TargetProperties(nullptr));
>>return *g_settings_sp_ptr;
>>  }
>>
>>
>> Modified: lldb/trunk/source/Target/Thread.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cpp?rev=284601&r1=284600&r2=284601&view=diff
>>
>> ==
>> --- lldb/trunk/source/Target/Thread.cpp (original)
>> +++ lldb/trunk/source/Target/Thread.cpp Wed Oct 19 10:12:45 2016
>> @@ -58,11 +58,8 @@ using namespace lldb_private;
>>  const ThreadPropertiesSP &Thread::GetGlobalProperties() {
>>// NOTE: intentional leak so we don't crash if global destructor chain
>> gets
>>// called as other threads still use the result of this function
>> -  static ThreadPropertiesSP *g_settings_sp_ptr = nullptr;
>> -  static std::once_flag g_once_flag;
>> -  std::call_once(g_once_flag, []() {
>> -g_settings_sp_ptr = new ThreadPropertiesSP(new
>> ThreadProperties(true));
>> -  });
>> +  static ThreadPropertiesSP *g_settings_sp_ptr =
>> +  new ThreadPropertiesSP(new ThreadProperties(true));
>>return *g_settings_sp_ptr;
>>  }
>>
>>
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r284601 - Simplify GetGlobalProperties functions of Thread/Process/Target

[Zachary Turner via lldb-commits]

This was using call_once because msvc didn't support function local static
thread safe initialization. But it does now in msvc 2015.

Can you fix HostInfoBase `g_flags` while you're at it?
On Wed, Oct 19, 2016 at 8:21 AM Pavel Labath via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: labath
> Date: Wed Oct 19 10:12:45 2016
> New Revision: 284601
>
> URL: http://llvm.org/viewvc/llvm-project?rev=284601&view=rev
> Log:
> Simplify GetGlobalProperties functions of Thread/Process/Target
>
> Summary:
> "Initialization of function-local statics is guaranteed to occur only once
> even when called from
> multiple threads, and may be more efficient than the equivalent code using
> std::call_once."
> 
>
> I'd add that it's also more readable.
>
> Reviewers: clayborg, zturner
>
> Subscribers: lldb-commits
>
> Differential Revision: http://reviews.llvm.org/D17710
>
> Modified:
> lldb/trunk/source/Target/Process.cpp
> lldb/trunk/source/Target/Target.cpp
> lldb/trunk/source/Target/Thread.cpp
>
> Modified: lldb/trunk/source/Target/Process.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=284601&r1=284600&r2=284601&view=diff
>
> ==
> --- lldb/trunk/source/Target/Process.cpp (original)
> +++ lldb/trunk/source/Target/Process.cpp Wed Oct 19 10:12:45 2016
> @@ -812,11 +812,8 @@ Process::~Process() {
>  const ProcessPropertiesSP &Process::GetGlobalProperties() {
>// NOTE: intentional leak so we don't crash if global destructor chain
> gets
>// called as other threads still use the result of this function
> -  static ProcessPropertiesSP *g_settings_sp_ptr = nullptr;
> -  static std::once_flag g_once_flag;
> -  std::call_once(g_once_flag, []() {
> -g_settings_sp_ptr = new ProcessPropertiesSP(new
> ProcessProperties(nullptr));
> -  });
> +  static ProcessPropertiesSP *g_settings_sp_ptr =
> +  new ProcessPropertiesSP(new ProcessProperties(nullptr));
>return *g_settings_sp_ptr;
>  }
>
>
> Modified: lldb/trunk/source/Target/Target.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=284601&r1=284600&r2=284601&view=diff
>
> ==
> --- lldb/trunk/source/Target/Target.cpp (original)
> +++ lldb/trunk/source/Target/Target.cpp Wed Oct 19 10:12:45 2016
> @@ -2643,11 +2643,8 @@ void Target::RunStopHooks() {
>  const TargetPropertiesSP &Target::GetGlobalProperties() {
>// NOTE: intentional leak so we don't crash if global destructor chain
> gets
>// called as other threads still use the result of this function
> -  static TargetPropertiesSP *g_settings_sp_ptr = nullptr;
> -  static std::once_flag g_once_flag;
> -  std::call_once(g_once_flag, []() {
> -g_settings_sp_ptr = new TargetPropertiesSP(new
> TargetProperties(nullptr));
> -  });
> +  static TargetPropertiesSP *g_settings_sp_ptr =
> +  new TargetPropertiesSP(new TargetProperties(nullptr));
>return *g_settings_sp_ptr;
>  }
>
>
> Modified: lldb/trunk/source/Target/Thread.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cpp?rev=284601&r1=284600&r2=284601&view=diff
>
> ==
> --- lldb/trunk/source/Target/Thread.cpp (original)
> +++ lldb/trunk/source/Target/Thread.cpp Wed Oct 19 10:12:45 2016
> @@ -58,11 +58,8 @@ using namespace lldb_private;
>  const ThreadPropertiesSP &Thread::GetGlobalProperties() {
>// NOTE: intentional leak so we don't crash if global destructor chain
> gets
>// called as other threads still use the result of this function
> -  static ThreadPropertiesSP *g_settings_sp_ptr = nullptr;
> -  static std::once_flag g_once_flag;
> -  std::call_once(g_once_flag, []() {
> -g_settings_sp_ptr = new ThreadPropertiesSP(new
> ThreadProperties(true));
> -  });
> +  static ThreadPropertiesSP *g_settings_sp_ptr =
> +  new ThreadPropertiesSP(new ThreadProperties(true));
>return *g_settings_sp_ptr;
>  }
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D17710: Simplify GetGlobalProperties functions of Thread/Process/Target

[Pavel Labath via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL284601: Simplify GetGlobalProperties functions of 
Thread/Process/Target (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D17710?vs=49356&id=75157#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D17710

Files:
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/source/Target/Target.cpp
  lldb/trunk/source/Target/Thread.cpp


Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -2643,11 +2643,8 @@
 const TargetPropertiesSP &Target::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static TargetPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new TargetPropertiesSP(new TargetProperties(nullptr));
-  });
+  static TargetPropertiesSP *g_settings_sp_ptr =
+  new TargetPropertiesSP(new TargetProperties(nullptr));
   return *g_settings_sp_ptr;
 }
 
Index: lldb/trunk/source/Target/Thread.cpp
===
--- lldb/trunk/source/Target/Thread.cpp
+++ lldb/trunk/source/Target/Thread.cpp
@@ -58,11 +58,8 @@
 const ThreadPropertiesSP &Thread::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static ThreadPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new ThreadPropertiesSP(new ThreadProperties(true));
-  });
+  static ThreadPropertiesSP *g_settings_sp_ptr =
+  new ThreadPropertiesSP(new ThreadProperties(true));
   return *g_settings_sp_ptr;
 }
 
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -812,11 +812,8 @@
 const ProcessPropertiesSP &Process::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static ProcessPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new ProcessPropertiesSP(new 
ProcessProperties(nullptr));
-  });
+  static ProcessPropertiesSP *g_settings_sp_ptr =
+  new ProcessPropertiesSP(new ProcessProperties(nullptr));
   return *g_settings_sp_ptr;
 }
 


Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -2643,11 +2643,8 @@
 const TargetPropertiesSP &Target::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static TargetPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new TargetPropertiesSP(new TargetProperties(nullptr));
-  });
+  static TargetPropertiesSP *g_settings_sp_ptr =
+  new TargetPropertiesSP(new TargetProperties(nullptr));
   return *g_settings_sp_ptr;
 }
 
Index: lldb/trunk/source/Target/Thread.cpp
===
--- lldb/trunk/source/Target/Thread.cpp
+++ lldb/trunk/source/Target/Thread.cpp
@@ -58,11 +58,8 @@
 const ThreadPropertiesSP &Thread::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static ThreadPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new ThreadPropertiesSP(new ThreadProperties(true));
-  });
+  static ThreadPropertiesSP *g_settings_sp_ptr =
+  new ThreadPropertiesSP(new ThreadProperties(true));
   return *g_settings_sp_ptr;
 }
 
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -812,11 +812,8 @@
 const ProcessPropertiesSP &Process::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static ProcessPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new ProcessPropertiesSP(new

[Lldb-commits] [PATCH] D25677: Minidump plugin: redesign the x86_64 register context

[Pavel Labath via lldb-commits]

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

Looks much better, thanks. :)


https://reviews.llvm.org/D25677



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

[Lldb-commits] [lldb] r284601 - Simplify GetGlobalProperties functions of Thread/Process/Target

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Oct 19 10:12:45 2016
New Revision: 284601

URL: http://llvm.org/viewvc/llvm-project?rev=284601&view=rev
Log:
Simplify GetGlobalProperties functions of Thread/Process/Target

Summary:
"Initialization of function-local statics is guaranteed to occur only once even 
when called from
multiple threads, and may be more efficient than the equivalent code using 
std::call_once."


I'd add that it's also more readable.

Reviewers: clayborg, zturner

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D17710

Modified:
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Target/Target.cpp
lldb/trunk/source/Target/Thread.cpp

Modified: lldb/trunk/source/Target/Process.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=284601&r1=284600&r2=284601&view=diff
==
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Wed Oct 19 10:12:45 2016
@@ -812,11 +812,8 @@ Process::~Process() {
 const ProcessPropertiesSP &Process::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static ProcessPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new ProcessPropertiesSP(new 
ProcessProperties(nullptr));
-  });
+  static ProcessPropertiesSP *g_settings_sp_ptr =
+  new ProcessPropertiesSP(new ProcessProperties(nullptr));
   return *g_settings_sp_ptr;
 }
 

Modified: lldb/trunk/source/Target/Target.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=284601&r1=284600&r2=284601&view=diff
==
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Wed Oct 19 10:12:45 2016
@@ -2643,11 +2643,8 @@ void Target::RunStopHooks() {
 const TargetPropertiesSP &Target::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static TargetPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new TargetPropertiesSP(new TargetProperties(nullptr));
-  });
+  static TargetPropertiesSP *g_settings_sp_ptr =
+  new TargetPropertiesSP(new TargetProperties(nullptr));
   return *g_settings_sp_ptr;
 }
 

Modified: lldb/trunk/source/Target/Thread.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cpp?rev=284601&r1=284600&r2=284601&view=diff
==
--- lldb/trunk/source/Target/Thread.cpp (original)
+++ lldb/trunk/source/Target/Thread.cpp Wed Oct 19 10:12:45 2016
@@ -58,11 +58,8 @@ using namespace lldb_private;
 const ThreadPropertiesSP &Thread::GetGlobalProperties() {
   // NOTE: intentional leak so we don't crash if global destructor chain gets
   // called as other threads still use the result of this function
-  static ThreadPropertiesSP *g_settings_sp_ptr = nullptr;
-  static std::once_flag g_once_flag;
-  std::call_once(g_once_flag, []() {
-g_settings_sp_ptr = new ThreadPropertiesSP(new ThreadProperties(true));
-  });
+  static ThreadPropertiesSP *g_settings_sp_ptr =
+  new ThreadPropertiesSP(new ThreadProperties(true));
   return *g_settings_sp_ptr;
 }
 


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

[Lldb-commits] [PATCH] D25677: Minidump plugin: redesign the x86_64 register context

[Dimitar Vlahovski via lldb-commits]

dvlahovski updated this revision to Diff 75154.
dvlahovski marked an inline comment as done.
dvlahovski added a comment.

Simplified writeRegister function

s/uint128_struct/Uint128/


https://reviews.llvm.org/D25677

Files:
  source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
  source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h

Index: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
===
--- source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
+++ source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -21,81 +21,145 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/Support/Endian.h"
 
 // C includes
 // C++ includes
 
 namespace lldb_private {
 
 namespace minidump {
 
-// The content of the Minidump register context is as follows:
-// (for reference see breakpad's source or WinNT.h)
-// Register parameter home addresses: (p1_home .. p6_home)
-// - uint64_t p1_home
-// - uint64_t p2_home
-// - uint64_t p3_home
-// - uint64_t p4_home
-// - uint64_t p5_home
-// - uint64_t p6_home
-//
-// - uint32_t context_flags - field that determines the layout of the structure
-// and which parts of it are populated
-// - uint32_t mx_csr
-//
-// - uint16_t cs - included if MinidumpContext_x86_64_Flags::Control
-//
-// - uint16_t ds - included if MinidumpContext_x86_64_Flags::Segments
-// - uint16_t es - included if MinidumpContext_x86_64_Flags::Segments
-// - uint16_t fs - included if MinidumpContext_x86_64_Flags::Segments
-// - uint16_t gs - included if MinidumpContext_x86_64_Flags::Segments
-//
-// - uint16_t ss - included if MinidumpContext_x86_64_Flags::Control
-// - uint32_t rflags - included if MinidumpContext_x86_64_Flags::Control
-//
-// Debug registers: (included if MinidumpContext_x86_64_Flags::DebugRegisters)
-// - uint64_t dr0
-// - uint64_t dr1
-// - uint64_t dr2
-// - uint64_t dr3
-// - uint64_t dr6
-// - uint64_t dr7
-//
-// The next 4 registers are included if MinidumpContext_x86_64_Flags::Integer
-// - uint64_t rax
-// - uint64_t rcx
-// - uint64_t rdx
-// - uint64_t rbx
-//
-// - uint64_t rsp - included if MinidumpContext_x86_64_Flags::Control
-//
-// The next 11 registers are included if MinidumpContext_x86_64_Flags::Integer
-// - uint64_t rbp
-// - uint64_t rsi
-// - uint64_t rdi
-// - uint64_t r8
-// - uint64_t r9
-// - uint64_t r10
-// - uint64_t r11
-// - uint64_t r12
-// - uint64_t r13
-// - uint64_t r14
-// - uint64_t r15
-//
-// - uint64_t rip - included if MinidumpContext_x86_64_Flags::Control
-//
-// TODO: add floating point registers here
-
 // This function receives an ArrayRef pointing to the bytes of the Minidump
 // register context and returns a DataBuffer that's ordered by the offsets
 // specified in the RegisterInfoInterface argument
 // This way we can reuse the already existing register contexts
 lldb::DataBufferSP
 ConvertMinidumpContextToRegIface(llvm::ArrayRef source_data,
  RegisterInfoInterface *target_reg_interface);
 
+struct Uint128 {
+  llvm::support::ulittle64_t high;
+  llvm::support::ulittle64_t low;
+};
+
+// Reference: see breakpad/crashpad source or WinNT.h
+struct MinidumpXMMSaveArea32AMD64 {
+  llvm::support::ulittle16_t control_word;
+  llvm::support::ulittle16_t status_word;
+  uint8_t tag_word;
+  uint8_t reserved1;
+  llvm::support::ulittle16_t error_opcode;
+  llvm::support::ulittle32_t error_offset;
+  llvm::support::ulittle16_t error_selector;
+  llvm::support::ulittle16_t reserved2;
+  llvm::support::ulittle32_t data_offset;
+  llvm::support::ulittle16_t data_selector;
+  llvm::support::ulittle16_t reserved3;
+  llvm::support::ulittle32_t mx_csr;
+  llvm::support::ulittle32_t mx_csr_mask;
+  Uint128 float_registers[8];
+  Uint128 xmm_registers[16];
+  uint8_t reserved4[96];
+};
+
+struct MinidumpContext_x86_64 {
+  // Register parameter home addresses.
+  llvm::support::ulittle64_t p1_home;
+  llvm::support::ulittle64_t p2_home;
+  llvm::support::ulittle64_t p3_home;
+  llvm::support::ulittle64_t p4_home;
+  llvm::support::ulittle64_t p5_home;
+  llvm::support::ulittle64_t p6_home;
+
+  // The context_flags field determines and which parts
+  // of the structure are populated (have valid values)
+  llvm::support::ulittle32_t context_flags;
+  llvm::support::ulittle32_t mx_csr;
+
+  // The next register is included with
+  // MinidumpContext_x86_64_Flags::Control
+  llvm::support::ulittle16_t cs;
+
+  // The next 4 registers are included with
+  // MinidumpContext_x86_64_Flags::Segments
+  llvm::support::ulittle16_t ds;
+  llvm::support::ulittle16_t es;
+  llvm::support::ulittle16_t fs;
+  llvm::support::ulittle16_t gs;
+
+  // The next 2 registers are included with
+  // MinidumpContext_x86_64_Flags::Control
+  llvm::support::ulittle16_t ss;
+  llvm::support::ulittle32_t eflags;
+
+  // The next 6 registers are included with
+  // MinidumpContext_x8

[Lldb-commits] [PATCH] D25393: Remove IntervalTimer class

[Pavel Labath via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL284599: Remove IntervalTimer class (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D25393?vs=74015&id=75153#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25393

Files:
  lldb/trunk/include/lldb/Core/Timer.h


Index: lldb/trunk/include/lldb/Core/Timer.h
===
--- lldb/trunk/include/lldb/Core/Timer.h
+++ lldb/trunk/include/lldb/Core/Timer.h
@@ -85,47 +85,6 @@
   DISALLOW_COPY_AND_ASSIGN(Timer);
 };
 
-class IntervalTimer {
-public:
-  IntervalTimer() : m_start(TimeValue::Now()) {}
-
-  ~IntervalTimer() = default;
-
-  uint64_t GetElapsedNanoSeconds() const { return TimeValue::Now() - m_start; }
-
-  void Reset() { m_start = TimeValue::Now(); }
-
-  int PrintfElapsed(const char *format, ...)
-  __attribute__((format(printf, 2, 3))) {
-TimeValue now(TimeValue::Now());
-const uint64_t elapsed_nsec = now - m_start;
-const char *unit = nullptr;
-float elapsed_value;
-if (elapsed_nsec < 1000) {
-  unit = "ns";
-  elapsed_value = (float)elapsed_nsec;
-} else if (elapsed_nsec < 100) {
-  unit = "us";
-  elapsed_value = (float)elapsed_nsec / 1000.0f;
-} else if (elapsed_nsec < 10) {
-  unit = "ms";
-  elapsed_value = (float)elapsed_nsec / 100.0f;
-} else {
-  unit = "sec";
-  elapsed_value = (float)elapsed_nsec / 10.0f;
-}
-int result = printf("%3.2f %s: ", elapsed_value, unit);
-va_list args;
-va_start(args, format);
-result += vprintf(format, args);
-va_end(args);
-return result;
-  }
-
-protected:
-  TimeValue m_start;
-};
-
 } // namespace lldb_private
 
 #endif // liblldb_Timer_h_


Index: lldb/trunk/include/lldb/Core/Timer.h
===
--- lldb/trunk/include/lldb/Core/Timer.h
+++ lldb/trunk/include/lldb/Core/Timer.h
@@ -85,47 +85,6 @@
   DISALLOW_COPY_AND_ASSIGN(Timer);
 };
 
-class IntervalTimer {
-public:
-  IntervalTimer() : m_start(TimeValue::Now()) {}
-
-  ~IntervalTimer() = default;
-
-  uint64_t GetElapsedNanoSeconds() const { return TimeValue::Now() - m_start; }
-
-  void Reset() { m_start = TimeValue::Now(); }
-
-  int PrintfElapsed(const char *format, ...)
-  __attribute__((format(printf, 2, 3))) {
-TimeValue now(TimeValue::Now());
-const uint64_t elapsed_nsec = now - m_start;
-const char *unit = nullptr;
-float elapsed_value;
-if (elapsed_nsec < 1000) {
-  unit = "ns";
-  elapsed_value = (float)elapsed_nsec;
-} else if (elapsed_nsec < 100) {
-  unit = "us";
-  elapsed_value = (float)elapsed_nsec / 1000.0f;
-} else if (elapsed_nsec < 10) {
-  unit = "ms";
-  elapsed_value = (float)elapsed_nsec / 100.0f;
-} else {
-  unit = "sec";
-  elapsed_value = (float)elapsed_nsec / 10.0f;
-}
-int result = printf("%3.2f %s: ", elapsed_value, unit);
-va_list args;
-va_start(args, format);
-result += vprintf(format, args);
-va_end(args);
-return result;
-  }
-
-protected:
-  TimeValue m_start;
-};
-
 } // namespace lldb_private
 
 #endif // liblldb_Timer_h_
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r284599 - Remove IntervalTimer class

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Oct 19 10:01:22 2016
New Revision: 284599

URL: http://llvm.org/viewvc/llvm-project?rev=284599&view=rev
Log:
Remove IntervalTimer class

Summary:
it was added back in 2013, but there are no uses of it. I started refactoring
it, but then it occured to me it would better to delete it.

Reviewers: clayborg, zturner

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/Core/Timer.h

Modified: lldb/trunk/include/lldb/Core/Timer.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Timer.h?rev=284599&r1=284598&r2=284599&view=diff
==
--- lldb/trunk/include/lldb/Core/Timer.h (original)
+++ lldb/trunk/include/lldb/Core/Timer.h Wed Oct 19 10:01:22 2016
@@ -85,47 +85,6 @@ private:
   DISALLOW_COPY_AND_ASSIGN(Timer);
 };
 
-class IntervalTimer {
-public:
-  IntervalTimer() : m_start(TimeValue::Now()) {}
-
-  ~IntervalTimer() = default;
-
-  uint64_t GetElapsedNanoSeconds() const { return TimeValue::Now() - m_start; }
-
-  void Reset() { m_start = TimeValue::Now(); }
-
-  int PrintfElapsed(const char *format, ...)
-  __attribute__((format(printf, 2, 3))) {
-TimeValue now(TimeValue::Now());
-const uint64_t elapsed_nsec = now - m_start;
-const char *unit = nullptr;
-float elapsed_value;
-if (elapsed_nsec < 1000) {
-  unit = "ns";
-  elapsed_value = (float)elapsed_nsec;
-} else if (elapsed_nsec < 100) {
-  unit = "us";
-  elapsed_value = (float)elapsed_nsec / 1000.0f;
-} else if (elapsed_nsec < 10) {
-  unit = "ms";
-  elapsed_value = (float)elapsed_nsec / 100.0f;
-} else {
-  unit = "sec";
-  elapsed_value = (float)elapsed_nsec / 10.0f;
-}
-int result = printf("%3.2f %s: ", elapsed_value, unit);
-va_list args;
-va_start(args, format);
-result += vprintf(format, args);
-va_end(args);
-return result;
-  }
-
-protected:
-  TimeValue m_start;
-};
-
 } // namespace lldb_private
 
 #endif // liblldb_Timer_h_


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

[Lldb-commits] [PATCH] D25569: Minidump plugin: functions parsing memory structures and filtering module list

[Dimitar Vlahovski via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL284593: Minidump plugin: functions parsing memory structures 
and filtering module list (authored by dvlahovski).

Changed prior to commit:
  https://reviews.llvm.org/D25569?vs=74824&id=75147#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25569

Files:
  lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
  lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
  lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
  lldb/trunk/unittests/Process/minidump/CMakeLists.txt
  lldb/trunk/unittests/Process/minidump/Inputs/fizzbuzz_wow64.dmp
  lldb/trunk/unittests/Process/minidump/Inputs/linux-x86_64_not_crashed.dmp
  lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -19,6 +19,7 @@
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataExtractor.h"
 #include "lldb/Host/FileSpec.h"
+#include "lldb/Target/MemoryRegionInfo.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
@@ -68,7 +69,11 @@
   ASSERT_EQ(1UL, thread_list.size());
 
   const MinidumpThread thread = thread_list[0];
-  ASSERT_EQ(16001UL, thread.thread_id);
+
+  EXPECT_EQ(16001UL, thread.thread_id);
+
+  llvm::ArrayRef context = parser->GetThreadContext(thread);
+  EXPECT_EQ(1232UL, context.size());
 }
 
 TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) {
@@ -131,14 +136,111 @@
   }
 }
 
+TEST_F(MinidumpParserTest, GetFilteredModuleList) {
+  SetUpData("linux-x86_64_not_crashed.dmp");
+  llvm::ArrayRef modules = parser->GetModuleList();
+  std::vector filtered_modules =
+  parser->GetFilteredModuleList();
+  EXPECT_EQ(10UL, modules.size());
+  EXPECT_EQ(9UL, filtered_modules.size());
+  // EXPECT_GT(modules.size(), filtered_modules.size());
+  bool found = false;
+  for (size_t i = 0; i < filtered_modules.size(); ++i) {
+llvm::Optional name =
+parser->GetMinidumpString(filtered_modules[i]->module_name_rva);
+ASSERT_TRUE(name.hasValue());
+if (name.getValue() == "/tmp/test/linux-x86_64_not_crashed") {
+  ASSERT_FALSE(found) << "There should be only one module with this name "
+ "in the filtered module list";
+  found = true;
+  ASSERT_EQ(0x40UL, filtered_modules[i]->base_of_image);
+}
+  }
+}
+
 TEST_F(MinidumpParserTest, GetExceptionStream) {
   SetUpData("linux-x86_64.dmp");
   const MinidumpExceptionStream *exception_stream =
   parser->GetExceptionStream();
   ASSERT_NE(nullptr, exception_stream);
   ASSERT_EQ(11UL, exception_stream->exception_record.exception_code);
 }
 
+void check_mem_range_exists(std::unique_ptr &parser,
+const uint64_t range_start,
+const uint64_t range_size) {
+  llvm::Optional range = parser->FindMemoryRange(range_start);
+  ASSERT_TRUE(range.hasValue()) << "There is no range containing this address";
+  EXPECT_EQ(range_start, range->start);
+  EXPECT_EQ(range_start + range_size, range->start + range->range_ref.size());
+}
+
+TEST_F(MinidumpParserTest, FindMemoryRange) {
+  SetUpData("linux-x86_64.dmp");
+  // There are two memory ranges in the file (size is in bytes, decimal):
+  // 1) 0x401d46 256
+  // 2) 0x7ffceb34a000 12288
+  EXPECT_FALSE(parser->FindMemoryRange(0x00).hasValue());
+  EXPECT_FALSE(parser->FindMemoryRange(0x2a).hasValue());
+
+  check_mem_range_exists(parser, 0x401d46, 256);
+  EXPECT_FALSE(parser->FindMemoryRange(0x401d46 + 256).hasValue());
+
+  check_mem_range_exists(parser, 0x7ffceb34a000, 12288);
+  EXPECT_FALSE(parser->FindMemoryRange(0x7ffceb34a000 + 12288).hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetMemory) {
+  SetUpData("linux-x86_64.dmp");
+
+  EXPECT_EQ(128UL, parser->GetMemory(0x401d46, 128).size());
+  EXPECT_EQ(256UL, parser->GetMemory(0x401d46, 512).size());
+
+  EXPECT_EQ(12288UL, parser->GetMemory(0x7ffceb34a000, 12288).size());
+  EXPECT_EQ(1024UL, parser->GetMemory(0x7ffceb34a000, 1024).size());
+
+  EXPECT_TRUE(parser->GetMemory(0x50, 512).empty());
+}
+
+TEST_F(MinidumpParserTest, FindMemoryRangeWithFullMemoryMinidump) {
+  SetUpData("fizzbuzz_wow64.dmp");
+
+  // There are a lot of ranges in the file, just testing with some of them
+  EXPECT_FALSE(parser->FindMemoryRange(0x00).hasValue());
+  EXPECT_FALSE(parser->FindMemoryRange(0x2a).hasValue());
+  check_mem_range_exists(parser, 0x1, 65536); // first range
+  check_mem_range_exists(parser, 0x4, 4096);
+  EXPECT_FALSE(parser->FindMemoryRange(0x4 + 4096).hasValue());
+  check_mem_range_exists(parser, 0x77c12000, 8192);
+  check_mem_range_exists(parser, 0x7ffe, 4096); // last range
+  EXPECT_FALSE(pars

[Lldb-commits] [lldb] r284593 - Minidump plugin: functions parsing memory structures and filtering module list

[Dimitar Vlahovski via lldb-commits]

Author: dvlahovski
Date: Wed Oct 19 09:14:18 2016
New Revision: 284593

URL: http://llvm.org/viewvc/llvm-project?rev=284593&view=rev
Log:
Minidump plugin: functions parsing memory structures and filtering module list

Summary:
Now the Minidump parser can parse the:
1) MemoryInfoList - containing region info about memory ranges (readable,
writable, executable)
2) Memory64List - this is the stuct used when the Minidump is a
full-memory one.
3) Adding filtering of the module list (shared libraries list) - there
can be mutliple records in the module list under the same name but with
different load address (e.g. when the binary has non contigious
sections). FilterModuleList eliminates the duplicated modules, leaving
the one with the lowest load addr.

Added unit tests for everything.

Reviewers: labath, zturner

Subscribers: beanz, mgorny, modocache, lldb-commits, amccarth

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


Added:
lldb/trunk/unittests/Process/minidump/Inputs/fizzbuzz_wow64.dmp
  - copied unchanged from r284590, 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/wow64_minidump/fizzbuzz_wow64.dmp
lldb/trunk/unittests/Process/minidump/Inputs/linux-x86_64_not_crashed.dmp   
(with props)
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp
lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h
lldb/trunk/unittests/Process/minidump/CMakeLists.txt
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=284593&r1=284592&r2=284593&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Wed Oct 19 
09:14:18 2016
@@ -11,8 +11,11 @@
 #include "MinidumpParser.h"
 
 // Other libraries and framework includes
+#include "lldb/Target/MemoryRegionInfo.h"
+
 // C includes
 // C++ includes
+#include 
 
 using namespace lldb_private;
 using namespace minidump;
@@ -100,6 +103,14 @@ llvm::ArrayRef MinidumpP
   return MinidumpThread::ParseThreadList(data);
 }
 
+llvm::ArrayRef
+MinidumpParser::GetThreadContext(const MinidumpThread &td) {
+  if (td.thread_context.rva + td.thread_context.data_size > GetData().size())
+return llvm::None;
+
+  return GetData().slice(td.thread_context.rva, td.thread_context.data_size);
+}
+
 const MinidumpSystemInfo *MinidumpParser::GetSystemInfo() {
   llvm::ArrayRef data = GetStream(MinidumpStreamType::SystemInfo);
 
@@ -216,6 +227,42 @@ llvm::ArrayRef MinidumpP
   return MinidumpModule::ParseModuleList(data);
 }
 
+std::vector MinidumpParser::GetFilteredModuleList() {
+  llvm::ArrayRef modules = GetModuleList();
+  // mapping module_name to pair(load_address, pointer to module struct in
+  // memory)
+  llvm::StringMap> lowest_addr;
+
+  std::vector filtered_modules;
+
+  llvm::Optional name;
+  std::string module_name;
+
+  for (const auto &module : modules) {
+name = GetMinidumpString(module.module_name_rva);
+
+if (!name)
+  continue;
+
+module_name = name.getValue();
+
+auto iter = lowest_addr.end();
+bool exists;
+std::tie(iter, exists) = lowest_addr.try_emplace(
+module_name, std::make_pair(module.base_of_image, &module));
+
+if (exists && module.base_of_image < iter->second.first)
+  iter->second = std::make_pair(module.base_of_image, &module);
+  }
+
+  filtered_modules.reserve(lowest_addr.size());
+  for (const auto &module : lowest_addr) {
+filtered_modules.push_back(module.second.second);
+  }
+
+  return filtered_modules;
+}
+
 const MinidumpExceptionStream *MinidumpParser::GetExceptionStream() {
   llvm::ArrayRef data = GetStream(MinidumpStreamType::Exception);
 
@@ -224,3 +271,156 @@ const MinidumpExceptionStream *MinidumpP
 
   return MinidumpExceptionStream::Parse(data);
 }
+
+llvm::Optional
+MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
+  llvm::ArrayRef data = GetStream(MinidumpStreamType::MemoryList);
+  llvm::ArrayRef data64 = GetStream(MinidumpStreamType::Memory64List);
+
+  if (data.empty() && data64.empty())
+return llvm::None;
+
+  if (!data.empty()) {
+llvm::ArrayRef memory_list =
+MinidumpMemoryDescriptor::ParseMemoryList(data);
+
+if (memory_list.empty())
+  return llvm::None;
+
+for (const auto &memory_desc : memory_list) {
+  const MinidumpLocationDescriptor &loc_desc = memory_desc.memory;
+  const lldb::addr_t range_start = memory_desc.start_of_memory_range;
+  const size_t range_size = loc_desc.data_size;
+
+  if (loc_desc.rva + loc_desc.data_size > GetData().size())
+return llvm::None;
+
+  

[Lldb-commits] [PATCH] D25677: Minidump plugin: redesign the x86_64 register context

[Pavel Labath via lldb-commits]

labath added a comment.

Just a couple of cleanups and then I think we're ready.




Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:78
+  if ((context_flags & ControlFlag) == ControlFlag) {
+writeRegister(&context->cs, getDestRegister(result_base, lldb_cs_x86_64,
+reg_info[lldb_cs_x86_64]));

These would be cleaner, if we made writeRegister call getDestRegister 
internally.
Then these two lines would become:
```
writeRegister(&context->cs, result_base, reg_info[lldb_cs_x86_64]);
```
Note that you don't have to pass in lldb_cs_x86_64 explicitly, as that 
information can be recovered via reg_info->kinds[eRegisterKindLLDB].



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:141
+  uint128_struct legacy[8];
+  uint128_struct xmm0;
+  uint128_struct xmm1;

labath wrote:
> `uint128_struct xmm[16];` ?
> 
> I'm not that sure about that struct and it's name though, I'll need to think 
> about that.
I don't really have a good idea about struct issue. Let's at least call it 
Uint128, so that it's consistent with the rest of the struct's in this class, 
and then call it a day.


https://reviews.llvm.org/D25677



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

[Lldb-commits] [PATCH] D25734: Add data formatter for libstdc++ unique_ptr

[Pavel Labath via lldb-commits]

labath added a comment.

Looks fine with the usual comments. :)




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/Makefile:5
+
+CXXFLAGS := -O0
+USE_LIBSTDCPP := 1

Same comments as the previous patch (no CXXFLAGS, simplify no-limit-debug-info 
handling)



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py:33
+
+self.expect("frame variable nup", substrs=['nup = nullptr'])
+self.expect("frame variable iup", substrs=['iup = 123', 'object = 
123'])

Can you add a test accessing `nup.deleter`, `iup.deleter` (I guess they should 
error out?)



Comment at: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp:110
+  if (name == ConstString("ptr") || name == ConstString("pointer")) return 2;
+  return UINT32_MAX;
+}

~0 ?


https://reviews.llvm.org/D25734



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

[Lldb-commits] [PATCH] D25733: Add data formatter for libstdc++ tuple

[Pavel Labath via lldb-commits]

labath added a comment.

Just minor nits from my side.




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/tuple/Makefile:5
+
+CXXFLAGS := -O0
+USE_LIBSTDCPP := 1

I don't think this is necessary.



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/tuple/Makefile:8
+
+# clang-3.5+ outputs FullDebugInfo by default for Darwin/FreeBSD
+# targets.  Other targets do not, which causes this test to fail.

Replace with `CFLAGS_EXTRAS += $(NO_LIMIT_DEBUG_INFO_FLAGS)`



Comment at: source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp:58
+
+  ValueObjectSP valobj_sp = valobj_backend_sp->GetNonSyntheticValue();
+  if (!valobj_sp) return false;

 



Comment at: source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp:77
+  StreamString name;
+  name.Printf("[%" PRIu64 "]", m_members.size());
+  value_sp->SetName(ConstString(name.GetData()));

You can use %z now.


https://reviews.llvm.org/D25733



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

[Lldb-commits] [lldb] r284584 - [cmake] Use LLVM_CMAKE_PATH for GetSVN script

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Wed Oct 19 07:54:28 2016
New Revision: 284584

URL: http://llvm.org/viewvc/llvm-project?rev=284584&view=rev
Log:
[cmake] Use LLVM_CMAKE_PATH for GetSVN script

Use the LLVM_CMAKE_PATH variable to locate the GetSVN.cmake script.
The variable was already available in stand-alone builds, and is also
set by LLVM since r284581.

Modified:
lldb/trunk/source/CMakeLists.txt

Modified: lldb/trunk/source/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/CMakeLists.txt?rev=284584&r1=284583&r2=284584&view=diff
==
--- lldb/trunk/source/CMakeLists.txt (original)
+++ lldb/trunk/source/CMakeLists.txt Wed Oct 19 07:54:28 2016
@@ -53,7 +53,7 @@ endforeach()
 
 if(DEFINED lldb_vc)
   set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/SVNVersion.inc")
-  set(get_svn_script "${LLVM_MAIN_SRC_DIR}/cmake/modules/GetSVN.cmake")
+  set(get_svn_script "${LLVM_CMAKE_PATH}/GetSVN.cmake")
 
   # Create custom target to generate the VC revision include.
   add_custom_command(OUTPUT "${version_inc}"


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

[Lldb-commits] [PATCH] D25668: [cmake] Respect LLVM_CMAKE_PATH in stand-alone builds for GetSVN.cmake

[Michał Górny via lldb-commits]

mgorny abandoned this revision.
mgorny added a comment.

Now handled as a followup of https://reviews.llvm.org/D25724.


https://reviews.llvm.org/D25668



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

[Lldb-commits] [PATCH] D25057: Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints

[Pavel Labath via lldb-commits]

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

LGTM, thanks.

BTW, it looks like the linux kernel is about to grow support for proper 
byte-range watchpoints 
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461036.html.


https://reviews.llvm.org/D25057



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

Re: [Lldb-commits] [lldb] r284565 - Revert back to the state before r284550

[Pavel Labath via lldb-commits]

Hi,

Let me know if you need help with this. I can give the patch a dry run
when you have the next version ready.

pl

On 19 October 2016 at 03:44, Chris Bieneman via lldb-commits
 wrote:
> Author: cbieneman
> Date: Tue Oct 18 21:44:20 2016
> New Revision: 284565
>
> URL: http://llvm.org/viewvc/llvm-project?rev=284565&view=rev
> Log:
> Revert back to the state before r284550
>
> This patch is causing a lot of issues on bots that I didn't see in local 
> testing. I'm going to have to work on this. Reverting for now while I sort it 
> out.
>
> Modified:
> lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules
>
> Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules?rev=284565&r1=284564&r2=284565&view=diff
> ==
> --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules (original)
> +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Tue Oct 18 
> 21:44:20 2016
> @@ -242,27 +242,27 @@ ifneq "$(DYLIB_NAME)" ""
>  endif
>
>  # Function that returns the counterpart C++ compiler, given $(CC) as arg.
> -cxx_compiler_notdir = $(if $(findstring icc,$(1)), \
> -$(subst icc,icpc,$(1)), \
> -$(if $(findstring llvm-gcc,$(1)), \
> - $(subst llvm-gcc,llvm-g++,$(1)), \
> - $(if $(findstring gcc,$(1)), \
> -  $(subst gcc,g++,$(1)), \
> -  $(subst cc,c++,$(1)
> +cxx_compiler_notdir = $(if $(findstring clang,$(1)), \
> +   $(subst clang,clang++,$(1)), \
> +   $(if $(findstring icc,$(1)), \
> +$(subst icc,icpc,$(1)), \
> +$(if $(findstring llvm-gcc,$(1)), \
> + $(subst llvm-gcc,llvm-g++,$(1)), \
> + $(if $(findstring gcc,$(1)), \
> +  $(subst gcc,g++,$(1)), \
> +  $(subst cc,c++,$(1))
>  cxx_compiler = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call 
> cxx_compiler_notdir,$(notdir $(1,$(call cxx_compiler_notdir,$(1)))
>
> -ifeq ($(findstring clang, $(cxx_compiler)), clang)
> -CXXFLAGS += --driver-mode=g++
> -endif
> -
>  # Function that returns the C++ linker, given $(CC) as arg.
> -cxx_linker_notdir = $(if $(findstring icc,$(1)), \
> -  $(subst icc,icpc,$(1)), \
> -  $(if $(findstring llvm-gcc,$(1)), \
> -   $(subst llvm-gcc,llvm-g++,$(1)), \
> -   $(if $(findstring gcc,$(1)), \
> -$(subst gcc,g++,$(1)), \
> -$(subst cc,c++,$(1)
> +cxx_linker_notdir = $(if $(findstring clang,$(1)), \
> + $(subst clang,clang++,$(1)), \
> + $(if $(findstring icc,$(1)), \
> +  $(subst icc,icpc,$(1)), \
> +  $(if $(findstring llvm-gcc,$(1)), \
> +   $(subst llvm-gcc,llvm-g++,$(1)), \
> +   $(if $(findstring gcc,$(1)), \
> +$(subst gcc,g++,$(1)), \
> +$(subst cc,c++,$(1))
>  cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call 
> cxx_linker_notdir,$(notdir $(1,$(call cxx_linker_notdir,$(1)))
>
>  ifneq "$(OS)" "Darwin"
> @@ -354,9 +354,6 @@ ifneq "$(strip $(DYLIB_CXX_SOURCES))" ""
>  DYLIB_OBJECTS +=$(strip $(DYLIB_CXX_SOURCES:.cpp=.o))
>  CXX = $(call cxx_compiler,$(CC))
>  LD = $(call cxx_linker,$(CC))
> -ifeq ($(findstring clang, $(cxx_linker)), clang)
> -  LDFLAGS += --driver-mode=g++
> -endif
>  endif
>
>  #--
> @@ -380,9 +377,6 @@ endif
>  ifneq "$(strip $(CXX_SOURCES))" ""
> OBJECTS +=$(strip $(CXX_SOURCES:.cpp=.o))
> CXX = $(call cxx_compiler,$(CC))
> -  ifeq ($(findstring clang, $(cxx_linker)), clang)
> -LDFLAGS += --driver-mode=g++
> -  endif
> LD = $(call cxx_linker,$(CC))
>  endif
>
> @@ -401,9 +395,6 @@ ifneq "$(strip $(OBJCXX_SOURCES))" ""
> OBJECTS +=$(strip $(OBJCXX_SOURCES:.mm=.o))
> CXX = $(call cxx_compiler,$(CC))
> LD = $(call cxx_linker,$(CC))
> -  ifeq ($(findstring clang, $(cxx_linker)), clang)
> -LDFLAGS += --driver-mode=g++
> -  endif
> ifeq "$(findstring lobjc,$(LDFLAGS))" ""
> LDFLAGS +=-lobjc
> endif
> @@ -423,9 +414,6 @@ ifneq "$(strip $(ARCHIVE_CXX_SOURCES))"
> ARCHIVE_OBJECTS +=$(strip $(ARCHI

[Lldb-commits] [PATCH] D25756: FreeBSD ARM support for software single step.

[Pavel Labath via lldb-commits]

labath added a comment.

I'd love to see freebsd switch to lldb-server based debugging. Then we could 
make sure this code is actually reused instead of copied around (e.g. we are 
right now preparing a patch to improve this, which you could have gotten for 
free if the code was shared). 


Repository:
  rL LLVM

https://reviews.llvm.org/D25756



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

Re: [Lldb-commits] [PATCH] debugserver: Disable four-char-constants and format-pedantic warnings

[Pavel Labath via lldb-commits]

On 19 October 2016 at 00:20, Justin Bogner  wrote:
> Pavel Labath  writes:
>> Thanks for the patch. Could you submit the patch through phabricator
>>  and add Greg Clayton as a reviewer.
>
> Maybe later. I don't have time to fight with phabricator today.
>
>> That said, my preference would be to actually fix those warnings
>> instead of silencing them.
>
> If you think that's best, but do note that neither of these warnings is
> flagging much of a problem:
>
> - The four-character-literal warning is flagging implementation defined
>   behaviour four spelling a 32 bit hex constant in ascii instead of
>   something like 0x46445343. This is a portability vs readability thing,
>   and pretty minor.

Agreed, but it also is pretty easily workaroundable without hurting
readability by defining a symbolic constant.

>
> - The format-pedantic is warning about passing typed pointers to a %p
>   format specifier. The "fix" is to cast these arguments to `void *`,
>   which IMO hurts readability for no gain. I'm pretty sure this warning
>   only exists to preserve portability to some hypothetical ABI whose
>   calling convention depends on the type of the pointer.

This one actually annoys me as well, but if we do that, I think it
should be done at the project level. BTW, does the flag disable
anything else apart from the void *-cast issue. I'd hate to lose other
checks, as it's extremely easy to write non-portable format strings.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D25745: [CMake] Rename lldb-launcher to darwin-debug

[Pavel Labath via lldb-commits]

labath accepted this revision.
labath added a reviewer: labath.
labath added a comment.

I have no idea what this executable does, but I am pretty sure we don't use it.

PS: there are some mentions of this in the 
`scripts/Python/finishSwigPythonLLDB.py` script. Does that need an update?


https://reviews.llvm.org/D25745



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

[Lldb-commits] [PATCH] D25726: Improve the libstdc++ smart pointer formatters

[Pavel Labath via lldb-commits]

labath added a comment.

Looks good from my side. Enrico, do you want to have a look at this?

A couple tiny comments below. Also, be sure to properly reformat and reorder 
the headers.




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py:41
+self.expect("frame variable ssp.count", substrs=['count = 1'])
+self.expect("frame variable ssp.weak_count", substrs=['weak_count = 
1'])
 

Could you add a test that accesses a non-existing subobject of `ssp`, to make 
sure it does something not-completely-broken.



Comment at: source/Plugins/Language/CPlusPlus/LibStdcppSmartPointer.cpp:27
+
+class LibStdcppSharedPtrSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+ public:

What do you think about renaming this class to something shorter 
(SharedPtrFrontEnd, SharedPtrSynthetic, simply FrontEnd ?). Since it's local to 
a cpp, I don't think we need to be so explicit, and it seems wasteful to use 
half of the line length for the class name.



Comment at: source/Plugins/Language/CPlusPlus/LibStdcppSmartPointer.cpp:89
+bool success = false;
+uint64_t count = m_weak_obj->GetValueAsUnsigned(0, &success) - 1;
+if (success) {

It would be useful to add a short comment explaining the `-1`



Comment at: source/Plugins/Language/CPlusPlus/LibStdcppSmartPointer.cpp:138
+return 3;
+  return UINT32_MAX;
+}

I know you just copied it, but this seems wrong (size_t can be 64-bit). Will 
this work if you just return `~0`.


https://reviews.llvm.org/D25726



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