`-fstack-size-section` doesn't emit `.stack_sizes` section when using any LTO

[pirama-arumuga-nainar]

# With ThinLTO (but Full LTO also skips the section the same way)
$ echo "int main() {
  return 0;
}" > main.cpp
$ clang -fstack-size-section main.cpp -fuse-ld=lld -v -flto=thin
$ llvm-readelf -a a.out | grep .stack_sizes

vs.

 # Without ThinLTO
$ clang -fstack-size-section main.cpp -fuse-ld=lld -v
$ llvm-readelf -a a.out | grep .stack_sizes
  [28] .stack_sizes      PROGBITS        0000000000000000 0009df 000009 00   L 15   0  1
   None   .comment .stack_sizes .symtab .shstrtab .strtab 

There is a workaround - passing -Wl,-mllvm,--stack-size-section to the linker [1] but this option should be a module attribute to make this work seamlessly with LTO (similar to, for e.g., -warn-frame-size)

[1]

$ clang++ -fstack-size-section main.cpp -fuse-ld=lld -flto=thin -Wl,-mllvm,--stack-size-section
$ llvm-readelf -a a.out | grep .stack_sizes
  [26] .stack_sizes      PROGBITS        0000000000000000 000a46 000009 00   L 13   0  1
   None   .comment .stack_sizes .symtab .shstrtab .strtab

[MaskRay]

-fstack-size-section affects a metadata section produced during IR to object file generation. The option does not change IR and there are many other options similar to -fstack-size-section.

You may use -Wl,-mllvm,--stack-size-section for now. Technically we can let Driver forward --stack-size-section for LTO, but every option is a special case and supporting them is a bit clumsy.

[llvmbot]

@llvm/issue-subscribers-clang-driver

[pirama-arumuga-nainar]

Thanks for the quick fix @MaskRay!

(Maybe a naive question): Is it feasible to find these options (clang FE options mapping to LLVM options) and have an automatic resolution for them to work with LTO - either forward them in the IR or have the driver do this automatically? Having a separate policy or action for each option (metadata, driver-forwarding, or linker flag) could be a pain point as we see wider (Thin)LTO adoption. If a better resolution is feasible, we could follow up on this in a separate issue/discussion.

(cc @stephenhines @teresajohnson )

[teresajohnson]

I am not a fan of the solution used here - ideally it should be passed through the IR, e.g. via module level metadata like for -warn-frame-size. There are some being passed through the driver for legacy reasons, but we've been trying to avoid new cases from getting added.

[MaskRay]

I do not agree that the current LTO handling of -g/-ffunction-sections/-fsplit-machine-function/-fcs-profile-generate/etc processing exists just for legacy reasons. There is a matter of practicality and desired granularity. Also, how well IPO applies to how different functions with different feature values, and how the feature applies to synthesized functions (see https://llvm.org/docs/LangRef.html#synthesized-functions-module-flags-metadata added by me). Function attributes and module flags metadata allow specifying certain merging behaviors, which can be handy when different merging behaviors are used. If such a granularity is not needed (which I believe is the case for -fstack-size-section), using -plugin-opt=-xxx is of a lower implementation overhead and looks ok to me.

I think the current driver handling definitely has room for improvement. Perhaps a more declarative Options.td can be leveraged.

[teresajohnson]

On Mon, Dec 12, 2022 at 2:13 PM Fangrui Song @.***> wrote:

I do not agree that the current LTO handling of -g/-ffunction-sections/-fsplit-machine-function/-fcs-profile-generate/etc processing exists just for legacy reasons. There is a matter of practicality and desired granularity. Also, how well IPO applies to how different functions with different feature values, and how the feature applies to synthesized functions (see https://llvm.org/docs/LangRef.html#synthesized-functions-module-flags-metadata added by me). Function attributes and module flags metadata allow specifying certain merging behaviors, which can be handy when different merging behaviors are used. If such a granularity is not needed (which I believe is the case for -fstack-size-section), using -plugin-opt=-xxx is of a lower implementation overhead and looks ok to me.

Is -fstack-size-section required to be passed to the link step of a compile? If not, then without embedding it in the IR, the options need to be changed when adding -flto[=thin] to the options of the compile steps, which is less than desirable (in an ideal world you simply add -flto to your options and the other options behave as before).

Also, what is the current behavior when one source file is compiled with -fstack-size-section and another is not, and they are linked together? If this is incorrect then a module flag would catch that in an LTO compile. If the constituent functions are compiled differently then it's ideal to use a function attribute to express the behavior. Maybe it doesn't really matter what we do with the code that was originally (non-LTO) compiled without the option, in which case the current driver solution works (a Max module flag would do the same), but then I would refer back to the first point above.

It's certainly faster implementation wise to pass through the driver, but not as clean.

I think the current driver handling definitely has room for improvement.

Perhaps a more declarative Options.td can be leveraged.

— Reply to this email directly, view it on GitHub https://github.com/llvm/llvm-project/issues/59424#issuecomment-1347240644, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE37ZQ3K5NARQLW5MGDRJU3WM6BPBANCNFSM6AAAAAASZSM3GY . You are receiving this because you were mentioned.Message ID: @.***>

-- Teresa Johnson | Software Engineer | @.*** |

[MaskRay]

I don't deny that it doesn't support fine granularity, but I also don't think it's necessary to add a function attribute, specify its merge behavior, and for synthesized functions add a module flag metadata for the last gap. For the use cases of -fstack-size-section, if people specify it for one TU, they actually specify it for all TUs. It does not change code/data generation and there is no point for users to apply it selectively to disable compiler bugs or others. This is likely more so for -fstack-size-section than some other options using the same mechanism (-ffunction-sections, -moutline, --save-stats, -gtune=xxx).

[teresajohnson]

On Mon, Dec 12, 2022 at 10:10 PM Fangrui Song @.***> wrote:

I don't deny that it doesn't support fine granularity, but I also don't think it's necessary to add a function attribute, specify its merge behavior, and for synthesized functions add a module flag metadata for the last gap. For the use cases of -fstack-size-section, if people specify it for one TU, they actually specify it for all TUs.

How does that work though - it looks like this info is generated during codegen. The linker will presumably blindly merge all generated .stack-size sections from TUs that generate them, but the linker (in a non-LTO link) wouldn't generate info for functions in TUs compiled without this option would it?

It does not change code/data generation and there is no point for users to apply it selectively to disable compiler bugs or others. This is likely more so for -fstack-size-section than some other options using the same mechanism (-ffunction-sections, -moutline, --save-stats, -gtune=xxx). Users will more likely to either specify for everything or nothing for this option than some other options that

Yes since it seems to be for tracking of size changes it seems fine to have the policy for LTO that if you specify for one TU it will get applied to all during LTO. However, this could be done with module flag metadata. Then you would get this section in a.out for both of the following 2 builds:

Non-LTO: clang++ -c A.cpp B.cpp -fstack-size-section clang++ A.o B.o

LTO: clang++ -c A.cpp B.cpp -fstack-size-section -flto clang++ A.o B.o -flto # For lld the -flto isn't even needed here

My understanding is that the latter will not currently get the .stack-size section in a.out without additionally passing -fstack-size-section to the link compile driver. While this use case may not be particularly critical since it is not affecting code correctness or even optimization, in general I think we should be going about this consistently and use IR whenever possible, which is what I've been asking people to do when I see driver patches get sent for review typically.

—

Reply to this email directly, view it on GitHub https://github.com/llvm/llvm-project/issues/59424#issuecomment-1347722054, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE37ZQZREV2DW5VYNZJPI73WM7ZLZANCNFSM6AAAAAASZSM3GY . You are receiving this because you were mentioned.Message ID: @.***>

-- Teresa Johnson | Software Engineer | @.*** |

[nickdesaulniers]

I am not a fan of the solution used here - ideally it should be passed through the IR, e.g. via module level metadata like for -warn-frame-size

+1