[SERVER-49798] Enforce correct library graph for ubsan vptr check builds Created: 22/Jul/20 Updated: 29/Oct/23 Resolved: 12/Oct/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Build |
| Affects Version/s: | None |
| Fix Version/s: | 4.9.0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Andrew Morrow (Inactive) | Assignee: | Daniel Moody |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Backport Requested: |
v4.4
|
||||||||||||||||||||
| Sprint: | Dev Platform 2020-10-05, Dev Platform 2020-10-19 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Description |
|
The -fsanitize=vptr option, which is enabled by default with -fsanitize=undefined, causes shared libraries to have dependencies on typeinfo symbols which they ordinarily would not. In an ordinary --link-model=dynamic build, if these symbols were left undefined at the time of linking the library in question, the link would fail because we normally link with -z,defs (or the local equivalent), requiring that each shared library fully express dependencies on all of the libraries required to satisfy all undefined symbols. However, in a sanitizer build, we disable -z,defs. If we don't, the link fails because the internal symbols the sanitizer injects are undefined. As a result, it is possible to link a libx.so that has undefined typeinfo references. Linking all of the programs is not sufficient to find these, because it may be the case that program y happens to also link to a library that provides the missing typeinfo definition that libx.so needs. If all uses of libx.so are always coincidentally accompanied by a library which provides the required symbols, then the entire tree will link. This however results in a very fragile state, as any perturbation of the link graph structure could suddenly result in libx.so's undefined symbols becoming unsatisfied, but only in the sanitizer build. The options to address this aren't great:
Here is a list at community/enterprise 86d94765b9030c9aa9adb3899277456e64bbd348/755fc267af1c4475a95e22159b90e0e428d23c28 of the current libraries that have undefined typeinfo references in an undefined behavior sanitizer build. The shared libraries were produced as follows:
The libraries were analyzed as follows (note that libmozjs is excluded because it does not honor our usual -z,defs type rules :
We find the following missing typeinfo nodes:
At least one of these was already known, per Adding the libraries required to satisfy these may be possible in most cases, but in some cases it may reveal what amount to new cycles in the graph, because satisfying them now introduces a stronger notion of acyclicity than had previously been in force: we now require that the dynamic link graph be acyclic with respect to typeinfo. Note that this is only true for --link-model=dynamic builds. We could also decide that --link-model=dynamic is inappropriate for undefined sanitizer builds. This would be somewhat unfortunate as we would like to use dynamic linking for the required waterfall builder that uses ubsan, per |
| Comments |
| Comment by Githook User [ 20/Jul/21 ] | ||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Andrew Morrow', 'email': 'acm@mongodb.com', 'username': 'acmorrow'}Message: This reverts the changes made for (cherry picked from commit 4122e4b7ddc05b49b35aab04ba0d4a70d73f7adf) | ||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 09/Jul/21 ] | ||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Andrew Morrow', 'email': 'acm@mongodb.com', 'username': 'acmorrow'}Message: This reverts the changes made for | ||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 09/Oct/20 ] | ||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Daniel Moody', 'email': 'daniel.moody@mongodb.com', 'username': 'dmoody256'}Message: | ||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 09/Oct/20 ] | ||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Daniel Moody', 'email': 'daniel.moody@mongodb.com', 'username': 'dmoody256'}Message: | ||||||||||||||||||||||||||||||||||||
| Comment by Andrew Morrow (Inactive) [ 29/Jul/20 ] | ||||||||||||||||||||||||||||||||||||
|
Here is a first idea at how we could build in tooling to identify such nodes and prevent the introduction of new instances:
It is pretty janky and could surely be improved (we should probably move that pipeline into an external script), but it would identify affected nodes which would allow us to fix them and then prevent the introduction of others. |