search

verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
989 stars 380 forks source link

Clean-up GCC Compilation Warings when IPO enabled #634

Closed kmurray closed 5 years ago

kmurray commented 5 years ago

Expected Behaviour

VTR should compile warning clean with gcc and Inter-Procedural/Link-Time Optimization (IPO/LTO).

Current Behaviour

VPR compiles warning clean when VTR_IPO_BUILD=off (i.e. without IPO which is not the default). While most of our code is warning clean there are some files which we do not control which are not. In particular:

  1. Flex/Bison generated parsers (libsdcparse, libblifparse)
  2. pugixml

To handle this we have historically disabled all warnings (pugixml) or the specific offending warnings (flex/bison).

However with IPO enabled, warnings similar to the following (from VPR) are produced at link time:

[100%] Linking CXX executable vpr
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In member function ‘save.constprop’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:4158:9: warning: potential null pointer dereference [-Wnull-dereference]
 4158 |     if (PUGI__NODETYPE(node) == node_element)
      |         ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:3: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |   ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:3: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |   ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:3: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |   ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:2: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘_ZN4pugi4impl12_GLOBAL__N_118compact_hash_table6rehashEv.isra.0’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:380:31: warning: potential null pointer dereference [-Wnull-dereference]
  380 |     *rt.insert(_items[i].key) = _items[i].value;
      |                               ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:3: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |   ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:3: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |   ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:3: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |   ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:2: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_set_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:788:3: warning: potential null pointer dereference [-Wnull-dereference]
  788 |   *compact_get_page(object, header_offset)->allocator->_hash->insert(object) = value;
      |   ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:2: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:2: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:2: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:13: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |             ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:13: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:13: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |                                                                                                  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |                                                                                                  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |                                                                                                  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |                                                                                                  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |                                                                                                  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |                                                                                                  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In function ‘compact_get_value’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
  783 |   return static_cast<T*>(*compact_get_page(object, header_offset)->allocator->_hash->find(object));
      |                                                                                                  ^
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:783:98: warning: potential null pointer dereference [-Wnull-dereference]
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp: In member function ‘offset_debug’:
/project/trees/vtr2/libs/EXTERNAL/libpugixml/src/pugixml.cpp:6151:12: warning: potential null pointer dereference [-Wnull-dereference]
 6151 |   if (!doc.buffer || doc.extra_buffers) return -1;
      |            ^
/project/trees/vtr2/build/libs/EXTERNAL/libsdcparse/sdc_lexer.gen.cpp: In function ‘yy_get_next_buffer’:
/project/trees/vtr2/build/libs/EXTERNAL/libsdcparse/sdc_lexer.gen.cpp:2102:9: warning: potential null pointer dereference [-Wnull-dereference]
 2102 |  b->yy_fill_buffer = 1;
      |         ^
/project/trees/vtr2/build/libs/EXTERNAL/libsdcparse/sdc_lexer.gen.cpp:2101:9: warning: potential null pointer dereference [-Wnull-dereference]
 2101 |  b->yy_input_file = file;
      |         ^
/project/trees/vtr2/build/libs/EXTERNAL/libsdcparse/sdc_lexer.gen.cpp:2102:9: warning: potential null pointer dereference [-Wnull-dereference]
 2102 |  b->yy_fill_buffer = 1;
      |         ^
/project/trees/vtr2/build/libs/EXTERNAL/libsdcparse/sdc_lexer.gen.cpp:2101:9: warning: potential null pointer dereference [-Wnull-dereference]
 2101 |  b->yy_input_file = file;
      |         ^
/project/trees/vtr2/build/libs/EXTERNAL/libblifparse/blif_parser.gen.cpp: In member function ‘yysyntax_error_’:
/project/trees/vtr2/build/libs/EXTERNAL/libblifparse/blif_parser.gen.cpp:1192:38: warning: potential null pointer dereference [-Wnull-dereference]
 1192 |     for (char const* yyp = yyformat; *yyp; ++yyp)
      |                                      ^
/project/trees/vtr2/build/libs/EXTERNAL/libsdcparse/sdc_parser.gen.cpp: In member function ‘yysyntax_error_’:
/project/trees/vtr2/build/libs/EXTERNAL/libsdcparse/sdc_parser.gen.cpp:1716:38: warning: potential null pointer dereference [-Wnull-dereference]
 1716 |     for (char const* yyp = yyformat; *yyp; ++yyp)
      |                                      ^
[100%] Built target vpr

Possible Solution

Not clear, it seems when run with LTO warning suppression options which were applied to specific source files are not carried over during linking. This is perhaps not so surprising since LTO breaks the historical single compilation unit model.

However it seems like GCC should really respect the warning flags when the source file was originally compiled even when doing LTO, so perhaps this is really a GCC bug.

Previously this was not an issue as we only applied LTO to vpr, but that caused crashes in GCC (#628), and the fix now causes all code (including the pugixml, libblifparse, and libsdcparse) to be built with LTO exposing this issue.

Steps to Reproduce

  1. CC=gcc-9 CXX=g++-9 make

Context

Keeping the code compiling with no warnings is important for code quality as it ensures newly introduced warnings about suspect code are immediately noticed.

Your Environment

kmurray commented 5 years ago

@mithro FYI

vaughnbetz commented 5 years ago

I think the best practical option is to turn these warnings off globally during the link phase. Maybe we miss a useful warning (bad), but we get rid of the spurious warnings (which are worse as they'll teach everyone to ignore these warnings).

kmurray commented 5 years ago

Should be fixed as of 4b56960b2.

We now provide the -Wno-null-dereference option (if supported by the compiler) at link time to the relevant executables when built with IPO.

This should ensure that we build warning clean (at least with GCC) even when IPO is enabled.