-
Notifications
You must be signed in to change notification settings - Fork 27
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
Call Frame Information (CFI) and frame pointer considerations #197
Comments
This is mostly just something we haven't prioritized so far, though I remember someone mentioning them as desirable before (perhaps it was Adam Langley). We pushed it to the background as something that "only matters for debugging", but I admit it's not ideal. There is currently no attempt to maintain actual frame pointers in the code, and it would be disruptive to do so. Quite a lot of the x86 functions (pretty much all the big ones I think) modify RBP internally; a few of the ARM ones also modify X29, though in both cases the value is saved and restored as per ABI. However, my understanding is that we could indeed provide suitable CFI directives, since no text/data segment changes (and hence no proof changes) would be needed. Given that it would be much better for debugging or related activities, this indeed seems like something we should do. We'd better take the time to make sure we know what we're doing though :-) |
It isn't really about debugging. Many environments, maybe even AWS internally, require the call frame information so they can do continuous live profiling of production systems, or at the very least on-demand profiling. The CFI directives help avoid the disruption caused by using ebp for a purpose other than the stack frame pointer. |
You're right of course, so this seems to be something we should do. If we make a first attempt, would you be willing to glance over it and see that it looks reasonable? Also, do you have any thoughts on practical validation? I guess it could be sanity-checked by running in some simulated environment like valgrind or gdb and deliberately putting breakpoints in, somehow or other. Do you know of any established methodology used by BoringSSL or anyone else? |
One way to validate it would be to write C functions with the same signature (parameters and return value) as every function you define in assembly, with a dummy body like this:
For each function, change the call to Of course, this is cringeworthy compared to your rigorous validation of correctness for your code. But, it may be a good first step towards eventually incorporating the validation of CFI into your proofs. |
Check out BoringSSL's CFI testing framework: https://issues.chromium.org/issues/42290050 |
This is the start of a transformation that will annotate the code properly with call frame information (CFI) directives, a lack in the current s2n-bignum codebase noted in awslabs#197 So far, this picks the principal CFI-relevant instructions such as calls and returns and those that modify the stack pointer, and simply wraps the existing instructions inside macros that can then later be augmented with CFI directives. The definitions of the macros in "include/_internal_s2n_bignum.h" show those instructions that are wrapped, for ARM and/or x86: both CFI_START <marker for entry point> both CFI_RET ret ARM CFI_BL(target) bl target ARM CFI_PUSH2(lo,hi) stp lo, hi, [sp, #-16]! ARM CFI_POP2(lo,hi) ldp lo, hi, [sp], awslabs#16 ARM CFI_INC_SP(offset) add sp, sp, #(offset) ARM CFI_DEC_SP(offset) sub sp, sp, #(offset) x86 CFI_CALL(target) call target x86 CFI_PUSH(reg) push reg x86 CFI_POP(reg) pop reg x86 CFI_INC_RSP(offset) add rsp, offset x86 CFI_DEC_RSP(offset) sub rsp, offset
Could you please expand the documentation regarding ABI compatibility with respect to call frame information (CFI) and frame pointers? In particular, are there no CFI directives in the assembly because they wouldn't be an improvement, or because it hasn't been a priority/desire to add them? This is something that comes up when comparing your assembly language code to other implementations like OpenSSL's and BoringSSL's.
See https://www.imperialviolet.org/2017/01/18/cfi.html for more clarification about what I'm referring to.
Thanks in advance.
The text was updated successfully, but these errors were encountered: