maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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

llvm-svn: 295651
This commit is contained in:
Pavel Labath 2017-02-20 11:35:33 +00:00
parent 55865432b4
commit c4a3395103
13 changed files with 121 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lldb/include/lldb/Target/Process.h
View File

@ -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;
};

14
lldb/include/lldb/Utility/NameMatches.h
View File

@ -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);
}

13
lldb/include/lldb/lldb-private-enumerations.h
View File

@ -115,19 +115,6 @@ typedef enum LazyBool {
eLazyBoolYes = 1
} LazyBool;
//------------------------------------------------------------------
/// Name matching
//------------------------------------------------------------------
typedef enum NameMatchType {
eNameMatchIgnore,
eNameMatchEquals,
eNameMatchContains,
eNameMatchStartsWith,
eNameMatchEndsWith,
eNameMatchRegularExpression
} NameMatchType;
//------------------------------------------------------------------
/// Instruction types
//------------------------------------------------------------------

24
lldb/source/Commands/CommandObjectPlatform.cpp
View File

@ -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();

2
lldb/source/Commands/CommandObjectProcess.cpp
View File

@ -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();

2
lldb/source/Core/ArchSpec.cpp
View File

@ -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 {

14
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
View File

@ -1962,29 +1962,29 @@ uint32_t GDBRemoteCommunicationClient::FindProcesses(
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;
}

17
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
View File

@ -360,16 +360,15 @@ GDBRemoteCommunicationServerCommon::Handle_qfProcessInfo(
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;

8
lldb/source/Target/Process.cpp
View File

@ -581,7 +581,7 @@ llvm::ArrayRef<OptionDefinition> ProcessLaunchCommandOptions::GetDefinitions() {
}
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::MatchAllProcesses() const {
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 &attach_info) {
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) {

25
lldb/source/Utility/NameMatches.cpp
View File

@ -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;
}

12
lldb/source/Utility/RegularExpression.cpp
View File

@ -81,14 +81,10 @@ RegularExpression::~RegularExpression() { Free(); }
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;
}

1
lldb/unittests/Utility/CMakeLists.txt
View File

@ -1,6 +1,7 @@
add_lldb_unittest(UtilityTests
ConstStringTest.cpp
ErrorTest.cpp
NameMatchesTest.cpp
StringExtractorTest.cpp
TaskPoolTest.cpp
TimeoutTest.cpp

58
lldb/unittests/Utility/NameMatchesTest.cpp Normal file
View File

@ -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"));
}