[PATCH] D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change.

[Dan Liew via Phabricator via cfe-commits]

delcypher added inline comments.


================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:225-228
+ENUM_CODEGENOPT(SanitizeAddressDetectStackUseAfterReturnMode,
+                llvm::AsanDetectStackUseAfterReturnMode, 2,
+                llvm::AsanDetectStackUseAfterReturnMode::Runtime
+                ) ///< Set detection mode for stack-use-after-return.
----------------
vitalybuka wrote:
> delcypher wrote:
> > vitalybuka wrote:
> > > vitalybuka wrote:
> > > > Mode->Kind to be consistent with the file
> > > I believe for consistency we need:
> > > 1. Var name (SanitizeAddressDetectStackUseAfterReturn) no Mode/Kind suffix
> > > 2. Enum type AsanDetectStackUseAfterReturnKind uses "kind"
> > > 3. Actual flag includes no "kind" -fsanitize-address-detect-stack-use-after-return=
> > > 
> > > BTW. @delcypher  -fsanitize-address-destructor-kind was not consistent with the rest of file, but I am not sure it's worth of fixing now.
> > > 
> > > 
> > @vitalybuka It's not too late to fix this because the `-fsanitize-address-destructor-kind=` flag hasn't been adopted yet AFAIK.
> > 
> > Am I right in understanding that what I landed doesn't match points 1 and 3 above? If you can confirm that's what you'd like to be changed I can put up some patches to fix this.
> Correct
> I'd expect 
> 
> ```
> ENUM_CODEGENOPT(SanitizeAddressDtor, llvm::AsanDtorKind, 2,
>                 llvm::AsanDtorKind::Global)
> ```
> 
> and 
> 
> ```
> def sanitize_address_destructor_EQ
>     : Joined<["-"], "fsanitize-address-destructor=">,
>       MetaVarName<"<kind>">,
>       Flags<[CC1Option]>,
>       HelpText<"Set destructor type used in ASan instrumentation">,
>       Group<f_clang_Group>,
>       Values<"none,global">,
>       NormalizedValuesScope<"llvm::AsanDtorKind">,
>       NormalizedValues<["None", "Global"]>,
>       MarshallingInfoEnum<CodeGenOpts<"SanitizeAddressDtor">, "Global">;
> ```
> 
> And one more thing, probably not very important, and matter of personal preferences.
> Maybe we don't need MetaVarName<"<kind>"> for both of these flags.
> Without MetaVarName documentation is going to be -fsanitize-address-destructor=<arg> 
> MetaVarName just lets use to replace <arg> with more common and specific <file>, <dir>.. etc.
> To me <kind> is almost generic as <arg>, so why not to stick to default.
@vitalybuka I put up the following patches for review

https://reviews.llvm.org/D101490
https://reviews.llvm.org/D101491


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101122