Support GCC's -fstack-usage flag

[pzheng]

Summary

This patch adds support for GCC's -fstack-usage flag. With this flag, a stack
usage file (i.e., .su file) is generated for each input source file. The format
of the stack usage file is also similar to what is used by GCC. For each
function defined in the source file, a line with the following information is
produced in the .su file.

<source_file>:<line_number>:<function_name> <size_in_byte> <static/dynamic>

"Static" means that the function's frame size is static and the size info is an
accurate reflection of the frame size. While "dynamic" means the function's
frame size can only be determined at run-time because the function manipulates
the stack dynamically (e.g., due to variable size objects). The size info only
reflects the size of the fixed size frame objects in this case and therefore is
not a reliable measure of the total frame size.

[lebedev.ri]

Might you be interested in implementing [[ https://bugs.llvm.org/show_bug.cgi?id=44418 | -Wstack-usage= ]] diag as a follow-up?

[pzheng]

In D100509#2689839, @lebedev.ri wrote:

Might you be interested in implementing [[ https://bugs.llvm.org/show_bug.cgi?id=44418 | -Wstack-usage= ]] diag as a follow-up?

Thanks for bringing it up, @lebedev.ri. Yes, I will look into supporting that next.

[jansvoboda11]

I left a couple of comments inline on the command-line parsing aspect of the patch. For more info, check https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option.

clang/include/clang/Driver/Options.td

2709

This argument is not necessary, MarshallingInfoString defaults to an empty string by default.

clang/lib/Frontend/CompilerInvocation.cpp

1934

Since OPT_fstack_usage has already been parsed into Opts.StackUsage at this point, it would make more sense to check that instead.

1938

This has already been parsed through MarshallingInfoString<CodeGenOpts<"StackUsageOutput">>, no need to duplicate it here.

[pzheng]

Address @jansvoboda11's comments.

[pzheng]

Thanks for your feedback, @jansvoboda11!

[xbolva00]

It would be great to also update release notes for clang to inform users about this new feature.

[pzheng]

Mention the flag in release notes based on @xbolva00's comment.

[xbolva00]

<source_file>:<line_number>:<function_name> <size_in_byte> <static/dynamic>

gcc also supports "bounded" - do you plan to somehow handle it? (https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html#Developer-Options), eg: parser.c:918:5:parse_statement 48 dynamic,bounded

clang/test/CodeGen/stack-usage.c

8

-fstack-usage=file.su is Clang only, right? Tried with GCC, gcc error: unrecognized command-line option ‘-fstack-usage=xxx’,

Do we need to also specify extra -fstack-usage if -fstack-usage=file.su is used? Seems quite redudant and -fstack-usage=file.su alone should be enough.

[pzheng]

In D100509#2692127, @xbolva00 wrote:

<source_file>:<line_number>:<function_name> <size_in_byte> <static/dynamic>

gcc also supports "bounded" - do you plan to somehow handle it? (https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html#Developer-Options), eg: parser.c:918:5:parse_statement 48 dynamic,bounded

I actually tried some test cases using GCC and never got it to output "dynamic,bounded." So, not sure how (or when) GCC actually determines a function is dynamic, but bounded. Any idea?

clang/test/CodeGen/stack-usage.c

8

Yes, -fstack-usage=file.su is Clang only and it is only a cc1 option, not a driver option. The main reason why I added both flags is to distinguish between the case where the user specified "-o" and the case where "-o" is not specified.

If only -fstack-usage is passed to cc1, we know the user did not pass "-o" on the command line and the name (with the extension removed) of the source file should be used to name the .su file. For example, with "clang -fstack-usage -c foo.c", foo.su is generated.

If both -fstack-usage and -fstack-usage= are present, we know the user specified "-o" and the name of the .su file should be based on that name instead. For example, with "clang -fstack-usage -c foo.c -o bar.o", bar.su is generated instead of foo.su.

[xbolva00]

Set current_function_has_unbounded_dynamic_stack_size to 1 when pushing a variable-sized argument onto the stack. 

      if (current_function_has_unbounded_dynamic_stack_size)
	stack_usage_kind = DYNAMIC;
      else
	stack_usage_kind = DYNAMIC_BOUNDED;

https://github.com/gcc-mirror/gcc/commit/d3c12306971946ab9a9d644ddf7b26e9383d2391

You can compile eg. zstd project with "CC="gcc -fstack-usage" make -j6 -B" and then grep "bounded" . -R and you will find some examples of dynamic,bounded.

[pzheng]

In D100509#2692259, @xbolva00 wrote:

Set current_function_has_unbounded_dynamic_stack_size to 1 when pushing a variable-sized argument onto the stack. 

      if (current_function_has_unbounded_dynamic_stack_size)
	stack_usage_kind = DYNAMIC;
      else
	stack_usage_kind = DYNAMIC_BOUNDED;

https://github.com/gcc-mirror/gcc/commit/d3c12306971946ab9a9d644ddf7b26e9383d2391

You can compile eg. zstd project with "CC="gcc -fstack-usage" make -j6 -B" and then grep "bounded" . -R and you will find some examples of dynamic,bounded.

Thanks, @xbolva00! This is really helpful. I tried compiling the zstd project with both gcc and clang. I found that in all those cases I checked where gcc outputs "dynamic,bounded", clang actually outputs "static" instead. Looks like LLVM already does a better job of determining the frame size statically. So, maybe there is no need to add the "dynamic,bounded" case to clang? Thoughts?

[xbolva00]

Great if better :)

Please consider adding a small testcase (eg from zstd) where llvm says static and gcc says dynamic,bound.

[pzheng]

I checked some of the functions in zstd where gcc outputs "dynamic,bounded", but did not find any straightforward way to simplify them into standalone tests. If anyone happen to have a simple test case, I would be more than happy to add here.

[xbolva00]

In D100509#2693388, @pzheng wrote:

I checked some of the functions in zstd where gcc outputs "dynamic,bounded", but did not find any straightforward way to simplify them into standalone tests. If anyone happen to have a simple test case, I would be more than happy to add here.

Me neither, no problem. Not a blocker.

[pzheng]

Thanks for all the great feedbacks, @jansvoboda11, @xbolva00 and @lebedev.ri! Is there any other concern that needs to be addressed?

[xbolva00]

Only nit. Added two more potential reviewers.

clang/include/clang/Basic/CodeGenOptions.h

374–379

Nit: comment? Like one in TargetOptions.h

[pzheng]

Add missing comments. Thanks for spotting it, @xbolva00.

[jansvoboda11]

This LGTM from command-line perspective, but I'll let others judge the rest of the patch.

[MaskRay]

clang/lib/Driver/ToolChains/Clang.cpp

5496

CC1 needs two options?

If you infer the filename in the driver, CC1 can use "whether StackUsageOutput is empty".

5502

"-fstack-usage=" + OutputFilename.str()

MakeArgString works with a Twine.

clang/test/CodeGen/stack-usage.c

2

Such llvm specific tests break layering but we don't have better way for testing so this might be ok...

However, not sure we need three targets. Usually one is sufficient.

4

%T is a deprecated lit replacement. Use rm -rf %t && mkdir %t

clang/test/Driver/stack-usage.c

2

Not sure we need three. One suffices.

10

Include quotes

llvm/include/llvm/CodeGen/AsmPrinter.h

186

Delete = nullptr

llvm/include/llvm/Target/TargetOptions.h

348

Delete = ""

[pzheng]

Addess @MaskRay's comments

[pzheng]

clang/lib/Driver/ToolChains/Clang.cpp

5496

Thanks for the suggestion! Addressed this in the latest update.

[pzheng]

Minor update to some comments.

[MaskRay]

clang/include/clang/Basic/CodeGenOptions.def

110 ↗

(On Diff #343795)

Unneeded

clang/test/CodeGen/stack-usage.c

8

8 -> [[#@LINE+1]]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1212

"" -> ''

[pzheng]

Address a few more comments from @MaskRay.

[pzheng]

clang/include/clang/Basic/CodeGenOptions.def

110 ↗

(On Diff #343795)

I tried removing this line, but TableGen doesn't seem to generate the fstack_usage flag anymore. Looks like this needs to be kept. Thoughts?

[MaskRay]

clang/lib/Driver/ToolChains/Clang.cpp

5496

You can compute the .su filename here and pass -fstack-usage=.... to CC1.

Then you can remove the boolean codegen option Opts.StackUsage.

[pzheng]

Address @MaskRay's comments.

[MaskRay]

LG with some nits. After fixes, please wait one day or so in case there are further comments.

clang/lib/Driver/ToolChains/Clang.cpp

5500

remove llvm::Twine

clang/test/CodeGen/stack-usage.c

4

%t/b.su can be simplified to b.su since you have changed cwd.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1189

const std::string &

otherwise there will be an unneeded copy.

[pzheng]

Address new comments from @MaskRay.

[pzheng]

Thanks for all the feedbacks, @MaskRay! I will wait another day before committing the change.

[MaskRay]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1208

Created https://github.com/llvm/llvm-project/pull/69896 to fix the filename for an included file, to address https://github.com/llvm/llvm-project/issues/69889

clang/docs/ReleaseNotes.rst

 - ...
 
 New Compiler Flags
 ------------------
 
 - ``-Wreserved-identifier`` emits warning when user code uses reserved
   identifiers.
 
+- ``-fstack-usage`` generates an extra .su file per input source file. The .su
+  file contains frame size information for each function defined in the source
+  file.
+
 Deprecated Compiler Flags
 -------------------------
 
 - ...
 
 Modified Compiler Flags
 -----------------------
 

clang/include/clang/Basic/CodeGenOptions.h

public:
   /// The TLS base register when StackProtectorGuard is "tls".
   /// On x86 this can be "fs" or "gs".
   std::string StackProtectorGuardReg;
 
   /// Path to ignorelist file specifying which objects
   /// (files, functions) listed for instrumentation by sanitizer
   /// coverage pass should actually not be instrumented.
   std::vector<std::string> SanitizeCoverageIgnorelistFiles;
 
+  /// Name of the stack usage file (i.e., .su file) if user passes
+  /// -fstack-usage. If empty, it can be implied that -fstack-usage is not
+  /// passed on the command line.
+  std::string StackUsageOutput;
+
   /// Executable and command-line used to create a given CompilerInvocation.
   /// Most of the time this will be the full -cc1 command.
   const char *Argv0 = nullptr;
   ArrayRef<const char *> CommandLineArgs;
 
   /// The minimum hotness value a diagnostic needs in order to be included in
   /// optimization diagnostics.
   ///

clang/include/clang/Driver/Options.td

def fbasic_block_sections_EQ : Joined<["-"], "fbasic-block-sections=">, Group<f_Group>,
   MarshallingInfoString<CodeGenOpts<"BBSections">, [{"none"}]>;
 defm data_sections : BoolFOption<"data-sections",
   CodeGenOpts<"DataSections">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Place each data in its own section">, NegFlag<SetFalse>>;
 defm stack_size_section : BoolFOption<"stack-size-section",
   CodeGenOpts<"StackSizeSection">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Emit section containing metadata on function stack sizes">,
   NegFlag<SetFalse>>;
+def fstack_usage : Flag<["-"], "fstack-usage">, Group<f_Group>,
+  HelpText<"Emit .su file containing information on function stack sizes">;
+def stack_usage_file : Separate<["-"], "stack-usage-file">,
+  Flags<[CC1Option, NoDriverOption]>,
+  HelpText<"Filename (or -) to write stack usage output to">,
+  MarshallingInfoString<CodeGenOpts<"StackUsageOutput">>;
 
 defm unique_basic_block_section_names : BoolFOption<"unique-basic-block-section-names",
   CodeGenOpts<"UniqueBasicBlockSectionNames">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Use unique names for basic block sections (ELF Only)">,
   NegFlag<SetFalse>>;
 defm unique_internal_linkage_names : BoolFOption<"unique-internal-linkage-names",
   CodeGenOpts<"UniqueInternalLinkageNames">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Uniqueify Internal Linkage Symbol Names by appending"

clang/lib/CodeGen/BackendUtil.cpp

Options.StackProtectorGuard =
           .Default(llvm::StackProtectorGuards::None);
   Options.StackProtectorGuardOffset = CodeGenOpts.StackProtectorGuardOffset;
   Options.StackProtectorGuardReg = CodeGenOpts.StackProtectorGuardReg;
   Options.TLSSize = CodeGenOpts.TLSSize;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
   Options.ExplicitEmulatedTLS = CodeGenOpts.ExplicitEmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
+  Options.StackUsageOutput = CodeGenOpts.StackUsageOutput;
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
   Options.ForceDwarfFrameSection = CodeGenOpts.ForceDwarfFrameSection;
   Options.EmitCallSiteInfo = CodeGenOpts.EmitCallSiteInfo;
   Options.EnableAIXExtendedAltivecABI = CodeGenOpts.EnableAIXExtendedAltivecABI;
   Options.PseudoProbeForProfiling = CodeGenOpts.PseudoProbeForProfiling;
   Options.ValueTrackingVariableLocations =
       CodeGenOpts.ValueTrackingVariableLocations;
   Options.XRayOmitFunctionIndex = CodeGenOpts.XRayOmitFunctionIndex;

clang/lib/Driver/ToolChains/Clang.cpp

if (Arg *A = Args.getLastArg(options::OPT_ftabstop_EQ)) {
     CmdArgs.push_back("-ftabstop");
     CmdArgs.push_back(A->getValue());
   }
 
   if (Args.hasFlag(options::OPT_fstack_size_section,
                    options::OPT_fno_stack_size_section, RawTriple.isPS4()))
     CmdArgs.push_back("-fstack-size-section");
 
+  if (Args.hasArg(options::OPT_fstack_usage)) {
+    CmdArgs.push_back("-stack-usage-file");
+
+    if (Arg *OutputOpt = Args.getLastArg(options::OPT_o)) {
+      SmallString<128> OutputFilename(OutputOpt->getValue());
+      llvm::sys::path::replace_extension(OutputFilename, llvm::Twine("su"));
+      CmdArgs.push_back(Args.MakeArgString(OutputFilename));
+    } else
+      CmdArgs.push_back(
+          Args.MakeArgString(Twine(getBaseInputStem(Args, Inputs)) + ".su"));
+  }
+
   CmdArgs.push_back("-ferror-limit");
   if (Arg *A = Args.getLastArg(options::OPT_ferror_limit_EQ))
     CmdArgs.push_back(A->getValue());
   else
     CmdArgs.push_back("19");
 
   if (Arg *A = Args.getLastArg(options::OPT_fmacro_backtrace_limit_EQ)) {
     CmdArgs.push_back("-fmacro-backtrace-limit");

clang/lib/Frontend/CompilerInvocation.cpp

#undef CODEGEN_OPTION_WITH_MARSHALLING
   }
 
   // If the user requested to use a sample profile for PGO, then the
   // backend will need to track source location information so the profile
   // can be incorporated into the IR.
   if (UsingSampleProfile)
     NeedLocTracking = true;
 
+  if (!Opts.StackUsageOutput.empty())
+    NeedLocTracking = true;
+
   // If the user requested a flag that requires source locations available in
   // the backend, make sure that the backend tracks source location information.
   if (NeedLocTracking && Opts.getDebugInfo() == codegenoptions::NoDebugInfo)
     Opts.setDebugInfo(codegenoptions::LocTrackingOnly);
 
   // Parse -fsanitize-recover= arguments.
   // FIXME: Report unrecoverable sanitizers incorrectly specified here.
   parseSanitizerKinds("-fsanitize-recover=",
                       Args.getAllArgValues(OPT_fsanitize_recover_EQ), Diags,
                       Opts.SanitizeRecover);

clang/test/CodeGen/stack-usage.c

+// REQUIRES: aarch64-registered-target
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clang_cc1 -triple aarch64-unknown -stack-usage-file %t/b.su -emit-obj %s -o %t/b.o
+// RUN: FileCheck %s < %t/b.su
+
+// CHECK: stack-usage.c:[[#@LINE+1]]:foo  {{[0-9]+}}  static
+int foo() {
+  char a[8];
+
+  return 0;
+}
+
+// CHECK: stack-usage.c:[[#@LINE+1]]:bar  {{[0-9]+}}  dynamic
+int bar(int len) {
+  char a[len];
+
+  return 1;
+}

clang/test/Driver/stack-usage.c

+// RUN: %clang -target aarch64-unknown %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ABSENT
+// CHECK-ABSENT-NOT: "-stack-usage-file"
+
+// RUN: %clang -target aarch64-unknown -fstack-usage %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PRESENT
+// CHECK-PRESENT: "-stack-usage-file"
+
+int foo() { return 42; }

llvm/include/llvm/CodeGen/AsmPrinter.h

private:
   MCSymbol *CurrentSectionBeginSym = nullptr;
 
   // The garbage collection metadata printer table.
   void *GCMetadataPrinters = nullptr; // Really a DenseMap.
 
   /// Emit comments in assembly output if this is true.
   bool VerboseAsm;
 
+  /// Output stream for the stack usage file (i.e., .su file).
+  std::unique_ptr<raw_fd_ostream> StackUsageStream;
+
   static char ID;
 
 protected:
   MCSymbol *CurrentFnBegin = nullptr;
 
   /// A vector of all debug/EH info emitters we should use. This vector
   /// maintains ownership of the emitters.
   std::vector<HandlerInfo> Handlers;
public:
   void emitFunctionBody();
 
   void emitCFIInstruction(const MachineInstr &MI);
 
   void emitFrameAlloc(const MachineInstr &MI);
 
   void emitStackSizeSection(const MachineFunction &MF);
 
+  void emitStackUsage(const MachineFunction &MF);
+
   void emitBBAddrMapSection(const MachineFunction &MF);
 
   void emitPseudoProbe(const MachineInstr &MI);
 
   void emitRemarksSection(remarks::RemarkStreamer &RS);
 
   /// Get the CFISection type for a function.
   CFISection getFunctionCFISectionType(const Function &F) const;

llvm/include/llvm/Target/TargetOptions.h

public:
 
     /// Stack protector guard mode to use, e.g. tls, global.
     StackProtectorGuards StackProtectorGuard =
                                          StackProtectorGuards::None;
 
     /// Stack protector guard reg to use, e.g. usually fs or gs in X86.
     std::string StackProtectorGuardReg = "None";
 
+    /// Name of the stack usage file (i.e., .su file) if user passes
+    /// -fstack-usage. If empty, it can be implied that -fstack-usage is not
+    /// passed on the command line.
+    std::string StackUsageOutput;
+
     /// FloatABIType - This setting is set by -float-abi=xxx option is specfied
     /// on the command line. This setting may either be Default, Soft, or Hard.
     /// Default selects the target's default behavior. Soft selects the ABI for
     /// software floating point, but does not indicate that FP hardware may not
     /// be used. Such a combination is unfortunately popular (e.g.
     /// arm-apple-darwin). Hard presumes that the normal FP ABI is used.
     FloatABI::ABIType FloatABIType = FloatABI::Default;
 

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::emitStackSizeSection(const MachineFunction &MF) {
   const MCSymbol *FunctionSymbol = getFunctionBegin();
   uint64_t StackSize = FrameInfo.getStackSize();
   OutStreamer->emitSymbolValue(FunctionSymbol, TM.getProgramPointerSize());
   OutStreamer->emitULEB128IntValue(StackSize);
 
   OutStreamer->PopSection();
 }
 
+void AsmPrinter::emitStackUsage(const MachineFunction &MF) {
+  auto OutputFilename = MF.getTarget().Options.StackUsageOutput;
+
+  // OutputFilename empty implies -fstack-usage is not passed.
+  if (OutputFilename.empty())
+    return;
+
+  const MachineFrameInfo &FrameInfo = MF.getFrameInfo();
+  uint64_t StackSize = FrameInfo.getStackSize();
+
+  if (StackUsageStream == nullptr) {
+    std::error_code EC;
+    StackUsageStream =
+        std::make_unique<raw_fd_ostream>(OutputFilename, EC, sys::fs::OF_Text);
+    if (EC) {
+      errs() << "Could not open file: " << EC.message();
+      return;
+    }
+  }
+
+  *StackUsageStream << MF.getFunction().getParent()->getName();
+  if (const DISubprogram *DSP = MF.getFunction().getSubprogram())
+    *StackUsageStream << ':' << DSP->getLine();
+
+  *StackUsageStream << ':' << MF.getName() << '\t' << StackSize << '\t';
+  if (FrameInfo.hasVarSizedObjects())
+    *StackUsageStream << "dynamic\n";
+  else
+    *StackUsageStream << "static\n";
+}
+
 static bool needFuncLabelsForEHOrDebugInfo(const MachineFunction &MF) {
   MachineModuleInfo &MMI = MF.getMMI();
   if (!MF.getLandingPads().empty() || MF.hasEHFunclets() || MMI.hasDebugInfo())
     return true;
 
   // We might emit an EH table that uses function begin and end labels even if
   // we don't have any landingpads.
   if (!MF.getFunction().hasPersonalityFn())
void AsmPrinter::emitFunctionBody() {
   // Emit section containing BB address offsets and their metadata, when
   // BB labels are requested for this function. Skip empty functions.
   if (MF->hasBBLabels() && HasAnyRealCode)
     emitBBAddrMapSection(*MF);
 
   // Emit section containing stack size metadata.
   emitStackSizeSection(*MF);
 
+  // Emit .su file containing function stack size information.
+  emitStackUsage(*MF);
+
   emitPatchableFunctionEntries();
 
   if (isVerbose())
     OutStreamer->GetCommentOS() << "-- End function\n";
 
   OutStreamer->AddBlankLine();
 }