[llvm-dev] Coverity Scan Needs to be Updated after GitHub Migration
David Blaikie via llvm-dev
llvm-dev at lists.llvm.org
Fri Jun 11 11:40:48 PDT 2021
On Fri, Jun 11, 2021 at 8:15 AM Tom Honermann <Thomas.Honermann at synopsys.com>
wrote:
> On 6/11/2021 9:58 AM, Tom Honermann via llvm-dev wrote:
>
> On 6/10/2021 2:54 PM, David Blaikie via llvm-dev wrote:
>
> On Thu, Jun 10, 2021 at 10:42 AM Mehdi AMINI <joker.eph at gmail.com> wrote:
>
>>
>>
>>
>> On Thu, Jun 10, 2021 at 10:20 AM David Blaikie via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>
>> From Coverity:
>>
>> Please find the latest report on new defect(s) introduced to llvm found
>> with Coverity Scan.
>>
>> 8 new defect(s) introduced to llvm found with Coverity Scan.
>> 19 defect(s), reported by Coverity Scan earlier, were marked fixed in the
>> recent build analyzed by Coverity Scan.
>>
>> New defect(s) Reported-by: Coverity Scan
>> Showing 8 of 8 defect(s)
>>
>>
>> ** CID 1457502: Uninitialized members (UNINIT_CTOR)
>> /lld/MachO/InputSection.h: 109 in
>> lld::macho::StringPiece::StringPiece(unsigned long, unsigned int)()
>>
>
> Is it easy for us to disable low-value findings (both on a per-instance,
> but also per-check-tye) basis in source (ie: without having to modify an
> external config)?
>
> Source annotations are available for suppressing per-instance issues, but
> no source annotations are available to disable checkers entirely.
>
> I don't see an option for disabling specific checkers in Coverity Scan.
> Checker enable/disable and tuning options are available when using a
> Coverity installation. I don't know why those capabilities wouldn't be
> exposed to Coverity Scan users.
>
> I confirmed with internal Coverity support that such options are not
> currently exposed to Coverity Scan users. Enhancement requests sent to
> scan-admin at coverity.com will be considered for future Coverity Scan
> updates (I encourage sending an enhancement request).
>
Having not setup the scanning/emails/etc I don't think I have enough
context (I'm not exactly a "user" - at best I've read one email produced by
the tool) to make a feature request.
>
> For instance, I'm not sure it's valuable for us to get notification on any
> member not initialized by a ctor. That could readily be detected by
> clang-tidy or clang warnings and we don't implement such checks in those
> places (which would be higher value because they can find the issue sooner
> rather than waiting for a long-running static analysis to come back with
> results).
>
> I'm going to push back on this a bit. I've had to debug problems in Clang
> that turned out to be due to failure to initialize a data member. I find
> the number of uninitialized data member issues that Coverity reports on
> LLVM to be out of line with respect to other projects I've seen scanned.
> I'm skeptical that omitting initializers has a significant impact on
> performance.
>
The tradeoff is that initializing values that aren't meant to be used
reduces msan's ability to identify bugs if such a value is really used.
> Coverity doesn't report every data member that isn't initialized by a
> member init; it looks for evidence that the data member is not initialized
> by any functions invoked by the constructor as well (whether in the same TU
> or not). It will therefore report fewer FPs than either clang-tidy or
> clang warnings.
>
> Tom.
>
>
> Keeping the warnings low-noise would be really important (so whoever set
> this up or requested it I hope is really pushing to reduce the noise until
> nearly all results have pretty broad agreement that they should be fixed).
>
> Yes, the general deployment recommendation is to tune to minimize FPs and
> low value results and then relax such tuning as issues are addressed.
>
> Tom.
>
>
>
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1457502: Uninitialized members (UNINIT_CTOR)
>> /lld/MachO/InputSection.h: 109 in
>> lld::macho::StringPiece::StringPiece(unsigned long, unsigned int)()
>> 103 // Offset from the start of the containing input section.
>> 104 uint32_t inSecOff;
>> 105 uint32_t hash;
>> 106 // Offset from the start of the containing output section.
>> 107 uint64_t outSecOff;
>> 108
>> >>> CID 1457502: Uninitialized members (UNINIT_CTOR)
>> >>> Non-static class member "outSecOff" is not initialized in this
>> constructor nor in any functions that it calls.
>> 109 StringPiece(uint64_t off, uint32_t hash) : inSecOff(off),
>> hash(hash) {}
>> 110 };
>> 111
>> 112 // CStringInputSections are composed of multiple null-terminated
>> string
>> 113 // literals, which we represent using StringPieces. These
>> literals can be
>> 114 // deduplicated and tail-merged, so translating offsets between
>> the input and
>>
>> ** CID 1457501: Uninitialized members (UNINIT_CTOR)
>> /llvm/lib/ObjectYAML/XCOFFEmitter.cpp: 36 in
>> <unnamed>::XCOFFWriter::XCOFFWriter(llvm::XCOFFYAML::Object &,
>> llvm::raw_ostream &, llvm::function_ref<void (const llvm::Twine &)>)()
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1457501: Uninitialized members (UNINIT_CTOR)
>> /llvm/lib/ObjectYAML/XCOFFEmitter.cpp: 36 in
>> <unnamed>::XCOFFWriter::XCOFFWriter(llvm::XCOFFYAML::Object &,
>> llvm::raw_ostream &, llvm::function_ref<void (const llvm::Twine &)>)()
>> 30
>> 31 class XCOFFWriter {
>> 32 public:
>> 33 XCOFFWriter(XCOFFYAML::Object &Obj, raw_ostream &OS,
>> yaml::ErrorHandler EH)
>> 34 : Obj(Obj), W(OS, support::big), ErrHandler(EH) {
>> 35 Is64Bit = Obj.Header.Magic ==
>> (llvm::yaml::Hex16)XCOFF::XCOFF64;
>> >>> CID 1457501: Uninitialized members (UNINIT_CTOR)
>> >>> Non-static class member "StartOffset" is not initialized in this
>> constructor nor in any functions that it calls.
>> 36 }
>> 37 bool writeXCOFF();
>> 38
>> 39 private:
>> 40 bool initFileHeader(uint64_t CurrentOffset);
>> 41 bool initSectionHeader(uint64_t &CurrentOffset);
>>
>> ** CID 1457500: Incorrect expression (SIZEOF_MISMATCH)
>> /compiler-rt/lib/dfsan/dfsan_custom.cpp: 2367 in format_buffer(char *,
>> unsigned long, const char *, unsigned char *, unsigned char *, unsigned int
>> *, unsigned int *, __va_list_tag *)()
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1457500: Incorrect expression (SIZEOF_MISMATCH)
>> /compiler-rt/lib/dfsan/dfsan_custom.cpp: 2367 in format_buffer(char *,
>> unsigned long, const char *, unsigned char *, unsigned char *, unsigned int
>> *, unsigned int *, __va_list_tag *)()
>> 2361 case 'n': {
>> 2362 int *ptr = va_arg(ap, int *);
>> 2363 *ptr = (int)formatter.str_off;
>> 2364 va_labels++;
>> 2365 if (va_origins)
>> 2366 va_origins++;
>> >>> CID 1457500: Incorrect expression (SIZEOF_MISMATCH)
>> >>> Passing argument "ptr" of type "int *" and argument "8UL /*
>> sizeof (ptr) */" to function "dfsan_set_label" is suspicious.
>> 2367 dfsan_set_label(0, ptr, sizeof(ptr));
>>
>
> I think clang has a sizeof warning for things like memcpy, right? I wonder
> if this more broad warning provides a lot of value, or not?
>
>
>> 2368 end_fmt = true;
>> 2369 break;
>> 2370 }
>> 2371
>> 2372 case '%':
>>
>> ** CID 1457499: Incorrect expression (DIVIDE_BY_ZERO)
>> /llvm/lib/Analysis/CFGPrinter.cpp: 308 in
>> llvm::DOTGraphTraits<llvm::DOTFuncInfo *>::isNodeHidden(const
>> llvm::BasicBlock *, const llvm::DOTFuncInfo *)()
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1457499: Incorrect expression (DIVIDE_BY_ZERO)
>> /llvm/lib/Analysis/CFGPrinter.cpp: 308 in
>> llvm::DOTGraphTraits<llvm::DOTFuncInfo *>::isNodeHidden(const
>> llvm::BasicBlock *, const llvm::DOTFuncInfo *)()
>> 302 const
>> DOTFuncInfo *CFGInfo) {
>> 303 if (HideColdPaths.getNumOccurrences() > 0)
>> 304 if (auto *BFI = CFGInfo->getBFI()) {
>> 305 uint64_t NodeFreq = BFI->getBlockFreq(Node).getFrequency();
>> 306 uint64_t EntryFreq = BFI->getEntryFreq();
>> 307 // Hide blocks with relative frequency below HideColdPaths
>> threshold.
>> >>> CID 1457499: Incorrect expression (DIVIDE_BY_ZERO)
>> >>> In expression "(double)NodeFreq / EntryFreq", division by
>> expression "EntryFreq" which may be zero has undefined behavior.
>> 308 if ((double)NodeFreq / EntryFreq < HideColdPaths)
>> 309 return true;
>> 310 }
>> 311 if (HideUnreachablePaths || HideDeoptimizePaths) {
>> 312 if (isOnDeoptOrUnreachablePath.find(Node) ==
>> 313 isOnDeoptOrUnreachablePath.end())
>>
>> ** CID 1457498: (DEADCODE)
>> /clang/lib/Sema/SemaDeclCXX.cpp: 11843 in
>> clang::Sema::CheckUsingShadowDecl(clang::BaseUsingDecl *, clang::NamedDecl
>> *, const clang::LookupResult &, clang::UsingShadowDecl *&)()
>> /clang/lib/Sema/SemaDeclCXX.cpp: 11789 in
>> clang::Sema::CheckUsingShadowDecl(clang::BaseUsingDecl *, clang::NamedDecl
>> *, const clang::LookupResult &, clang::UsingShadowDecl *&)()
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1457498: (DEADCODE)
>> /clang/lib/Sema/SemaDeclCXX.cpp: 11843 in
>> clang::Sema::CheckUsingShadowDecl(clang::BaseUsingDecl *, clang::NamedDecl
>> *, const clang::LookupResult &, clang::UsingShadowDecl *&)()
>> 11837 return true;
>> 11838 }
>> 11839
>> 11840 // No conflict between a tag and a non-tag.
>> 11841 if (!NonTag) return false;
>> 11842
>> >>> CID 1457498: (DEADCODE)
>> >>> Execution cannot reach this statement: "<temporary> =
>> this->Diag(cl...".
>> 11843 Diag(BUD->getLocation(), diag::err_using_decl_conflict);
>> 11844 Diag(Target->getLocation(), diag::note_using_decl_target);
>> 11845 Diag(NonTag->getLocation(), diag::note_using_decl_conflict);
>> 11846 BUD->setInvalidDecl();
>> 11847 return true;
>> 11848 }
>> /clang/lib/Sema/SemaDeclCXX.cpp: 11789 in
>> clang::Sema::CheckUsingShadowDecl(clang::BaseUsingDecl *, clang::NamedDecl
>> *, const clang::LookupResult &, clang::UsingShadowDecl *&)()
>> 11783 // Always emit a diagnostic for a mismatch between an
>> unresolved
>> 11784 // using_if_exists and a resolved using declaration in either
>> direction.
>> 11785 if (isa<UnresolvedUsingIfExistsDecl>(Target) !=
>> 11786 (isa_and_nonnull<UnresolvedUsingIfExistsDecl>(NonTag))) {
>> 11787 if (!NonTag && !Tag)
>> 11788 return false;
>> >>> CID 1457498: (DEADCODE)
>> >>> Execution cannot reach this statement: "<temporary> =
>> this->Diag(cl...".
>> 11789 Diag(BUD->getLocation(), diag::err_using_decl_conflict);
>> 11790 Diag(Target->getLocation(), diag::note_using_decl_target);
>> 11791 Diag((NonTag ? NonTag : Tag)->getLocation(),
>> 11792 diag::note_using_decl_conflict);
>> 11793 BUD->setInvalidDecl();
>> 11794 return true;
>>
>> ** CID 1457497: Integer handling issues (NEGATIVE_RETURNS)
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1457497: Integer handling issues (NEGATIVE_RETURNS)
>> /lld/MachO/InputSection.cpp: 117 in
>> lld::macho::CStringInputSection::getStringPiece(unsigned long) const()
>> 111 const StringPiece &CStringInputSection::getStringPiece(uint64_t
>> off) const {
>> 112 if (off >= data.size())
>> 113 fatal(toString(this) + ": offset is outside the section");
>> 114
>> 115 auto it =
>> 116 partition_point(pieces, [=](StringPiece p) { return
>> p.inSecOff <= off; });
>> >>> CID 1457497: Integer handling issues (NEGATIVE_RETURNS)
>> >>> A negative constant "-1L" is passed as an argument to a parameter
>> that cannot be negative.
>>
>
> Doesn't sound correct - negatively indexing from an iterator is valid, I
> believe? (though perhaps this check is using some info about the nature of
> `partition_point` being able to return the begin iterator)
>
>
>> 117 return it[-1];
>> 118 }
>> 119
>> 120 uint64_t CStringInputSection::getFileOffset(uint64_t off) const {
>> 121 return parent->fileOff + getOffset(off);
>> 122 }
>>
>> ** CID 1457496: Possible Control flow issues (DEADCODE)
>> /clang/lib/Sema/SemaDeclCXX.cpp: 11833 in
>> clang::Sema::CheckUsingShadowDecl(clang::BaseUsingDecl *, clang::NamedDecl
>> *, const clang::LookupResult &, clang::UsingShadowDecl *&)()
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1457496: Possible Control flow issues (DEADCODE)
>> /clang/lib/Sema/SemaDeclCXX.cpp: 11833 in
>> clang::Sema::CheckUsingShadowDecl(clang::BaseUsingDecl *, clang::NamedDecl
>> *, const clang::LookupResult &, clang::UsingShadowDecl *&)()
>> 11827 // Target is not a function.
>> 11828
>> 11829 if (isa<TagDecl>(Target)) {
>> 11830 // No conflict between a tag and a non-tag.
>> 11831 if (!Tag) return false;
>> 11832
>> >>> CID 1457496: Possible Control flow issues (DEADCODE)
>> >>> Execution cannot reach this statement: "<temporary> =
>> this->Diag(cl...".
>> 11833 Diag(BUD->getLocation(), diag::err_using_decl_conflict);
>> 11834 Diag(Target->getLocation(), diag::note_using_decl_target);
>> 11835 Diag(Tag->getLocation(), diag::note_using_decl_conflict);
>> 11836 BUD->setInvalidDecl();
>> 11837 return true;
>> 11838 }
>>
>> ** CID 1419078: Resource leaks (VIRTUAL_DTOR)
>> /llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp: 541 in ()
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1419078: Resource leaks (VIRTUAL_DTOR)
>> /llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp: 541 in ()
>> 535 return (Twine(Removed ? "-" : " ") + (InMST ? " " : "*") +
>> 536 (IsCritical ? "c" : " ") + " W=" +
>> Twine(Weight)).str();
>> 537 }
>> 538 };
>> 539
>> 540 // This class stores the auxiliary information for each BB.
>> >>> CID 1419078: Resource leaks (VIRTUAL_DTOR)
>> >>> Class "<unnamed>::BBInfo" does not have a virtual destructor.
>> 541 struct BBInfo {
>> 542 BBInfo *Group;
>> 543 uint32_t Index;
>> 544 uint32_t Rank = 0;
>> 545
>> 546 BBInfo(unsigned IX) : Group(this), Index(IX) {}
>>
>>
>>
>> ________________________________________________________________________________________________________
>> To view the defects in Coverity Scan visit,
>> https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yqtGMuad6pPsL7inW23sAqZCWZD0rQ5FZsyk18zSjnBpg-3D-3Dqen7_nlj59xHPRAo5NMSpMZh-2B1UYnQ4IBJNE2FCxtFGv5-2FfRZ2ZNTfin-2BJg3vqHM-2BrKWO-2BwAgQVf1GGFXh4xVX9UzXjq3jPiI59xzjIzanRxv0XSIVZFqDyJd-2BdQm4cXqcdS7Dt0L1fNQpWyw15e-2BbU4YMt1YpKZVa4kbM7Bjl6hGDatG6tnUuz5zUAdmlq-2B7z2QmYSc2DTghseWofWoq-2Bn7ssA-3D-3D
>> <https://urldefense.com/v3/__https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yqtGMuad6pPsL7inW23sAqZCWZD0rQ5FZsyk18zSjnBpg-3D-3Dqen7_nlj59xHPRAo5NMSpMZh-2B1UYnQ4IBJNE2FCxtFGv5-2FfRZ2ZNTfin-2BJg3vqHM-2BrKWO-2BwAgQVf1GGFXh4xVX9UzXjq3jPiI59xzjIzanRxv0XSIVZFqDyJd-2BdQm4cXqcdS7Dt0L1fNQpWyw15e-2BbU4YMt1YpKZVa4kbM7Bjl6hGDatG6tnUuz5zUAdmlq-2B7z2QmYSc2DTghseWofWoq-2Bn7ssA-3D-3D__;!!A4F2R9G_pg!PQwy8rAjiKMI_cBfDwnfPHyGD0k6TmR-QczpyNJJYgHISD-Qk1-6wPLjy535GVbE$>
>>
>> To manage Coverity Scan email notifications for "joker.eph at gmail.com",
>> click
>> https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxUrjVeIJ0Cfeziujhnhh3yxzc7w9MgExjQKEssnVrR9tYRoPYlaXXfdUjwRQLJCdFixsrT7mUhUA9ixc9DPUdquU2MMNgdrF247xaBicB0V4-3Dht-M_nlj59xHPRAo5NMSpMZh-2B1UYnQ4IBJNE2FCxtFGv5-2FfRZ2ZNTfin-2BJg3vqHM-2BrKWOP6G0-2FiydipnYxIKl-2Bk7AFFr9CcUjQXx7tq4qtWbpBdzz7-2Bib4DWeMAf-2Bv0hl9c0qiwYnDVi4C3uD0F0P9wRuATKCNq-2FJGcnjmQ51zJUZdom9QcwJ2QwYMBBjOk8G2ylW4oH3PujCpojyn5dsLN7qdQ-3D-3D
>> <https://urldefense.com/v3/__https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXxUrjVeIJ0Cfeziujhnhh3yxzc7w9MgExjQKEssnVrR9tYRoPYlaXXfdUjwRQLJCdFixsrT7mUhUA9ixc9DPUdquU2MMNgdrF247xaBicB0V4-3Dht-M_nlj59xHPRAo5NMSpMZh-2B1UYnQ4IBJNE2FCxtFGv5-2FfRZ2ZNTfin-2BJg3vqHM-2BrKWOP6G0-2FiydipnYxIKl-2Bk7AFFr9CcUjQXx7tq4qtWbpBdzz7-2Bib4DWeMAf-2Bv0hl9c0qiwYnDVi4C3uD0F0P9wRuATKCNq-2FJGcnjmQ51zJUZdom9QcwJ2QwYMBBjOk8G2ylW4oH3PujCpojyn5dsLN7qdQ-3D-3D__;!!A4F2R9G_pg!PQwy8rAjiKMI_cBfDwnfPHyGD0k6TmR-QczpyNJJYgHISD-Qk1-6wPLjy8RFYZYc$>
>>
>>
>>>
>>> On Thu, Jun 10, 2021 at 7:37 AM Luke Benes via llvm-dev <
>>> llvm-dev at lists.llvm.org> wrote:
>>>
>>>> > https://scan.coverity.com/projects/llvm
>>>> > Should run once a day.
>>>>
>>>> Sylvester,
>>>> The report seems to be working perfectly. Thank you for taking the time
>>>> to get this up and running again!
>>>>
>>>> My only concern is that there is no visibly on these reports. Without
>>>> the new issues being reported here, it is highly unlikely that they will
>>>> get addressed.
>>>>
>>>> Since there was interest and no objections, could you please add the
>>>> [llvm-dev] list to the email?
>>>>
>>>> You can do this by going to the "Project Settings" page:
>>>> https://scan.coverity.com/projects/llvm?tab=project_settings
>>>>
>>>> "Additional Emails for New Defect Notifications"
>>>> -> llvm-dev at lists.llvm.org
>>>>
>>>> Then could you please lower the report frequency to once or twice a
>>>> week? With that we will receive weekly reports like this:
>>>>
>>>>
>>>> http://document-foundation-mail-archive.969070.n3.nabble.com/New-Defects-reported-by-Coverity-Scan-for-LibreOffice-td4301203.html
>>>> <https://urldefense.com/v3/__http://document-foundation-mail-archive.969070.n3.nabble.com/New-Defects-reported-by-Coverity-Scan-for-LibreOffice-td4301203.html__;!!A4F2R9G_pg!PQwy8rAjiKMI_cBfDwnfPHyGD0k6TmR-QczpyNJJYgHISD-Qk1-6wPLjy7OhpJPm$>
>>>>
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>> <https://urldefense.com/v3/__https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!A4F2R9G_pg!PQwy8rAjiKMI_cBfDwnfPHyGD0k6TmR-QczpyNJJYgHISD-Qk1-6wPLjy7Im7VZf$>
>>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>> <https://urldefense.com/v3/__https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!A4F2R9G_pg!PQwy8rAjiKMI_cBfDwnfPHyGD0k6TmR-QczpyNJJYgHISD-Qk1-6wPLjy7Im7VZf$>
>>>
>>
> _______________________________________________
> LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://urldefense.com/v3/__https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!A4F2R9G_pg!PQwy8rAjiKMI_cBfDwnfPHyGD0k6TmR-QczpyNJJYgHISD-Qk1-6wPLjy7Im7VZf$
>
>
> _______________________________________________
> LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://urldefense.com/v3/__https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!A4F2R9G_pg!OqS--6HaZxcTx5Dt_tzjIpH37pYd3W0dBZqRxguX9IqfSIEm0LR2kmIPuSCtBYp1$
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210611/3bbe57b1/attachment.html>
More information about the llvm-dev
mailing list