Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

"global constructors keyed to..." function does not respect frame pointer settings #21810

Open Quuxplusone opened 9 years ago

Quuxplusone commented 9 years ago
Bugzilla Link PR21811
Status NEW
Importance P normal
Reported by Dario Domizioli (dario.domizioli@gmail.com)
Reported on 2014-12-10 12:35:53 -0800
Last modified on 2015-10-09 15:14:54 -0700
Version unspecified
Hardware PC Linux
CC aaron@aaronballman.com, ahatanak@gmail.com, gaoyunzhong+2@gmail.com, isanbard@gmail.com, llvm-bugs@lists.llvm.org, rafael@espindo.la, rnk@google.com
Fixed by commit(s)
Attachments global-init-frame-ptr-settings.cpp (1431 bytes, text/plain)
Blocks
Blocked by
See also
Created attachment 13452
Reproducible - see description for how to compile and detect the issue

I have found an instance where the global initializers function for the
compilation unit (the "global constructors keyed to..." function) does not
respect the settings for whether to eliminate the frame pointer.

For example, compile the attached code with:
    clang -S -emit-llvm -O2 -fno-frame-pointer-elim <file>

I am using the "x86_64-unknown-linux" triple, and clang r223927 (but as far as
I can tell, Clang 3.4 and 3.5 show the same behaviour).

If you look at the .ll file, you should notice that main() is marked with
attributes #0 which include settings for the frame pointer elimination; however
the global constructors function is just created with attributes #2 which do
NOT include said settings.

I think this causes the global constructors function to be compiled with
different settings than the ones provided by the user. Specifically, the frame
pointer will be eliminated. On x86_64 this is detectable from looking at the
usage of %rbp.

If a lot of constructors are inlined, the register pressure will cause the
frame pointer register to be reused as a general purpose register. It will then
be stomped and (if there are later calls to constructors with a deep call tree)
what appears to be the initial frame can potentially be a value that has
nothing to do with frames.

Strangely, if we pass both -fno-frame-pointer-elim and -mno-omit-leaf-frame-
pointer then the global construtors function does have a frame pointer.
However, that function is not a leaf because it definitely calls __cxa_atexit()
to register the array dtors, so this shouldn't make a difference.

So there may even be two issues here...
Quuxplusone commented 9 years ago

Attached global-init-frame-ptr-settings.cpp (1431 bytes, text/plain): Reproducible - see description for how to compile and detect the issue

Quuxplusone commented 9 years ago

I don't think -fno-frame-pointer-elim is a valid Clang flag? Did you mean -fno-omit-frame-pointer?

Quuxplusone commented 9 years ago
I think "no-frame-pointer-elim" is the name of the function attribute. I usually
get this attribute if I pass "-Xclang -mdisable-fp-elim" on the command line,
and
I believe it is equivalent to "-fno-omit-frame-pointer".

Commit r187092+r187093 replaced the codegen option NoFramePointerElimNonLeaf
with
a function attribute "no-frame-pointer-elim-non-leaf", but the function
attribute
is not attached to all the functions. For example, what is created from
CodeGenModule::CreateGlobalInitOrDestructFunction() does not have this
attribute.
Oddly, the related codegen option NoFramePointerElim is still being used, and
the
corresponding function attribute "no-frame-pointer-elim" is attached but not
checked in the backend; maybe one of these should be removed.

Fixing the bug probably involves either making clang more diligent at attaching
this attributes, or to revert r187093. The latter is actually quite appealing
since I do not have to worry about forgetting to attach this attribute somewhere
in the front end.
Quuxplusone commented 9 years ago

We should fix the frontend to apply the attribute. Relying on global options doesn't work when you move to LTO.

Quuxplusone commented 9 years ago

Was this fixed in r235537?