search

weiqk / cld2

Automatically exported from code.google.com/p/cld2
0 stars 0 forks source link

Windows compile process for Chromium unhappy with zero-length array declarations #9

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
In these files (and any others, obviously):
cld2_generated_distinctoctachrome0122
cld2_generated_deltaoctachrome0122

The Windows compile chain for Chromium is upset because there is an attempt to 
declare a zero-length array. Dick has noted this as a concern when we let the 
size be zero, and it seems the concern is valid under the Chromium build chain 
on Windows.

From Chromium's buildbots, here are the error messages from compilation:

FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma\gomacc.exe 
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo 
/showIncludes /FC 
@obj\third_party\cld_2\src\internal\cld_2.cld2_generated_distinctoctachrome0122.
obj.rsp /c 
..\..\third_party\cld_2\src\internal\cld2_generated_distinctoctachrome0122.cc 
/Foobj\third_party\cld_2\src\internal\cld_2.cld2_generated_distinctoctachrome012
2.obj /Fdobj\third_party\cld_2\cld_2.cc.pdb 
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_generated_dis
tinctoctachrome0122.cc(2184) : error C2466: cannot allocate an array of 
constant size 0
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_generated_dis
tinctoctachrome0122.cc(2186) : error C2466: cannot allocate an array of 
constant size 0
FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma\gomacc.exe 
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo 
/showIncludes /FC 
@obj\third_party\cld_2\src\internal\cld_2.cld2_generated_deltaoctachrome0122.obj
.rsp /c 
..\..\third_party\cld_2\src\internal\cld2_generated_deltaoctachrome0122.cc 
/Foobj\third_party\cld_2\src\internal\cld_2.cld2_generated_deltaoctachrome0122.o
bj /Fdobj\third_party\cld_2\cld_2.cc.pdb 
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_generated_del
taoctachrome0122.cc(4577) : error C2466: cannot allocate an array of constant 
size 0
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_generated_del
taoctachrome0122.cc(4579) : error C2466: cannot allocate an array of constant 
size 0
ninja: build stopped: subcommand failed.

The workaround we had in place before was to have the constants for size *say* 
zero, i.e. the code will never read anything from the array and the dynamic 
data tool will just skip it. We'd then actually allocate an array of size one 
(however many bytes, usually 4 for our use cases of uint32). This makes the 
compiler happy at a cost of a few bytes of overhead in non-dynamic mode. Seems 
like we don't really have a choice here, so I'll prepare the patch.

Original issue reported on code.google.com by andrewha...@google.com on 12 Mar 2014 at 11:32

GoogleCodeExporter commented 9 years ago 
Patch submitted as r155, and I've once again verified that dynamic mode 
produces the same binary file before and after the change, all unit tests pass 
in all compilation scripts, and the world should be good again.

Original comment by andrewha...@google.com on 12 Mar 2014 at 11:55

GoogleCodeExporter commented 9 years ago 
I missed two files when I did this:
  cld2_generated_quad0122.cc
  cld2_generated_quadchrome0122_2.cc

I hadn't noticed them because they weren't getting compiled in Chromium, but 
I've found them now. I did some creative grepping for other places where we set 
array sizes of zero, but haven't found any other instances. The _16 and _19 
table files do not share this problem.

Reopening. Proposed patch attached.

Original comment by andrewha...@google.com on 14 Mar 2014 at 8:47

Attachments:

GoogleCodeExporter commented 9 years ago 
I have verified again that the dynamic mode data file is unchanged by this 
patch and that all unit tests of all flavors run successfully, so I've gone 
ahead and submitted this as r156.

Original comment by andrewha...@google.com on 14 Mar 2014 at 8:55