boinc icon indicating copy to clipboard operation
boinc copied to clipboard

On C++11 and up support

Open dimhotepus opened this issue 5 years ago • 22 comments

Does BOINC support C++11?

For example, i want to fix undefined behavior (which may lead to security issues) when printf-like functions get incompatible format specifier and actual parameter type: https://github.com/BOINC/boinc/pull/3633 In order to do that, i either need to include <cinttypes> (C++11 thing) for portable format specifiers, or add ugly macroses to duplicate such functionality in BOINC headers.

If we support MinGW builds, things become even more complicated. MinGW uses old msvcrt runtime, and requires special define __USE_MINGW_ANSI_STDIO=1 to mimic C11 behavior (%zu format specifier for size_t). This also can lead to changes in how other format specifiers work on MinGW builds.

So possible solution is to

  1. Define all needed format specifier macroses manually (including special cases for MinGW) in BOINC headers. Hard to do correctly, but possible (Visual Studio added support of different format specifiers from different versions, MinGW with its own formatters, 32 vs 64 bit types, etc).

or

Allow C++11 in gcc builds (as it's done for Windows builds on vs2019) and

  1. Define special portable format specifiers based on <cinttypes> for MinGW. or
  2. Define __USE_MINGW_ANSI_STDIO=1 for MinGW builds and use standard format specifiers from <cinttypes>. or
  3. Drop support of MinGW and use VsCode or Community versions of Visual Studio to compile BOINC on Windows. So cross-compiling Windows builds on Linux will not work.

What do you think?

dimhotepus avatar Apr 24 '20 12:04 dimhotepus

Currently c++11 is not supported. The reason for this is that current Windows release builds are still done on MS VS2010 that is not support c++11. Also, do we really need ? Is there another way to print values without these weird format specifiers like PRIuMAX ?

AenBleidd avatar Apr 24 '20 13:04 AenBleidd

It is hard to do in a cross-(platform/compiler) way. Especially if we support old Visual Studio versions, like vs2010.

Portable format specifiers were added in inttypes.h/cinttypes as a part of C++11. So we stuck with preprocessor definitions similar to PRIuMAX. Something like this (add MinGW and MSC_VER to the mix):

#ifdef _WIN32
#define PR_SIZET 
#elif defined(_WIN64)
#define PR_SIZET "I"
#elif defined(__linux__) 
#if defined(__x86_64__)
#define PR_SIZET  "z"
#else
#define PR_SIZET
#endif
#else
#error Please, define PR_SIZET for your platform
#endif



size_t u;
printf("%" PR_SIZET "x", u);

dimhotepus avatar Apr 24 '20 13:04 dimhotepus

Is there a bug here, or is it just about compiler warnings?

davidpanderson avatar Apr 25 '20 02:04 davidpanderson

In general, if the syntax of a conversion specification is invalid, behavior is undefined, and can cause program termination.

For example, at https://github.com/BOINC/boinc/pull/3633/files#diff-af2978c2e9a6dcbd81e11d25b397ecadR1284 we interpret actual data (size_t, unsigned, 8 bytes long on x86-64) as int (4 bytes, signed), which can lead to dumping incorrect process statistics values when they are > INT_MAX.

at https://github.com/BOINC/boinc/pull/3633/files#diff-af2978c2e9a6dcbd81e11d25b397ecadR1434 and https://github.com/BOINC/boinc/pull/3633/files#diff-af2978c2e9a6dcbd81e11d25b397ecadR1444 we output incorrect addresses of exception sources on x86-64 when they > UINT_MAX.

at https://github.com/BOINC/boinc/pull/3633/files#diff-2100491c40a00d1c0a6c54ed6a816619R732 we dump incorrect cpu registers / stack frame values on x86-64 when they > UINT_MAX.

dimhotepus avatar Apr 25 '20 19:04 dimhotepus

I recall that I used inttypes.h around year 2007 on Linux and Solaris, before I even heard about C++11. BOINC could check in ./configure if this header and macros are present and use them. If not, it could provide its own ones.

sirzooro avatar Apr 30 '20 21:04 sirzooro

Problem is gcc rejects to build with inttypes.h if -std=c++11 not passed as compiler option.

dimhotepus avatar May 02 '20 06:05 dimhotepus

Decided to define portable size_t printf format specifier across different compilers.

dimhotepus avatar May 02 '20 06:05 dimhotepus

Wondering what the reason is to still support Visual Studio 2010.

cminnoy avatar Feb 10 '21 21:02 cminnoy

Ain't there a way to upgrade to a newer Visual Studio?

Secondly, I noticed that clearly on Ubuntu C++11 features compile fine for BOINC. So if no C++11 features are allowed, than the compiler options should specify the standard to enforce compliance.

cnergyone avatar Feb 16 '21 12:02 cnergyone

@cnergyone,

Ain't there a way to upgrade to a newer Visual Studio?

It's already done. But while there is no new release for Windows with VS2019 - we don't drop VS2010 support.

So if no C++11 features are allowed, than the compiler options should specify the standard to enforce compliance.

Because next release we want to make with VS2019, so no need to add this restriction now.

So this ticket will leave for a while and soon will be closed without any fix.

AenBleidd avatar Feb 16 '21 12:02 AenBleidd

I'm implementing ticket 4180. You're saying that I can now use C++11 statements?

What will be the new standard after the next release? C++11, C++14 or C++17? I vote for C++17

cnergyone avatar Feb 16 '21 12:02 cnergyone

I'm implementing ticket 4180. You're saying that I can now use C++11 statements?

After BOINC will be released for Windows and I close this ticket - yes.

Regarding next standard - I don't know yet.

AenBleidd avatar Feb 16 '21 13:02 AenBleidd

I'm implementing ticket 4180. You're saying that I can now use C++11 statements?

After BOINC will be released for Windows and I close this ticket - yes.

Regarding next standard - I don't know yet.

Is it correct that after #4207 be merged I can use <cinttypes> header from C++11 standard library to deal with #3633 long running PR?

dimhotepus avatar Mar 01 '21 21:03 dimhotepus

@dimhotepus, I'd ask you to wait until next Windows release. Then coding style document should be updated with new requirements. #4207 was merged because it touched only linux code. For Windows we still want to have a fallback solution in case smth goes wrong. I'll inform you when there will be a green light for your PR ;)

AenBleidd avatar Mar 01 '21 21:03 AenBleidd

Any progress on the C++11 adoption? I noticed my pull request still fails on Android and OSX manager builds because of lack of C++11 support (auto keyword and lambda).

cnergyone avatar Mar 30 '21 12:03 cnergyone

@cnergyone, no updates right now. I'll notify you about it

AenBleidd avatar Mar 30 '21 12:03 AenBleidd

@cnergyone My understanding is that current versions of Xcode on MacOS enable C++11 support by default. But I'm not certain of that. You can test this by modifying mac_build/buildMacBOINC-CI.sh to pass the argument -c++11 to each call of source mac_build/buildMacBOINC.sh.

Let me know if making those changes fixes the errors; that would indicate that apparently C++11 support is not enabled by default. If so, I'll update the Xcode project and buildMacBOINC.sh script accordingly.

CharlieFenton avatar Mar 30 '21 13:03 CharlieFenton

I noticed my pull request still fails on Android and OSX manager builds because of lack of C++11 support (auto keyword and lambda).

@cnergyone Which pull request is failing on MacOS Manager due to those keywords? The only PR I'm finding from you is #4181 and the error is not due to either of those keywords.

CharlieFenton avatar Mar 30 '21 13:03 CharlieFenton

@cnergyone Actually, GitHub is a bit confusing when I search for Pull Requests authored by you, it says #4181 but it seems to be #4207. Either way, the error for MacOS Manager is not due to either the auto keyword or lambda.

CharlieFenton avatar Mar 30 '21 23:03 CharlieFenton

@cnergyone Sorry, as I've dug into this a bit more I realize I'm not familiar with more advanced concepts in C++11 such as lambda expressions. But I have confirmed that by specifying C++11 support, your code does compile on MacOS without generating any errors. However, specifying C++11 support on MacOS does generate warnings and errors in other source files.

I will try to submit a PR in the next week or two to enable C++11 support on macOS.

CharlieFenton avatar Mar 31 '21 09:03 CharlieFenton

@cnergyone @AenBleidd I have completed the changes for C++11 support on MacOS in my PR #4304.

CharlieFenton avatar Mar 31 '21 11:03 CharlieFenton

Please be aware that we're not ready now to accept any c++11 specific features. When next release for Windows will be released with no issues using VS2019, then c++11 support will be officially announced. Please be patient and stay tuned.

AenBleidd avatar Mar 31 '21 12:03 AenBleidd