This is an archive of the discontinued LLVM Phabricator instance.

[scan-build] fix warnings emitted on Clang Format code base
AcceptedPublic

Authored by apelete on Apr 21 2016, 12:32 PM.

Details

Reviewers
djasper
rsmith
Summary

Fix "Logic error" warnings of the type "Called C++ object pointer is
null" reported by Clang Static Analyzer on the following file:

  • lib/Format/AffectedRangeManager.cpp.

Signed-off-by: Apelete Seketeli <apelete@seketeli.net>

Diff Detail

Event Timeline

apelete updated this revision to Diff 54564.Apr 21 2016, 12:32 PM
apelete retitled this revision from to [scan-build] fix logic error warnings emitted on clang code base.
apelete added reviewers: rjmccall, zaks.anna, rsmith.
apelete updated this object.
apelete added a subscriber: cfe-commits.
rjmccall added inline comments.Apr 22 2016, 11:23 AM
lib/Frontend/CompilerInstance.cpp
763 ↗(On Diff #54564)

What's the justification for this one?

apelete added inline comments.Apr 22 2016, 12:30 PM
lib/Frontend/CompilerInstance.cpp
763 ↗(On Diff #54564)

CompilerInstance::InitializeSourceManager() could call CompilerInstance::InitializeSourceManager() and pass a null pointer value via the 5th parameter 'HS':

  1. bool CompilerInstance::InitializeSourceManager(const FrontendInputFile &Input){
  2. return InitializeSourceManager(
  3. Input, getDiagnostics(), getFileManager(), getSourceManager(),
  4. hasPreprocessor() ? &getPreprocessor().getHeaderSearchInfo() : nullptr,
  5. getDependencyOutputOpts(), getFrontendOpts());
  6. }

In that case, 'HS' object pointer would be null at this point.
I chose not to assert 'HS' and just check it because we already check if 'File' is assigned a null pointer as the result of HS->LookupFile() below.

Should it be fixed otherwise ?

rjmccall added inline comments.Apr 22 2016, 1:20 PM
lib/Frontend/CompilerInstance.cpp
763 ↗(On Diff #54564)

I don't really know.

The Sema and AST changes LGTM.

apelete updated this revision to Diff 55103.Apr 26 2016, 3:00 PM

[scan-build] fix logic error warnings emitted on clang code base

Chnages since last revision:

  • lib/Format/Format.cpp: removed all changes, redenderd obsolete by upstream.
apelete updated this revision to Diff 55327.Apr 27 2016, 2:37 PM

[scan-build] fix logic error warnings emitted on clang code base

Changes since last revision:

  • lib/Format/AffectedRangeManager.cpp: moved the fixes from lib/Format/Format.cpp:AffectedRangeManager::nonPPLineAffected() here since the code appaears to have been moved.
zaks.anna added inline comments.Apr 27 2016, 11:34 PM
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
300–301 ↗(On Diff #55327)

LGTM

klimek added inline comments.
lib/Format/AffectedRangeManager.cpp
103

Perhaps we should change this to take a Line& instead? Daniel, thoughts?

djasper added inline comments.Apr 28 2016, 7:46 AM
lib/Format/AffectedRangeManager.cpp
103

Yeah, that makes sense to me.

109

Here, we should probably just return false.

apelete updated this revision to Diff 55483.Apr 28 2016, 2:14 PM

[scan-build] fix logic error warnings emitted on clang code base

Changes since last revison:

  • lib/Format/AffectedRangeManager.h: fix declaration of nonPPLineAffected() to pass a reference to AnnotatedLine as first parameter.
  • lib/Format/AffectedRangeManager.cpp: fix nonPPLineAffected() definition accordingly.
apelete updated this revision to Diff 55487.Apr 28 2016, 2:21 PM
apelete marked 3 inline comments as done.

[scan-build] fix logic error warnings emitted on clang code base

Changes since last revision:

  • lib/Format/AffectedRangeManager.cpp: fix the call to AffectedRangeManager::nonPPLineAffected() from AffectedRangeManager::computeAffectedLines() that was left out of previous revision somehow.

Thanks for your reviews.

apelete updated this revision to Diff 55748.May 1 2016, 9:14 AM

[scan-build] fix logic error warnings emitted on clang code base

Changes since last revision:

  • fast forward rebase on git master branch.

Waiting for review, could someone please have a look at this one ?

apelete updated this revision to Diff 55757.May 1 2016, 1:03 PM

[scan-build] fix logic error warnings emitted on clang code base

Changes since last revision:

  • lib/Format/AffectedRangeManager.cpp: fix typo in class name.
apelete updated this revision to Diff 56259.May 5 2016, 3:40 AM

[scan-build] fix warnings emitted on Clang Format code base

Changes since last revision:

  • split patch into Format unit to ease review process.
apelete retitled this revision from [scan-build] fix logic error warnings emitted on clang code base to [scan-build] fix warnings emitted on Clang Format code base.May 5 2016, 3:43 AM
apelete updated this object.
apelete edited reviewers, added: ioeric; removed: zaks.anna, rjmccall.
ioeric removed a reviewer: ioeric.Sep 6 2016, 8:39 AM
djasper accepted this revision.Oct 4 2016, 11:32 PM
djasper edited edge metadata.

Looks good.

This revision is now accepted and ready to land.Oct 4 2016, 11:32 PM
shafik added a subscriber: shafik.Sep 21 2023, 8:35 AM

Is this relevant anymore or should it be closed?

Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2023, 8:35 AM

I guess it can be closed, thanks.