steakhal added inline comments.
================ Comment at: clang/docs/analyzer/checkers.rst:69-74 +Check for dereferences of null pointers. This checker specifically does +not report null pointer dereferences for x86 and x86-64 targets when the +address space is 256 (x86 GS Segment), 257 (x86 FS Segment), or 258 (x86 SS +segment). See `X86/X86-64 Language Extensions +<https://clang.llvm.org/docs/LanguageExtensions.html#memory-references-to-specified-segments>`__ +for reference. ---------------- IMO this should be in a separate paragraph. It's such a niche use-case. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:119 + // and #258. Those address spaces are used when dereferencing address spaces + // relative to the GS, FS, and SS segments on x86/x86-64 targets. + // Dereferencing a null pointer in these address spaces is not defined ---------------- Shouldn't we check for this target? Add a test if we have `x86` and a non-`x86` target. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:125-133 + if (isTargetAddressSpace(E->getType().getAddressSpace())) { + switch (toTargetAddressSpace(E->getType().getAddressSpace())) { + case 256: + case 257: + case 258: + return true; + } ---------------- Even though `DetectAllNullDereferences=true`, `address_space(256)` would be suppressed, which seems to be a contradiction to me. We should either pick a different name or implement a different semantic. I would vote for both, personally. Then name should encode that it's purely about address-space attributes. The implementation should check if we have attributes or not, and bail out in the generic case. I would do something like this: ``` QualType Ty = E->getType(); if (!Ty.hasAddressSpace()) return false; if (SuppressAddressSpaces) return true; // On X86 addr spaces ...., thus ignored. return target == x86 && addressspace in {256,257,258}; ``` ================ Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:331 + auto *Chk = mgr.registerChecker<DereferenceChecker>(); + // auto *Chk = mgr.getChecker<DereferenceChecker>(); + Chk->DetectAllNullDereferences = ---------------- dead-code? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/ https://reviews.llvm.org/D122841 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
