-f[no-]omit-frame-pointer

Hi,

It looks like the default for -fomit-frame-pointer has recently changed from “no” to “yes at O1 and higher”, but I did not see the commit.
Was that intentional?

(AddressSanitizer uses frame pointers to unwind stack, that’s how we noticed).

Thanks,

–kcc

Yes; r146586.

-Eli

Hi,

It looks like the default for -fomit-frame-pointer has recently changed from
“no” to “yes at O1 and higher”, but I did not see the commit.
Was that intentional?

Yes; r146586.

What would be the best fix for asan?
Force -mdisable-fp-elim when asan is enabled or document that asan requires -fomit-frame-pointer for better warning messages?

Thanks!

–kcc

It looks like the default for -fomit-frame-pointer has recently changed from
"no" to "yes at O1 and higher", but I did not see the commit.
Was that intentional?

Yes. See r146586 and PR8186

Can you be explicit what you need to asan? Just the equivalent of
__builtin_return_address(0) or do you really need a full stack trace?

Joerg

What would be the best fix for asan?

Can you be explicit what you need to asan? Just the equivalent of
__builtin_return_address(0) or do you really need a full stack trace?

asan-rt uses __builtin_return_address(0) to get the full stack trace.
See compiler-rt/lib/asan/asan_stack.cc (AsanStackTrace::FastUnwindStack)
It checks the current thread’s stack bounds to avoid a wild dereference.

Asan does not use unsafe __builtin_return_address(N, N>0), although it would be nice if __builtin_return_address(N, N>0) had safer semantics.

–kcc

What would be the best fix for asan?
Force -mdisable-fp-elim when asan is enabled or document that asan
requires -fomit-frame-pointer for better warning messages?

Just messages? I would suggest documentation.

If it is needed by some of the checks then enabling frame pointers by
default when asan is enabled and rejecting or warning when someone tries
to use both asan and -fomit-frame-pointer is probably better.

Thanks!

--kcc

Cheers,
Rafael

What would be the best fix for asan?
Force -mdisable-fp-elim when asan is enabled or document that asan
requires -fomit-frame-pointer for better warning messages?

Just messages?

Yes.
Asan unwinds stack on every malloc/free and on failure.
w/o frame pointers the warning message will be less useful, but the ability to find bugs will not be affected.

I would suggest documentation.

Ok, I’ll do that later (if no one else has an opposite opinion).

–kcc

That's inconsistent :slight_smile: __builtin_return_address(0) works with or without
frame pointer. What doesn't work is depending on the frame pointer on
the stack to "speed up" the unwinding further. So what is it?

Joerg

Not sure what your question is.
afaict, the method used by asan is the only fast way to get stack traces (but it requires frame pointers).
All other methods (e.g. libunwind) are much slower.

–kcc

I think it does not. What you're doing there is traversing the stack frames.
__builtin_return_address(0) is generated for each function
individually. It just reads the return address from the known position
on the stack and does not help to unwind further.

I believe the best option for us is to add the
--fno-omit-frame-pointer depending on the -faddress-sanitizer flag.
But in order to do this reliably, we need to remove all the instances
of -fomit-frame-pointer from the command line.
IIUC Clang doesn't support this now.

I believe the best option for us is to add the
--fno-omit-frame-pointer depending on the -faddress-sanitizer flag.
But in order to do this reliably, we need to remove all the instances
of -fomit-frame-pointer from the command line.

Why? Just add -fno-omit-frame-pointer last.

Yea, this should be easy to do inside the Clang driver. Feel free to CC me on any patch, and send i to cfe-commits@

I believe the best option for us is to add the
–fno-omit-frame-pointer depending on the -faddress-sanitizer flag.
But in order to do this reliably, we need to remove all the instances
of -fomit-frame-pointer from the command line.

Why? Just add -fno-omit-frame-pointer last.

Yea, this should be easy to do inside the Clang driver. Feel free to CC me on any patch, and send i to cfe-commits@

One thing in favor of documenting the need for -fno-omit-frame-pointer instead of enforcing it in the driver:
suppose a program, part of which is compiled with -faddress-sanitizer, but some other part is compiled w/o it.
If -fno-omit-frame-pointer is implied by -faddress-sanitizer and not explicitly given by users, some part of the program will not have frame pointers and stacks will be short.
I am inclined to document the need to -fno-omit-frame-pointer.
Other opinions?

–kcc

If some part of a program isn't compiled with -faddress-sanitizer, we
are likely to run into trouble if any of the uninstrumented
constructors call instrumented code. OTOH it is possibly legal to
dlopen a shared library that is not compiled with -faddress-sanitizer,
so it's a good idea to document this.