[sframe] Add assembler option --gsframe #165322
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This plumbs the option --gsframe through the various levels needed to support it in the assembler. This is the final step in assembler-level sframe support. With it in place, clang can produce sframe-sections that successfully link with gnu-ld. LLD support is pending some discussion.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: None (Sterling-Augustine) ChangesThis plumbs the option --gsframe through the various levels needed to support it in the assembler. This is the final step in assembler-level sframe support for x86. With it in place, clang produces sframe-sections that successfully link with gnu-ld. LLD support is pending some discussion. Full diff: https://github.com/llvm/llvm-project/pull/165322.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 90e1f8d1eb5e9..99199c3a6c34c 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -120,6 +120,8 @@ CODEGENOPT(StackSizeSection , 1, 0, Benign) ///< Set when -fstack-size-section
///< Set when -femit-compact-unwind-non-canonical is enabled.
CODEGENOPT(EmitCompactUnwindNonCanonical, 1, 0, Benign)
+CODEGENOPT(EmitSFrameUnwind, 1, 0, Benign) ///< Set when -sframe is enabled.
+
///< Set when -fxray-always-emit-customevents is enabled.
CODEGENOPT(XRayAlwaysEmitCustomEvents , 1, 0, Benign)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 93aeb22b18e92..06365cef77bf0 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4701,11 +4701,13 @@ def femit_dwarf_unwind_EQ : Joined<["-"], "femit-dwarf-unwind=">,
NormalizedValues<["Always", "NoCompactUnwind", "Default"]>,
NormalizedValuesScope<"llvm::EmitDwarfUnwindType">,
MarshallingInfoEnum<CodeGenOpts<"EmitDwarfUnwind">, "Default">;
-defm emit_compact_unwind_non_canonical : BoolFOption<"emit-compact-unwind-non-canonical",
- CodeGenOpts<"EmitCompactUnwindNonCanonical">, DefaultFalse,
- PosFlag<SetTrue, [], [ClangOption, CC1Option, CC1AsOption],
- "Try emitting Compact-Unwind for non-canonical entries. Maybe overridden by other constraints">,
- NegFlag<SetFalse>>;
+defm emit_compact_unwind_non_canonical
+ : BoolFOption<"emit-compact-unwind-non-canonical",
+ CodeGenOpts<"EmitCompactUnwindNonCanonical">, DefaultFalse,
+ PosFlag<SetTrue, [], [ClangOption, CC1Option, CC1AsOption],
+ "Try emitting Compact-Unwind for non-canonical "
+ "entries. Maybe overridden by other constraints">,
+ NegFlag<SetFalse>>;
def g_Flag : Flag<["-"], "g">, Group<g_Group>,
Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
HelpText<"Generate source-level debug information">;
@@ -7729,6 +7731,11 @@ def massembler_fatal_warnings : Flag<["-"], "massembler-fatal-warnings">,
def crel : Flag<["--"], "crel">,
HelpText<"Enable CREL relocation format (ELF only)">,
MarshallingInfoFlag<CodeGenOpts<"Crel">>;
+// The leading 'g' is misleading. This is an unwind tables option, not
+// a debug option. But uses this name for gnu compatibility.
+def gsframe : Flag<["--"], "gsframe">,
+ HelpText<"Generate .sframe unwind sections (ELF only)">,
+ MarshallingInfoFlag<CodeGenOpts<"EmitSFrameUnwind">>;
def mmapsyms_implicit : Flag<["-"], "mmapsyms=implicit">,
HelpText<"Allow mapping symbol at section beginning to be implicit, "
"lowering number of mapping symbols at the expense of some "
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 468c930acacbd..4b988103e3cd1 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -500,6 +500,7 @@ static bool initTargetOptions(const CompilerInstance &CI,
Options.MCOptions.EmitDwarfUnwind = CodeGenOpts.getEmitDwarfUnwind();
Options.MCOptions.EmitCompactUnwindNonCanonical =
CodeGenOpts.EmitCompactUnwindNonCanonical;
+ Options.MCOptions.EmitSFrameUnwind = CodeGenOpts.EmitSFrameUnwind;
Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;
Options.MCOptions.MCSaveTempLabels = CodeGenOpts.SaveTempLabels;
Options.MCOptions.MCUseDwarfDirectory =
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 79edc561c551f..a033e8f708306 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2617,6 +2617,8 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
llvm::codegenoptions::DebugInfoConstructor,
DwarfVersion, llvm::DebuggerKind::Default);
}
+ } else if (Value == "--gsframe") {
+ CmdArgs.push_back("--gsframe");
} else if (Value.starts_with("-mcpu") || Value.starts_with("-mfpu") ||
Value.starts_with("-mhwdiv") || Value.starts_with("-march")) {
// Do nothing, we'll validate it later.
@@ -5920,6 +5922,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
else if (UnwindTables)
CmdArgs.push_back("-funwind-tables=1");
+ // Sframe unwind tables are independent of the other types, and only defined
+ // for these two targets.
+ if (Arg *A = Args.getLastArg(options::OPT_gsframe)) {
+ if ((Triple.isAArch64() || Triple.isX86()) && Triple.isOSBinFormatELF())
+ CmdArgs.push_back("--gsframe");
+ else
+ D.Diag(diag::err_drv_unsupported_opt_for_target)
+ << A->getOption().getName() << TripleStr;
+ }
+
// Prepare `-aux-target-cpu` and `-aux-target-feature` unless
// `--gpu-use-aux-triple-only` is specified.
if (!Args.getLastArg(options::OPT_gpu_use_aux_triple_only) &&
diff --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp
index 50da2f8449a22..3187a5b5c571b 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -163,6 +163,10 @@ struct AssemblerInvocation {
LLVM_PREFERRED_TYPE(bool)
unsigned EmitCompactUnwindNonCanonical : 1;
+ // Whether to emit sframe unwind sections.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned EmitSFrameUnwind : 1;
+
LLVM_PREFERRED_TYPE(bool)
unsigned Crel : 1;
LLVM_PREFERRED_TYPE(bool)
@@ -388,6 +392,7 @@ bool AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts,
Opts.EmitCompactUnwindNonCanonical =
Args.hasArg(OPT_femit_compact_unwind_non_canonical);
+ Opts.EmitSFrameUnwind = Args.hasArg(OPT_gsframe);
Opts.Crel = Args.hasArg(OPT_crel);
Opts.ImplicitMapsyms = Args.hasArg(OPT_mmapsyms_implicit);
Opts.X86RelaxRelocations = !Args.hasArg(OPT_mrelax_relocations_no);
@@ -450,6 +455,7 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
MCOptions.MCRelaxAll = Opts.RelaxAll;
MCOptions.EmitDwarfUnwind = Opts.EmitDwarfUnwind;
MCOptions.EmitCompactUnwindNonCanonical = Opts.EmitCompactUnwindNonCanonical;
+ MCOptions.EmitSFrameUnwind = Opts.EmitSFrameUnwind;
MCOptions.MCSaveTempLabels = Opts.SaveTemporaryLabels;
MCOptions.Crel = Opts.Crel;
MCOptions.ImplicitMapSyms = Opts.ImplicitMapsyms;
|
Member
|
@llvm/pr-subscribers-clang-driver Author: None (Sterling-Augustine) ChangesThis plumbs the option --gsframe through the various levels needed to support it in the assembler. This is the final step in assembler-level sframe support for x86. With it in place, clang produces sframe-sections that successfully link with gnu-ld. LLD support is pending some discussion. Full diff: https://github.com/llvm/llvm-project/pull/165322.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 90e1f8d1eb5e9..99199c3a6c34c 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -120,6 +120,8 @@ CODEGENOPT(StackSizeSection , 1, 0, Benign) ///< Set when -fstack-size-section
///< Set when -femit-compact-unwind-non-canonical is enabled.
CODEGENOPT(EmitCompactUnwindNonCanonical, 1, 0, Benign)
+CODEGENOPT(EmitSFrameUnwind, 1, 0, Benign) ///< Set when -sframe is enabled.
+
///< Set when -fxray-always-emit-customevents is enabled.
CODEGENOPT(XRayAlwaysEmitCustomEvents , 1, 0, Benign)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 93aeb22b18e92..06365cef77bf0 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4701,11 +4701,13 @@ def femit_dwarf_unwind_EQ : Joined<["-"], "femit-dwarf-unwind=">,
NormalizedValues<["Always", "NoCompactUnwind", "Default"]>,
NormalizedValuesScope<"llvm::EmitDwarfUnwindType">,
MarshallingInfoEnum<CodeGenOpts<"EmitDwarfUnwind">, "Default">;
-defm emit_compact_unwind_non_canonical : BoolFOption<"emit-compact-unwind-non-canonical",
- CodeGenOpts<"EmitCompactUnwindNonCanonical">, DefaultFalse,
- PosFlag<SetTrue, [], [ClangOption, CC1Option, CC1AsOption],
- "Try emitting Compact-Unwind for non-canonical entries. Maybe overridden by other constraints">,
- NegFlag<SetFalse>>;
+defm emit_compact_unwind_non_canonical
+ : BoolFOption<"emit-compact-unwind-non-canonical",
+ CodeGenOpts<"EmitCompactUnwindNonCanonical">, DefaultFalse,
+ PosFlag<SetTrue, [], [ClangOption, CC1Option, CC1AsOption],
+ "Try emitting Compact-Unwind for non-canonical "
+ "entries. Maybe overridden by other constraints">,
+ NegFlag<SetFalse>>;
def g_Flag : Flag<["-"], "g">, Group<g_Group>,
Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
HelpText<"Generate source-level debug information">;
@@ -7729,6 +7731,11 @@ def massembler_fatal_warnings : Flag<["-"], "massembler-fatal-warnings">,
def crel : Flag<["--"], "crel">,
HelpText<"Enable CREL relocation format (ELF only)">,
MarshallingInfoFlag<CodeGenOpts<"Crel">>;
+// The leading 'g' is misleading. This is an unwind tables option, not
+// a debug option. But uses this name for gnu compatibility.
+def gsframe : Flag<["--"], "gsframe">,
+ HelpText<"Generate .sframe unwind sections (ELF only)">,
+ MarshallingInfoFlag<CodeGenOpts<"EmitSFrameUnwind">>;
def mmapsyms_implicit : Flag<["-"], "mmapsyms=implicit">,
HelpText<"Allow mapping symbol at section beginning to be implicit, "
"lowering number of mapping symbols at the expense of some "
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 468c930acacbd..4b988103e3cd1 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -500,6 +500,7 @@ static bool initTargetOptions(const CompilerInstance &CI,
Options.MCOptions.EmitDwarfUnwind = CodeGenOpts.getEmitDwarfUnwind();
Options.MCOptions.EmitCompactUnwindNonCanonical =
CodeGenOpts.EmitCompactUnwindNonCanonical;
+ Options.MCOptions.EmitSFrameUnwind = CodeGenOpts.EmitSFrameUnwind;
Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;
Options.MCOptions.MCSaveTempLabels = CodeGenOpts.SaveTempLabels;
Options.MCOptions.MCUseDwarfDirectory =
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 79edc561c551f..a033e8f708306 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2617,6 +2617,8 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
llvm::codegenoptions::DebugInfoConstructor,
DwarfVersion, llvm::DebuggerKind::Default);
}
+ } else if (Value == "--gsframe") {
+ CmdArgs.push_back("--gsframe");
} else if (Value.starts_with("-mcpu") || Value.starts_with("-mfpu") ||
Value.starts_with("-mhwdiv") || Value.starts_with("-march")) {
// Do nothing, we'll validate it later.
@@ -5920,6 +5922,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
else if (UnwindTables)
CmdArgs.push_back("-funwind-tables=1");
+ // Sframe unwind tables are independent of the other types, and only defined
+ // for these two targets.
+ if (Arg *A = Args.getLastArg(options::OPT_gsframe)) {
+ if ((Triple.isAArch64() || Triple.isX86()) && Triple.isOSBinFormatELF())
+ CmdArgs.push_back("--gsframe");
+ else
+ D.Diag(diag::err_drv_unsupported_opt_for_target)
+ << A->getOption().getName() << TripleStr;
+ }
+
// Prepare `-aux-target-cpu` and `-aux-target-feature` unless
// `--gpu-use-aux-triple-only` is specified.
if (!Args.getLastArg(options::OPT_gpu_use_aux_triple_only) &&
diff --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp
index 50da2f8449a22..3187a5b5c571b 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -163,6 +163,10 @@ struct AssemblerInvocation {
LLVM_PREFERRED_TYPE(bool)
unsigned EmitCompactUnwindNonCanonical : 1;
+ // Whether to emit sframe unwind sections.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned EmitSFrameUnwind : 1;
+
LLVM_PREFERRED_TYPE(bool)
unsigned Crel : 1;
LLVM_PREFERRED_TYPE(bool)
@@ -388,6 +392,7 @@ bool AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts,
Opts.EmitCompactUnwindNonCanonical =
Args.hasArg(OPT_femit_compact_unwind_non_canonical);
+ Opts.EmitSFrameUnwind = Args.hasArg(OPT_gsframe);
Opts.Crel = Args.hasArg(OPT_crel);
Opts.ImplicitMapsyms = Args.hasArg(OPT_mmapsyms_implicit);
Opts.X86RelaxRelocations = !Args.hasArg(OPT_mrelax_relocations_no);
@@ -450,6 +455,7 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
MCOptions.MCRelaxAll = Opts.RelaxAll;
MCOptions.EmitDwarfUnwind = Opts.EmitDwarfUnwind;
MCOptions.EmitCompactUnwindNonCanonical = Opts.EmitCompactUnwindNonCanonical;
+ MCOptions.EmitSFrameUnwind = Opts.EmitSFrameUnwind;
MCOptions.MCSaveTempLabels = Opts.SaveTemporaryLabels;
MCOptions.Crel = Opts.Crel;
MCOptions.ImplicitMapSyms = Opts.ImplicitMapsyms;
|
tarunprabhu
requested changes
Oct 29, 2025
Contributor
tarunprabhu
left a comment
tarunprabhu
left a comment
There was a problem hiding this comment.
Please add tests to check that:
- the option is passed on to cc1
- the correct error is emitted when an unsupported platform is used
- the assembler behaves as expected (even if this has been tested elsewhere, an end-to-end "integration" test would be useful)
| NormalizedValues<["Always", "NoCompactUnwind", "Default"]>, | ||
| NormalizedValuesScope<"llvm::EmitDwarfUnwindType">, | ||
| MarshallingInfoEnum<CodeGenOpts<"EmitDwarfUnwind">, "Default">; | ||
| defm emit_compact_unwind_non_canonical : BoolFOption<"emit-compact-unwind-non-canonical", |
Contributor
There was a problem hiding this comment.
Is there a reason that this has been reformatted?
Contributor
Author
There was a problem hiding this comment.
git clang-format really, really wants to reformat these lines, even if I check the file out again and start from scratch.
I have reverted for now, but we will see if the formatters complain.
…162553) This fixes [issue 162148](llvm#162148). Common symbols are intended to have only a single version of the data present in the final executable. The MSVC linker is able to successfully deduplicate these chunks. If you have an application with a large number of translation units with a large block of common data (this is possible, for example, with Fortran code), then failing to deduplicate these chunks can make the data size so large that the resulting executable fails to load. The logic in this patch doesn't catch all of the potential cases for deduplication, but it should catch the most common ones.
…64285) This pr updates `DXILPrepare` and `DXILTranslateMetadata` by moving all the removal of metadata from `DXILPrepare` to `DXILTranslateMetadata` to have a more consistent definition of what each pass is doing. It restricts the `DXILPrepare` to only update function attributes and insert bitcasts, and moves the removal of metadata to `DXILTranslateMetadata` so that all manipulation of metadata is done in a single pass.
…San false negatives (llvm#165028) MSan currently crashes at compile-time when it encounters target("aarch64.svcount") (e.g., llvm#164315). This patch duct-tapes MSan so that it won't crash at compile-time, and instead propagates a clean shadow (resulting in false negatives but not false positives).
…ed_callee (llvm#164592) Commit b194cf1 changed this function for the case where attribute `cfi_unchecked_callee` is added in a function conversion. But this introduces a hole (issue llvm#162798), and it seems the change was unnecessary: the preceding `TryFunctionConversion` will already allow adding the `cfi_unchecked_callee` attribute, and will update `FromType` if it succeeds. So we revert the changes to `IsStandardConversion`. We also remove the helper function `AddingCFIUncheckedCallee` which is no longer needed, and simplify the corresponding `DiscardingCFIUncheckedCallee`. Fixes: llvm#162798
llvm#165038) … correctly" (llvm#164670)" This reverts commit 50ca1f4. Reverting because this leads to the bug on ToT described in llvm#164866. The original fix addresses an old regression which we'd still like to land eventually. See the discussion in llvm#164670 for more context.
VPlan::SCEVToExpansion isn't needed any longer, as SCEV expansion de-duplication is handled locally in expandSCEVs. PR: llvm#164490
This patch renames variable names to conform to the LLVM Coding Standards. The public interface remains the same.
Now that the old llvm::identity has moved into IndexedMap.h under a different name, this patch renames identity_cxx20 to identity. Note that llvm::identity closely models std::identity from C++20.
Note that "override" makes "virtual" redundant. Identified with modernize-use-override.
Now that the Windows container contains clang, use it for building the premerge tests. Measurements show this is significantly faster than using msvc cl. Note we had to disable four warnings -Wc++98-compat, -Wc++14-compat, -Wunsafe-buffer-usage, and -Wold-style-cast to make this work with 'check-mlir' on Windows (clang generates a lot of warnings that msvc cl does not).
Introduce the concept of internal stop hooks. These are similar to LLDB's internal breakpoints: LLDB itself will add them and users of LLDB will not be able to add or remove them. This change adds the following 3 independently-useful concepts: * Maintain a list of internal stop hooks that will be populated by LLDB and cannot be added to or removed from by users. They are managed in a separate list in `Target::m_internal_stop_hooks`. * `StopHookKind:CodeBased` and `StopHookCoded` represent a stop hook defined by a C++ code callback (instead of command line expressions or a Python class). * Stop hooks that do not print any output can now also suppress the printing of their header and description when they are hit via `StopHook::GetSuppressOutput`. Combining these 3 concepts we can model "internal stop hooks" which serve the same function as LLDB's internal breakpoints: executing built-in, LLDB-defined behavior, leveraging the existing mechanism of stop hooks. This change also simplifies `Target::RunStopHooks`. We already have to materialize a new list for combining internal and user stop hooks. Filter and only add active hooks to this list to avoid the need for "isActive?" checks later on.
Before this patch we were passing in the previous commit rather than the current commit due to a copy and paste adjustment failure from the PR flow. We want the base SHA to just be the commit SHA for postcommit. We also were not attaching the run number which made the source ID the first JUnit XML file rather than the buildbot run number.
Sterling-Augustine
requested review from
a team,
PeimingLiu,
aartbik,
andykaylor,
antiagainst,
ayermolo,
bcardosolopes,
ftynse,
hanhanW,
krzysz00,
kuhar,
lanza,
maksfb,
makslevental,
matthias-springer,
nicolasvasilache,
nikic,
paschalis-mpeis,
rafaelauler,
rolfmorel,
stellaraccident,
xlauko,
yinying-lisa-li,
yota9 and
yozhu
as code owners
Contributor
|
Can this be split into multiple PRs per project? |
Contributor
Author
Tests added for all those cases. Not sure what the best "assembler behaves as expected" test is, but for now the check is, "Was an sframe section emitted?" without any details on the format.
Somehow jcramer-intel added a bunch of changes to this PR. I have no idea how or why. Trying to figure out why. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
103 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This plumbs the option --gsframe through the various levels needed to support it in the assembler.
This is the final step in assembler-level sframe support for x86. With it in place, clang produces sframe-sections that successfully link with gnu-ld.
LLD support is pending some discussion.