Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
GoogleCodeExporter opened this issue Jun 9, 2015 · 3 comments

Comments

@GoogleCodeExporter
Copy link

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
Copy link
Author

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

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

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

  • Changed state: Accepted

Attachments:

@GoogleCodeExporter
Copy link
Author

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

  • Changed state: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant