Revert 289067 "Use RE string pattern matching for blacklist stri..."

Revert 289067 "Use RE string pattern matching for blacklist stri..."

Reason for revert: uninitialize memory access

http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20Mac%20%28valgrind%29%282%29/builds/29469/steps/memory%20test%3A%20content/logs/stdio

> Use RE string pattern matching for blacklist strings.
> 
> Including cpu_brand, gl_vendor, gl_renderer, gl_extension, driver_vendor,
> and machine_model_name.
> 
> BUG=396578
> TBR=piman@chromium.org
> TEST=gpu_unittests
> 
> Review URL: https://codereview.chromium.org/452293002

TBR=zmo@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289154

[oshima, 2014-08-13 01:47:44]

Committed patchset #1 manually as r289154.

[Zhenyao Mo, 2014-08-13 02:09:58]

On 2014/08/13 01:47:44, oshima wrote:
> Committed patchset #1 manually as r289154.

Are you sure this is the right action?  I searched chromium codebase and there
are multiple use cases of re2::FullMatch().  Whatever issue you are seeing, it's
likely false alarm.

[oshima, 2014-08-13 02:34:07]

On 2014/08/13 02:09:58, Zhenyao Mo wrote:
> On 2014/08/13 01:47:44, oshima wrote:
> > Committed patchset #1 manually as r289154.
> 
> Are you sure this is the right action?  I searched chromium codebase and there
> are multiple use cases of re2::FullMatch().  Whatever issue you are seeing,
it's
> likely false alarm.

Yes, this is the right action, given that:

a) It's uninitialized memory access, not just a leak.
b) The stack has gpu::GpuControlList::GpuControlListEntry
c) and your CL is the only CL in the blame list that touches it.

I'll reland if it didn't fix the issue.

[Zhenyao Mo, 2014-08-13 02:46:36]

On 2014/08/13 02:34:07, oshima wrote:
> On 2014/08/13 02:09:58, Zhenyao Mo wrote:
> > On 2014/08/13 01:47:44, oshima wrote:
> > > Committed patchset #1 manually as r289154.
> > 
> > Are you sure this is the right action?  I searched chromium codebase and
there
> > are multiple use cases of re2::FullMatch().  Whatever issue you are seeing,
> it's
> > likely false alarm.
> 
> Yes, this is the right action, given that:
> 
> a) It's uninitialized memory access, not just a leak.
> b) The stack has gpu::GpuControlList::GpuControlListEntry
> c) and your CL is the only CL in the blame list that touches it.
> 
> I'll reland if it didn't fix the issue.

Look at the code:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/re2/ut...

It is apparently not an access of uninitialized variabled.  It is to initialize
the variable.

I think this CL should be relanded and the failures should be supressed.

[oshima, 2014-08-13 03:46:27]

On 2014/08/13 02:46:36, Zhenyao Mo wrote:
> On 2014/08/13 02:34:07, oshima wrote:
> > On 2014/08/13 02:09:58, Zhenyao Mo wrote:
> > > On 2014/08/13 01:47:44, oshima wrote:
> > > > Committed patchset #1 manually as r289154.
> > > 
> > > Are you sure this is the right action?  I searched chromium codebase and
> there
> > > are multiple use cases of re2::FullMatch().  Whatever issue you are
seeing,
> > it's
> > > likely false alarm.
> > 
> > Yes, this is the right action, given that:
> > 
> > a) It's uninitialized memory access, not just a leak.
> > b) The stack has gpu::GpuControlList::GpuControlListEntry
> > c) and your CL is the only CL in the blame list that touches it.
> > 
> > I'll reland if it didn't fix the issue.
> 
> Look at the code:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/re2/ut...
> 
> It is apparently not an access of uninitialized variabled.  It is to
initialize
> the variable.
> 
> I think this CL should be relanded and the failures should be supressed.

This is still right *action* to do. Please understand that sheriff's
responsibility is to
keep bots green, not to identify the root cause nor fixing the bug. If the
suppression is
the right thing to do, I'm fine with that.

That's being said, I don't fully understand why we don't have suppression for
this already
even though there are other usages of RE2::FullMatch like you said (although
there seems to be not many).

Do you have good explanation for this?

[oshima, 2014-08-13 06:38:01]

On 2014/08/13 03:46:27, oshima wrote:
> On 2014/08/13 02:46:36, Zhenyao Mo wrote:
> > On 2014/08/13 02:34:07, oshima wrote:
> > > On 2014/08/13 02:09:58, Zhenyao Mo wrote:
> > > > On 2014/08/13 01:47:44, oshima wrote:
> > > > > Committed patchset #1 manually as r289154.
> > > > 
> > > > Are you sure this is the right action?  I searched chromium codebase and
> > there
> > > > are multiple use cases of re2::FullMatch().  Whatever issue you are
> seeing,
> > > it's
> > > > likely false alarm.
> > > 
> > > Yes, this is the right action, given that:
> > > 
> > > a) It's uninitialized memory access, not just a leak.
> > > b) The stack has gpu::GpuControlList::GpuControlListEntry
> > > c) and your CL is the only CL in the blame list that touches it.
> > > 
> > > I'll reland if it didn't fix the issue.
> > 
> > Look at the code:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/re2/ut...
> > 
> > It is apparently not an access of uninitialized variabled.  It is to
> initialize
> > the variable.
> > 
> > I think this CL should be relanded and the failures should be supressed.
> 
> This is still right *action* to do. Please understand that sheriff's
> responsibility is to
> keep bots green, not to identify the root cause nor fixing the bug. If the
> suppression is
> the right thing to do, I'm fine with that.
> 
> That's being said, I don't fully understand why we don't have suppression for
> this already
> even though there are other usages of RE2::FullMatch like you said (although
> there seems to be not many).
> 
> Do you have good explanation for this?

Ok, I found the bug crbug.com/252641, so this seems to be known. I'll suppress
and reland your CL.

[oshima, 2014-08-13 15:53:22]

On 2014/08/13 06:38:01, oshima wrote:
> On 2014/08/13 03:46:27, oshima wrote:
> > On 2014/08/13 02:46:36, Zhenyao Mo wrote:
> > > On 2014/08/13 02:34:07, oshima wrote:
> > > > On 2014/08/13 02:09:58, Zhenyao Mo wrote:
> > > > > On 2014/08/13 01:47:44, oshima wrote:
> > > > > > Committed patchset #1 manually as r289154.
> > > > > 
> > > > > Are you sure this is the right action?  I searched chromium codebase
and
> > > there
> > > > > are multiple use cases of re2::FullMatch().  Whatever issue you are
> > seeing,
> > > > it's
> > > > > likely false alarm.
> > > > 
> > > > Yes, this is the right action, given that:
> > > > 
> > > > a) It's uninitialized memory access, not just a leak.
> > > > b) The stack has gpu::GpuControlList::GpuControlListEntry
> > > > c) and your CL is the only CL in the blame list that touches it.
> > > > 
> > > > I'll reland if it didn't fix the issue.
> > > 
> > > Look at the code:
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/re2/ut...
> > > 
> > > It is apparently not an access of uninitialized variabled.  It is to
> > initialize
> > > the variable.
> > > 
> > > I think this CL should be relanded and the failures should be supressed.
> > 
> > This is still right *action* to do. Please understand that sheriff's
> > responsibility is to
> > keep bots green, not to identify the root cause nor fixing the bug. If the
> > suppression is
> > the right thing to do, I'm fine with that.
> > 
> > That's being said, I don't fully understand why we don't have suppression
for
> > this already
> > even though there are other usages of RE2::FullMatch like you said (although
> > there seems to be not many).
> > 
> > Do you have good explanation for this?
> 
> Ok, I found the bug crbug.com/252641, so this seems to be known. I'll suppress
> and reland your CL.

I'm working on this right now.