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
snprintf() in verilator.h not supported in MSVC++ 2010 #927
Comments
Original Redmine Comment File name in title should be verilated_vpi.h |
Original Redmine Comment Workaround added to verilated.h (and not verilated_vpi.h). |
Original Redmine Comment Fixed in git. Please give the git version a try as I can't test it myself thanks. |
Original Redmine Comment In 3.874. |
Original Redmine Comment Unfortunately your fix didn't work and broke more. Now both verilated_vpi.h and verilatedos.h are impacted. For the sake of clarity please note that it is only about building a verilated project with MSVC++ 2010 (not building verilator itself - I wouldn't dare nor attempt with MSVC++). A fix like I proposed earlier in file verilated.h did help me to build my project (but I don't have the overview nor knowledge like you have to judge impact overall). Note that your fix also impacted building verilator with MinGW since you used _WIN32 compiler flags only and not _MSC_VER. I could build release 3.872 verilator with MinGW by simply working around setenv() in file V3Os.cpp, which is not supported by MinGW. Will post separate on that one. |
Original Redmine Comment Can you provide a patch that fixes it? Is it just using _MSC_VER? |
Original Redmine Comment A better patch than previous specified has been found in URL [[http://www.juce.com/forum/topic/solved-error-vs2015-regarding-snprintf]]. The file verilated_vpi.h is impacted only. Add following code snippet just after the #include section of file verilated_vpi.h:
That's all. Note that the _WIN32 macro in the compiler switch is probably redundant but cannot hurt either. |
Original Redmine Comment verilatedos.h is really the right point. Can you please edit the #ifdef and replace the function appropriately (to define VL_SNPRINTF) and test/attach the patch? |
Original Redmine Comment With all due respect but I don't see immediately the point in updating verilatedos.h except from reversing or adapting a previous fix you made with respect of this topic (macro VL_VSNPRINTF in verilatedos.h breaks building verilator with MinGW currently). In verilatedos.h the macro VL_VSNPRINTF is defined. However, this macro is not used in verilated_vpi.h at lines 923-926. There snprintf() is called directly without being redefined by macro as far as I can judge. If verilatedos.h is indeed the right point then a new macro VL_SNPRINTF should be defined with a dependency on _WIN32 and _MSC_VER and subsequently used in verilated_vpi.h at lines 923-926. Also macro VL_VSNPRINTF must either be removed or redefined to prevent breaking build of verilator with MinGW. Both verilatedos.h and verilated_vpi.h files will be touched. Is that what you want? |
Original Redmine Comment Please give the trunk a try. I got confused by the web reference in your original message and only thought vsnprintf was wrong. BTW it's important to put portability stuff in verilatedos.h otherwise future code is likely to misuse vsnprintf leading to later complaints. Likewise it's better to define VL_SNPRINTF than just 'snprintf' because when Microsoft finally fixes their silly limitation it won't override their snprintf. |
Original Redmine Comment Saw meanwhile that you already updated the trunk. Thanks for that! What is missing is the removal of the original fix (it was all about snprintf and not vsnprintf!) and the _MSC_VER version check to prevent potential future PRs of MSVC++ 2015 RC users. The code snippet around VL_SNPRINTF macro in file verilatedos.h should be adjusted as follows:
Would appreciate if you could again make an update of the trunk. (I don't dear and like to make an update directly in the trunk with the risk to mess things up. Would normally use a branch for that, which merges into trunk if approved.). After applying above change, I tested a build of verilator with MinGW, which went Ok. Verilating an example with the new (MinGW) verilator.exe and building the result in a MSVC++ 2010 environment (Amir Gonnen's approach and original contribution - see URL [[http://www.veripool.org/boards/1/topics/945-Verilator-Verilator-binaries-for-windows]]) went smoothless. |
Original Redmine Comment Slip of the finger "went smooth" :-). |
Original Redmine Comment Patched as suggested, thanks. Fixed in git towards 3.875. |
Original Redmine Comment In 3.876. |
Author Name: Hans Tichelaar
Original Redmine Issue: 927 from https://www.veripool.org
Original Date: 2015-06-04
Original Assignee: Wilson Snyder (@wsnyder)
Verilator 3.873 (git checkout HEAD yesterday) has file include/verilated_vpi.h with snprintf() calls in lines 923-926. The snprintf() is a C++11 feature and not supported in MSVC++ 2010 (and successors most likely although not checked by me). Workaround is given by URL http://stackoverflow.com/questions/2915672/snprintf-and-visual-studio-2010. Adding workaround at the beginning of include/verilated_vpi.h solved my issue temporarily (until next update of verilator).
The text was updated successfully, but these errors were encountered: