Issue #29 new

clReflectScan runtime errors in visual studio 2013

Sander van Rossen
created an issue

I'm trying to extend clReflectExport and the binary versions are unfortunately outdated, so I need to get everything to compile because the binary file format has changed in between versions.

Unfortunately clReflectScan gives runtime errors in the clang/llvm source-code (assertion failed: isa(X)(Val) && "cast_or_null(Ty)() argument of incopatible type!", file E:...extern\llvm\include\llvm/casting.h, line 205")

This happens every time I run clReflectScan, even before it hits main.cpp.

.. and this is after days of trying to compile llvm/clang itself because apparently it causes race conditions while trying to compile lots of visual studio 2013 projects at the same time. (eventually just compiled every project one by one by hand, took hours) MSBuild is apparently really buggy.

I give up.

Comments (16)

  1. Don Williamson repo owner

    As far as I can see, there is no guarantee that clang 3.1 will even compile with VS2013.

    Latest release is clang 3.4, which supposedly supports VS2012 and up.

    The only way this will work is if you use VS2010 or get clReflect to build with clang 3.4.

    That, again, is another tricky prospect as there is no guarantee that the patches I submitted for clang to get member offsets to work with the Win32 ABI haven't been accidentally deleted again. Original thread is here:

    http://clang-developers.42468.n3.nabble.com/double-virtual-offsets-for-MSVC-broken-in-3-1-td4026435.html

    As a work-around, clang is only hosted here because it contains my fixes. The project needs to switch to a far simpler way of linking with clang. For example:

    http://clang.llvm.org/docs/LibTooling.html

  2. Sander van Rossen reporter

    Tried it with latest (well latest on llvm website) version of clang (3.4.1) and I managed to get everything compiled (needed to add some header files and lib files since some things seemed to have been moved around in the clang project) .. The only problem is that I can't actually compile clReflectGenCppbin because it relies on clReflectScan .. which crashes sigh Perhaps due to that patch you where referring to?

  3. Sander van Rossen reporter

    Ok, so crash was caused to some clang methods having changed caused wrong method to be called in ClangParser (createDiagnostics doesn't have Argc/Argv anymore). So now it seems to work, except that the clang parser has a fatal error because it can't find 'cstdio' ... so it seems the paths aren't set up correctly somehow? Any idea where I should look to fix that?

  4. Sander van Rossen reporter

    MSVC_INSTALL_PATH:PATH=../..

    maybe that has something to do with it ...? Apparently it depends on an environment variable that doesn't exist? VS80COMNTOOLS should be VS120COMNTOOLS ...

    There's probably a better way of handling this.

  5. Sander van Rossen reporter

    Now I only seem to still be getting errors in windows header files because clang doesn't recognize a keyword called "__is_constructible" .. the puzzling thing is, I can find patches online, committed to clang, that added that keyword? But it doesn't seem to be in the source now .. still investigating

  6. Sander van Rossen reporter

    I managed to get everything compiling with the current unstable version of LLVM/Clang .. One thing that does make me uneasy, however, is that I can't get it to work with lang_options.MSBitfields = 1 in ClangFrontEnd.cpp. If I use it I get the error "ms_struct may not produce MSVC-compatible layouts for classes with base classes or virtual functions" for a lot of structs/classes, in 3.4.1 these where warnings. If I comment the MSBitFields out, then it works and all the tests seem to pass .. (as far as I can see)

  7. Don Williamson repo owner

    That may be a valid fix. I'm scanning the mailing lists and can't find much reference to these issues beyond http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-October/024725.html

    In short, the clang record layout builder should not check MSBitFields anywhere, instead using it as a hint that it can override.

    If the clReflectTest tests are passing then you are highly likely to be safe - it exhaustively tries to test all possible types of C++ struct layout.

  8. Sander van Rossen reporter

    Well to be honest, I should have said I think the tests pass .. since the test executable doesn't actually say "test passed" for every test (one of the tests just dump out hash values, for instance) and I can't compare it to the original version.. :)

  9. Don Williamson repo owner

    With the layout tests you don't need to compare with previous as it compares what clReflect thinks is a field address with the compiler's (MSVC) result from address_of.

    I've just changed mine locally and it all seems to work... will run some heavier tests today on the game.

  10. Don Williamson repo owner

    I use serialisation everywhere so will be testing the record layout changes first by undoing my double-patch and running through the game. If this succeeds then it means you've found the actual bug (incorrect MSBitFields setting) and my patch to clang was not reverted.

    I can't get latest clang until I move up to at least VS2012 (I'm on 2010). I will be doing that soon - my copy is just sitting gathering dust on a shelf.

  11. Sander van Rossen reporter

    I actually made cmake work (I think) with both visual studio 2010 and 2013 .. the only problem is that I don't know how to detect the version of clang that's being used, and a couple of things changed in clang. Like the location of the lib directory .. it used to be lib/Debug/ but now it's Debug/lib/ ... sigh

  12. Don Williamson repo owner

    So... I have mixed results.

    This is my "patch patch" to clang 3.1 to ensure clReflectTest passes its tests: commit 323a297 I had to do this because somebody partially destroyed my older patch to clang 3.0 that fixed lots of Win32 ABI issues. I never submitted this to clang 3.1 because I got a little tired of my changes being lost in the noise and didn't have the time to push through the patch process again.

    It fixes data types like this: DoubleInPolymorphicStruct

    The output from clReflectTest before the fix was:

    Offsets::DoubleInPolymorphicStruct          24    16 FAILED
    ----------------------------------------
    a                                            8     4 FAILED
    b                                           16     8 FAILED
    

    The values on the left are the sizeof and offsetof the type and fields of the struct as calculated by MSVC. The values on the right are what clang calculates.

    The bad news is that reverting my changes to clang 3.1 and removing the use of MSBitfields does not fix the problem. So 3.1 is still broken and I wasn't setting it up incorrectly.

    (it's stuff like this that highlights how hokey clang Windows ABI support has been for a long time)

    The good news is that if you have been building and running clReflectTest correctly with 3.4 and it passes these tests, I can go ahead with an update (after switching to MSVC 2012 myself) and no longer have to distribute clang with clReflect.

    I can start shipping prebuilt binaries instead!

  13. Sander van Rossen reporter

    I just double checked to see if that particular test didn't fail, and it generates the correct results! I'm not using 3.4.1 though, I'm using the latest version from the trunk of their subversion server. I haven't been able to get 3.4.1 to compile with visual studio 2013 because a certain internal MSVC keyword was missing in clangs' implementation :-/ They seemed to have added it later on.

    So ... I can't guarantee you that it'll work with 3.4.1 ... but it works with the latest version, for sure

  14. Log in to comment