Check for undefined symbols in .so and warn about run-time errors

[dmakarov]

Problem

Several math functions not yet supported by the BPF toolchain may be used in users' code and trigger undefined function run-time errors.

Summary of Changes

Add a post processing step to check undefined symbols against known syscalls and issue a warning if unknown undefined symbols are found in the generated .so file.

Fixes #16552

[codecov]

Codecov Report

Merging #17850 (928607b) into master (d272468) will increase coverage by 0.0%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #17850   +/-   ##
=======================================
  Coverage    82.6%    82.6%           
=======================================
  Files         431      431           
  Lines      120996   121001    +5     
=======================================
+ Hits        99988   100003   +15     
+ Misses      21008    20998   -10     

[dmakarov]

@jackcmay do you need more time to review, or ok to merge? If this is merged, are you ok with #16552 being closed or would prefer to keep it around for the second option -- checking the undefs in the loader?

[jackcmay]

Hey @dmakarov , sorry about the late review, feeling like a pinball recently. My comment above is not necessarily a blocker but I think we are on thin ground by throwing out an error that might not be real.

[dmakarov]

I agree that issuing an error about a valid program may be bad. I tried to phrase the warning in a cautious way, so that users can still proceed with deploying their program. The list of known syscalls will indeed get out of sync. If users build their programs with a toolchain older than the runtime they’re going to run them on, I’m not sure we can catch that. For example, how does the tool even know which network to query for a list of syscalls? The build can be run on a host where no solana network (runtime system) is running/installed. Like you mentioned in the original issue, an alternative is that the warning can be issued at loading time. Is that much different than the undefined symbol error that the RBPF reports? I think it’s possible to write build.rs script that would extract the list of syscalls before cargo-build-bpf is built and the list can be built into cargo-build-bpf. This would at least guarantee that cargo-build-bpf knowledge of available syscalls matches the runtime which was built from the same source base as cargo-build-bpf.

On Thu, Jun 10, 2021 at 22:20 Jack May ***@***.***> wrote: Hey @dmakarov <https://github.com/dmakarov> , sorry about the late review, feeling like a pinball recently. My comment above is not necessarily a blocker but I think we are on thin ground by throwing out an error that might not be real. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17850 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZIRWMHVKSH6QGSO5SOALTSGMJRANCNFSM46K3ZESA> .

-- Dmitri

[jackcmay]

What if we keep a list in the sdk that the build tools read? That way the list will at least be tied to the sdk version the developer is building with which hopefully is also close to the one they are deploying to.

[dmakarov]

The list would be generated automatically when solana-bpf-loader-program crate is built, correct? This sounds good to me.