sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-24 19:42:46 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #18088: build: ensure we aren't using GNU extensions

Browse Source 
0ae8f18dfe143051fec6ae10ea7df10142e3ff2f build: add -Wgnu to compile flags (fanquake)
3a0fd7726b8b916de6cce33bb67f48990575f923 Remove use of non-standard zero variadic macros (Ben Woosley)
49f6178c3e5e3ad54a419da9d8523207da17fc64 Drop unused LOG_TIME_MICROS helper (Ben Woosley)
5d4999951ee32e333b511245862628e80f83b703 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes)

Pull request description:

  Since we [started using](https://github.com/bitcoin/bitcoin/pull/7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile".

  However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former.

  #### anonymous structs
  ```bash
  ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct]
          struct {
  ```

  This is fixed in b849212c1e.

  #### variadic macros

  ```bash
  ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]
              ::Unserialize(s, VARINT(nVersionDummy));
  ```

  This is taken care of in #18087.

  The `LOG_TIME_*` macros introduced in #16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html).

  ```bash
  In file included from validation.cpp:22:
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                                    ^
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                             ^
  6 warnings generated.
  ```

  This is fixed in 081a0ab64eb442bc85c4d4a4d3bc2c8e97ac2a6d and 612e8e138b97fc5ad2f38847300132a8fc423c3f.

  #### prevention
  To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions.

  This would close #14130.
  Also related to #17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute.

ACKs for top commit:
  practicalswift:
    ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f -- diff looks correct
  MarcoFalke:
    ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f
  vasild:
    utACK 0ae8f18df
  dongcarl:
    ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f

Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
This commit is contained in:
fanquake 2020-05-05 07:19:44 +08:00 committed by Vijay Das Manikpuri
parent cb3be0fd3b
commit afdb731442
No known key found for this signature in database
GPG Key ID: DB1D81B01DB7C46E
4 changed files with 22 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
configure.ac
View File

@ -440,6 +440,7 @@ fi
if test "x$CXXFLAGS_overridden" = "xno"; then if test "x$CXXFLAGS_overridden" = "xno"; then
AX_CHECK_COMPILE_FLAG([-Wall],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wall],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wextra],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wextra"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wextra],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wextra"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wgnu],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wgnu"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wformat],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wformat],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wvla],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wvla],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wshadow-field],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-field"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wshadow-field],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-field"],,[[$CXXFLAG_WERROR]])

10
src/logging/timer.h
View File

@ -93,12 +93,10 @@ private:
} // namespace BCLog } // namespace BCLog
#define LOG_TIME_MICROS(end_msg, ...) \ #define LOG_TIME_MILLIS_WITH_CATEGORY(end_msg, log_category) \
BCLog::Timer<std::chrono::microseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, log_category)
#define LOG_TIME_MILLIS(end_msg, ...) \ #define LOG_TIME_SECONDS(end_msg) \
BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg)
#define LOG_TIME_SECONDS(end_msg, ...) \
BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
#endif // BITCOIN_LOGGING_TIMER_H #endif // BITCOIN_LOGGING_TIMER_H

24
src/prevector.h
View File

@ -152,7 +152,7 @@ private:
struct { struct {
char* indirect; char* indirect;
size_type capacity; size_type capacity;
}; } indirect_contents;
}; };
#pragma pack(pop) #pragma pack(pop)
alignas(char*) direct_or_indirect _union = {}; alignas(char*) direct_or_indirect _union = {};
@ -163,8 +163,8 @@ private:
T* direct_ptr(difference_type pos) { return reinterpret_cast<T*>(_union.direct) + pos; } T* direct_ptr(difference_type pos) { return reinterpret_cast<T*>(_union.direct) + pos; }
const T* direct_ptr(difference_type pos) const { return reinterpret_cast<const T*>(_union.direct) + pos; } const T* direct_ptr(difference_type pos) const { return reinterpret_cast<const T*>(_union.direct) + pos; }
T* indirect_ptr(difference_type pos) { return reinterpret_cast<T*>(_union.indirect) + pos; } T* indirect_ptr(difference_type pos) { return reinterpret_cast<T*>(_union.indirect_contents.indirect) + pos; }
const T* indirect_ptr(difference_type pos) const { return reinterpret_cast<const T*>(_union.indirect) + pos; } const T* indirect_ptr(difference_type pos) const { return reinterpret_cast<const T*>(_union.indirect_contents.indirect) + pos; }
bool is_direct() const { return _size <= N; } bool is_direct() const { return _size <= N; }
void change_capacity(size_type new_capacity) { void change_capacity(size_type new_capacity) {
@ -182,17 +182,17 @@ private:
/* FIXME: Because malloc/realloc here won't call new_handler if allocation fails, assert /* FIXME: Because malloc/realloc here won't call new_handler if allocation fails, assert
success. These should instead use an allocator or new/delete so that handlers success. These should instead use an allocator or new/delete so that handlers
are called as necessary, but performance would be slightly degraded by doing so. */ are called as necessary, but performance would be slightly degraded by doing so. */
_union.indirect = static_cast<char*>(realloc(_union.indirect, ((size_t)sizeof(T)) * new_capacity)); _union.indirect_contents.indirect = static_cast<char*>(realloc(_union.indirect_contents.indirect, ((size_t)sizeof(T)) * new_capacity));
assert(_union.indirect); assert(_union.indirect_contents.indirect);
_union.capacity = new_capacity; _union.indirect_contents.capacity = new_capacity;
} else { } else {
char* new_indirect = static_cast<char*>(malloc(((size_t)sizeof(T)) * new_capacity)); char* new_indirect = static_cast<char*>(malloc(((size_t)sizeof(T)) * new_capacity));
assert(new_indirect); assert(new_indirect);
T* src = direct_ptr(0); T* src = direct_ptr(0);
T* dst = reinterpret_cast<T*>(new_indirect); T* dst = reinterpret_cast<T*>(new_indirect);
memcpy(dst, src, size() * sizeof(T)); memcpy(dst, src, size() * sizeof(T));
_union.indirect = new_indirect; _union.indirect_contents.indirect = new_indirect;
_union.capacity = new_capacity; _union.indirect_contents.capacity = new_capacity;
_size += N + 1; _size += N + 1;
} }
} }
@ -318,7 +318,7 @@ public:
if (is_direct()) { if (is_direct()) {
return N; return N;
} else { } else {
return _union.capacity; return _union.indirect_contents.capacity;
} }
} }
@ -485,8 +485,8 @@ public:
clear(); clear();
} }
if (!is_direct()) { if (!is_direct()) {
free(_union.indirect); free(_union.indirect_contents.indirect);
_union.indirect = nullptr; _union.indirect_contents.indirect = nullptr;
} }
} }
@ -538,7 +538,7 @@ public:
if (is_direct()) { if (is_direct()) {
return 0; return 0;
} else { } else {
return ((size_t)(sizeof(T))) * _union.capacity; return ((size_t)(sizeof(T))) * _union.indirect_contents.capacity;
} }
} }

10
src/validation.cpp
View File

@ -2488,11 +2488,11 @@ bool CChainState::FlushStateToDisk(
LOCK(cs_LastBlockFile); LOCK(cs_LastBlockFile);
if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) {
if (nManualPruneHeight > 0) { if (nManualPruneHeight > 0) {
LOG_TIME_MILLIS("find files to prune (manual)", BCLog::BENCHMARK); LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune (manual)", BCLog::BENCHMARK);
FindFilesToPruneManual(g_chainman, setFilesToPrune, nManualPruneHeight); FindFilesToPruneManual(g_chainman, setFilesToPrune, nManualPruneHeight);
} else { } else {
LOG_TIME_MILLIS("find files to prune", BCLog::BENCHMARK); LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune", BCLog::BENCHMARK);
FindFilesToPrune(g_chainman, setFilesToPrune, chainparams.PruneAfterHeight()); FindFilesToPrune(g_chainman, setFilesToPrune, chainparams.PruneAfterHeight());
fCheckForPruning = false; fCheckForPruning = false;
@ -2533,7 +2533,7 @@ bool CChainState::FlushStateToDisk(
} }
// First make sure all block and undo data is flushed to disk. // First make sure all block and undo data is flushed to disk.
{ {
LOG_TIME_MILLIS("write block and undo data to disk", BCLog::BENCHMARK); LOG_TIME_MILLIS_WITH_CATEGORY("write block and undo data to disk", BCLog::BENCHMARK);
// First make sure all block and undo data is flushed to disk. // First make sure all block and undo data is flushed to disk.
FlushBlockFile(); FlushBlockFile();
@ -2541,7 +2541,7 @@ bool CChainState::FlushStateToDisk(
// Then update all block file information (which may refer to block and undo files). // Then update all block file information (which may refer to block and undo files).
{ {
LOG_TIME_MILLIS("write block index to disk", BCLog::BENCHMARK); LOG_TIME_MILLIS_WITH_CATEGORY("write block index to disk", BCLog::BENCHMARK);
std::vector<std::pair<int, const CBlockFileInfo*> > vFiles; std::vector<std::pair<int, const CBlockFileInfo*> > vFiles;
vFiles.reserve(setDirtyFileInfo.size()); vFiles.reserve(setDirtyFileInfo.size());
@ -2561,7 +2561,7 @@ bool CChainState::FlushStateToDisk(
} }
// Finally remove any pruned files // Finally remove any pruned files
if (fFlushForPrune) { if (fFlushForPrune) {
LOG_TIME_MILLIS("unlink pruned files", BCLog::BENCHMARK); LOG_TIME_MILLIS_WITH_CATEGORY("unlink pruned files", BCLog::BENCHMARK);
UnlinkPrunedFiles(setFilesToPrune); UnlinkPrunedFiles(setFilesToPrune);
} }