[Lldb-commits] [lldb] r373925 - ProcessInstanceInfoMatch: Don't match processes with no name if a name match was requested

[Jonas Devlieghere via lldb-commits]

Hey Pavel,

I've reverted your change in r374008. This was breaking
TestProcessAttach and TestHelloWorld on Darwin.

-- Jonas

On Mon, Oct 7, 2019 at 1:57 PM Shafik Yaghmour via lldb-commits
<lldb-commits at lists.llvm.org> wrote:
>
> Pavel,
>
> It looks like this change breaks a couple of tests:
>
>    http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/2315/
>
> Reverting it fixes those on my end.
>
> -Shafik
>
> On Oct 7, 2019, at 10:17 AM, Pavel Labath via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>
> Author: labath
> Date: Mon Oct  7 10:17:53 2019
> New Revision: 373925
>
> URL: http://llvm.org/viewvc/llvm-project?rev=373925&view=rev
> Log:
> ProcessInstanceInfoMatch: Don't match processes with no name if a name match was requested
>
> Since D68289, a couple of tests on linux started being extremely flaky.
> All of them were doing name-based attaching and were failing because
> they couldn't find an unambiguous process to attach to.
>
> The patch above changed the process finding logic, so that failure to
> find a process name does not constitute an error. This meant that a lot
> more transient processes showed up in the process list during the test
> suite run. Previously, these processes would not appear as they would be
> gone by the time we went to read their executable name, arguments, etc.
>
> Now, this alone should not cause an issue were it not for the fact that
> we were considering a process with no name as if it matched by default
> (even if we were explicitly searching for a process with a specified
> name). This meant that any of the "transient" processes with no name
> would make the name match ambiguous. That clearly seems like a bug to me
> so I fix that.
>
> Modified:
>    lldb/trunk/source/Utility/ProcessInfo.cpp
>    lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
>
> Modified: lldb/trunk/source/Utility/ProcessInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ProcessInfo.cpp?rev=373925&r1=373924&r2=373925&view=diff
> ==============================================================================
> --- lldb/trunk/source/Utility/ProcessInfo.cpp (original)
> +++ lldb/trunk/source/Utility/ProcessInfo.cpp Mon Oct  7 10:17:53 2019
> @@ -244,7 +244,7 @@ void ProcessInstanceInfo::DumpAsTableRow
> }
>
> bool ProcessInstanceInfoMatch::NameMatches(const char *process_name) const {
> -  if (m_name_match_type == NameMatch::Ignore || process_name == nullptr)
> +  if (m_name_match_type == NameMatch::Ignore)
>     return true;
>   const char *match_name = m_match_info.GetName();
>   if (!match_name)
>
> Modified: lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp?rev=373925&r1=373924&r2=373925&view=diff
> ==============================================================================
> --- lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp (original)
> +++ lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp Mon Oct  7 10:17:53 2019
> @@ -91,3 +91,20 @@ TEST(ProcessInstanceInfo, DumpTable_inva
> )",
>       s.GetData());
> }
> +
> +TEST(ProcessInstanceInfoMatch, Name) {
> +  ProcessInstanceInfo info_bar, info_empty;
> +  info_bar.GetExecutableFile().SetFile("/foo/bar", FileSpec::Style::posix);
> +
> +  ProcessInstanceInfoMatch match;
> +  match.SetNameMatchType(NameMatch::Equals);
> +  match.GetProcessInfo().GetExecutableFile().SetFile("bar",
> +                                                     FileSpec::Style::posix);
> +
> +  EXPECT_TRUE(match.Matches(info_bar));
> +  EXPECT_FALSE(match.Matches(info_empty));
> +
> +  match.GetProcessInfo().GetExecutableFile() = FileSpec();
> +  EXPECT_TRUE(match.Matches(info_bar));
> +  EXPECT_TRUE(match.Matches(info_empty));
> +}
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits