vitalybuka added a comment. No need to recover my snapshot, I'll just comment here.
================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:20 +using GlobalVariable = llvm::GlobalVariable; +using GVSanitizerMetadata = GlobalVariable::SanitizerMetadata; ---------------- Seems inconsistent with the rest. it no so frequently use here to justify indirection. ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:55 + + bool IsExcluded = CGM.isInNoSanitizeList(GV, Loc, Ty); + IsExcluded |= (NoSanitizeMask == SanitizerKind::All); ---------------- it can be in some weird ubsan check ignore list, and this way it will propagate on asan/hwasan I don't think you can avoid extending isInNoSanitizeList (in a separate patch) ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:60 + SanitizerKind::KernelAddress)) { + IsExcluded |= CGM.isInNoSanitizeList(GV, Loc, Ty, "address"); + IsExcluded |= NoSanitizeSet.hasOneOf(SanitizerKind::Address | ---------------- the last arg is not sanitizer, just a particulate check of it like "init" here llvm-project/compiler-rt/lib/asan/asan_ignorelist.txt ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:64 bool IsDynInit) { - if (!isAsanHwasanOrMemTag(CGM.getLangOpts().Sanitize)) - return; ---------------- May be early isAsanHwasanOrMemTag check here is useful to avoid string stuff below for compilation without sanitizers. ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:70 - auto getNoSanitizeMask = [](const VarDecl &D) { - if (D.hasAttr<DisableSanitizerInstrumentationAttr>()) ---------------- I don't insist but one it's cleaner with lambda and return if you prefer your way please revert lambda in a separate patch ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:91-93 + llvm::LLVMContext &VMContext = CGM.getLLVMContext(); llvm::Metadata *LocDescr = nullptr; llvm::Metadata *GlobalName = nullptr; ---------------- undo this line move ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:118 + SanitizerMask NoSanitizeMask; + for (auto *Attr : D.specific_attrs<NoSanitizeAttr>()) { + NoSanitizeMask |= Attr->getMask(); ---------------- don't use {} for one liners ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:122 + + if (D.hasAttr<DisableSanitizerInstrumentationAttr>()) { + NoSanitizeMask = SanitizerKind::All; ---------------- this if probably should go before the loop ================ Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:126 + std::string QualName; llvm::raw_string_ostream OS(QualName); ---------------- moving QualName calculation is unnecessary, please move it back before NoSanitizeMask Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126929/new/ https://reviews.llvm.org/D126929 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits