Plug leaks so sanatizers can be used to look for other bugs

[oharboe]

Description

I made this modification because I want to check if there's some non-determinism going on:

./build_openroad.sh --local --clean-force --openroad-args -DCMAKE_CXX_FLAGS=-fsanitize=address

I get false positives(compared to what I'm looking for):

=================================================================
==422676==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 15040 byte(s) in 188 object(s) allocated from:
    #0 0x7f05206b4867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x55a9d62eceaf in LefDefParser::lefMalloc(unsigned long) /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/src/odb/src/lef/lef/lef_keywords.cpp:1352
    #2 0x55a9d63df88c in LefDefParser::lefyyparse() /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/src/odb/src/lef/lef/lef.y:5873
    #3 0x55a9d630e0a4 in LefDefParser::lefrRead(_IO_FILE*, char const*, void*) /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/src/odb/src/lef/lef/lefrReader.cpp:297
    #4 0x55a9d567dc9f in odb::lefin_parse(odb::lefin*, utl::Logger*, char const*) /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/src/odb/src/lefin/reader.cpp:560
    #5 0x55a9d5665ff5 in odb::lefin::readLef(char const*) /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/src/odb/src/lefin/lefin.cpp:2106
    #6 0x55a9d566947e in odb::lefin::createLib(char const*, char const*) /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/src/odb/src/lefin/lefin.cpp:2242
    #7 0x55a9c8df3d72 in ord::OpenRoad::readLef(char const*, char const*, bool, bool) /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/src/OpenRoad.cc:285
    #8 0x55a9c90a9fef in read_lef_cmd(char const*, char const*, bool, bool) /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/build/src/CMakeFiles/openroad_swig.dir/OpenRoadTCL_wrap.cxx:2001
    #9 0x55a9c90ac5fa in _wrap_read_lef_cmd /home/oyvind/OpenROAD-flow-scripts/tools/OpenROAD/build/src/CMakeFiles/openroad_swig.dir/OpenRoadTCL_wrap.cxx:2465
    #10 0x7f052048cd31 in TclNRRunCallbacks (/lib/x86_64-linux-gnu/libtcl8.6.so+0x3cd31)

Suggested Solution

No response

Additional Context

No response

[oharboe]

@antonblanchard FYI, tried your tip... Trying again without memory sanatizer...

[oharboe]

@maliberty More actionable, command line given to build with address sanatizer.

[maliberty]

We can fix it but a memory leak won't cause non-determinism.

[maliberty]

is this on mock-array?

[oharboe]

is this on mock-array?

I think so. I was juggling a lot today....

[maliberty]

I don't see any ERROR statements in the logs (still finishing top level drt but I guess this should have happened long ago).

[antonblanchard]

I don't see any ERROR statements in the logs (still finishing top level drt but I guess this should have happened long ago).

The memory leak errors likely wont appear until the process exits. There are still quite a few small memory leaks (eg in LEF parsing) that I haven't had time to investigate.

[maliberty]

OR exits after every step

[antonblanchard]

mkdir build
cd build

ASAN_CFLAGS="-fsanitize=address -fno-omit-frame-pointer -g -O2"
ASAN_LDFLAGS=""

cmake -GNinja ../ \
	-DALLOW_WARNINGS=ON \
	-DCMAKE_C_COMPILER=clang \
	-DCMAKE_CXX_COMPILER=clang++ \
	-DCMAKE_C_FLAGS="$ASAN_CFLAGS" \
	-DCMAKE_CXX_FLAGS="$ASAN_CFLAGS" \
	-DCMAKE_EXE_LINKER_FLAGS="$ASAN_LDFLAGS" \

ninja

cd ../src/odb/test/ && ../../../build/src/openroad db_read_write.tcl

Direct leak of 1920 byte(s) in 24 object(s) allocated from:
    #0 0xcda9d7 in malloc (OpenROAD/build-dist/src/openroad+0xcda9d7) (BuildId: 91eaf3aa8981d4b458bd98e8835a293d749c48ce)
    #1 0x62a2538 in LefDefParser::lefMalloc(unsigned long) OpenROAD/src/odb/src/lef/lef/lef_keywords.cpp:1352:25
    #2 0x63062d6 in LefDefParser::lefyyparse() OpenROAD/src/odb/src/lef/lef/lef.y:5873:55
    #3 0x607e7fc in odb::lefin_parse(odb::lefin*, utl::Logger*, char const*) OpenROAD/src/odb/src/lefin/reader.cpp:560:11
    #4 0x607593a in odb::lefin::readLef(char const*) OpenROAD/src/odb/src/lefin/lefin.cpp:2109:12
    #5 0x6078515 in odb::lefin::createTechAndLib(char const*, char const*) OpenROAD/src/odb/src/lefin/lefin.cpp:2283:8
    #6 0xd60f33 in ord::OpenRoad::readLef(char const*, char const*, bool, bool) OpenROAD/src/OpenRoad.cc:280:22
    #7 0xe25ac1 in read_lef_cmd(char const*, char const*, bool, bool) OpenROAD/build-dist/src/CMakeFiles/openroad_swig.dir/OpenRoadTCL_wrap.cxx:1947:8
    #8 0xe25ac1 in _wrap_read_lef_cmd(void*, Tcl_Interp*, int, Tcl_Obj* const*) OpenROAD/build-dist/src/CMakeFiles/openroad_swig.dir/OpenRoadTCL_wrap.cxx:2411:7
    #9 0x7fa60554572f in TclNRRunCallbacks (/lib64/libtcl8.6.so+0x3b72f) (BuildId: 7bba4bd4283da69c8741ef3654eb4d3062f6d906)
...

[antonblanchard]

At least one issue maps back to memory leaked from addProp. Lots of logic here, shouldn't we just be using vectors and strings?

void lefiLayer::addProp(const char* name, const char* value, const char type)
{ 
  int len = strlen(name) + 1;
  // char*  tvalue;
  // int    vlen, i;
  if (numProps_ == propsAllocated_) {
    int i;
    int max;
    int lim = numProps_;
    char** nn;
    char** nv;
    double* nd;
    char* nt;

    if (propsAllocated_ == 0)
      max = propsAllocated_ = 2;
    else
      max = propsAllocated_ *= 2;
    nn = (char**) lefMalloc(sizeof(char*) * max);
    nv = (char**) lefMalloc(sizeof(char*) * max);
    nd = (double*) lefMalloc(sizeof(double) * max);
    nt = (char*) lefMalloc(sizeof(char) * max);
    for (i = 0; i < lim; i++) {
      nn[i] = names_[i];
      nv[i] = values_[i];
      nd[i] = dvalues_[i];
      nt[i] = types_[i];
    }
    lefFree((char*) (names_));
    lefFree((char*) (values_));
    lefFree((char*) (dvalues_));
    lefFree((char*) (types_));
    names_ = nn;
    values_ = nv; 
    dvalues_ = nd;
    types_ = nt;
  }
  names_[numProps_] = (char*) lefMalloc(sizeof(char) * len);
  strcpy(names_[numProps_], name);
  len = strlen(value) + 1;
  values_[numProps_] = (char*) lefMalloc(sizeof(char) * len);
  strcpy(values_[numProps_], value);
  dvalues_[numProps_] = 0;

[maliberty]

Fixed a number in #3257 but some still are tbd

[maliberty]

@antonblanchard I don't want to rewrite the Si2 lef/def parsers too much as it makes taking updates more difficult.

[QuantamHD]

As soon as we update to the latest version we were going to fork and then rewrite.