kstoimenov added inline comments.
================
Comment at: clang/lib/CodeGen/SanitizerMetadataFactory.h:33
+
+class SanitizerMetadataFactory {
+ SanitizerMetadataFactory(const SanitizerMetadataFactory &) = delete;
----------------
Not sure if this class follows the 'factory' design pattern, which typically it
would have some sort of 'produce' or 'create' method. Maybe
SanitizerMatadataWriter or SanitizerMatadataCreator?
================
Comment at: clang/lib/CodeGen/SanitizerMetadataFactory.h:46-48
+ void setASanSpecificMetadata(llvm::GlobalVariable::SanitizerMetadata &Meta,
+ llvm::GlobalVariable *GV, SourceLocation Loc,
+ QualType Ty, bool IsDynInit = false);
----------------
This could be private I think.
================
Comment at: compiler-rt/lib/asan/asan_globals.cpp:90
+
+ DataInfo info;
+ Symbolizer::GetOrInit()->SymbolizeData(g.beg, &info);
----------------
Outside of the scope of this change. It looks like DataInfo's constructor is
calling memset on it's fields. I would have probably designed this differently
because now it looks like potential uninitialized when we check for 'info.line
!= 0'.
================
Comment at: llvm/include/llvm/AsmParser/LLToken.h:205
kw_nest,
+ kw_no_sanitize,
+ kw_no_sanitize_address,
----------------
Why do we need to add these to lex? I am surprised that didn't have the need so
far.
================
Comment at: llvm/include/llvm/IR/GlobalValue.h:312
+ void RemoveSanitizer(GlobalSanitizer S) { Sanitizer &= ~S; }
+ bool HasSanitizer(GlobalSanitizer S) const {
+ if (Sanitizer == GlobalSanitizer::NoSanitize)
----------------
I suspect that there might be another place where that logic exists. I am
worried that we are duplicating it. Do you might looking for it in the code
base?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124493/new/
https://reviews.llvm.org/D124493
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
