hctim marked 10 inline comments as done.
hctim added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2767-2781
+ if (NoSanitizeL.containsGlobal(LangOpts.Sanitize.Mask, GV->getName(),
Category))
return true;
- if (NoSanitizeL.containsLocation(EnabledAsanMask, Loc, Category))
+ if (NoSanitizeL.containsLocation(LangOpts.Sanitize.Mask, Loc, Category))
return true;
// Check global type.
if (!Ty.isNull()) {
// Drill down the array types: if global variable of a fixed type is
----------------
hctim wrote:
> vitalybuka wrote:
> > can this lines be landed separately?
> sure
(now we do touch a little bit here regardless)
================
Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:55
+
+ bool IsExcluded = CGM.isInNoSanitizeList(GV, Loc, Ty);
+ IsExcluded |= (NoSanitizeMask == SanitizerKind::All);
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > 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)
> you you can introduce:
>
> ```
> bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind,
> llvm::GlobalVariable *GV,
> SourceLocation Loc) const {
> ```
>
> similar to
>
> ```
> bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind, llvm::Function *Fn,
> SourceLocation Loc) const {
> ```
done, but bearing in mind if you have some global `src:` exclude in an
`-fsanitize-ignorelist` that's designed to ignore some file for UBSan, and then
you compile with `-fsanitize=address,undefined` and use that
`-fsanitize-ignorelist`, then those GVs would also be ignored in ASan. The
right way to go about that is to have the creator of the ignorelist make sure
that the `src:` rule is in a `[undefined]` block. Added the expected use case
to `sanitizer-special-case-list-globals.txt`.
i think it's small enough a change + relevant enough to this CL to not fork it
out to a different patch and then have to do the cleanup twice.
================
Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:64
bool IsDynInit) {
- if (!isAsanHwasanOrMemTag(CGM.getLangOpts().Sanitize))
- return;
----------------
vitalybuka wrote:
> May be early isAsanHwasanOrMemTag check here is useful to avoid string stuff
> below for compilation without sanitizers.
sure, also hoisted the other check up
================
Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:70
- auto getNoSanitizeMask = [](const VarDecl &D) {
- if (D.hasAttr<DisableSanitizerInstrumentationAttr>())
----------------
vitalybuka wrote:
> 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
reverted it back, lambda here seems very fancy for a simple farmer like me, but
i can't deny that it's pretty.
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
