[llvm-dev] Coverity Scan Needs to be Updated after GitHub Migration

David Blaikie via llvm-dev
2021-06-10T18:54:08Z

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:
>
>> Do the reports have deltas? (highlighting new defects with as fine
>> revision granularity as possible) or do they only show the total set of
>> findings at a given revision?
>>
>
> Yes (see below sample email from today), which is why I'd prefer to
keep
> this daily rather than weekly.
>

Ah, cool - thanks!


> llvm-commits@ may be more suitable than llvm-dev@ for this?
>

Yeah, not sure. I don't feel too strongly either way.


>
>
> 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)?

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).

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).


>
>
>
>
________________________________________________________________________________________________________
> *** 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
>
>   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
>
>
>>
>> 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
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210610/c640a83d/attachment.html>