Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The BPF tools report blown stacks but provide no information about how to track down the offenders #13290

Closed
jackcmay opened this issue Oct 29, 2020 · 8 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@jackcmay
Copy link
Contributor

jackcmay commented Oct 29, 2020

Problem

If a function exceeds the supported stack size LLVM will warn the user but not provide information on what function is the offender.

Background: We've found that in Rust, there are often packages that a program depends on that exceed the stack usage even though the program is not actually using the offending functionality. (or maybe a dependency of a dependency). Instead of halting program compilation LLVM posts a warning.

Proposed Solution

  • Provide more useful information about what function/package is blowing the stack.
  • Limit the warnings to only the code pulled into the final program.
@dmakarov dmakarov added the enhancement New feature or request label Feb 27, 2021
@dmakarov
Copy link
Contributor

The second requirement Limit the warnings to only the code pulled into the final program seems a bit unrealistic, because when the compiler is processing a function and determines that the function's stack size exceeds the limit, it cannot be known whether this function's code will be pulled into the final program.

@dmakarov
Copy link
Contributor

llvm already outputs the name of the function that blows the stack. I added printing the filename and line where the function is defined, when debug information is available. I also reduced the number of messages to one only for each function

For example,

$ ./rust/build/x86_64-unknown-linux-gnu/llvm/build/bin/llc lib-957c99f7a6aaad93.ll |& rustfilt
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/montgomery.rs:273 Function <&lib::montgomery::MontgomeryPoint as core::ops::arith::Mul<&lib::scalar::Scalar>>::mul Stack offset of -4232 exceeded max offset of -4096 by 136 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/edwards.rs:607 Function <&lib::edwards::EdwardsPoint as core::ops::arith::Mul<&lib::scalar::Scalar>>::mul Stack offset of -10744 exceeded max offset of -4096 by 6648 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/edwards.rs:737 Function lib::edwards::EdwardsPoint::vartime_double_scalar_mul_basepoint Stack offset of -10960 exceeded max offset of -4096 by 6864 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/edwards.rs:808 Function <&lib::edwards::EdwardsBasepointTable as core::ops::arith::Mul<&lib::scalar::Scalar>>::mul Stack offset of -4592 exceeded max offset of -4096 by 496 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/field.rs:99 Function lib::field::<impl lib::backend::serial::u64::field::FieldElement51>::pow22501 Stack offset of -4496 exceeded max offset of -4096 by 400 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/window.rs:88 Function <lib::window::LookupTable<lib::backend::serial::curve_models::ProjectiveNielsPoint> as core::convert::From<&lib::edwards::EdwardsPoint>>::from Stack offset of -4368 exceeded max offset of -4096 by 272 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/window.rs:138 Function <lib::window::NafLookupTable5<lib::backend::serial::curve_models::ProjectiveNielsPoint> as core::convert::From<&lib::edwards::EdwardsPoint>>::from Stack offset of -6120 exceeded max offset of -4096 by 2024 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/window.rs:150 Function <lib::window::NafLookupTable5<lib::backend::serial::curve_models::AffineNielsPoint> as core::convert::From<&lib::edwards::EdwardsPoint>>::from Stack offset of -5784 exceeded max offset of -4096 by 1688 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/window.rs:185 Function <lib::window::NafLookupTable8<lib::backend::serial::curve_models::ProjectiveNielsPoint> as core::convert::From<&lib::edwards::EdwardsPoint>>::from Stack offset of -14072 exceeded max offset of -4096 by 9976 bytes, please minimize large stack variables
Error: /home/dmitri_solana_com/.cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-3.0.0/src/window.rs:197 Function <lib::window::NafLookupTable8<lib::backend::serial::curve_models::AffineNielsPoint> as core::convert::From<&lib::edwards::EdwardsPoint>>::from Stack offset of -12504 exceeded max offset of -4096 by 8408 bytes, please minimize large stack variables

Unfortunately llvm cannot demangle rustc's name mangling, so it may be necessary to filter the messages through rustfilt.

This fix will be included in the bpf-tools v1.4 release.

@jackcmay
Copy link
Contributor Author

@dmakarov Can we defer the stack size check to the end where we know which functions are pulled into the final binary? That info is probably not retained all the way to the final link, but maybe there is a way to do so?

@dmakarov
Copy link
Contributor

Not sure if it's possible. This check is done as part of the frame index elimination. The frame index is used to compute the offsets encoded in instructions referring to the corresponding stack entries. By the link time, this information is available only in instructions encoding, it seems.

If the warning is for the function in an external package that is not controlled by the user, then it's not useful, but not much can be done about it anyway. If it warns about the code that the user controls, it seems the warning is useful at this point as well as it would be at the link time.

@jackcmay
Copy link
Contributor Author

It is still useful in an external package if the function is pulled into the final binary because the developer has the choice to not call it and do something else.

Not recommending this because it's pretty ugly but we could someone store the stack usage of each function and then lookup and report during link or post-link?

@dmakarov
Copy link
Contributor

Maybe it's better at link time to have a pass that analyzes offsets encoded in instructions and reports those that exceed the stack size limit, but that probably requires LTO to be enabled.

@jackcmay
Copy link
Contributor Author

Can we add a pass to the linker?

@dmakarov
Copy link
Contributor

This is resolved by enabling the variable size stack in the RBPF VM. The support for variable size stacks has been added to the VM. solana-labs/rbpf#274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants