victorv / google-breakpad

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

r1370 breaks build of breakpad_client on MSVC 12 #606

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
*What steps will reproduce the problem?*

1. Check out r1370 or any later commit
2. In a command prompt, run the following commands: 

call "C:\Program Files (x86)\Microsoft Visual Studio 
12.0\Common7\Tools\VsDevCmd.bat"
C:\python27\python src\tools\gyp\gyp_main.py --no-circular-check 
src\client\windows\breakpad_client.gyp
msbuild src\client\windows\build_all.vcxproj /p:Configuration=Debug;Platform=x86

*What is the expected output? What do you see instead?*

Up to r1369, the build completes successfully. Starting with r1370, the 
following error occurs (excerpt only, full error message can be found in 
attachment):

"C:\Users\the_user_name\workspace\google-breakpad\src\client\windows\build_all.
vcxproj" (default target) (1) ->
"C:\Users\the_user_name\workspace\google-breakpad\src\client\windows\unittests\
client_tests.vcxproj" (default target) (6) ->
(Link target) ->
  exception_handler_death_test.obj : error LNK2019: unresolved external symbol
"public: bool __thiscall google_breakpad::DumpContext::GetInstructionPointer(un
signed __int64 *)const " (?GetInstructionPointer@DumpContext@google_breakpad@@Q
BE_NPA_K@Z) referenced in function "private: virtual void __thiscall `anonymous
 namespace'::ExceptionHandlerDeathTest_InstructionPointerMemoryMaxBound_Test::T
estBody(void)" (?TestBody@ExceptionHandlerDeathTest_InstructionPointerMemoryMax
Bound_Test@?A0xe2557791@@EAEXXZ) [C:\Users\the_user_name\workspace\google-break
pad\src\client\windows\unittests\client_tests.vcxproj]

*What version of the product are you using? On what operating system?*

The build is broken for at least r1370 and r1375. I'm using Windows Server 2012 
R2 with Visual Studio Express 2013 for Windows Desktop.

Original issue reported on code.google.com by tiwocph...@gmail.com on 16 Sep 2014 at 12:05

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by thestig@chromium.org on 16 Sep 2014 at 5:23

GoogleCodeExporter commented 9 years ago
I apologize for the inconvenience.

I just sent a fix for review and will submit it as soon as it is approved.

Original comment by mmandlis@chromium.org on 16 Sep 2014 at 5:57

GoogleCodeExporter commented 9 years ago
I believe the issue should be fixed now - r1377.

Could you, please, verify and let me know if you're still experiencing 
problem(s).

Thanks.

Original comment by mmandlis@chromium.org on 16 Sep 2014 at 7:18

GoogleCodeExporter commented 9 years ago
Thank you for the quick response.

With r1377, another problem surfaces (full log in the attachments):

"C:\Users\the_user_name\workspace\google-breakpad\src\client\windows\build_all.
vcxproj" (default target) (1) ->
"C:\Users\the_user_name\workspace\google-breakpad\src\client\windows\unittests\
client_tests.vcxproj" (default target) (6) ->
"C:\Users\the_user_name\workspace\google-breakpad\src\client\windows\unittests\
processor_bits.vcxproj" (default target) (10) ->
(ClCompile target) ->
  ..\..\..\processor\dump_context.cc(520): error C2146: syntax error : missing
')' before identifier 'PRIx32' [C:\Users\the_user_name\workspace\google-breakpa
d\src\client\windows\unittests\processor_bits.vcxproj]
  ..\..\..\processor\dump_context.cc(521): error C2059: syntax error : ')' [C:\
Users\the_user_name\workspace\google-breakpad\src\client\windows\unittests\proc
essor_bits.vcxproj]

Original comment by tiwocph...@gmail.com on 16 Sep 2014 at 7:43

Attachments:

GoogleCodeExporter commented 9 years ago
+primiano

I apologize for the inconvenience again.

However, the lines that are broken were pulled out exactly from the existing 
implementation in minidump.cc:
https://breakpad.appspot.com/5684002/diff/1390001/src/processor/minidump.cc 
(line 1622)

So, I'm not sure what the proper fix would be. Primiano, any ideas?

Original comment by mmandlis@chromium.org on 17 Sep 2014 at 1:26

GoogleCodeExporter commented 9 years ago
Oh yes, MSVC has its own idea of C99 and misses inttypes. As a matter of facts 
you need to move also the lines 45-53 (#ifdef _WIN32 ... #define PRIx32 ....) 
to dump_context.cc. Sorry that was hard to spot during the review :/

P.S: Can we check that this works before committing to SVN (just to avoid too 
much svn log spam)?

Original comment by primi...@chromium.org on 17 Sep 2014 at 9:22

GoogleCodeExporter commented 9 years ago
Sure, I'm willing to test any patch that you provide to me.

Original comment by tiwocph...@gmail.com on 24 Sep 2014 at 4:03

GoogleCodeExporter commented 9 years ago
I'm so sorry, I forgot to update the bug.
Actually I already submitted that CL - r1381 
(https://code.google.com/p/google-breakpad/source/detail?r=1381).
My colleague tested it and it builds successfully, but 3 tests fail. We checked 
with an earlier revision r1369 (before any of my changes were submitted) and 
found that 2 tests were failing then.
Please, let me know if you find that it is any of my changes that is causing 
the additional failure.

Original comment by mashka...@gmail.com on 24 Sep 2014 at 4:14

GoogleCodeExporter commented 9 years ago
Yes, r1385 compiles :)

On my machine, only 2 unittests fail:

[  FAILED  ] ExceptionHandlerTest.InvalidParameterMiniDumpTest
[  FAILED  ] ExceptionHandlerTest.PureVirtualCallMiniDumpTest

These are the exact same tests that already failed with r1369, so I guess you 
can close this issue. Thanks a lot!

Original comment by tiwocph...@gmail.com on 24 Sep 2014 at 4:58

GoogleCodeExporter commented 9 years ago
Thanks!

Original comment by mmandlis@chromium.org on 24 Sep 2014 at 5:08