ABI/API Breaking Change Detection Tools

[bitonality]

Regardless of what branching/release strategy we decide to use, I think we agree that we need some sort of automatic detection if ABI/API breaks will occur as a result of a PR being merged.

What kind of changes break the ABI?

This article has a fairly comprehensive list of the do's and don'ts of binary compatibility. I'll include an inline snippet to give us a general sense.

You cannot...

For existing classes:
    Unexport or remove an exported class.
    Change the class hierachy in any way (add, remove, or reorder base classes).
    Remove finality
For template classes:
    Change the template arguments in any way (add, remove or reorder).
For existing functions of any type:
    Unexport it.
    Remove it.
        Remove the implementation of existing declared functions. The symbol comes from the implementation of the function, so this is effectively the function.
    Inline it (this includes moving a member function's body to the class definition, even without the inline keyword).
    Add an overload (binary compatible, but not source compatible: it makes &func ambiguous). Adding overloads to already overloaded functions is ok (since any use of &func already needed a cast).

We should probably end up on the same page about what degree of ABI breaks we want our tool to detect as well as provide documentation to developers about what kind of changes they can expect to break the ABI.

Existing Tooling

ABI Compliance Checker

Requires ELF binaries if we don't want to compile the headers with an ancient version of GCC for analysis. I played around trying to get it working purely on windows, but ran into mingw/gcc issues... Maybe someone could make more headway on this?

I really like the reports it generates. Example report for netapi32.dll

Libabigail

Requires ELF binaries
Simple example

$ make abi-check
../tools/check-abi ../abi-ref/libslicksoft.so.abi libslicksoft.so || echo "ABI compatibility issue detected!"
An incompatible ABI change was detected:

Functions changes summary: 1 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 Removed function:

  'function void function_1()'    {function_1}

ABI compatibility issue detected!
$

Custom Tooling

Dumpbin

We could leverage dumpbin /exports to compare API level symbols. This is not full ABI compatibility, but it's a start that could work for us pretty much out of the box.

Other tools

I'll update this original post as people make more suggestions/considerations.

[Buckminsterfullerene02]

Nice post, this will be useful :D

So it's more of a compatibility scale rather than it is or it isn't. So for C++ mods, if the ABI changes on the UE4SS.dll at all, they would break, or only if they are trying to touch certain parts of it? It seems to me like it would be a binary classification for this scenario.

For "other tools", there aren't any more I know of, however, in the worst case scenario where none of the above work out, I'm pretty sure the only option involves building a C++ mod and UE4SS and then installing both onto a blank shipping game on a VM and checking the logs to see if the C++ mod gets the 0x7f error that means it failed to load due to ABI incompatibility.

[UE4SS]

Didn't someone at some point a while back look into running a headless UE instance in a github action ?
We could presumably still do that with our own hosted runner if the builtin ones are too tightly locked down.

Would using our own hosted runner help with the PR/CI problem as well ? I don't know how much freedom a self hosted runner has compared to the ones provide by github but if we can set it up to use its own set of keys and executing the checks in its own repo instead of trying to use the actual PR branch we'd sidestep this problem.
If not, we'd need to have a VM or something and just use the action as a trigger for the VM to sidestep the problem.
Otherwise we have to fix this problem somehow.

[bitonality]

Currently working on the ABI breakage detector in rust by using https://crates.io/crates/pdb
It uses the PDB to generate an intermediate snapshot of the ABI at a point in time. We can then feed two of these snapshots to the tool in order to diff and detect ABI breakage.

I'm fond of the idea of generating an intermediate file since the comparison tool will only have to download two of the intermediate files instead of two PDBs. It also let us run the build wherever we can (the pseudo licensing requirement), generate the ABI intermediates from that same machine, and then upload the intermediates as an artifact. We can then just download artifacts for the new and old versions of the API and run the rust tool to detect breakage against those artifacts.

I've wired up all the following cases that should flag ABI breakage

pub enum ClassProblem {
    RemovedOrUnexported,
    HierarchyChanged,
    FinalityRemoved,
    FuncProblem(FuncSigProblem),
    FieldProblem(FieldSigProblem),
    TemplateProblem(TemplateProblem),
}

pub enum FuncSigProblem {
    RemovedOrUnexported,
    InlineAdded,
    OverloadIntroduced,
    CvQualifierChanged,
    AccessRightsChanged,
    ReturnTypeChanged,
    VirtualProblem(VirtualMemberFuncProblem)
}

pub enum VirtualMemberFuncProblem {
    Introduced,
    Added,
    Removed,
    OverrideNonPrimaryBase,
    OverrideCovariantReturnType,
}

pub enum FieldSigProblem {
    Added,
    Removed,
    TypeChanged,
    StaticSigFieldProblem(StaticFieldSigProblem)
}

pub enum StaticFieldSigProblem {
    RemovedOrUnexported,
    TypeChanged,
    CvQualifierChanged,
}

pub enum TemplateProblem {
    ArgumentsChanged
}

[narknon]

How does this work if someone PRs to uepseudo from their own fork?

[UE4SS]

According to @bitonality nothing runs on the fork, it runs on UE4SS/UEPseudo.
I want to test it so that we know for sure because I have my doubts, but I hope those are wrong.

[narknon]

But the fork is where the changes are, does it create a branch in the main repo with the changes from the PR merged in?

[UE4SS]

No, that's why I doubt it will work.

[bitonality]

It might benefit this discussion to look into the process that git uses under the hood during pull requests if you are unfamiliar.

In short, each PR has its own remote ref that's created by github in the form of refs/pull/239/merge and refs/pull/239/head. These are in the base repo (UE4SS repo in this case).

This is how workflows/runners are able to run against the correct PR context when they are triggered.