Add -Wp,-D_GLIBCXX_ASSERTIONS

cbalint13 commented 1 year ago

This PR adds runtime boundaries checks via glibc.

See https://github.com/The-OpenROAD-Project/OpenROAD/issues/1764 for details. Some short notes about and summary docs regarding this flag.

Currently as of fbcc26374d (master) all regresion tests pass (unlike few weeks ago).
Using standalone regular builds having regression tests on test and example folder.
Having it as standalone library (separate from openroad) means flags from OR's top CMake won't propagate.

These are the tests used for mentioned builds:

%check
export LD_LIBRARY_PATH=%{buildroot}%{_libdir}
pushd test
%{buildroot}%{_bindir}/sta -exit regression.tcl
popd
pushd examples
for t in $(ls *.tcl)
do
  %{buildroot}%{_bindir}/sta -exit $t
done
popd

Cc @jjcherry56 , @maliberty , please help with the review.

maliberty commented 1 year ago

FYI @openroadie is managing updates to sta. Its best not to involve jjcherry56.

openroadie commented 1 year ago

@cbalint13 Looks fine to me. The performance penalty seems to be small and consolidating copied options is always :+1: .

jjcherry56 commented 1 year ago

Except it is pointless to pass "options" that are gcc specific to clang. The checks are pointless because I use valgrind on the private regressions and this does not catch anything valgrind does not catch. I factored the options but ignored the D_GLIBCXX_ASSERTIONS addtion.

maliberty commented 1 year ago

@jjcherry56 It isn't gcc specific and works in clang too. The assertion is in the stl implementation (glibcxx) which is common to both.

You could still have a user input that triggers such an error even if your regressions don't.

The-OpenROAD-Project / OpenSTA

Add -Wp,-D_GLIBCXX_ASSERTIONS #149