[libfuzzer] Added script for dictionary generation.

[libfuzzer] Added script for dictionary generation.

BUG=624752
R=inferno@chromium.org, ochang@chromium.org, sky@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/1a6bef1675e05626a4692ab6fa43cbbc5515299b

[mmoroz, 2016-06-30 14:41:10]

Description was changed from

==========
[libfuzzer] Added script for dictionary generation.

BUG=624752
==========

to

==========
[libfuzzer] Added script for dictionary generation.

BUG=624752
==========

[mmoroz, 2016-06-30 14:41:10]

mmoroz@chromium.org changed reviewers:
+ aizatsky@chromium.org, inferno@chromium.org, ochang@chromium.org

[mmoroz, 2016-06-30 14:43:06]

On 2016/06/30 14:41:10, mmoroz wrote:
> mailto:mmoroz@chromium.org changed reviewers:
> + mailto:aizatsky@chromium.org, mailto:inferno@chromium.org,
mailto:ochang@chromium.org

Please check the bug for additional context.

[mmoroz, 2016-06-30 15:31:33]

On 2016/06/30 14:43:06, mmoroz wrote:
> On 2016/06/30 14:41:10, mmoroz wrote:
> > mailto:mmoroz@chromium.org changed reviewers:
> > + mailto:aizatsky@chromium.org, mailto:inferno@chromium.org,
> mailto:ochang@chromium.org
> 
> Please check the bug for additional context.

I guess I also should upload updated dictionaries for some fuzzers.

[aizatsky, 2016-06-30 22:00:27]

On 2016/06/30 at 15:31:33, mmoroz wrote:
> On 2016/06/30 14:43:06, mmoroz wrote:
> > On 2016/06/30 14:41:10, mmoroz wrote:
> > > mailto:mmoroz@chromium.org changed reviewers:
> > > + mailto:aizatsky@chromium.org, mailto:inferno@chromium.org,
> > mailto:ochang@chromium.org
> > 
> > Please check the bug for additional context.
> 
> I guess I also should upload updated dictionaries for some fuzzers.

Is it possible to generate dictionaries rather than check them in? This way they
will be updated whenever new code is written. How long is dictionary generation
process?

[mmoroz, 2016-07-01 09:19:03]

On 2016/06/30 22:00:27, aizatsky wrote:
> On 2016/06/30 at 15:31:33, mmoroz wrote:
> > On 2016/06/30 14:43:06, mmoroz wrote:
> > > On 2016/06/30 14:41:10, mmoroz wrote:
> > > > mailto:mmoroz@chromium.org changed reviewers:
> > > > + mailto:aizatsky@chromium.org, mailto:inferno@chromium.org,
> > > mailto:ochang@chromium.org
> > > 
> > > Please check the bug for additional context.
> > 
> > I guess I also should upload updated dictionaries for some fuzzers.
> 
> Is it possible to generate dictionaries rather than check them in? This way
they
> will be updated whenever new code is written. How long is dictionary
generation
> process?

The process is no longer than ~1 second. I agree that it would be nice to
generate dictionaries on ClusterFuzz, but there are some issues:
1) need to provide a format specification (RFC or any other text document), this
need to be done once for every fuzzer, but I'm not sure that we can store it in
the repo
2) I recommend to use a binary built without ASan and libFuzzer
("use_libfuzzer=false is_asan=false") for generation, it produces dictionaries
2-3 times smaller with the same coverage (check "ASan vs. noSan (fixed seed)"
sheet:
https://docs.google.com/spreadsheets/d/1r-dz8b5nG7SIRT1p1LDwF_r9J68UTkHgSU0_j...)

Also I would say that fuzzers get updated not so often to re-generate
dictionaries. The most important would be dynamic analysis to find new
recommended dictionary elements to increase coverage  and improve dictionaries
over time.

Basically, this script is just the first step for now. The next step will be a
dynamically updated dictionary stored outside of the repository (the same way we
do with corpus).

[Oliver Chang, 2016-07-01 19:10:46]

lgtm with some nits

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
File testing/libfuzzer/dictionary_generator.py (right):

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:124: rodata_path =
PreprocessBinary(filepath)
is rodata_path ever deleted?

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:145: temp_filepath =
os.path.join('/tmp', os.path.basename(filepath) + '_copy')
can you use the tempfile instead?

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:197: parser.add_argument('--spec',
required=True,
can the spec option be optional? is it reasonable to fallback to only using
strings for dictionary generation in the case that we have no spec?

[aizatsky, 2016-07-01 22:28:55]

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
File testing/libfuzzer/dictionary_generator.py (right):

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:28: def
EscapeDictionaryElement(element):
Consider using string_escape:

https://docs.python.org/2/library/codecs.html

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:60: """Find text with an offset, e.g.
code samples in RFCs or data examples."""
Find space-indented text blocks, e.g. ...

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:129: cmd = 'strings -e %s -n %d %s' %
(encoding, min_length, rodata_path)
Wouldn't it be simpler to simply use regexp to extract words rather than spawn?

[inferno, 2016-07-04 00:21:56]

https://codereview.chromium.org/2115563002/diff/80001/net/BUILD.gn
File net/BUILD.gn (right):

https://codereview.chromium.org/2115563002/diff/80001/net/BUILD.gn#newcode1970
net/BUILD.gn:1970: dict =
"//testing/libfuzzer/fuzzers/dicts/generated/net_http_stream_parser_fuzzer.dict"
we should probably prefer to keep these dicts next to the code unittest dirs. in
this case probably data/dicts/generated

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
File testing/libfuzzer/dictionary_generator.py (right):

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:25: ALLOWED_CHARS = string.letters +
string.digits + '#$%&()*+,-./:;<=>?@[]^_`{|}~ '
maybe better to just use ascii table 32-126 ?

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:30: res = ''
s/res/result. full variable names are preferred.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:43: for l in lines:
nit: s/l/line, s/w/word

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:52: n = 0
nit: maybe just line.count(' ') ? or we care about other whitespace chars.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:67: # Go through every line and
concatenate quoted lines into a one string.
Can you more explanation on algorithm and add an example here in comment. This
code is hard to understand what is actually doing. maybe even rename some
variable names.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:136: def ParseSpec(filepath):
nit: s/ParseSpec/ExtractWordsFromSpec

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:145: temp_filepath =
os.path.join('/tmp', os.path.basename(filepath) + '_copy')
On 2016/07/01 19:10:45, Oliver Chang wrote:
> can you use the tempfile instead?

+1

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:149: os.popen('strip --strip-all %s' %
temp_filepath)
os.popen is deprecated, use subprocess module.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:160: data = io.open(filepath, 'r',
encoding='utf-8').read()
forgot to close file :(, also just plain open should work too. maybe also add a
os.path.exists condition.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:167: """Read a specification file and
convert it to ASCII text if needed."""
description part "convert it to ASCII text if needed" is confusing, please
rephrase.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:168: if filepath.endswith('.txt'):
This seems too hacky, i wonder if people will know how to put file extension
correctly, can we parse the data to make a judgement on this automatically.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:172: data =
ReadAndDecodeHTML(filepath)
maybe split ReadAndDecodeHtml into ReadFile and decodeHtml functions. then use
ReadFile on line 169.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:204: dictionary =
GenerateDictionary(args.fuzzer, args.spec, args.strategy)
I think you should check the platform so that binaries like strip, etc
dependencies work. Otherwise you will error out.

I think we should also think about making this script work on windows and mac as
well.

[mmoroz, 2016-07-06 15:06:19]

https://codereview.chromium.org/2115563002/diff/80001/net/BUILD.gn
File net/BUILD.gn (right):

https://codereview.chromium.org/2115563002/diff/80001/net/BUILD.gn#newcode1970
net/BUILD.gn:1970: dict =
"//testing/libfuzzer/fuzzers/dicts/generated/net_http_stream_parser_fuzzer.dict"
On 2016/07/04 00:21:55, inferno wrote:
> we should probably prefer to keep these dicts next to the code unittest dirs.
in
> this case probably data/dicts/generated

Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
File testing/libfuzzer/dictionary_generator.py (right):

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:25: ALLOWED_CHARS = string.letters +
string.digits + '#$%&()*+,-./:;<=>?@[]^_`{|}~ '
On 2016/07/04 00:21:56, inferno wrote:
> maybe better to just use ascii table 32-126 ?

Removed this white-listing at all.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:28: def
EscapeDictionaryElement(element):
On 2016/07/01 22:28:55, aizatsky wrote:
> Consider using string_escape:
> 
> https://docs.python.org/2/library/codecs.html

Thank you, it is almost exactly what I need!

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:30: res = ''
On 2016/07/04 00:21:55, inferno wrote:
> s/res/result. full variable names are preferred.

Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:43: for l in lines:
On 2016/07/04 00:21:55, inferno wrote:
> nit: s/l/line, s/w/word

Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:52: n = 0
On 2016/07/04 00:21:55, inferno wrote:
> nit: maybe just line.count(' ') ? or we care about other whitespace chars.

I'm afraid that specification in textual form may have tabs for indentation. I
agree that this looks a bit hacky, but should be safer + count() gives number of
characters through all line, not only from the beginning.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:60: """Find text with an offset, e.g.
code samples in RFCs or data examples."""
On 2016/07/01 22:28:55, aizatsky wrote:
> Find space-indented text blocks, e.g. ...

Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:67: # Go through every line and
concatenate quoted lines into a one string.
On 2016/07/04 00:21:55, inferno wrote:
> Can you more explanation on algorithm and add an example here in comment. This
> code is hard to understand what is actually doing. maybe even rename some
> variable names.

Made it a bit simpler, added comments and renamed variables. Thanks!

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:124: rodata_path =
PreprocessBinary(filepath)
On 2016/07/01 19:10:45, Oliver Chang wrote:
> is rodata_path ever deleted?

Done. Now it works via tempfile, thanks for the suggestion.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:129: cmd = 'strings -e %s -n %d %s' %
(encoding, min_length, rodata_path)
On 2016/07/01 22:28:55, aizatsky wrote:
> Wouldn't it be simpler to simply use regexp to extract words rather than
spawn?

Nice idea! Looks like it gives even a bit "cleaner" results.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:136: def ParseSpec(filepath):
On 2016/07/04 00:21:56, inferno wrote:
> nit: s/ParseSpec/ExtractWordsFromSpec

Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:145: temp_filepath =
os.path.join('/tmp', os.path.basename(filepath) + '_copy')
On 2016/07/04 00:21:56, inferno wrote:
> On 2016/07/01 19:10:45, Oliver Chang wrote:
> > can you use the tempfile instead?
> 
> +1

Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:149: os.popen('strip --strip-all %s' %
temp_filepath)
On 2016/07/04 00:21:56, inferno wrote:
> os.popen is deprecated, use subprocess module.

Thanks! Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:160: data = io.open(filepath, 'r',
encoding='utf-8').read()
On 2016/07/04 00:21:55, inferno wrote:
> forgot to close file :(, also just plain open should work too. maybe also add
a
> os.path.exists condition.

My bad, I thought that it is not mandatory to explicitly close the file, because
GC will close the file when exit or earlier. Done.

Good point about os.path.exists, will add it for both binary and spec, thanks!

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:167: """Read a specification file and
convert it to ASCII text if needed."""
On 2016/07/04 00:21:55, inferno wrote:
> description part "convert it to ASCII text if needed" is confusing, please
> rephrase.

Done.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:168: if filepath.endswith('.txt'):
On 2016/07/04 00:21:56, inferno wrote:
> This seems too hacky, i wonder if people will know how to put file extension
> correctly, can we parse the data to make a judgement on this automatically.

Good point, but we don't know exactly should file be HTML-decoded or not. May be
we are processing HTML RFC which contains HTML-encoded entities, and we want to
use them as-is without decoding.

I leave only reading as default behavior, but add an argument to enable HTML
decoding explicitly. I used it only once per 10+ fuzzers, so this is a rare
case.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:172: data =
ReadAndDecodeHTML(filepath)
On 2016/07/04 00:21:55, inferno wrote:
> maybe split ReadAndDecodeHtml into ReadFile and decodeHtml functions. then use
> ReadFile on line 169.

Yeah, but regular open() works fine for everything except of HTMLParser - data
has to be utf-8 encoded for him.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:197: parser.add_argument('--spec',
required=True,
On 2016/07/01 19:10:45, Oliver Chang wrote:
> can the spec option be optional? is it reasonable to fallback to only using
> strings for dictionary generation in the case that we have no spec?

I don't think it will be reasonable, there are too many strings even if we take
a binary without memory tools and use only rodata section. 

For example:
url_parse_fuzzer - 12744 strings, while Intersection gives only 139 words.
sqlite3_prepare_v2_fuzzer - 9333 strings, but only 354 words after Intersection.

https://codereview.chromium.org/2115563002/diff/80001/testing/libfuzzer/dicti...
testing/libfuzzer/dictionary_generator.py:204: dictionary =
GenerateDictionary(args.fuzzer, args.spec, args.strategy)
On 2016/07/04 00:21:55, inferno wrote:
> I think you should check the platform so that binaries like strip, etc
> dependencies work. Otherwise you will error out.
> 
> I think we should also think about making this script work on windows and mac
as
> well. 

Good point, thanks!

My plan is to focus on dynamic part in libFuzzer and continuous dictionary
update on CF side. Then, estimate effectiveness of this generation script
against libFuzzer+CF.

I assume that it won't be too useful after continuous dictionary update thing.
Though it should be useful for initial generation of "seed" dictionary. That
seed dictionary will be cleaned up by CF and upgraded over time.

[inferno, 2016-07-06 15:33:23]

lgtm with nits, can commit after fixing nits. Also, add reviewer for /net code,
maybe eroman.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
File testing/libfuzzer/dictionary_generator.py (right):

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:118: def HexEscapeSingleChar(c):
unused, please remove.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:123: def
ExtractWordsFromBinary(filepath, min_length=4):
nit: Put 4 in a global

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:132: strings_raw =
strings_re.findall(data)
s/strings_raw/raw_strings

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:193: data = open(filepath).read()
nit: forgot to close file.

with open(filepath, 'r') as file_handle:
  data = file_handle.read()

if is_html:
  data = DecodeHTML(data)

No need to read file in ReadAndDecodeHTML then, just rename to DecodeHTML.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/fuzz...
File testing/libfuzzer/fuzzers/BUILD.gn (right):

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/fuzz...
testing/libfuzzer/fuzzers/BUILD.gn:193: dict =
"dicts/generated/sqlite3_prepare_v2_fuzzer.dict"
Do we need to delete dicts/sql.dict ? and similarly others ?

[mmoroz, 2016-07-06 16:12:37]

Thanks for the review!

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
File testing/libfuzzer/dictionary_generator.py (right):

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:118: def HexEscapeSingleChar(c):
On 2016/07/06 15:33:23, inferno wrote:
> unused, please remove.

Done.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:123: def
ExtractWordsFromBinary(filepath, min_length=4):
On 2016/07/06 15:33:23, inferno wrote:
> nit: Put 4 in a global

Done.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:132: strings_raw =
strings_re.findall(data)
On 2016/07/06 15:33:23, inferno wrote:
> s/strings_raw/raw_strings

Done.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:193: data = open(filepath).read()
On 2016/07/06 15:33:23, inferno wrote:
> nit: forgot to close file.
> 
> with open(filepath, 'r') as file_handle:
>   data = file_handle.read()
> 
> if is_html:
>   data = DecodeHTML(data)
> 
> No need to read file in ReadAndDecodeHTML then, just rename to DecodeHTML.

HTMLParser.unescape fails in this case. But decode('ascii', 'ignore') prevents
HTMLParser from failure and produces the same results, so I add it.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/dict...
testing/libfuzzer/dictionary_generator.py:193: data = open(filepath).read()
On 2016/07/06 15:33:23, inferno wrote:
> nit: forgot to close file.
> 
> with open(filepath, 'r') as file_handle:
>   data = file_handle.read()
> 
> if is_html:
>   data = DecodeHTML(data)
> 
> No need to read file in ReadAndDecodeHTML then, just rename to DecodeHTML.

Done.

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/fuzz...
File testing/libfuzzer/fuzzers/BUILD.gn (right):

https://codereview.chromium.org/2115563002/diff/140001/testing/libfuzzer/fuzz...
testing/libfuzzer/fuzzers/BUILD.gn:193: dict =
"dicts/generated/sqlite3_prepare_v2_fuzzer.dict"
On 2016/07/06 15:33:23, inferno wrote:
> Do we need to delete dicts/sql.dict ? and similarly others ?

Hm, yes, probably would be better to delete. It will not be hard if we would
like to restore them later.

[mmoroz, 2016-07-06 16:14:01]

mmoroz@chromium.org changed reviewers:
+ eroman@chromium.org, sky@chromium.org

[mmoroz, 2016-07-06 16:15:04]

Added eroman@ for //net review and sky@ for //content (since jam@ is OOO).
Please take a look once you get a chance.

[aizatsky, 2016-07-06 18:26:24]

On 2016/07/06 at 16:15:04, mmoroz wrote:
> Added eroman@ for //net review and sky@ for //content (since jam@ is OOO).
Please take a look once you get a chance.

I suggest you move dictionaries to separate changelist(s).

[eroman, 2016-07-06 19:37:30]

https://codereview.chromium.org/2115563002/diff/180001/net/data/dicts/generat...
File net/data/dicts/generated/net_http_proxy_client_socket_fuzzer.dict (right):

https://codereview.chromium.org/2115563002/diff/180001/net/data/dicts/generat...
net/data/dicts/generated/net_http_proxy_client_socket_fuzzer.dict:166: #
Dictionary elements below have been generated automatically.
General design comments:

(1) Rather than name these files:
      net/data/dicts/generated

Can you use a name that makes it clearer these are for fuzzing.
How about:

      net/data/fuzzer_dictionaries

I also don't think "generated" needs to necessarily be in the path name, but can
be conveyed through a comment in the file itself.

(2) Was this file wholly generated? I don't believe so. In that case I don't
think it belongs in a 'generated' directory since it contains manual entries.

How about instead removing "generated" from the path and including a comment at
the top of the files that explains what part of the file was generated, and what
script was run to generate it. Otherwise it is unclear how to edit these, or if
editing them is even expected.

Thanks!

[sky, 2016-07-06 23:47:00]

LGTM

[mmoroz, 2016-07-07 08:43:44]

https://codereview.chromium.org/2115563002/diff/180001/net/data/dicts/generat...
File net/data/dicts/generated/net_http_proxy_client_socket_fuzzer.dict (right):

https://codereview.chromium.org/2115563002/diff/180001/net/data/dicts/generat...
net/data/dicts/generated/net_http_proxy_client_socket_fuzzer.dict:166: #
Dictionary elements below have been generated automatically.
On 2016/07/06 19:37:30, eroman wrote:
> General design comments:
> 
> (1) Rather than name these files:
>       net/data/dicts/generated
> 
> Can you use a name that makes it clearer these are for fuzzing.
> How about:
> 
>       net/data/fuzzer_dictionaries
> 
> I also don't think "generated" needs to necessarily be in the path name, but
can
> be conveyed through a comment in the file itself.
> 
> (2) Was this file wholly generated? I don't believe so. In that case I don't
> think it belongs in a 'generated' directory since it contains manual entries.
> 
> How about instead removing "generated" from the path and including a comment
at
> the top of the files that explains what part of the file was generated, and
what
> script was run to generate it. Otherwise it is unclear how to edit these, or
if
> editing them is even expected.
> 
> Thanks!

Good points, agree on both! Thanks for the suggestion!
I moved //net changes to another CL: https://codereview.chromium.org/2128583006
as Mike suggested, to don't block here.

[mmoroz, 2016-07-07 09:01:53]

The CQ bit was checked by mmoroz@chromium.org

[mmoroz, 2016-07-07 09:01:54]

The patchset sent to the CQ was uploaded after l-g-t-m from ochang@chromium.org,
inferno@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2115563002/#ps220001
(title: "Update comments in dictionaries.")

[commit-bot: I haz the power, 2016-07-07 09:02:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 09:23:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-07 09:23:48]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmoroz, 2016-07-07 11:21:33]

The CQ bit was checked by mmoroz@chromium.org

[mmoroz, 2016-07-07 11:21:34]

The patchset sent to the CQ was uploaded after l-g-t-m from ochang@chromium.org,
sky@chromium.org, inferno@chromium.org
Link to the patchset: https://codereview.chromium.org/2115563002/#ps240001
(title: "Restore xml.dict, will replace with generated one in a separate CL.")

[mmoroz, 2016-07-07 11:21:38]

The CQ bit was checked by mmoroz@chromium.org

[commit-bot: I haz the power, 2016-07-07 11:21:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 11:26:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-07 11:26:11]

Try jobs failed on following builders:
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)

[mmoroz, 2016-07-07 12:07:23]

On 2016/07/07 11:26:11, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)

Looks fine, except of mac trybot failed to start goma. Landing manually.

[commit-bot: I haz the power, 2016-07-07 12:11:21]

Description was changed from

==========
[libfuzzer] Added script for dictionary generation.

BUG=624752
==========

to

==========
[libfuzzer] Added script for dictionary generation.

BUG=624752
R=inferno@chromium.org, ochang@chromium.org, sky@chromium.org

Committed: https://crrev.com/1a6bef1675e05626a4692ab6fa43cbbc5515299b
Cr-Commit-Position: refs/heads/master@{#404133}
==========

[commit-bot: I haz the power, 2016-07-07 12:11:23]

Patchset 13 (id:??) landed as
https://crrev.com/1a6bef1675e05626a4692ab6fa43cbbc5515299b
Cr-Commit-Position: refs/heads/master@{#404133}

[mmoroz, 2016-07-07 12:12:57]

Description was changed from

==========
[libfuzzer] Added script for dictionary generation.

BUG=624752
R=inferno@chromium.org, ochang@chromium.org, sky@chromium.org

Committed: https://crrev.com/1a6bef1675e05626a4692ab6fa43cbbc5515299b
Cr-Commit-Position: refs/heads/master@{#404133}
==========

to

==========
[libfuzzer] Added script for dictionary generation.

BUG=624752
R=inferno@chromium.org, ochang@chromium.org, sky@chromium.org

Committed:
https://chromium.googlesource.com/chromium/src/+/1a6bef1675e05626a4692ab6fa43...
==========

[mmoroz, 2016-07-07 12:12:59]

Committed patchset #13 (id:240001) manually as
1a6bef1675e05626a4692ab6fa43cbbc5515299b (presubmit successful).