[Lldb-commits] [lldb] r353503 - [NFC] Fix license headers after r352845

[Aleksandr Urakov via lldb-commits]

Author: aleksandr.urakov
Date: Fri Feb  8 00:48:15 2019
New Revision: 353503

URL: http://llvm.org/viewvc/llvm-project?rev=353503&view=rev
Log:
[NFC] Fix license headers after r352845

Modified:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h

lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.h

lldb/trunk/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp

Modified: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp?rev=353503&r1=353502&r2=353503&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp 
Fri Feb  8 00:48:15 2019
@@ -1,9 +1,8 @@
 //===-- CodeViewRegisterMapping.cpp -*- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
 

Modified: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h?rev=353503&r1=353502&r2=353503&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h 
Fri Feb  8 00:48:15 2019
@@ -1,9 +1,8 @@
 //===-- CodeViewRegisterMapping.h ---*- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
 

Modified: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp?rev=353503&r1=353502&r2=353503&view=diff
==
--- 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
 Fri Feb  8 00:48:15 2019
@@ -1,9 +1,8 @@
 //===-- PDBFPOProgramToDWARFExpression.cpp --*- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
 

Modified: 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.h?rev=353503&r1=353502&r2=353503&view=diff
==
--- 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.h 
(original)
+++ 
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.h 
Fri Feb  8 00:48:15 2019
@@ -1,9 +1,8 @@
 //===-- PDBFPOProgramToDWARFExpression.h *- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===---

[Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Fixed in r353503, thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55122



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

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

I don't remember if this tests some super-critical functionality (i.e., whether 
you will be able to function reasonably without it), but in case it does, it 
should be fairly easy to get this class to use one of the other event sources 
(pselect, ppoll), if kevent is not functioning well on netbsd.

And to answer your IRC question, gtest has a concept of a disabled test, which 
you do by prefixing the test with "DISABLED_". The typical way to achieve that 
would be via something like:

  #ifdef __NetBSD__
  #define SKIP_ON_NETBSD(x) DISABLED_ ## x
  #else
  #define SKIP_ON_NETBSD(x) x
  #endif
  TEST(foo, SKIP_ON_NETBSD(bar)) { ... }

However, that is very tedious, so people usually do exactly what you have done 
here.


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

https://reviews.llvm.org/D57912



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

[Lldb-commits] [PATCH] D57928: Fix x86 return pattern detection

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Could you please write a test for this? You can take a look at 
unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp for other 
x86AssemblyInspectionEngine tests.

Also, I'm curious how you found this bug. (i.e. which functionality was broken 
with the old implementation).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57928



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

[Lldb-commits] [PATCH] D57895: Breakpad: auto-detect path style of file entries

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: include/lldb/Utility/FileSpec.h:250
+  /// unreliable (e.g. "c:\foo.txt" is a valid relative posix path).
+  static llvm::Optional

[Lldb-commits] [PATCH] D57895: Breakpad: auto-detect path style of file entries

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 185937.
labath added a comment.

Rename arg to `absolute_path`.


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

https://reviews.llvm.org/D57895

Files:
  include/lldb/Utility/FileSpec.h
  lit/SymbolFile/Breakpad/Inputs/line-table-mixed-path-styles.syms
  lit/SymbolFile/Breakpad/line-table-discontinuous-file-ids.test
  lit/SymbolFile/Breakpad/line-table-edgecases.test
  lit/SymbolFile/Breakpad/line-table-missing-file.test
  lit/SymbolFile/Breakpad/line-table-mixed-path-styles.test
  lit/SymbolFile/Breakpad/line-table.test
  source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Utility/FileSpec.cpp
  unittests/Utility/FileSpecTest.cpp

Index: unittests/Utility/FileSpecTest.cpp
===
--- unittests/Utility/FileSpecTest.cpp
+++ unittests/Utility/FileSpecTest.cpp
@@ -189,6 +189,17 @@
   }
 }
 
+TEST(FileSpecTest, GuessPathStyle) {
+  EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("/foo/bar.txt"));
+  EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
+  EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
+  EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
+}
+
 TEST(FileSpecTest, GetNormalizedPath) {
   std::pair posix_tests[] = {
   {"/foo/.././bar", "/bar"},
Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -365,6 +365,17 @@
   return a == b;
 }
 
+llvm::Optional FileSpec::GuessPathStyle(llvm::StringRef absolute_path) {
+  if (absolute_path.startswith("/"))
+return Style::posix;
+  if (absolute_path.startswith(R"(\\)"))
+return Style::windows;
+  if (absolute_path.size() > 3 && llvm::isAlpha(absolute_path[0]) &&
+  absolute_path.substr(1, 2) == R"(:\)")
+return Style::windows;
+  return llvm::None;
+}
+
 //--
 // Dump the object to the supplied stream. If the object contains a valid
 // directory name, it will be displayed followed by a directory delimiter, and
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -820,22 +820,19 @@
   if (!die)
 return;
 
-  auto guess = [](llvm::StringRef str) {
-if (str.startswith("/"))
-  return FileSpec::Style::posix;
-if (str.size() > 3 && llvm::isAlpha(str[0]) && str.substr(1, 2) == ":\\")
-  return FileSpec::Style::windows;
-return FileSpec::Style::native;
-  };
   llvm::StringRef comp_dir = removeHostnameFromPathname(
   die->GetAttributeValueAsString(m_dwarf, this, DW_AT_comp_dir, NULL));
   if (!comp_dir.empty()) {
-m_comp_dir = resolveCompDir(FileSpec(comp_dir, guess(comp_dir)));
+FileSpec::Style comp_dir_style =
+FileSpec::GuessPathStyle(comp_dir).getValueOr(FileSpec::Style::native);
+m_comp_dir = resolveCompDir(FileSpec(comp_dir, comp_dir_style));
   } else {
 // Try to detect the style based on the DW_AT_name attribute, but just store
 // the detected style in the m_comp_dir field.
-m_comp_dir = FileSpec("", guess(die->GetAttributeValueAsString(
-  m_dwarf, this, DW_AT_name, NULL)));
+const char *name =
+die->GetAttributeValueAsString(m_dwarf, this, DW_AT_name, NULL);
+m_comp_dir = FileSpec(
+"", FileSpec::GuessPathStyle(name).getValueOr(FileSpec::Style::native));
   }
 }
 
Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -398,7 +398,9 @@
 
 if (record->Number >= m_files->size())
   m_files->resize(record->Number + 1);
-(*m_files)[record->Number] = FileSpec(record->Name);
+FileSpec::Style style = FileSpec::GuessPathStyle(record->Name)
+.getValueOr(FileSpec::Style::native);
+(*m_files)[record->Number] = FileSpec(record->Name, style);
   }
 }
 
Index: lit/SymbolFile/Breakpad/line-table.test
===
--- lit/SymbolFile/Breakpad/line-table.test
+++ lit/SymbolFile/Breakpad/line-table.test
@@ -1,5 +1,3 @@
-# XFAIL: system-windows
-
 # RUN: yaml2obj %S/Inputs/basic-elf.yaml > %T/line-table.out
 # RUN: %lldb %T/line-table.out -o "target symbols add -s line-table.out %S/Inputs/line-table.syms" \
 # RUN:   -s %s -o exit | FileCheck %s
Index: lit/S

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Hi Greg, what do you think of my replies to your `CreateModuleFromObjectFile` 
comment?


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

https://reviews.llvm.org/D57751



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

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Ok, thanks.  I consider this only temporary; I have a kernel patch ready 
already (and waiting for review), so we're going to look into reenabling it 
soon enough.


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

https://reviews.llvm.org/D57912



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

[Lldb-commits] [PATCH] D57928: Fix x86 return pattern detection

[Todd Mortimer via Phabricator via lldb-commits]

mortimer added a comment.

For sure I can add a test.

I found this only because I was looking to fix something else (prologue 
detection on OpenBSD with -fret-protector) and noticed that the return pattern 
detect function was wrong. So nothing was broken that I found, though the 
inclusion of 0xc9 as a return pattern could conceivably cause problems under 
some circumstances. In any event, I figured it would be easy enough to submit a 
fix, since it’s a tiny change.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57928



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

[Lldb-commits] [PATCH] D57895: Breakpad: auto-detect path style of file entries

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.

I think `absolute_path` is great.  Thanks for checking on the `native`/`None`.  
I didn't want there to be any latent surprises.


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

https://reviews.llvm.org/D57895



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

[Lldb-commits] [PATCH] D57956: [www] Add ASTImporter fuzzer project.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added subscribers: friss, LLDB.
teemperor added a comment.

Just trying to get some early feedback before I subscribe the mailing list. 
Also let me know if someone here also wants to mentor for this project and I'll 
add you.


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

https://reviews.llvm.org/D57956



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

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: zturner, labath, krytarowski.
Herald added a project: LLDB.

Fix MainLoop::RunImpl::get_sigmask() to correctly return empty sigset_t
when SIGNAL_POLLING_UNSUPPORTED is true.  On NetBSD (and probably
on some other platforms), integers are not implicitly convertible to
sigset_t, so 'return 0' is erraneous.  Instead, sigset_t should be reset
through sigemptyset().

While at it, move common parts out of the #ifdef.

// FTR: I don't know if this wouldn't break Windows. I'd appreciate if somebody 
could test it. Alternatively, I can watch buildbots after merging.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D57959

Files:
  lldb/source/Host/common/MainLoop.cpp


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -137,18 +137,18 @@
 }
 
 sigset_t MainLoop::RunImpl::get_sigmask() {
+  sigset_t sigmask;
 #if SIGNAL_POLLING_UNSUPPORTED
-  return 0;
+  sigemptyset(&sigmask);
 #else
-  sigset_t sigmask;
   int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
   assert(ret == 0);
   (void) ret;
 
   for (const auto &sig : loop.m_signals)
 sigdelset(&sigmask, sig.first);
-  return sigmask;
 #endif
+  return sigmask;
 }
 
 #ifdef __ANDROID__


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -137,18 +137,18 @@
 }
 
 sigset_t MainLoop::RunImpl::get_sigmask() {
+  sigset_t sigmask;
 #if SIGNAL_POLLING_UNSUPPORTED
-  return 0;
+  sigemptyset(&sigmask);
 #else
-  sigset_t sigmask;
   int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
   assert(ret == 0);
   (void) ret;
 
   for (const auto &sig : loop.m_signals)
 sigdelset(&sigmask, sig.first);
-  return sigmask;
 #endif
+  return sigmask;
 }
 
 #ifdef __ANDROID__
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57956: [www] Add ASTImporter fuzzer project.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Raphael, can you please also update the corresponding RST file?


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

https://reviews.llvm.org/D57956



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

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

In D57751#1390345 , @labath wrote:

> Hi Greg, what do you think of my replies to your `CreateModuleFromObjectFile` 
> comment?


Your explanation makes sense. I missed the fact that this function was creating 
a module from an object file!


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

https://reviews.llvm.org/D57751



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

[Lldb-commits] [lldb] r353545 - [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Fri Feb  8 10:56:11 2019
New Revision: 353545

URL: http://llvm.org/viewvc/llvm-project?rev=353545&view=rev
Log:
[lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

The NetBSD kernel currently does not support detecting closed slave pty
via kevent on master pty.  This causes the test to hang forever.
To avoid that, disable the test until the kernel is fixed.

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

Modified:
lldb/trunk/unittests/Host/MainLoopTest.cpp

Modified: lldb/trunk/unittests/Host/MainLoopTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/MainLoopTest.cpp?rev=353545&r1=353544&r2=353545&view=diff
==
--- lldb/trunk/unittests/Host/MainLoopTest.cpp (original)
+++ lldb/trunk/unittests/Host/MainLoopTest.cpp Fri Feb  8 10:56:11 2019
@@ -108,7 +108,11 @@ TEST_F(MainLoopTest, TerminatesImmediate
 }
 
 #ifdef LLVM_ON_UNIX
+// NetBSD currently does not report slave pty EOF via kevent
+// causing this test to hang forever.
+#ifndef __NetBSD__
 TEST_F(MainLoopTest, DetectsEOF) {
+
   PseudoTerminal term;
   ASSERT_TRUE(term.OpenFirstAvailableMaster(O_RDWR, nullptr, 0));
   ASSERT_TRUE(term.OpenSlave(O_RDWR | O_NOCTTY, nullptr, 0));
@@ -125,6 +129,7 @@ TEST_F(MainLoopTest, DetectsEOF) {
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
+#endif
 
 TEST_F(MainLoopTest, Signal) {
   MainLoop loop;


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

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz created this revision.
sgraenitz added a reviewer: jingham.

As seen in a crash report, the C-string returned for the directory component of 
`target_file` can null. It should not be assigned to `std::string` directly as 
this is undefined behavior.


https://reviews.llvm.org/D57964

Files:
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp


Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2780,7 +2780,13 @@
   basename = pathname; // not a filename, probably a package of some sort,
// let it go through
 } else if (is_directory(st) || is_regular_file(st)) {
-  std::string directory = target_file.GetDirectory().GetCString();
+  const char *dir_cstr = target_file.GetDirectory().GetCString();
+  if (!dir_cstr) {
+error.SetErrorString("invalid directory name");
+return false;
+  }
+
+  std::string directory(dir_cstr);
   replace_all(directory, "\\", "");
   replace_all(directory, "'", "\\'");
 
@@ -2843,6 +2849,7 @@
 }
 
 // now actually do the import
+
 command_stream.Clear();
 
 if (was_imported) {


Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2780,7 +2780,13 @@
   basename = pathname; // not a filename, probably a package of some sort,
// let it go through
 } else if (is_directory(st) || is_regular_file(st)) {
-  std::string directory = target_file.GetDirectory().GetCString();
+  const char *dir_cstr = target_file.GetDirectory().GetCString();
+  if (!dir_cstr) {
+error.SetErrorString("invalid directory name");
+return false;
+  }
+
+  std::string directory(dir_cstr);
   replace_all(directory, "\\", "");
   replace_all(directory, "'", "\\'");
 
@@ -2843,6 +2849,7 @@
 }
 
 // now actually do the import
+
 command_stream.Clear();
 
 if (was_imported) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353545: [lldb] [unittests] Disable MainLoopTest::DetectsEOF 
on NetBSD (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57912?vs=185815&id=186000#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57912

Files:
  lldb/trunk/unittests/Host/MainLoopTest.cpp


Index: lldb/trunk/unittests/Host/MainLoopTest.cpp
===
--- lldb/trunk/unittests/Host/MainLoopTest.cpp
+++ lldb/trunk/unittests/Host/MainLoopTest.cpp
@@ -108,7 +108,11 @@
 }
 
 #ifdef LLVM_ON_UNIX
+// NetBSD currently does not report slave pty EOF via kevent
+// causing this test to hang forever.
+#ifndef __NetBSD__
 TEST_F(MainLoopTest, DetectsEOF) {
+
   PseudoTerminal term;
   ASSERT_TRUE(term.OpenFirstAvailableMaster(O_RDWR, nullptr, 0));
   ASSERT_TRUE(term.OpenSlave(O_RDWR | O_NOCTTY, nullptr, 0));
@@ -125,6 +129,7 @@
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
+#endif
 
 TEST_F(MainLoopTest, Signal) {
   MainLoop loop;


Index: lldb/trunk/unittests/Host/MainLoopTest.cpp
===
--- lldb/trunk/unittests/Host/MainLoopTest.cpp
+++ lldb/trunk/unittests/Host/MainLoopTest.cpp
@@ -108,7 +108,11 @@
 }
 
 #ifdef LLVM_ON_UNIX
+// NetBSD currently does not report slave pty EOF via kevent
+// causing this test to hang forever.
+#ifndef __NetBSD__
 TEST_F(MainLoopTest, DetectsEOF) {
+
   PseudoTerminal term;
   ASSERT_TRUE(term.OpenFirstAvailableMaster(O_RDWR, nullptr, 0));
   ASSERT_TRUE(term.OpenSlave(O_RDWR | O_NOCTTY, nullptr, 0));
@@ -125,6 +129,7 @@
   ASSERT_TRUE(loop.Run().Success());
   ASSERT_EQ(1u, callback_count);
 }
+#endif
 
 TEST_F(MainLoopTest, Signal) {
   MainLoop loop;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I think that sigemptyset(2) is unsupported on Windows.




Comment at: lldb/source/Host/common/MainLoop.cpp:149
   for (const auto &sig : loop.m_signals)
 sigdelset(&sigmask, sig.first);
 #endif

Shouldn't we initialize sigmask always before adding/deleting particular 
signals?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57959



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

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Host/common/MainLoop.cpp:149
   for (const auto &sig : loop.m_signals)
 sigdelset(&sigmask, sig.first);
 #endif

krytarowski wrote:
> Shouldn't we initialize sigmask always before adding/deleting particular 
> signals?
Ah, I can see it now. It's passed to `pthread_sigmask(3)` so it's fine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57959



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

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz updated this revision to Diff 186002.
sgraenitz added a comment.

Remove useless newline


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

https://reviews.llvm.org/D57964

Files:
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp


Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2780,7 +2780,13 @@
   basename = pathname; // not a filename, probably a package of some sort,
// let it go through
 } else if (is_directory(st) || is_regular_file(st)) {
-  std::string directory = target_file.GetDirectory().GetCString();
+  const char *dir_cstr = target_file.GetDirectory().GetCString();
+  if (!dir_cstr) {
+error.SetErrorString("invalid directory name");
+return false;
+  }
+
+  std::string directory(dir_cstr);
   replace_all(directory, "\\", "");
   replace_all(directory, "'", "\\'");
 


Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2780,7 +2780,13 @@
   basename = pathname; // not a filename, probably a package of some sort,
// let it go through
 } else if (is_directory(st) || is_regular_file(st)) {
-  std::string directory = target_file.GetDirectory().GetCString();
+  const char *dir_cstr = target_file.GetDirectory().GetCString();
+  if (!dir_cstr) {
+error.SetErrorString("invalid directory name");
+return false;
+  }
+
+  std::string directory(dir_cstr);
   replace_all(directory, "\\", "");
   replace_all(directory, "'", "\\'");
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

@labath our short-term goal is to enable execution of LLDB tests on the NetBSD 
buildbot. We are stuck temporarily with an older release of NetBSD on the 
machine for some time (1-2 months) so we need to live with it for now. No need 
to make it better than sufficient as of now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57912



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

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Might be clearer to do:

  if (target_file.GetDirectory().IsEmpty()) {
  error...
  }
  std::string directory = ...

Avoids the extra variable and also adds the check for "", which I don't think 
we want to add to sys.path either.


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

https://reviews.llvm.org/D57964



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

[Lldb-commits] [lldb] r353549 - [opaque pointer types] Update calls to CreateCall to pass the function

[James Y Knight via lldb-commits]

Author: jyknight
Date: Fri Feb  8 11:30:46 2019
New Revision: 353549

URL: http://llvm.org/viewvc/llvm-project?rev=353549&view=rev
Log:
[opaque pointer types] Update calls to CreateCall to pass the function
type in lldb and polly.

Modified:
lldb/trunk/source/Expression/IRDynamicChecks.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.h

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp

Modified: lldb/trunk/source/Expression/IRDynamicChecks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/IRDynamicChecks.cpp?rev=353549&r1=353548&r2=353549&view=diff
==
--- lldb/trunk/source/Expression/IRDynamicChecks.cpp (original)
+++ lldb/trunk/source/Expression/IRDynamicChecks.cpp Fri Feb  8 11:30:46 2019
@@ -254,7 +254,7 @@ protected:
   /// @return
   /// The function pointer, for use in a CallInst.
   //--
-  llvm::Value *BuildPointerValidatorFunc(lldb::addr_t start_address) {
+  llvm::FunctionCallee BuildPointerValidatorFunc(lldb::addr_t start_address) {
 llvm::Type *param_array[1];
 
 param_array[0] = const_cast(GetI8PtrTy());
@@ -266,7 +266,7 @@ protected:
 PointerType *fun_ptr_ty = PointerType::getUnqual(fun_ty);
 Constant *fun_addr_int =
 ConstantInt::get(GetIntptrTy(), start_address, false);
-return ConstantExpr::getIntToPtr(fun_addr_int, fun_ptr_ty);
+return {fun_ty, ConstantExpr::getIntToPtr(fun_addr_int, fun_ptr_ty)};
   }
 
   //--
@@ -279,7 +279,7 @@ protected:
   /// @return
   /// The function pointer, for use in a CallInst.
   //--
-  llvm::Value *BuildObjectCheckerFunc(lldb::addr_t start_address) {
+  llvm::FunctionCallee BuildObjectCheckerFunc(lldb::addr_t start_address) {
 llvm::Type *param_array[2];
 
 param_array[0] = const_cast(GetI8PtrTy());
@@ -292,7 +292,7 @@ protected:
 PointerType *fun_ptr_ty = PointerType::getUnqual(fun_ty);
 Constant *fun_addr_int =
 ConstantInt::get(GetIntptrTy(), start_address, false);
-return ConstantExpr::getIntToPtr(fun_addr_int, fun_ptr_ty);
+return {fun_ty, ConstantExpr::getIntToPtr(fun_addr_int, fun_ptr_ty)};
   }
 
   PointerType *GetI8PtrTy() {
@@ -382,7 +382,7 @@ protected:
   }
 
 private:
-  llvm::Value *m_valid_pointer_check_func;
+  llvm::FunctionCallee m_valid_pointer_check_func;
 };
 
 class ObjcObjectChecker : public Instrumenter {
@@ -544,7 +544,7 @@ protected:
   }
 
 private:
-  llvm::Value *m_objc_object_check_func;
+  llvm::FunctionCallee m_objc_object_check_func;
 };
 
 IRDynamicChecks::IRDynamicChecks(DynamicCheckerFunctions &checker_functions,

Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp?rev=353549&r1=353548&r2=353549&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp Fri Feb  8 
11:30:46 2019
@@ -477,15 +477,15 @@ bool IRForTarget::RewriteObjCConstString
 
 ArrayRef CFSCWB_arg_types(arg_type_array, 5);
 
-llvm::Type *CFSCWB_ty =
+llvm::FunctionType *CFSCWB_ty =
 FunctionType::get(ns_str_ty, CFSCWB_arg_types, false);
 
 // Build the constant containing the pointer to the function
 PointerType *CFSCWB_ptr_ty = PointerType::getUnqual(CFSCWB_ty);
 Constant *CFSCWB_addr_int =
 ConstantInt::get(m_intptr_ty, CFStringCreateWithBytes_addr, false);
-m_CFStringCreateWithBytes =
-ConstantExpr::getIntToPtr(CFSCWB_addr_int, CFSCWB_ptr_ty);
+m_CFStringCreateWithBytes = {
+CFSCWB_ty, ConstantExpr::getIntToPtr(CFSCWB_addr_int, CFSCWB_ptr_ty)};
   }
 
   ConstantDataSequential *string_array = NULL;
@@ -880,14 +880,15 @@ bool IRForTarget::RewriteObjCSelector(In
 
 ArrayRef srN_arg_types(type_array, 1);
 
-llvm::Type *srN_type =
+llvm::FunctionType *srN_type =
 FunctionType::get(sel_ptr_type, srN_arg_types, false);
 
 // Build the constant containing the pointer to the function
 PointerType *srN_ptr_ty = PointerType::getUnqual(srN_type);
 Constant *srN_addr_int =
 ConstantInt::get(m_intptr_ty, sel_registerName_addr, false);
-m_sel_registerName = ConstantExpr::getIntToPtr(srN_addr_int, srN_ptr_ty);
+m_sel_registerName = {srN_type,
+  ConstantExpr::getIntToPtr(srN_addr_int, srN_ptr_ty)};
   }
 
   Value *argument_array[1];
@@ -1042,14 +1043,15 @@ bool IRForTarget::RewriteObjCClassRefere
 
 ArrayRef ogC_arg_

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 186011.
mgorny added a comment.

Ok, I see that `_WIN32` actually redefines `sigset_t`, so I've added a separate 
branch for it.


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

https://reviews.llvm.org/D57959

Files:
  lldb/source/Host/common/MainLoop.cpp


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -137,18 +137,20 @@
 }
 
 sigset_t MainLoop::RunImpl::get_sigmask() {
-#if SIGNAL_POLLING_UNSUPPORTED
-  return 0;
-#else
   sigset_t sigmask;
+#if defined(_WIN32)
+  sigmask = 0;
+#elif SIGNAL_POLLING_UNSUPPORTED
+  sigemptyset(&sigmask);
+#else
   int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
   assert(ret == 0);
   (void) ret;
 
   for (const auto &sig : loop.m_signals)
 sigdelset(&sigmask, sig.first);
-  return sigmask;
 #endif
+  return sigmask;
 }
 
 #ifdef __ANDROID__


Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -137,18 +137,20 @@
 }
 
 sigset_t MainLoop::RunImpl::get_sigmask() {
-#if SIGNAL_POLLING_UNSUPPORTED
-  return 0;
-#else
   sigset_t sigmask;
+#if defined(_WIN32)
+  sigmask = 0;
+#elif SIGNAL_POLLING_UNSUPPORTED
+  sigemptyset(&sigmask);
+#else
   int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
   assert(ret == 0);
   (void) ret;
 
   for (const auto &sig : loop.m_signals)
 sigdelset(&sigmask, sig.first);
-  return sigmask;
 #endif
+  return sigmask;
 }
 
 #ifdef __ANDROID__
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D57959#1390929 , @mgorny wrote:

> Ok, I see that `_WIN32` actually redefines `sigset_t`, so I've added a 
> separate branch for it.


Maybe `#ifdef LLVM_ON_UNIX`?


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

https://reviews.llvm.org/D57959



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

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

But to what purpose? I think it's better to use consistent macros to refer to 
the same scenario.


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

https://reviews.llvm.org/D57959



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

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D57959#1390992 , @mgorny wrote:

> But to what purpose? I think it's better to use consistent macros to refer to 
> the same scenario.


I had an impression that LLVM_ON_UNIX is LLVM homegrown symbol for this exact 
purpose to differentiate Windows (out of UNIX [and probably next to Fuchsia 
today, but it's not supported in LLDB].


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

https://reviews.llvm.org/D57959



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

[Lldb-commits] [lldb] r353581 - Tiny fix spotted by static analyzer; GetPath() returns a std::string,

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Fri Feb  8 15:36:25 2019
New Revision: 353581

URL: http://llvm.org/viewvc/llvm-project?rev=353581&view=rev
Log:
Tiny fix spotted by static analyzer; GetPath() returns a std::string,
we get a pointer to the c-string rep and then the temporary object
is destructed and we still refer to the c-string after that.


Modified:
lldb/trunk/source/Target/Platform.cpp

Modified: lldb/trunk/source/Target/Platform.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Platform.cpp?rev=353581&r1=353580&r2=353581&view=diff
==
--- lldb/trunk/source/Target/Platform.cpp (original)
+++ lldb/trunk/source/Target/Platform.cpp Fri Feb  8 15:36:25 2019
@@ -1059,11 +1059,11 @@ Status Platform::LaunchProcess(ProcessLa
   uint32_t num_resumes = GetResumeCountForLaunchInfo(launch_info);
   if (log) {
 const FileSpec &shell = launch_info.GetShell();
-const char *shell_str = (shell) ? shell.GetPath().c_str() : "";
+std::string shell_str = (shell) ? shell.GetPath() : "";
 log->Printf(
 "Platform::%s GetResumeCountForLaunchInfo() returned %" PRIu32
 ", shell is '%s'",
-__FUNCTION__, num_resumes, shell_str);
+__FUNCTION__, num_resumes, shell_str.c_str());
   }
 
   if (!launch_info.ConvertArgumentsForLaunchingInShell(


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

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

[Eugene Zelenko via Phabricator via lldb-commits]

Eugene.Zelenko added inline comments.



Comment at: lldb/source/API/SBData.cpp:9
 
 #include 
 

cinttypes should be used instead. See Clang-tidy modernize-deprecated-headers. 
Same in other places.



Comment at: lldb/source/API/SBData.cpp:12
+#include 
+
 #include "lldb/API/SBData.h"

Spaces between include statements interfere with Clang-format. Same in other 
places.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57990



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

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.



Comment at: lldb/source/API/SBData.cpp:12
+#include 
+
 #include "lldb/API/SBData.h"

Eugene.Zelenko wrote:
> Spaces between include statements interfere with Clang-format. Same in other 
> places.
How?

We always separate system header includes from lldb includes from llvm includes 
by putting a blank line.  That's done all over the place in lldb.  I haven't 
heard of this causing problems before now.  If this does cause problems, that 
should be fixed in clang-format.  It seems like a really unreasonable 
requirement.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57990



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

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

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



Comment at: lldb/source/API/SBData.cpp:12
+#include 
+
 #include "lldb/API/SBData.h"

jingham wrote:
> Eugene.Zelenko wrote:
> > Spaces between include statements interfere with Clang-format. Same in 
> > other places.
> How?
> 
> We always separate system header includes from lldb includes from llvm 
> includes by putting a blank line.  That's done all over the place in lldb.  I 
> haven't heard of this causing problems before now.  If this does cause 
> problems, that should be fixed in clang-format.  It seems like a really 
> unreasonable requirement.
Indeed, and as far as I know llvm does exactly the same thing. clang-format 
sorts headers within the same group, which is exactly what we want. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57990



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

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

[Eugene Zelenko via Phabricator via lldb-commits]

Eugene.Zelenko added inline comments.



Comment at: lldb/source/API/SBData.cpp:12
+#include 
+
 #include "lldb/API/SBData.h"

JDevlieghere wrote:
> jingham wrote:
> > Eugene.Zelenko wrote:
> > > Spaces between include statements interfere with Clang-format. Same in 
> > > other places.
> > How?
> > 
> > We always separate system header includes from lldb includes from llvm 
> > includes by putting a blank line.  That's done all over the place in lldb.  
> > I haven't heard of this causing problems before now.  If this does cause 
> > problems, that should be fixed in clang-format.  It seems like a really 
> > unreasonable requirement.
> Indeed, and as far as I know llvm does exactly the same thing. clang-format 
> sorts headers within the same group, which is exactly what we want. 
Is there any good reason to be different from LLVM/Clang? Wasn't formatting 
style was changed in not so recent past to reduce differences?

Examples of problems visible in this patch:

lldb/source/API/SBInstruction.cpp
lldb/source/API/SBThread.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Interpreter/CommandInterpreter.cpp


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57990



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

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

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



Comment at: lldb/source/API/SBData.cpp:12
+#include 
+
 #include "lldb/API/SBData.h"

Eugene.Zelenko wrote:
> JDevlieghere wrote:
> > jingham wrote:
> > > Eugene.Zelenko wrote:
> > > > Spaces between include statements interfere with Clang-format. Same in 
> > > > other places.
> > > How?
> > > 
> > > We always separate system header includes from lldb includes from llvm 
> > > includes by putting a blank line.  That's done all over the place in 
> > > lldb.  I haven't heard of this causing problems before now.  If this does 
> > > cause problems, that should be fixed in clang-format.  It seems like a 
> > > really unreasonable requirement.
> > Indeed, and as far as I know llvm does exactly the same thing. clang-format 
> > sorts headers within the same group, which is exactly what we want. 
> Is there any good reason to be different from LLVM/Clang? Wasn't formatting 
> style was changed in not so recent past to reduce differences?
> 
> Examples of problems visible in this patch:
> 
> lldb/source/API/SBInstruction.cpp
> lldb/source/API/SBThread.cpp
> lldb/source/Commands/CommandObjectMemory.cpp
> lldb/source/Interpreter/CommandInterpreter.cpp
I'm saying it's exactly the same in LLDB as it is in LLVM. I'm afraid I don't 
understand what you mean here.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57990



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

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

[Eugene Zelenko via Phabricator via lldb-commits]

Eugene.Zelenko added inline comments.



Comment at: lldb/source/API/SBData.cpp:12
+#include 
+
 #include "lldb/API/SBData.h"

JDevlieghere wrote:
> Eugene.Zelenko wrote:
> > JDevlieghere wrote:
> > > jingham wrote:
> > > > Eugene.Zelenko wrote:
> > > > > Spaces between include statements interfere with Clang-format. Same 
> > > > > in other places.
> > > > How?
> > > > 
> > > > We always separate system header includes from lldb includes from llvm 
> > > > includes by putting a blank line.  That's done all over the place in 
> > > > lldb.  I haven't heard of this causing problems before now.  If this 
> > > > does cause problems, that should be fixed in clang-format.  It seems 
> > > > like a really unreasonable requirement.
> > > Indeed, and as far as I know llvm does exactly the same thing. 
> > > clang-format sorts headers within the same group, which is exactly what 
> > > we want. 
> > Is there any good reason to be different from LLVM/Clang? Wasn't formatting 
> > style was changed in not so recent past to reduce differences?
> > 
> > Examples of problems visible in this patch:
> > 
> > lldb/source/API/SBInstruction.cpp
> > lldb/source/API/SBThread.cpp
> > lldb/source/Commands/CommandObjectMemory.cpp
> > lldb/source/Interpreter/CommandInterpreter.cpp
> I'm saying it's exactly the same in LLDB as it is in LLVM. I'm afraid I don't 
> understand what you mean here.
I don't remember empty lines between headers groups in LLVM and Clang code base.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57990



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

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

[JF Bastien via Phabricator via lldb-commits]

jfb added a comment.

Do you ever use weak pointers? The single allocation is good until you start 
holding on to more things (because you can't just keep the block around, you 
need the entire allocation), so you should make sure you won’t be delaying 
memory being reclaimed.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57990



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