Page MenuHomePhabricator

[analyzer] Add analyzer option to limit the number of imported TUs
ClosedPublic

Authored by gamesh411 on Mar 25 2019, 1:42 PM.

Details

Summary

During CTU analysis of complex projects, the loaded AST-contents of
imported TUs can grow bigger than available system memory. This option
introduces a threshold on the number of TUs to be imported for a single
TU in order to prevent such cases.

Diff Detail

Repository
rL LLVM

Event Timeline

gamesh411 created this revision.Mar 25 2019, 1:42 PM
Herald added a project: Restricted Project. · View Herald Transcript
gamesh411 retitled this revision from Add analyzer option to limit the number of imported TUs to [WIP] Add analyzer option to limit the number of imported TUs.Mar 25 2019, 1:56 PM

Updated option handling location to use AnalyzerOptions instead of CC1

gamesh411 retitled this revision from [WIP] Add analyzer option to limit the number of imported TUs to Add analyzer option to limit the number of imported TUs.Mar 28 2019, 12:33 AM
gamesh411 retitled this revision from Add analyzer option to limit the number of imported TUs to [analyzer] Add analyzer option to limit the number of imported TUs.Mar 28 2019, 12:41 AM

@xazax.hun Could you please take a look?

Mostly looks good, I have two slightly related nits.

include/clang/CrossTU/CrossTranslationUnit.h
127 ↗(On Diff #192571)

I would prefer to keep the default argument for DisplayCTUProgress. You could do this either by swapping the arguments or adding a default argument to CTULoadThreshold as well. I think I might prefer the latter.

176 ↗(On Diff #192571)

Does it make sense to call getCrossTUDefinition with different thresholds for the same CTUContext? I suspect it is not, so it might make more sense to set the threshold in the context's constructor.

gamesh411 updated this revision to Diff 199277.May 13 2019, 9:17 AM

Apply review suggestions by Xazax

gamesh411 marked an inline comment as done.May 13 2019, 9:22 AM

I could greatly simplify the API of getCrossTUDefinition by injecting the threshold value in the constructor. A minor drawback with this approach is that testing code is a bit more complicated, as I had to patch the CompilerInstance to override the AnalyzerOption value for CTUImportThreshold. Thanks for the suggestion!

gamesh411 marked an inline comment as done.May 13 2019, 9:23 AM
gamesh411 updated this revision to Diff 199278.May 13 2019, 9:28 AM

Revert unnecessary clang-formating of AnalysisConsumer.cpp

gamesh411 updated this revision to Diff 207711.Jul 2 2019, 11:41 PM

Rebase onto current master

gamesh411 updated this revision to Diff 207712.Jul 3 2019, 12:20 AM

Rebase again

martong accepted this revision.Jul 3 2019, 3:03 AM
martong added inline comments.
include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
297 ↗(On Diff #207712)

burder -> burden ?

This revision is now accepted and ready to land.Jul 3 2019, 3:03 AM
Szelethus added inline comments.Jul 3 2019, 3:35 AM
test/Analysis/ctu-import-threshold.c
1–5 ↗(On Diff #207712)

There's no need for such validation, AnalyzerOptions' interface is very well guarded against misuse.

If you insist, prefer modifying clang/test/Analysis/invalid-analyzer-config-value.c.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2019, 5:37 AM