vitalybuka added a subscriber: delcypher.
vitalybuka added a comment.
In D101122#2711962 <https://reviews.llvm.org/D101122#2711962>, @joerg wrote:
> There are two approaches that work for reviewing: starting from clang and
> going down or starting from compiler-rt and going up the layers. I'd prefer
> to do the latter as it means meaningful testing can be done easier. There are
> two natural steps here: clang flag+IR generation is one, llvm codegen and
> compiler-rt is the other. The clang step should include tests that ensure
> that proper IR is generated for the different flag combination (including
> documentation for the IR changes). The LLVM step should ensure that the
> different attributes (either function or module scope) are correctly lowered
> in the instrumentation pass and unit tests on the compiler-rt side to do
> functional testing. The unit testing might also be done as a third step if it
> goes the full way from C to binary code.
Today on chat I advised to start with clang, but now I agree with Joerg. More
precisely you can go with following 4 patches:
1. LLVM Detailed IR tests of effect of this flag on transformation
2. compiler-rt (tests will have to uses -mllvm -<internal asan copt>)
3. clang
- driver tests
- basic IR tests to see that flag makes a difference of generated code. see
llvm-project/clang/test/CodeGen/asan-*.cpp
4. compiler-rt: replace -mllvm copt from patch2 with with clang flag from
patch3.
================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:225
///< destructors are emitted.
+ENUM_CODEGENOPT(SanitizeAddressDetectStackUseAfterReturnMode,
+ llvm::AsanDetectStackUseAfterReturnMode, 2,
----------------
Mode->Kind to be consistent with the file
================
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:
> 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.
================
Comment at: clang/include/clang/Driver/Options.td:1555
+ NormalizedValuesScope<"llvm::AsanDetectStackUseAfterReturnMode">,
+ NormalizedValues<["Always", "Runtime", "Never"]>,
+
MarshallingInfoEnum<CodeGenOpts<"SanitizeAddressDetectStackUseAfterReturnMode">,
"Runtime">;
----------------
same order as enum
================
Comment at:
llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h:29
+ ///< (ASAN_OPTIONS=detect_stack_use_after_return=1)
+ Never, ///< Never detect stack use after return.
+ Invalid, ///< Not a valid detect mode.
----------------
Could you order them in acceding order by "strictness", this will make Never ==
0, and the current default == 1.
Never = 0, (maybe None for consistency?)
Runtime=1,
Always=2,
Invalid=<whatever>
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101122/new/
https://reviews.llvm.org/D101122
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
