[Lldb-commits] [lldb] r295651 - Fix a couple of corner cases in NameMatches
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 20 03:35:33 PST 2017
Author: labath
Date: Mon Feb 20 05:35:33 2017
New Revision: 295651
URL: http://llvm.org/viewvc/llvm-project?rev=295651&view=rev
Log:
Fix a couple of corner cases in NameMatches
Summary:
I originally set out to move the NameMatches closer to the relevant
function and add some unit tests. However, in the process I've found a
couple of bugs in the implementation:
- the early exits where not always correct:
- (test==pattern) does not mean the match will always suceed because
of regular expressions
- pattern.empty() does not mean the match will fail because the "" is
a valid prefix of any string
So I cleaned up those and added some tests. The only tricky part here
was that regcomp() implementation on darwin did not recognise the empty
string as a regular expression and returned an REG_EMPTY error instead.
The simples fix here seemed to be to replace the empty expression with
an equivalent non-empty one.
Reviewers: clayborg, zturner
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D30094
Added:
lldb/trunk/unittests/Utility/NameMatchesTest.cpp
Modified:
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/include/lldb/Utility/NameMatches.h
lldb/trunk/include/lldb/lldb-private-enumerations.h
lldb/trunk/source/Commands/CommandObjectPlatform.cpp
lldb/trunk/source/Commands/CommandObjectProcess.cpp
lldb/trunk/source/Core/ArchSpec.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Utility/NameMatches.cpp
lldb/trunk/source/Utility/RegularExpression.cpp
lldb/trunk/unittests/Utility/CMakeLists.txt
Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Feb 20 05:35:33 2017
@@ -48,6 +48,7 @@
#include "lldb/Target/QueueList.h"
#include "lldb/Target/ThreadList.h"
#include "lldb/Utility/Error.h"
+#include "lldb/Utility/NameMatches.h"
#include "lldb/lldb-private.h"
#include "llvm/ADT/ArrayRef.h"
@@ -305,11 +306,11 @@ public:
class ProcessInstanceInfoMatch {
public:
ProcessInstanceInfoMatch()
- : m_match_info(), m_name_match_type(eNameMatchIgnore),
+ : m_match_info(), m_name_match_type(NameMatch::Ignore),
m_match_all_users(false) {}
ProcessInstanceInfoMatch(const char *process_name,
- NameMatchType process_name_match_type)
+ NameMatch process_name_match_type)
: m_match_info(), m_name_match_type(process_name_match_type),
m_match_all_users(false) {
m_match_info.GetExecutableFile().SetFile(process_name, false);
@@ -323,9 +324,9 @@ public:
void SetMatchAllUsers(bool b) { m_match_all_users = b; }
- NameMatchType GetNameMatchType() const { return m_name_match_type; }
+ NameMatch GetNameMatchType() const { return m_name_match_type; }
- void SetNameMatchType(NameMatchType name_match_type) {
+ void SetNameMatchType(NameMatch name_match_type) {
m_name_match_type = name_match_type;
}
@@ -338,7 +339,7 @@ public:
protected:
ProcessInstanceInfo m_match_info;
- NameMatchType m_name_match_type;
+ NameMatch m_name_match_type;
bool m_match_all_users;
};
Modified: lldb/trunk/include/lldb/Utility/NameMatches.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/NameMatches.h?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Utility/NameMatches.h (original)
+++ lldb/trunk/include/lldb/Utility/NameMatches.h Mon Feb 20 05:35:33 2017
@@ -9,12 +9,20 @@
#ifndef LLDB_UTILITY_NAMEMATCHES_H
#define LLDB_UTILITY_NAMEMATCHES_H
-#include "lldb/lldb-private-enumerations.h"
-
#include "llvm/ADT/StringRef.h"
namespace lldb_private {
-bool NameMatches(llvm::StringRef name, NameMatchType match_type,
+
+enum class NameMatch {
+ Ignore,
+ Equals,
+ Contains,
+ StartsWith,
+ EndsWith,
+ RegularExpression
+};
+
+bool NameMatches(llvm::StringRef name, NameMatch match_type,
llvm::StringRef match);
}
Modified: lldb/trunk/include/lldb/lldb-private-enumerations.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-private-enumerations.h?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/include/lldb/lldb-private-enumerations.h (original)
+++ lldb/trunk/include/lldb/lldb-private-enumerations.h Mon Feb 20 05:35:33 2017
@@ -116,19 +116,6 @@ typedef enum LazyBool {
} LazyBool;
//------------------------------------------------------------------
-/// Name matching
-//------------------------------------------------------------------
-typedef enum NameMatchType {
- eNameMatchIgnore,
- eNameMatchEquals,
- eNameMatchContains,
- eNameMatchStartsWith,
- eNameMatchEndsWith,
- eNameMatchRegularExpression
-
-} NameMatchType;
-
-//------------------------------------------------------------------
/// Instruction types
//------------------------------------------------------------------
typedef enum InstructionType {
Modified: lldb/trunk/source/Commands/CommandObjectPlatform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectPlatform.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectPlatform.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectPlatform.cpp Mon Feb 20 05:35:33 2017
@@ -1182,21 +1182,21 @@ protected:
m_options.match_info.GetProcessInfo().GetName();
if (match_name && match_name[0]) {
switch (m_options.match_info.GetNameMatchType()) {
- case eNameMatchIgnore:
+ case NameMatch::Ignore:
break;
- case eNameMatchEquals:
+ case NameMatch::Equals:
match_desc = "matched";
break;
- case eNameMatchContains:
+ case NameMatch::Contains:
match_desc = "contained";
break;
- case eNameMatchStartsWith:
+ case NameMatch::StartsWith:
match_desc = "started with";
break;
- case eNameMatchEndsWith:
+ case NameMatch::EndsWith:
match_desc = "ended with";
break;
- case eNameMatchRegularExpression:
+ case NameMatch::RegularExpression:
match_desc = "matched the regular expression";
break;
}
@@ -1342,31 +1342,31 @@ protected:
case 'n':
match_info.GetProcessInfo().GetExecutableFile().SetFile(option_arg,
false);
- match_info.SetNameMatchType(eNameMatchEquals);
+ match_info.SetNameMatchType(NameMatch::Equals);
break;
case 'e':
match_info.GetProcessInfo().GetExecutableFile().SetFile(option_arg,
false);
- match_info.SetNameMatchType(eNameMatchEndsWith);
+ match_info.SetNameMatchType(NameMatch::EndsWith);
break;
case 's':
match_info.GetProcessInfo().GetExecutableFile().SetFile(option_arg,
false);
- match_info.SetNameMatchType(eNameMatchStartsWith);
+ match_info.SetNameMatchType(NameMatch::StartsWith);
break;
case 'c':
match_info.GetProcessInfo().GetExecutableFile().SetFile(option_arg,
false);
- match_info.SetNameMatchType(eNameMatchContains);
+ match_info.SetNameMatchType(NameMatch::Contains);
break;
case 'r':
match_info.GetProcessInfo().GetExecutableFile().SetFile(option_arg,
false);
- match_info.SetNameMatchType(eNameMatchRegularExpression);
+ match_info.SetNameMatchType(NameMatch::RegularExpression);
break;
case 'A':
@@ -1585,7 +1585,7 @@ public:
if (partial_name) {
match_info.GetProcessInfo().GetExecutableFile().SetFile(
partial_name, false);
- match_info.SetNameMatchType(eNameMatchStartsWith);
+ match_info.SetNameMatchType(NameMatch::StartsWith);
}
platform_sp->FindProcesses(match_info, process_infos);
const uint32_t num_matches = process_infos.GetSize();
Modified: lldb/trunk/source/Commands/CommandObjectProcess.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectProcess.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectProcess.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectProcess.cpp Mon Feb 20 05:35:33 2017
@@ -416,7 +416,7 @@ public:
if (partial_name) {
match_info.GetProcessInfo().GetExecutableFile().SetFile(
partial_name, false);
- match_info.SetNameMatchType(eNameMatchStartsWith);
+ match_info.SetNameMatchType(NameMatch::StartsWith);
}
platform_sp->FindProcesses(match_info, process_infos);
const size_t num_matches = process_infos.GetSize();
Modified: lldb/trunk/source/Core/ArchSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ArchSpec.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Core/ArchSpec.cpp (original)
+++ lldb/trunk/source/Core/ArchSpec.cpp Mon Feb 20 05:35:33 2017
@@ -259,7 +259,7 @@ struct ArchDefinition {
size_t ArchSpec::AutoComplete(llvm::StringRef name, StringList &matches) {
if (!name.empty()) {
for (uint32_t i = 0; i < llvm::array_lengthof(g_core_definitions); ++i) {
- if (NameMatches(g_core_definitions[i].name, eNameMatchStartsWith, name))
+ if (NameMatches(g_core_definitions[i].name, NameMatch::StartsWith, name))
matches.AppendString(g_core_definitions[i].name);
}
} else {
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Mon Feb 20 05:35:33 2017
@@ -1962,29 +1962,29 @@ uint32_t GDBRemoteCommunicationClient::F
bool has_name_match = false;
if (name && name[0]) {
has_name_match = true;
- NameMatchType name_match_type = match_info.GetNameMatchType();
+ NameMatch name_match_type = match_info.GetNameMatchType();
switch (name_match_type) {
- case eNameMatchIgnore:
+ case NameMatch::Ignore:
has_name_match = false;
break;
- case eNameMatchEquals:
+ case NameMatch::Equals:
packet.PutCString("name_match:equals;");
break;
- case eNameMatchContains:
+ case NameMatch::Contains:
packet.PutCString("name_match:contains;");
break;
- case eNameMatchStartsWith:
+ case NameMatch::StartsWith:
packet.PutCString("name_match:starts_with;");
break;
- case eNameMatchEndsWith:
+ case NameMatch::EndsWith:
packet.PutCString("name_match:ends_with;");
break;
- case eNameMatchRegularExpression:
+ case NameMatch::RegularExpression:
packet.PutCString("name_match:regex;");
break;
}
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp Mon Feb 20 05:35:33 2017
@@ -360,16 +360,15 @@ GDBRemoteCommunicationServerCommon::Hand
extractor.GetHexByteString(file);
match_info.GetProcessInfo().GetExecutableFile().SetFile(file, false);
} else if (key.equals("name_match")) {
- NameMatchType name_match =
- llvm::StringSwitch<NameMatchType>(value)
- .Case("equals", eNameMatchEquals)
- .Case("starts_with", eNameMatchStartsWith)
- .Case("ends_with", eNameMatchEndsWith)
- .Case("contains", eNameMatchContains)
- .Case("regex", eNameMatchRegularExpression)
- .Default(eNameMatchIgnore);
+ NameMatch name_match = llvm::StringSwitch<NameMatch>(value)
+ .Case("equals", NameMatch::Equals)
+ .Case("starts_with", NameMatch::StartsWith)
+ .Case("ends_with", NameMatch::EndsWith)
+ .Case("contains", NameMatch::Contains)
+ .Case("regex", NameMatch::RegularExpression)
+ .Default(NameMatch::Ignore);
match_info.SetNameMatchType(name_match);
- if (name_match == eNameMatchIgnore)
+ if (name_match == NameMatch::Ignore)
return SendErrorResponse(2);
} else if (key.equals("pid")) {
lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Feb 20 05:35:33 2017
@@ -581,7 +581,7 @@ llvm::ArrayRef<OptionDefinition> Process
}
bool ProcessInstanceInfoMatch::NameMatches(const char *process_name) const {
- if (m_name_match_type == eNameMatchIgnore || process_name == nullptr)
+ if (m_name_match_type == NameMatch::Ignore || process_name == nullptr)
return true;
const char *match_name = m_match_info.GetName();
if (!match_name)
@@ -627,7 +627,7 @@ bool ProcessInstanceInfoMatch::Matches(
}
bool ProcessInstanceInfoMatch::MatchAllProcesses() const {
- if (m_name_match_type != eNameMatchIgnore)
+ if (m_name_match_type != NameMatch::Ignore)
return false;
if (m_match_info.ProcessIDIsValid())
@@ -659,7 +659,7 @@ bool ProcessInstanceInfoMatch::MatchAllP
void ProcessInstanceInfoMatch::Clear() {
m_match_info.Clear();
- m_name_match_type = eNameMatchIgnore;
+ m_name_match_type = NameMatch::Ignore;
m_match_all_users = false;
}
@@ -3024,7 +3024,7 @@ Error Process::Attach(ProcessAttachInfo
if (platform_sp) {
ProcessInstanceInfoMatch match_info;
match_info.GetProcessInfo() = attach_info;
- match_info.SetNameMatchType(eNameMatchEquals);
+ match_info.SetNameMatchType(NameMatch::Equals);
platform_sp->FindProcesses(match_info, process_infos);
const uint32_t num_matches = process_infos.GetSize();
if (num_matches == 1) {
Modified: lldb/trunk/source/Utility/NameMatches.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/NameMatches.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Utility/NameMatches.cpp (original)
+++ lldb/trunk/source/Utility/NameMatches.cpp Mon Feb 20 05:35:33 2017
@@ -13,32 +13,23 @@
using namespace lldb_private;
-bool lldb_private::NameMatches(llvm::StringRef name, NameMatchType match_type,
+bool lldb_private::NameMatches(llvm::StringRef name, NameMatch match_type,
llvm::StringRef match) {
- if (match_type == eNameMatchIgnore)
- return true;
-
- if (name == match)
- return true;
-
- if (name.empty() || match.empty())
- return false;
-
switch (match_type) {
- case eNameMatchIgnore: // This case cannot occur: tested before
+ case NameMatch::Ignore:
return true;
- case eNameMatchEquals:
+ case NameMatch::Equals:
return name == match;
- case eNameMatchContains:
+ case NameMatch::Contains:
return name.contains(match);
- case eNameMatchStartsWith:
+ case NameMatch::StartsWith:
return name.startswith(match);
- case eNameMatchEndsWith:
+ case NameMatch::EndsWith:
return name.endswith(match);
- case eNameMatchRegularExpression: {
+ case NameMatch::RegularExpression: {
RegularExpression regex(match);
return regex.Execute(name);
- } break;
+ }
}
return false;
}
Modified: lldb/trunk/source/Utility/RegularExpression.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/RegularExpression.cpp?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/source/Utility/RegularExpression.cpp (original)
+++ lldb/trunk/source/Utility/RegularExpression.cpp Mon Feb 20 05:35:33 2017
@@ -81,14 +81,10 @@ RegularExpression::~RegularExpression()
bool RegularExpression::Compile(llvm::StringRef str) {
Free();
- if (!str.empty()) {
- m_re = str;
- m_comp_err = ::regcomp(&m_preg, m_re.c_str(), DEFAULT_COMPILE_FLAGS);
- } else {
- // No valid regular expression
- m_comp_err = 1;
- }
-
+ // regcomp() on darwin does not recognize "" as a valid regular expression, so
+ // we substitute it with an equivalent non-empty one.
+ m_re = str.empty() ? "()" : str;
+ m_comp_err = ::regcomp(&m_preg, m_re.c_str(), DEFAULT_COMPILE_FLAGS);
return m_comp_err == 0;
}
Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=295651&r1=295650&r2=295651&view=diff
==============================================================================
--- lldb/trunk/unittests/Utility/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Utility/CMakeLists.txt Mon Feb 20 05:35:33 2017
@@ -1,6 +1,7 @@
add_lldb_unittest(UtilityTests
ConstStringTest.cpp
ErrorTest.cpp
+ NameMatchesTest.cpp
StringExtractorTest.cpp
TaskPoolTest.cpp
TimeoutTest.cpp
Added: lldb/trunk/unittests/Utility/NameMatchesTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/NameMatchesTest.cpp?rev=295651&view=auto
==============================================================================
--- lldb/trunk/unittests/Utility/NameMatchesTest.cpp (added)
+++ lldb/trunk/unittests/Utility/NameMatchesTest.cpp Mon Feb 20 05:35:33 2017
@@ -0,0 +1,58 @@
+//===-- NameMatchesTest.cpp -------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Utility/NameMatches.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(NameMatchesTest, Ignore) {
+ EXPECT_TRUE(NameMatches("foo", NameMatch::Ignore, "bar"));
+}
+
+TEST(NameMatchesTest, Equals) {
+ EXPECT_TRUE(NameMatches("foo", NameMatch::Equals, "foo"));
+ EXPECT_FALSE(NameMatches("foo", NameMatch::Equals, "bar"));
+}
+
+TEST(NameMatchesTest, Contains) {
+ EXPECT_TRUE(NameMatches("foobar", NameMatch::Contains, "foo"));
+ EXPECT_TRUE(NameMatches("foobar", NameMatch::Contains, "oob"));
+ EXPECT_TRUE(NameMatches("foobar", NameMatch::Contains, "bar"));
+ EXPECT_TRUE(NameMatches("foobar", NameMatch::Contains, "foobar"));
+ EXPECT_TRUE(NameMatches("", NameMatch::Contains, ""));
+ EXPECT_FALSE(NameMatches("", NameMatch::Contains, "foo"));
+ EXPECT_FALSE(NameMatches("foobar", NameMatch::Contains, "baz"));
+}
+
+TEST(NameMatchesTest, StartsWith) {
+ EXPECT_TRUE(NameMatches("foo", NameMatch::StartsWith, "f"));
+ EXPECT_TRUE(NameMatches("foo", NameMatch::StartsWith, ""));
+ EXPECT_TRUE(NameMatches("", NameMatch::StartsWith, ""));
+ EXPECT_FALSE(NameMatches("foo", NameMatch::StartsWith, "b"));
+ EXPECT_FALSE(NameMatches("", NameMatch::StartsWith, "b"));
+}
+
+TEST(NameMatchesTest, EndsWith) {
+ EXPECT_TRUE(NameMatches("foo", NameMatch::EndsWith, "o"));
+ EXPECT_TRUE(NameMatches("foo", NameMatch::EndsWith, ""));
+ EXPECT_TRUE(NameMatches("", NameMatch::EndsWith, ""));
+ EXPECT_FALSE(NameMatches("foo", NameMatch::EndsWith, "b"));
+ EXPECT_FALSE(NameMatches("", NameMatch::EndsWith, "b"));
+}
+
+TEST(NameMatchesTest, RegularExpression) {
+ EXPECT_TRUE(NameMatches("foobar", NameMatch::RegularExpression, "foo"));
+ EXPECT_TRUE(NameMatches("foobar", NameMatch::RegularExpression, "f[oa]o"));
+ EXPECT_TRUE(NameMatches("foo", NameMatch::RegularExpression, ""));
+ EXPECT_TRUE(NameMatches("", NameMatch::RegularExpression, ""));
+ EXPECT_FALSE(NameMatches("foo", NameMatch::RegularExpression, "b"));
+ EXPECT_FALSE(NameMatches("", NameMatch::RegularExpression, "b"));
+ EXPECT_FALSE(NameMatches("^a", NameMatch::RegularExpression, "^a"));
+}
More information about the lldb-commits
mailing list