Skip to content

Commit

Permalink
[tools] Add test to flag hazardous exported symbols
Browse files Browse the repository at this point in the history
  • Loading branch information
jwnimmer-tri committed Sep 29, 2023
1 parent bb1fbe4 commit a78cf1e
Show file tree
Hide file tree
Showing 2 changed files with 212 additions and 0 deletions.
13 changes: 13 additions & 0 deletions tools/install/libdrake/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,19 @@ drake_cc_googletest(
],
)

drake_py_unittest(
name = "exported_symbols_test",
args = select({
# Don't run the test on Debug builds. They have a lot more symbols
# (no dead code elimination) so are too much work to tidy up.
"//tools/cc_toolchain:debug": ["--help"],
"//conditions:default": [],
}),
data = [
":libdrake.so",
],
)

drake_py_unittest(
name = "header_dependency_test",
args = [
Expand Down
199 changes: 199 additions & 0 deletions tools/install/libdrake/test/exported_symbols_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
from collections import namedtuple
from pathlib import Path
import subprocess
import sys
import unittest

# Any symbols whose name starts with one of these strings are okay.
_GOOD_SYMBOLS_PREFIX = [
# For now, allow any typeinfo symbols.
"_ZTI",
"_ZTS",
# For now, allow anything in `namespace std {}`. (We should
# probably be checking for unwanted template arguments.)
"_ZGVZNKSt8__detail",
"_ZN9__gnu_cxx",
"_ZNKRSt",
"_ZNKSt",
"_ZNSt",
"_ZSt",
"_ZTVNSt",
"_ZTVSt",
"_ZZNKSt",
"_ZZNSt",
"_ZZSt",
]

# Any symbols whose name contains one of these strings are okay.
_GOOD_SYMBOLS_SUBSTR = [
# Symbols from Drake are fine.
"N5drake",
"NK5drake",
"NO5drake",
"drake_set_assertion_failure_to_throw_exception",
# Symbols from Drake's public externals are fine.
"N3fmt",
"N5Eigen",
"N6spdlog",
"NK3fmt",
"NK5Eigen",
"NK6spdlog",
# Symbols from Drake's vendored externals are fine. (It would be better
# for performance if they could be hidden, but they are not hazardous.)
"N12drake_vendor",
"NK12drake_vendor",
]

# Any symbols whose name contains one of these are undesirable, but for now
# will not cause this test to fail.
_KNOWN_BAD_SYMBOLS_SUBSTR = [
"5bazel5tools3cpp8runfiles",
"AbslInternal",
"Ampl",
"BitVector128",
"Clp",
"Coin",
"EventHandler",
"FactorPointers",
"GLEW",
"GLXEW",
"Idiot",
"MessageHandler",
"N3uWS",
"N5ofats10any_detail",
"N7tinyobj",
"Realpath",
"WindowsError",
"action",
"alternativeEnvironment",
"ampl_obj_prec",
"boundary_sort",
"charToStatus",
"clp_",
"coin",
"ekk",
"fileAbsPath",
"freeArgs",
"freeArrays",
"getFunctionValueFromString",
"getNorms",
"glew",
"innerProduct",
"maximumAbsElement",
"maximumIterations",
"multiplyAdd",
"presolve",
"setElements",
"setupForSolve",
"slack_value",
"sortOnOther",
"usage",
"vtkdouble_conversion",
"vtkglew",
"vtkpugixml",
"wrapper",
# This fix is pending a deprecation removal (#20115) 2024-01-01.
"N3lcm",
# This fix is pending a deprecation removal (#19866) 2023-11-01.
"optitrack",
]


class ExportedSymbolsTest(unittest.TestCase):

@unittest.skipIf(sys.platform == "darwin", "Ubuntu only")
def test_exported_symbols(self):
"""Confirms that the symbols exported by libdrake.so are only:
- Drake API (`namespace drake { ... }`).
- Vendored externals (`namespace drake_vendor { ... }`).
Note that many vendored externals are hidden (no exported symbols) but
in some cases that's not possible and the symbols end up being public.
For simplicity, we only test on Ubuntu. Hopefully, macOS is similar.
"""
libdrake = "tools/install/libdrake/libdrake.so"
command = ["readelf", "--wide", "--symbols"]
output = subprocess.check_output(command + [libdrake]).decode("utf8")
lines = output.splitlines()

# Next, we'll parse the readelf output. The lines look like this:
#
# Symbol table '.dynsym' contains 58984 entries:
# Num: Value Size Type Bind Vis Ndx Name
# 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
# 1: 0000000000000000 0 FUNC GLOBAL DEFAULT UND hypot@...
#
# The first time we see a "Num:" header, we'll make it a namedtuple.
# After that, each row of data will be parsed into that namedtuple.
Row = None
symbols = []
for i, line in enumerate(lines):
# Skip over useless lines.
if not line or line.startswith("Symbol table"):
continue
values = line.split()
# Deal with the table header row.
if line.strip().startswith("Num:"):
column_names = tuple([x.strip(":") for x in values])
assert column_names[-1] == "Name", line
name_col_start = line.index("Name")
assert name_col_start > 0, line
if Row is None:
Row = namedtuple("Row", column_names, rename=True)
else:
assert column_names == Row._fields
continue
# Skip over any lines of junk (e.g., with no symbol name).
if len(line) == name_col_start:
continue
# Discard any symbol versions like "(3)" at end-of-line.
if values[-1].endswith(")"):
assert values[-1].startswith("("), line
values.pop()
# Add this line to the list of symbols.
assert len(values) == len(Row._fields), line
symbols.append(Row._make(values))

# Check the symbols against our policy.
bad_rows = [
row
for row in symbols
if not self._is_symbol_ok(row)
]
bad_rows = sorted(bad_rows, key=lambda x: (x.Type, x.Name))

# Report the first few errors.
for row in bad_rows[:25]:
print(f"{row.Type} {row.Bind} {row.Vis}")
print(f" {self._demangle(row.Name)}")
print(f" {row.Name}")
print()
self.assertEqual(len(bad_rows), 0)

@staticmethod
def _is_symbol_ok(row):
# Local symbols don't matter.
if row.Bind == "LOCAL":
return True
# BSS start / end /etc don't matter.
if row.Type == "NOTYPE":
return True
# Undefined references don't matter.
if row.Ndx == "UND":
return True
name = row.Name
for prefix in _GOOD_SYMBOLS_PREFIX:
if name.startswith(prefix):
return True
for needle in _GOOD_SYMBOLS_SUBSTR:
if needle in name:
return True
for needle in _KNOWN_BAD_SYMBOLS_SUBSTR:
if needle in name:
return True
return False

@staticmethod
def _demangle(x):
return subprocess.check_output(["c++filt"], input=x, encoding="utf-8")

0 comments on commit a78cf1e

Please sign in to comment.