arenadata / gpdb

Arenadata DB
https://docs.arenadata.io/en/ADB/current/introduction/intro.html
Apache License 2.0
40 stars 22 forks source link

Add -Werror to compile GPDB #1005

Closed red1452 closed 2 months ago

red1452 commented 3 months ago

Add -Werror to compile GPDB

All warning were fixed. 1) checking for not-NULL was added before calling strlen(). 2) converting long int to float8 was added for comparison between long int and float8. 3) Unused variables were removed. 4) Useless memset's were removed from the insertsetbit function, because the buf variable was zeroed before sending to the insertsetbit function. 5) Assert was fixed in the CreateDXLProjectNullsForGroupingSets function. Execution of the Find method is extra, because it is checked below in the condition and it may return nullptr.

andr-sokolov commented 3 months ago

typo in the description: LOAT8. Should be in lower case: float8

andr-sokolov commented 2 months ago

I get this error:

gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wimplicit-fallthrough=3 -Wvla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-unused-but-set-variable -Wno-format-truncation -Wno-stringop-truncation -O3  -Werror -I. -I. -I../../../../src/interfaces/libpq -I../../../../src/include   -D_GNU_SOURCE -I/usr/include/libxml2   -c -o tsvector_op.o tsvector_op.c -MMD -MP -MF .deps/tsvector_op.Po
In function ‘tsCompareString’,
    inlined from ‘tsvector_bsearch’ at tsvector_op.c:1171:1:
tsvector_op.c:1188:9: error: ‘memcmp’ specified size between 18446744073709551612 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
 1188 |   cmp = memcmp(a, b, Min(lena, lenb));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘tsCompareString’,
    inlined from ‘tsvector_bsearch’ at tsvector_op.c:1171:1,
    inlined from ‘tsvector_delete_arr’ at tsvector_op.c:625:13:
tsvector_op.c:1188:9: error: ‘memcmp’ specified size between 18446744073709551612 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
 1188 |   cmp = memcmp(a, b, Min(lena, lenb));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Let's fix it too

red1452 commented 2 months ago

I get this error:

gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wimplicit-fallthrough=3 -Wvla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-unused-but-set-variable -Wno-format-truncation -Wno-stringop-truncation -O3  -Werror -I. -I. -I../../../../src/interfaces/libpq -I../../../../src/include   -D_GNU_SOURCE -I/usr/include/libxml2   -c -o tsvector_op.o tsvector_op.c -MMD -MP -MF .deps/tsvector_op.Po
In function ‘tsCompareString’,
    inlined from ‘tsvector_bsearch’ at tsvector_op.c:1171:1:
tsvector_op.c:1188:9: error: ‘memcmp’ specified size between 18446744073709551612 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
 1188 |   cmp = memcmp(a, b, Min(lena, lenb));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘tsCompareString’,
    inlined from ‘tsvector_bsearch’ at tsvector_op.c:1171:1,
    inlined from ‘tsvector_delete_arr’ at tsvector_op.c:625:13:
tsvector_op.c:1188:9: error: ‘memcmp’ specified size between 18446744073709551612 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
 1188 |   cmp = memcmp(a, b, Min(lena, lenb));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Let's fix it too

It is bug at GCC less than 11: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91397 I do not think, that it is good idea to fix this. And I have not found the right way. You can add flag -Wno-error=stringop-overflow to your build.

andr-sokolov commented 2 months ago

make[4]: вход в каталог «/home/keremet/compile/gpdb_arenadata/src/backend/access/bitmap» gcc-13 -Wall -Wmissing-prototypes -Wpointer-arith -Wimplicit-fallthrough=3 -Wvla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-unused-but-set-variable -Wno-format-truncation -Wno-stringop-truncation -O3 -Werror -I../../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o bitmapinsert.o bitmapinsert.c -MMD -MP -MF .deps/bitmapinsert.Po In file included from /usr/include/string.h:532, from ../../../../src/include/c.h:63, from ../../../../src/include/postgres.h:46, from bitmapinsert.c:18: In function ‘memset’, inlined from ‘insertsetbit’ at bitmapinsert.c:2148:2, inlined from ‘inserttuple.isra’ at bitmapinsert.c:2518:2: /usr/include/bits/string_fortified.h:56:10: error: ‘builtin___memset_chk’ forming offset [560, 1080] is out of the bounds [0, 560] of object ‘buf’ with type ‘BMTIDBuffer’ [-Werror=array-bounds=] 56 | return builtin_memset_chk (dest, ch, len, bos0 (dest)); | ^~~~~~~~~~~~~ bitmapinsert.c: In function ‘inserttuple.isra’: bitmapinsert.c:2447:25: note: ‘buf’ declared here 2447 | BMTIDBuffer buf; | ^~~ cc1: all warnings being treated as errors

andr-sokolov commented 2 months ago
gcc-13 -Wall -Wmissing-prototypes -Wpointer-arith -Wimplicit-fallthrough=3 -Wvla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-unused-but-set-variable -Wno-format-truncation -Wno-stringop-truncation -O3  -Werror -I../../../../src/interfaces/libpq -I../../../../src/include   -D_GNU_SOURCE -I/usr/include/libxml2   -c -o ic_udpifc.o ic_udpifc.c -MMD -MP -MF .deps/ic_udpifc.Po
In function ‘sendControlMessage’,
    inlined from ‘sendAck’ at ic_udpifc.c:1875:2,
    inlined from ‘doSendStopMessageUDPIFC’ at ic_udpifc.c:5805:6,
    inlined from ‘doSendStopMessageUDPIFC’ at ic_udpifc.c:5728:1:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘doSendStopMessageUDPIFC’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendAck’ at ic_udpifc.c:1875:2,
    inlined from ‘putRxBufferAndSendAck’ at ic_udpifc.c:1983:4,
    inlined from ‘doSendStopMessageUDPIFC’ at ic_udpifc.c:5774:6,
    inlined from ‘doSendStopMessageUDPIFC’ at ic_udpifc.c:5728:1:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘doSendStopMessageUDPIFC’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendDisorderAck’ at ic_udpifc.c:1906:2,
    inlined from ‘handleDisorderPacket’ at ic_udpifc.c:4923:2,
    inlined from ‘handleDataPacket’ at ic_udpifc.c:6159:4:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘handleDataPacket’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendAck’ at ic_udpifc.c:1875:2,
    inlined from ‘handleMismatch’ at ic_udpifc.c:6624:4,
    inlined from ‘rxThreadFunc’ at ic_udpifc.c:6416:10:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘rxThreadFunc’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendAckWithParam’ at ic_udpifc.c:1851:2,
    inlined from ‘rxThreadFunc’ at ic_udpifc.c:6431:5:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘rxThreadFunc’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendStatusQueryMessage’ at ic_udpifc.c:1932:2,
    inlined from ‘checkDeadlock’ at ic_udpifc.c:5292:4,
    inlined from ‘checkExceptions’ at ic_udpifc.c:5431:3:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘checkExceptions’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendAckWithParam’ at ic_udpifc.c:1851:2,
    inlined from ‘MlPutRxBufferIFC’ at ic_udpifc.c:2026:3:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘MlPutRxBufferIFC’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendAckWithParam’ at ic_udpifc.c:1851:2,
    inlined from ‘handleCachedPackets’ at ic_udpifc.c:2948:6,
    inlined from ‘SetupUDPIFCInterconnect_Internal’ at ic_udpifc.c:3225:3,
    inlined from ‘SetupUDPIFCInterconnect’ at ic_udpifc.c:3250:15:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘SetupUDPIFCInterconnect’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
In function ‘sendControlMessage’,
    inlined from ‘sendAck’ at ic_udpifc.c:1875:2,
    inlined from ‘putRxBufferAndSendAck’ at ic_udpifc.c:1983:4,
    inlined from ‘TeardownUDPIFCInterconnect_Internal’ at ic_udpifc.c:3561:7,
    inlined from ‘TeardownUDPIFCInterconnect’ at ic_udpifc.c:3679:3:
ic_udpifc.c:1824:12: error: ‘n’ may be used uninitialized [-Werror=maybe-uninitialized]
 1824 |         if (n < pkt->len)
      |            ^
ic_udpifc.c: In function ‘TeardownUDPIFCInterconnect’:
ic_udpifc.c:1791:33: note: ‘n’ was declared here
 1791 |         int                     n;
      |                                 ^
cc1: all warnings being treated as errors
andr-sokolov commented 2 months ago

CTranslatorQueryToDXL.cpp is compiled without -Werror

g++-13 -std=c++14 -Wall -Wpointer-arith -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -fexcess-precision=standard -O3 -I../../../../src/backend/gporca/libgpdbcost/include -I../../../../src/backend/gporca/libnaucrates/include -I../../../../src/backend/gporca/libgpopt/include -I../../../../src/backend/gporca/libgpos/include -I../../../../src/include   -D_GNU_SOURCE -I/usr/include/libxml2   -c -o CTranslatorQueryToDXL.o CTranslatorQueryToDXL.cpp -MMD -MP -MF .deps/CTranslatorQueryToDXL.Po
In file included from ../../../../src/backend/gporca/libgpopt/include/gpopt/base/CColRef.h:16,
                 from ../../../../src/backend/gporca/libgpopt/include/gpopt/base/CUtils.h:16,
                 from CTranslatorQueryToDXL.cpp:33:
In member function ‘T* gpos::CHashMap<K, T, HashFn, EqFn, DestroyKFn, DestroyTFn>::Find(const K*) const [with K = int; T = unsigned int; gpos::ULONG (* HashFn)(const K*) = gpos::HashValue<int>; gpos::BOOL (* EqFn)(const K*, const K*) = gpos::Equals<int>; void (* DestroyKFn)(K*) = gpos::CleanupDelete<int>; void (* DestroyTFn)(T*) = gpos::CleanupDelete<unsigned int>]’,
    inlined from ‘gpdxl::CDXLNode* gpdxl::CTranslatorQueryToDXL::CreateDXLProjectNullsForGroupingSets(List*, gpdxl::CDXLNode*, gpos::CBitSet*, gpnaucrates::IntToUlongMap*, gpnaucrates::IntToUlongMap*, gpos::UlongToUlongMap*) const’ at CTranslatorQueryToDXL.cpp:4490:3:
../../../../src/backend/gporca/libgpos/include/gpos/common/CHashMap.h:240:44: warning: ‘this’ pointer is null [-Wnonnull]
  240 |                 CHashMapElem *elem = Lookup(key);
      |                                      ~~~~~~^~~~~
../../../../src/backend/gporca/libgpos/include/gpos/common/CHashMap.h: In member function ‘gpdxl::CDXLNode* gpdxl::CTranslatorQueryToDXL::CreateDXLProjectNullsForGroupingSets(List*, gpdxl::CDXLNode*, gpos::CBitSet*, gpnaucrates::IntToUlongMap*, gpnaucrates::IntToUlongMap*, gpos::UlongToUlongMap*) const’:
../../../../src/backend/gporca/libgpos/include/gpos/common/CHashMap.h:165:9: note: in a call to non-static member function ‘gpos::CHashMap<K, T, HashFn, EqFn, DestroyKFn, DestroyTFn>::CHashMapElem* gpos::CHashMap<K, T, HashFn, EqFn, DestroyKFn, DestroyTFn>::Lookup(const K*) const [with K = int; T = unsigned int; gpos::ULONG (* HashFn)(const K*) = gpos::HashValue<int>; gpos::BOOL (* EqFn)(const K*, const K*) = gpos::Equals<int>; void (* DestroyKFn)(K*) = gpos::CleanupDelete<int>; void (* DestroyTFn)(T*) = gpos::CleanupDelete<unsigned int>]’
  165 |         Lookup(const K *key) const
      |         ^~~~~~
andr-sokolov commented 2 months ago

"memset was removed" -> "memset-s were removed"

andr-sokolov commented 2 months ago

"Initialization for variables were added." - where?

dkovalev1 commented 2 months ago
/usr/bin/clang -Wno-ignored-attributes -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Xclang -no-opaque-pointers -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -Wno-deprecated-non-prototype -O2  -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_CONSTANT_MACROS -D_GNU_SOURCE -I/usr/lib/llvm-10/include  -I../../../../src/include   -D_GNU_SOURCE -I/usr/include/libxml2  -flto=thin -emit-llvm -c -o llvmjit_types.bc llvmjit_types.c
In file included from /usr/lib/llvm-10/include/llvm/Analysis/ModuleSummaryAnalysis.h:17,
                 from llvmjit_inline.cpp:51:
/usr/lib/llvm-10/include/llvm/IR/ModuleSummaryIndex.h: In constructor ‘llvm::modulesummaryindex::ModuleSummaryIndex(bool, bool)’:
/usr/lib/llvm-10/include/llvm/IR/ModuleSummaryIndex.h:995:73: error: member ‘llvm::modulesummaryindex::Alloc’ is used uninitialized [-Werror=uninitialized]
  995 |       : HaveGVs(HaveGVs), EnableSplitLTOUnit(EnableSplitLTOUnit), Saver(Alloc) {
      |                                                                         ^~~~~
cc1plus: all warnings being treated as errors
make[2]: * [<builtin>: llvmjit_inline.o] Error 1
make[2]: Leaving directory '/home/denis/arena/gpdb7/src/backend/jit/llvm'
make[1]: * [Makefile:45: all-backend/jit/llvm-recurse] Error 2
make[1]: Leaving directory '/home/denis/arena/gpdb7/src'
make: *** [GNUmakefile:11: all-src-recurse] Error 2
denis@deli:~/arena/gpdb7$ /usr/bin/clang --version
Ubuntu clang version 16.0.6 (++20231112100455+7cbf1a259152-1~exp1~20231112100542.106)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
denis@deli:~/arena/gpdb7$ gcc --version
gcc (Ubuntu 13.1.0-8ubuntu1~20.04.2) 13.1.0
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

It's ubuntu 20 with manually upgraded gcc and clang.