[Lldb-commits] [PATCH] D41086: [lldb] Stop searching for a symbol in a pdb by regex

2017-12-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This is making the windows unit tests fail: 
http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7378/steps/run%20unit%20tests/logs/stdio.
 If I understand correctly, this changes FindTypes to **not** search by regex.
If that's the case, then the `SymbolFilePDBTests::TestRegexNameMatch` needs to 
be updated to not expect that `.*` will find a match.
Also, while technically not failing, the `SymbolFilePDBTests::TestMaxMatches` 
also looks weird  in this new world, and probably needs updating.

Could you look into those?


Repository:
  rL LLVM

https://reviews.llvm.org/D41086



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


[Lldb-commits] [lldb] r321353 - Enable TestReadMemCString on non-darwin targets

2017-12-22 Thread Pavel Labath via lldb-commits
Author: labath
Date: Fri Dec 22 02:26:59 2017
New Revision: 321353

URL: http://llvm.org/viewvc/llvm-project?rev=321353&view=rev
Log:
Enable TestReadMemCString on non-darwin targets

The test works fine on linux, and I believe other targets should not
have an issue with as well. If they do, we can start blacklisting
instead of whitelisting.

The idea of using "-1" as the value of the pointer on non-apple targets
backfired, as it fails the "address != LLDB_INVALID_ADDRESS" test (-1 is
the value of LLDB_INVALID_ADDRESS).  However, it should be safe to use
0x100 for other targets as well. The first page of memory is generally
kept unreadable to catch null pointer dereferences.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py?rev=321353&r1=321352&r2=321353&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py
 Fri Dec 22 02:26:59 2017
@@ -12,13 +12,8 @@ from lldbsuite.test import lldbutil
 class TestReadMemCString(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
-def setUp(self):
-TestBase.setUp(self)
-
-# Need to have a char* pointer that points to unmapped memory to run
-# this test on other platforms -- Darwin only for now.
-@skipUnlessDarwin
 def test_read_memory_c_string(self):
 """Test corner case behavior of SBProcess::ReadCStringFromMemory"""
 self.build()

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c?rev=321353&r1=321352&r2=321353&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/main.c
 Fri Dec 22 02:26:59 2017
@@ -3,11 +3,9 @@ int main ()
 {
const char *empty_string = "";
const char *one_letter_string = "1";
-#if defined (__APPLE__)
-   const char *invalid_memory_string = (char*)0x100; // lower 4k is always 
PAGEZERO & unreadable on darwin
-#else
-   const char *invalid_memory_string = -1ULL; // maybe an invalid address on 
other platforms?
-#endif
+   // This expects that lower 4k of memory will be mapped unreadable, which 
most
+   // OSs do (to catch null pointer dereferences).
+   const char *invalid_memory_string = (char*)0x100;
 
return empty_string[0] + one_letter_string[0]; // breakpoint here
 }


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


[Lldb-commits] [lldb] r321355 - debugserver: Propagate environment in launch-mode (pr35671)

2017-12-22 Thread Pavel Labath via lldb-commits
Author: labath
Date: Fri Dec 22 03:09:21 2017
New Revision: 321355

URL: http://llvm.org/viewvc/llvm-project?rev=321355&view=rev
Log:
debugserver: Propagate environment in launch-mode (pr35671)

Summary:
Make sure we propagate environment when starting debugserver with a pre-loaded
inferior. AFAIK, RNBRunLoopLaunchInferior is only called in pre-loaded inferior
scenario, so we can just pick up the debugserver environment instead of trying
to construct an envp from the (empty) context.

This makes debugserver pass an test added for an equivalent lldb-server fix.

Reviewers: jasonmolenda, clayborg

Subscribers: JDevlieghere, lldb-commits

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

Modified:
lldb/trunk/tools/debugserver/source/RNBContext.cpp
lldb/trunk/tools/debugserver/source/RNBContext.h
lldb/trunk/tools/debugserver/source/debugserver.cpp
lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h

Modified: lldb/trunk/tools/debugserver/source/RNBContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBContext.cpp?rev=321355&r1=321354&r2=321355&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBContext.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBContext.cpp Fri Dec 22 03:09:21 2017
@@ -42,6 +42,25 @@ const char *RNBContext::EnvironmentAtInd
 return NULL;
 }
 
+static std::string GetEnvironmentKey(const std::string &env) {
+  std::string key = env.substr(0, env.find('='));
+  if (!key.empty() && key.back() == '=')
+key.pop_back();
+  return key;
+}
+
+void RNBContext::PushEnvironmentIfNeeded(const char *arg) {
+  if (!arg)
+return;
+  std::string arg_key = GetEnvironmentKey(arg);
+
+  for (const std::string &entry: m_env_vec) {
+if (arg_key == GetEnvironmentKey(entry))
+  return;
+  }
+  m_env_vec.push_back(arg);
+}
+
 const char *RNBContext::ArgumentAtIndex(size_t index) {
   if (index < m_arg_vec.size())
 return m_arg_vec[index].c_str();

Modified: lldb/trunk/tools/debugserver/source/RNBContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBContext.h?rev=321355&r1=321354&r2=321355&view=diff
==
--- lldb/trunk/tools/debugserver/source/RNBContext.h (original)
+++ lldb/trunk/tools/debugserver/source/RNBContext.h Fri Dec 22 03:09:21 2017
@@ -86,6 +86,7 @@ public:
 if (arg)
   m_env_vec.push_back(arg);
   }
+  void PushEnvironmentIfNeeded(const char *arg);
   void ClearEnvironment() {
 m_env_vec.erase(m_env_vec.begin(), m_env_vec.end());
   }

Modified: lldb/trunk/tools/debugserver/source/debugserver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/debugserver.cpp?rev=321355&r1=321354&r2=321355&view=diff
==
--- lldb/trunk/tools/debugserver/source/debugserver.cpp (original)
+++ lldb/trunk/tools/debugserver/source/debugserver.cpp Fri Dec 22 03:09:21 2017
@@ -1020,6 +1020,7 @@ int main(int argc, char *argv[]) {
   optind = 1;
 #endif
 
+  bool forward_env = false;
   while ((ch = getopt_long_only(argc, argv, short_options, g_long_options,
 &long_option_index)) != -1) {
 DNBLogDebug("option: ch == %c (0x%2.2x) --%s%c%s\n", ch, (uint8_t)ch,
@@ -1251,14 +1252,7 @@ int main(int argc, char *argv[]) {
   break;
 
 case 'F':
-  // Pass the current environment down to the process that gets launched
-  {
-char **host_env = *_NSGetEnviron();
-char *env_entry;
-size_t i;
-for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
-  remote->Context().PushEnvironment(env_entry);
-  }
+  forward_env = true;
   break;
 
 case '2':
@@ -1420,6 +1414,18 @@ int main(int argc, char *argv[]) {
   if (start_mode == eRNBRunLoopModeExit)
 return -1;
 
+  if (forward_env || start_mode == eRNBRunLoopModeInferiorLaunching) {
+// Pass the current environment down to the process that gets launched
+// This happens automatically in the "launching" mode. For the rest, we
+// only do that if the user explicitly requested this via --forward-env
+// argument.
+char **host_env = *_NSGetEnviron();
+char *env_entry;
+size_t i;
+for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
+  remote->Context().PushEnvironmentIfNeeded(env_entry);
+  }
+
   RNBRunLoopMode mode = start_mode;
   char err_str[1024] = {'\0'};
 

Modified: lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt?rev=321355&r1=321354&r2=321355&view=diff

[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

2017-12-22 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL321355: debugserver: Propagate environment in launch-mode 
(pr35671) (authored by labath, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D41352

Files:
  lldb/trunk/tools/debugserver/source/RNBContext.cpp
  lldb/trunk/tools/debugserver/source/RNBContext.h
  lldb/trunk/tools/debugserver/source/debugserver.cpp
  lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
  lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
  lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
  lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h

Index: lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
===
--- lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
+++ lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
@@ -13,9 +13,9 @@
 add_lldb_test_executable(environment_check inferior/environment_check.cpp)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  add_definitions(-DLLDB_SERVER="$")
+  add_definitions(-DLLDB_SERVER="$" -DLLDB_SERVER_IS_DEBUGSERVER=1)
 else()
-  add_definitions(-DLLDB_SERVER="$")
+  add_definitions(-DLLDB_SERVER="$" -DLLDB_SERVER_IS_DEBUGSERVER=0)
 endif()
 
 add_definitions(
Index: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h
===
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h
@@ -21,12 +21,20 @@
 #include 
 #include 
 
+#if LLDB_SERVER_IS_DEBUGSERVER
+#define LLGS_TEST(x) DISABLED_ ## x
+#define DS_TEST(x) x
+#else
+#define LLGS_TEST(x) x
+#define DS_TEST(x) DISABLED_ ## x
+#endif
+
 namespace llgs_tests {
 class TestClient
 : public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
 public:
-  static bool IsDebugServer();
-  static bool IsLldbServer();
+  static bool IsDebugServer() { return LLDB_SERVER_IS_DEBUGSERVER; }
+  static bool IsLldbServer() { return !IsDebugServer(); }
 
   /// Launches the server, connects it to the client and returns the client. If
   /// Log is non-empty, the server will write it's log to this file.
@@ -37,6 +45,13 @@
   static llvm::Expected>
   launch(llvm::StringRef Log, llvm::ArrayRef InferiorArgs);
 
+  /// Allows user to pass additional arguments to the server. Be careful when
+  /// using this for generic tests, as the two stubs have different
+  /// command-line interfaces.
+  static llvm::Expected>
+  launchCustom(llvm::StringRef Log, llvm::ArrayRef ServerArgs, llvm::ArrayRef InferiorArgs);
+
+
   ~TestClient() override;
   llvm::Error SetInferior(llvm::ArrayRef inferior_args);
   llvm::Error ListThreadsInStopReply();
Index: lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
===
--- lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -16,11 +16,6 @@
 using namespace llvm;
 
 TEST_F(TestBase, LaunchModePreservesEnvironment) {
-  if (TestClient::IsDebugServer()) {
-GTEST_LOG_(WARNING) << "Test fails with debugserver: llvm.org/pr35671";
-return;
-  }
-
   putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
 
   auto ClientOr = TestClient::launch(getLogFileName(),
@@ -34,3 +29,20 @@
   HasValue(testing::Property(&StopReply::getKind,
  WaitStatus{WaitStatus::Exit, 0})));
 }
+
+TEST_F(TestBase, DS_TEST(DebugserverEnv)) {
+  // Test that --env takes precedence over inherited environment variables.
+  putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=foobar"));
+
+  auto ClientOr = TestClient::launchCustom(getLogFileName(),
+  { "--env", "LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE" },
+ {getInferiorPath("environment_check")});
+  ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
+  auto &Client = **ClientOr;
+
+  ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
+  ASSERT_THAT_EXPECTED(
+  Client.GetLatestStopReplyAs(),
+  HasValue(testing::Property(&StopReply::getKind,
+ WaitStatus{WaitStatus::Exit, 0})));
+}
Index: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
===
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -27,11 +27,6 @@
 using namespace llvm;
 
 namespace llgs_tests {
-bool TestClient::IsDebugServer() {
-  return sys::path::filename(LLDB_SERVER).contains("debugserver");
-}
-
-bool TestClient::IsLldbServer() { return !IsDebugServer(); }
 
 TestClient::TestClient(std::unique_ptr Conn) {
   SetConnection(Conn.release());
@@ -56,6 +51,10 @@
 }
 
 Expected> TestClient::launch(StringRef Log, ArrayRef InferiorArgs) {
+  return launchCustom(Log, {}, InferiorArgs);
+}
+
+Exp

Re: [Lldb-commits] [PATCH] D41086: [lldb] Stop searching for a symbol in a pdb by regex

2017-12-22 Thread Greg Clayton via lldb-commits
We should fix the test case to not require regex, or add 
SymbolFile::FindTypesByRegex(...) and pass that through to public API if 
needed. Not sure where the test is failing (gtest, or public API test).

> On Dec 22, 2017, at 2:01 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> This is making the windows unit tests fail: 
> http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7378/steps/run%20unit%20tests/logs/stdio.
>  If I understand correctly, this changes FindTypes to **not** search by regex.
> If that's the case, then the `SymbolFilePDBTests::TestRegexNameMatch` needs 
> to be updated to not expect that `.*` will find a match.
> Also, while technically not failing, the `SymbolFilePDBTests::TestMaxMatches` 
> also looks weird  in this new world, and probably needs updating.
> 
> Could you look into those?
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D41086
> 
> 
> 

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


Re: [Lldb-commits] [PATCH] D41086: [lldb] Stop searching for a symbol in a pdb by regex

2017-12-22 Thread Aaron Smith via lldb-commits
The url provided times out but my guess is these two unit tests are
what are failing.

lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
- TEST_F(SymbolFilePDBTests, TestRegexNameMatch)
- TEST_F(SymbolFilePDBTests, TestMaxMatches)

The first one is testing regex name matching which was decided in this
code review to not support right now. The second one is using a regex
to return all symbol names and checking that the number of results
returned by FindTypes() is consistent.

The first one can be updated to use FindTypesByRegex() if the API is
made public as Greg recommends. The second needs more thought.

On Fri, Dec 22, 2017 at 7:54 AM, Greg Clayton  wrote:
> We should fix the test case to not require regex, or add 
> SymbolFile::FindTypesByRegex(...) and pass that through to public API if 
> needed. Not sure where the test is failing (gtest, or public API test).
>
>> On Dec 22, 2017, at 2:01 AM, Pavel Labath via Phabricator 
>>  wrote:
>>
>> labath added a comment.
>>
>> This is making the windows unit tests fail: 
>> http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7378/steps/run%20unit%20tests/logs/stdio.
>>  If I understand correctly, this changes FindTypes to **not** search by 
>> regex.
>> If that's the case, then the `SymbolFilePDBTests::TestRegexNameMatch` needs 
>> to be updated to not expect that `.*` will find a match.
>> Also, while technically not failing, the 
>> `SymbolFilePDBTests::TestMaxMatches` also looks weird  in this new world, 
>> and probably needs updating.
>>
>> Could you look into those?
>>
>>
>> Repository:
>>  rL LLVM
>>
>> https://reviews.llvm.org/D41086
>>
>>
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D41550: Update failing PDB unit tests that are searching for symbols by regex to use FindTypesByRegex instead of FindTypes

2017-12-22 Thread Aaron Smith via Phabricator via lldb-commits
asmith created this revision.
asmith added reviewers: zturner, lldb-commits, labath, clayborg.

https://reviews.llvm.org/D41086 fixed an exception in 
FindTypes()/FindTypesByRegex() and caused two lldb unit test to fail. This 
change updates the unit tests to pass again.

Related review:
https://reviews.llvm.org/D41086


Repository:
  rL LLVM

https://reviews.llvm.org/D41550

Files:
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp


Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -512,13 +512,14 @@
   SymbolVendor *plugin = module->GetSymbolVendor();
   SymbolFilePDB *symfile =
   static_cast(plugin->GetSymbolFile());
-  SymbolContext sc;
-  llvm::DenseSet searched_files;
   TypeMap results;
-  uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr,
-false, 0, searched_files, results);
-  EXPECT_GT(num_results, 1u);
-  EXPECT_EQ(num_results, results.GetSize());
+  symfile->FindTypesByRegex(RegularExpression(".*"), 0, results);
+  EXPECT_GT(results.GetSize(), 1u);
+
+  // We expect no exception thrown if the given regex can't be compiled
+  results.Clear();
+  symfile->FindTypesByRegex(RegularExpression("**"), 0, results);
+  EXPECT_EQ(0u, results.GetSize());
 }
 
 TEST_F(SymbolFilePDBTests, TestMaxMatches) {
@@ -532,7 +533,8 @@
   SymbolContext sc;
   llvm::DenseSet searched_files;
   TypeMap results;
-  uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr,
+  const ConstString name("ClassTypedef");
+  uint32_t num_results = symfile->FindTypes(sc, name, nullptr,
 false, 0, searched_files, results);
   // Try to limit ourselves from 1 to 10 results, otherwise we could be doing
   // this thousands of times.
@@ -542,7 +544,7 @@
   uint32_t iterations = std::min(num_results, 10u);
   for (uint32_t i = 1; i <= iterations; ++i) {
 uint32_t num_limited_results = symfile->FindTypes(
-sc, ConstString(".*"), nullptr, false, i, searched_files, results);
+sc, name, nullptr, false, i, searched_files, results);
 EXPECT_EQ(i, num_limited_results);
 EXPECT_EQ(num_limited_results, results.GetSize());
   }
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -140,6 +140,10 @@
   size_t FindTypes(const std::vector &context,
bool append, lldb_private::TypeMap &types) override;
 
+  void FindTypesByRegex(const lldb_private::RegularExpression ®ex,
+uint32_t max_matches,
+lldb_private::TypeMap &types);
+
   lldb_private::TypeList *GetTypeList() override;
 
   size_t GetTypes(lldb_private::SymbolContextScope *sc_scope,
@@ -172,9 +176,6 @@
   const llvm::pdb::PDBSymbolCompiland &cu,
   llvm::DenseMap &index_map) const;
 
-  void FindTypesByRegex(const std::string ®ex, uint32_t max_matches,
-lldb_private::TypeMap &types);
-
   void FindTypesByName(const std::string &name, uint32_t max_matches,
lldb_private::TypeMap &types);
 


Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -512,13 +512,14 @@
   SymbolVendor *plugin = module->GetSymbolVendor();
   SymbolFilePDB *symfile =
   static_cast(plugin->GetSymbolFile());
-  SymbolContext sc;
-  llvm::DenseSet searched_files;
   TypeMap results;
-  uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr,
-false, 0, searched_files, results);
-  EXPECT_GT(num_results, 1u);
-  EXPECT_EQ(num_results, results.GetSize());
+  symfile->FindTypesByRegex(RegularExpression(".*"), 0, results);
+  EXPECT_GT(results.GetSize(), 1u);
+
+  // We expect no exception thrown if the given regex can't be compiled
+  results.Clear();
+  symfile->FindTypesByRegex(RegularExpression("**"), 0, results);
+  EXPECT_EQ(0u, results.GetSize());
 }
 
 TEST_F(SymbolFilePDBTests, TestMaxMatches) {
@@ -532,7 +533,8 @@
   SymbolContext sc;
   llvm::DenseSet searched_files;
   TypeMap results;
-  uint32_t num_results = symfile->FindTypes(sc, ConstString(".*"), nullptr,
+  const ConstString name("ClassTypedef");
+  uint32_t num_results = symfile->FindTypes(sc, name, nullptr,
 false, 0, searched_files, results);
   // Try to limit ourselves from 1 to 10 results, otherwise we could be doing
   // this thousands of times.
@@ -542,7 +544,7 @@
   uint32_t iterations = std::min

[Lldb-commits] [PATCH] D41427: Fix crash when parsing the type of a function without any arguments

2017-12-22 Thread Aaron Smith via Phabricator via lldb-commits
asmith updated this revision to Diff 128069.
asmith retitled this revision from "[lldb] Fix crash when parsing the type of a 
function without any arguments" to "Fix crash when parsing the type of a 
function without any arguments".
asmith edited the summary of this revision.

Repository:
  rL LLVM

https://reviews.llvm.org/D41427

Files:
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
  unittests/SymbolFile/PDB/Inputs/test-pdb-types.exe
  unittests/SymbolFile/PDB/Inputs/test-pdb-types.pdb
  unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -484,7 +484,10 @@
   llvm::DenseSet searched_files;
   TypeMap results;
 
-  const char *TypedefsToCheck[] = {"ClassTypedef", "NSClassTypedef"};
+  const char *TypedefsToCheck[] = {
+  "ClassTypedef", "NSClassTypedef",
+  "FuncPointerTypedef", "VariadicFuncPointerTypedef"
+  };
   for (auto Typedef : TypedefsToCheck) {
 TypeMap results;
 EXPECT_EQ(1u, symfile->FindTypes(sc, ConstString(Typedef), nullptr, false,
Index: unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
===
--- unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
+++ unittests/SymbolFile/PDB/Inputs/test-pdb-types.cpp
@@ -48,6 +48,10 @@
 
 typedef Class ClassTypedef;
 typedef NS::NSClass NSClassTypedef;
+typedef int(*FuncPointerTypedef)();
+typedef int(*VariadicFuncPointerTypedef)(char,...);
+FuncPointerTypedef GlobalFunc;
+VariadicFuncPointerTypedef GlobalVariadicFunc;
 int GlobalArray[10];
 
 static const int sizeof_NSClass = sizeof(NS::NSClass);
@@ -57,6 +61,8 @@
 static const int sizeof_ShortEnum = sizeof(ShortEnum);
 static const int sizeof_ClassTypedef = sizeof(ClassTypedef);
 static const int sizeof_NSClassTypedef = sizeof(NSClassTypedef);
+static const int sizeof_FuncPointerTypedef = sizeof(FuncPointerTypedef);
+static const int sizeof_VariadicFuncPointerTypedef = sizeof(VariadicFuncPointerTypedef);
 static const int sizeof_GlobalArray = sizeof(GlobalArray);
 
 int main(int argc, char **argv) {
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -26,12 +26,12 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionArg.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
 using namespace lldb;
 using namespace lldb_private;
-using namespace llvm;
 using namespace llvm::pdb;
 
 namespace {
@@ -141,8 +141,14 @@
   } else if (auto func_sig = llvm::dyn_cast(&type)) {
 auto arg_enum = func_sig->getArguments();
 uint32_t num_args = arg_enum->getChildCount();
+bool is_variadic = false;
 std::vector arg_list;
 while (auto arg = arg_enum->getNext()) {
+  if (arg->getSymTag() == PDB_SymType::BuiltinType &&
+  arg->getRawSymbol().getLength() == 0) {
+is_variadic = true;
+continue;
+  }
   lldb_private::Type *arg_type =
   m_ast.GetSymbolFile()->ResolveTypeUID(arg->getSymIndexId());
   // If there's some error looking up one of the dependent types of this
@@ -168,7 +174,8 @@
 if (func_sig->isVolatileType())
   type_quals |= clang::Qualifiers::Volatile;
 CompilerType func_sig_ast_type = m_ast.CreateFunctionType(
-return_ast_type, arg_list.data(), arg_list.size(), false, type_quals);
+return_ast_type, arg_list.data(), arg_list.size(), is_variadic,
+type_quals);
 
 return std::make_shared(
 func_sig->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(), 0,
@@ -188,6 +195,35 @@
 array_type->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(),
 bytes, nullptr, LLDB_INVALID_UID, lldb_private::Type::eEncodingIsUID,
 decl, array_ast_type, lldb_private::Type::eResolveStateFull);
+  } else if (auto *builtin_type = llvm::dyn_cast(&type)) {
+uint64_t bytes = builtin_type->getLength();
+PDB_BuiltinType pdb_builtin = builtin_type->getBuiltinType();
+Encoding encoding = TranslateBuiltinEncoding(pdb_builtin);
+
+// A 'void' builtin type has zero byte and invalid encoding type.
+if (bytes || encoding != eEncodingInvalid ||
+pdb_builtin == PDB_BuiltinType::Void) {
+  CompilerType builtin_ast_type =
+  m_ast.GetBuiltinTypeForEncodingAndBitSize(encoding, bytes * 8);
+
+  return std::make_shared(
+  builtin_type->getSymIndexId(), m_ast.GetSymbolFile(), ConstString(),
+  bytes, nullptr, LLDB_INV

[Lldb-commits] [PATCH] D41427: Fix crash when parsing the type of a function without any arguments

2017-12-22 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

This was originally written as a unit test because at the time we didn't have 
`lldb-test`.  To be honest I think it's time to remove these checked in 
binaries and convert everything to FileCheck tests.  There's a couple of 
reasons this is more practical.  For starters, the binaries can be really large 
and having lots of large binaries in the repo is a real drag for developers.  
Secondly, we've recently had some bug reports where some virus scan software is 
flagging our executables as malicious due to some heuristics.  So while you're 
just modifying an existing binary / test, if it's not a ton of work I would 
really prefer standardizing on `lldb-test` + `FileCheck` for this sort of 
thing.  Did you explore that at all?

While there's a little higher up front cost because you have to go update the 
`lldb-test` tool to support this, after that work is done it will make writing 
future tests very easy.  All you would need is something like this:

  REQUIRES: system-windows
  RUN: clang-cl /Z7 /c %S/Inputs/foo.cpp /o %T/foo.cpp.obj
  RUN: link /DEBUG /PDB:%T/foo.cpp.pdb /Fo:%T/foo.cpp.exe
  RUN: lldb-test symbols -types foo.cpp.exe | FileCheck %s
  
  CHECK: int(*FuncPointerTypedef)();
  CHECK: int(*VariadicFuncPointerTypedef)(char,...);


Repository:
  rL LLVM

https://reviews.llvm.org/D41427



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