nickdesaulniers planned changes to this revision.
nickdesaulniers added a comment.

Will work on explicit LTO tests.



================
Comment at: clang/include/clang/Driver/Options.td:3429
   HelpText<"Use the given reg for addressing the stack-protector guard">,
-  MarshallingInfoString<CodeGenOpts<"StackProtectorGuardReg">, [{"none"}]>;
+  MarshallingInfoString<CodeGenOpts<"StackProtectorGuardReg">>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at 
function entry (x86/SystemZ only)">,
----------------
tejohnson wrote:
> nickdesaulniers wrote:
> > tejohnson wrote:
> > > What's the effect of or reason for this change?
> > Of the 3 options added in D88631 (`mstack_protector_guard_EQ`, 
> > `mstack_protector_guard_offset_EQ`, `mstack_protector_guard_reg_EQ`) 2 are 
> > strings (`mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ`).  
> > It was inconsistent that one could be unspecified, where as the other could 
> > be unspecified or `"none"` (but those 2 values were equivalent).
> > 
> > Without this change, in clang/lib/CodeGen/CodeGenModule.cpp I'd need to 
> > check that `StackProtectorGuardReg != "none"` rather than 
> > `!StackProtectorGuardReg.empty()` below.
> > 
> > I can change it back, but I think the asymmetry between 
> > `mstack_protector_guard_EQ` and `mstack_protector_guard_reg_EQ` in D88631, 
> > and I missed that in code review.
> > 
> > I don't think there would be any other observers of such a change.
> I see. Does unspecified mean something like just 
> "-mstack-protector-guard-reg=" with nothing after the =? I didn't realize 
> that was supported.
It looks like we validate for that case in the front end already. Specifically, 
`RenderAnalyzerOptions` in clang/lib/Driver/ToolChains/Clang.cpp.

    $ clang -mstack-protector-guard-reg= ...
    clang-13: error: invalid value '' in 'mstack-protector-guard-reg='


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102742/new/

https://reviews.llvm.org/D102742

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to