NoQ added a comment.
Aha, cool, so it's probably just virtual functions.
Repository:
rC Clang
https://reviews.llvm.org/D30691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
martong added a comment.
Just added a new entry to our roadmap:
https://github.com/Ericsson/clang/issues/435
Repository:
rC Clang
https://reviews.llvm.org/D30691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-b
martong added a comment.
@NoQ , @dkrupp
CallEvent::getRuntimeDefinition is overwritten in
- AnyFunctionCall
- CXXInstanceCall
- CXXMemberCall
- CXXDestructorCall
- ObjCMethodCall
AnyFunctionCall handles the CTU logic.
CXXInstanceCall calls into AnyFunctionCall if the function is not virtual.
If
dkrupp added a comment.
> Which means that for some calls we aren't even trying to make a CTU lookup.
Thanks @NoQ, we will take a look at it!
Repository:
rC Clang
https://reviews.llvm.org/D30691
___
cfe-commits mailing list
cfe-commits@lists.llv
NoQ added a comment.
Herald added a subscriber: mikhail.ramalho.
Just noticed: `getRuntimeDefinition()` has a lot of overrides in `CallEvent`
sub-classes, and there paths that don't defer to
`AnyFunctionCall::getRuntimeDefinition()`, eg., `
CXXInstanceCall::getRuntimeDefinition()` => `if (MD->i
xazax.hun abandoned this revision.
xazax.hun added a comment.
Resubmitted in https://reviews.llvm.org/rL326439
Phabricator did not display the mailing list conversation. The point is, the
circular dependency does not exist in the upstream version of clang. The reason
is that CMake does not trac
Resubmitted as r326439. Sorry for all the trouble.
We need to hack around the Analyses.def being required by Frontend, but it
would nice to remove this dependency upstream.
On Thu, Mar 1, 2018 at 3:34 PM Benjamin Kramer wrote:
> Frontend depends on StaticAnalyzerCore by
> including "clang/Stat
Frontend depends on StaticAnalyzerCore by
including "clang/StaticAnalyzer/Core/Analyses.def". That's a clear layering
violation, but cmake doesn't model header dependencies. Maybe Analyses.def
should move into its own library.
On Thu, Mar 1, 2018 at 3:07 PM Gábor Horváth via llvm-commits <
llvm-c
I am away from my workstation so I would really appreciate if you could
recommit.
Thanks in advance,
Gábor
2018. márc. 1. 15:28 ezt írta ("Ilya Biryukov" ):
> You're right. We have this extra dependency in our internal build files
> for some reason and I missed that.
> It's totally my fault.
>
>
You're right. We have this extra dependency in our internal build files for
some reason and I missed that.
It's totally my fault.
Should I resubmit the patch that I reverted or you would rather do it
yourself?
On Thu, Mar 1, 2018 at 3:07 PM Gábor Horváth wrote:
>
>
> 2018. márc. 1. 14:58 ezt
2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):
I replied to a commit in the wrong thread (https://reviews.llvm.org/rL326323),
sorry.
Here are the important bits:
This change introduced the following cyclic dependency in the build system:
StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnaly
I replied to a commit in the wrong thread (https://reviews.llvm.org/rL326323),
sorry.
Here are the important bits:
This change introduced the following cyclic dependency in the build system:
StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.
I'm sorry, but I had to revert the commit
xazax.hun added a reviewer: ilya-biryukov.
xazax.hun added a subscriber: ilya-biryukov.
xazax.hun added a comment.
@ilya-biryukov
Could you please provide some more details where the cyclic dependency is? I
cannot reproduce the problem and usually cmake fails when there is a cyclic
dependency a
a.sidorin reopened this revision.
a.sidorin added a comment.
The changes were reverted:
http://llvm.org/viewvc/llvm-project?rev=326432&view=rev
Gabor, could you take a look?
Repository:
rC Clang
https://reviews.llvm.org/D30691
___
cfe-commits ma
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326323: [analyzer] Support for naive cross translation unit
analysis (authored by xazax, committed by ).
Herald added a su
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC326323: [analyzer] Support for naive cross translation unit
analysis (authored by xazax, committed by ).
Changed prior to
dcoughlin accepted this revision.
dcoughlin added a comment.
Thanks Gabor, this looks good to me. Please commit!
https://reviews.llvm.org/D30691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#1003514, @george.karpenkov wrote:
> Python code looks OK to me, I have one last request: could we have a small
> documentation how the whole thing is supposed work in integration, preferably
> on an available open-source project any
xazax.hun updated this revision to Diff 134431.
xazax.hun added a comment.
- Rebased to current ToT
- Fixed a problem that the scan-build-py used an old version of the ctu
configuration option
- Added a short guide how to use CTU
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAna
gerazo added inline comments.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:702
+
# To have good results from static analyzer certain compiler options shall be
george.karpenkov wrote:
> This blank line should not be in this PR.
Scheduled to be done.
george.karpenkov added a comment.
Python code looks OK to me, I have one last request: could we have a small
documentation how the whole thing is supposed work in integration, preferably
on an available open-source project any reader could check out?
I am asking because I have actually tried and
nikhgupt added inline comments.
Herald added a subscriber: hintonda.
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:395
+return XFE && !YFE;
+ return XFE->getName() < YFE->getName();
+}
getName could yield incorrect results if two files in the projec
xazax.hun updated this revision to Diff 129509.
xazax.hun marked 5 inline comments as done.
xazax.hun added a comment.
- Fixed review comments
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngin
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:418-423
SourceLocation XDL = XD->getLocation();
SourceLocation YDL = YD->getLocation();
if (XDL != YDL) {
const SourceManager &SM = XL.getManager();
- return SM.isBe
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:418-423
SourceLocation XDL = XD->getLocation();
SourceLocation YDL = YD->getLocation();
if (XDL != YDL) {
const SourceManager &SM = XL.getManager();
- return SM.isBeforeIn
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:418-423
SourceLocation XDL = XD->getLocation();
SourceLocation YDL = YD->getLocation();
if (XDL != YDL) {
const SourceManager &SM = XL.getManager();
- return SM.isBe
NoQ added a comment.
Had a fresh look on the C++ part, it is super clean now, i'm very impressed :)
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:373-374
- return RuntimeDefinition();
+ auto Engine = static_cast(
+ getState()->getStateManager().getOwningEngine());
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:407
+ if (!NaiveCTU.hasValue()) {
+NaiveCTU = getBooleanOption("experimental-enable-naive-ctu-analysis",
+/*Default=*/false);
This option
xazax.hun updated this revision to Diff 127525.
xazax.hun marked an inline comment as not done.
xazax.hun added a comment.
- Address some review comments
- Rebased on ToT
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/
xazax.hun marked 6 inline comments as done.
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:372
+
+ cross_tu::CrossTranslationUnitContext &CTUCtx =
+ Engine->getCrossTranslationUnitContext();
dcoughlin wrote:
> Can this lo
gerazo added a comment.
In https://reviews.llvm.org/D30691#954740, @george.karpenkov wrote:
> I've tried using the patch, and I got blocked at the following: CTU options
> are only exposed when one goes through `analyze-build` frontend, which
> requires `compile_commands.json` to be present. I'
dcoughlin added a comment.
Some comments on the C++ inline.
Comment at: include/clang/AST/ASTContext.h:82
class APValue;
+class ASTImporter;
class ASTMutationListener;
Is this forward declaration needed?
Comment at: include/clang/StaticAnal
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
I've tried using the patch, and I got blocked at the following: CTU options are
only exposed when one goes through `analyze-build` frontend, which requires
`compi
george.karpenkov added inline comments.
Comment at: tools/scan-build-py/libscanbuild/report.py:257
def read_bugs(output_dir, html):
+# type: (str, bool) -> Generator[Dict[str, Any], None, None]
""" Generate a unique sequence of bugs from given output directory.
xazax.hun updated this revision to Diff 126543.
xazax.hun added a comment.
- Further improvements to python script part.
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/PathSensitiv
gerazo added inline comments.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:44
+CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt'
+CTU_TEMP_FNMAP_FOLDER = 'tmpExternalFnMaps'
george.karpenkov wrote:
> gerazo wrote:
> > george.karpenkov wrote:
> > > What
george.karpenkov added a comment.
Python part looks good to me. I don't know whether @dcoughlin or @NoQ would
want to insert additional comments on C++ parts.
https://reviews.llvm.org/D30691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
xazax.hun updated this revision to Diff 125986.
xazax.hun added a comment.
Herald added a subscriber: rnkovacs.
- @gerazo addressed some review comments in scan-build-py.
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
include/clang/StaticAnalyzer/Core/AnalyzerOptions.
george.karpenkov added inline comments.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:44
+CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt'
+CTU_TEMP_FNMAP_FOLDER = 'tmpExternalFnMaps'
gerazo wrote:
> george.karpenkov wrote:
> > What would happen when m
gerazo added a comment.
The code modifications are coming soon (after doing some extensive testing) for
the scan-build part.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:223
+ctu_config = get_ctu_config(args)
+if ctu_config.collect:
+shutil.rmtree(ct
gerazo added a comment.
Thanks George for the review. I will start working on the code right away. I've
tried to answer the simpler cases.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:44
+CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt'
+CTU_TEMP_FNMAP_FOLDER = 'tm
george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:44
+CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt'
+CTU_TEMP_FNMAP_FOLDER = 'tmpE
xazax.hun updated this revision to Diff 121700.
xazax.hun added a comment.
- Rebased on ToT
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
xazax.hun updated this revision to Diff 119458.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.
- Update the scan-build part to work correctly with the accepted version of
libCrossTU
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptio
dkrupp requested changes to this revision.
dkrupp added a comment.
This revision now requires changes to proceed.
Please fix the incompatibility between analyze-build and lib/CrossTU in the
format of externalFnMap.txt mappfing file.
Comment at: tools/scan-build-py/libscanbuild
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#878830, @r.stahl wrote:
> For my purposes I replaced the return statement of the
> compareCrossTUSourceLocs function with:
>
> return XL.getFileID() < YL.getFileID();
>
>
> A more correct fix would create only one unique diagnostic
xazax.hun updated this revision to Diff 116502.
xazax.hun added a comment.
Herald added a subscriber: baloghadamsoftware.
- Fixed an issue with source locations
- Updated to latest trunk
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#878711, @r.stahl wrote:
> If either of the FullSourceLocs is a MacroID, the call
> SM.getFileEntryForID(XL.getFileID()) will return a null pointer. The null
> pointer will crash the program when attempting to call ->getName() on it.
r.stahl added a comment.
In a similar case, static inline functions are an issue.
inc.h
void foo();
static inline void wow()
{
int a = *(int*)0;
}
main.c
#include "inc.h"
void moo()
{
foo();
wow();
}
other.c
#include "inc.h"
void foo()
{
wow();
r.stahl added a comment.
While testing this I stumbled upon a crash with the following test case:
inc.h
#define BASE ((int*)0)
void foo();
main.c:
#include "inc.h"
void moo()
{
int a = BASE[0];
foo();
}
other.c
#include "inc.h"
void foo()
{
int a = BASE[0]
danielmarjamaki accepted this revision.
danielmarjamaki added inline comments.
This revision is now accepted and ready to land.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:165
+with open(filename, 'r') as in_file:
+for line in in_file:
+
whisperity added a comment.
The Python code here still uses `mangled name` in their wording. Does this mean
this patch is yet to be updated with the USR management in the parent patch?
Comment at: tools/scan-build-py/libscanbuild/analyze.py:165
+with open(filename,
danielmarjamaki added inline comments.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:165
+with open(filename, 'r') as in_file:
+for line in in_file:
+yield line
I believe you can write:
for line in op
xazax.hun updated this revision to Diff 105053.
xazax.hun added a comment.
- Patch scan-build instead of using custom scripts
- Rebase patch based on the proposed LibTooling CTU code
https://reviews.llvm.org/D30691
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/Sta
klimek added a reviewer: rsmith.
klimek added a comment.
Richard (added as reviewer) usually owns decisions around clang itself. Writing
an email to cfe-dev with the numbers and wait for whether others have concerns
would probably also be good.
https://reviews.llvm.org/D30691
__
NoQ added a comment.
Regarding serializing vs not serializing and now vs later.
1. I think we eventually need to provide a reasonable default approach
presented to the user. This approach shouldn't be hurting the user dramatically
in any sense. Because //serializing// hurts the user's disk spac
xazax.hun added a comment.
Some of the CTU related analyzer independent parts are being factored out.
The review is ongoing here: https://reviews.llvm.org/D34512
Another small and independent part is under review here:
https://reviews.llvm.org/D34506
https://reviews.llvm.org/D30691
dkrupp added a comment.
Thanks.
> It would be best to just add the scan-build-py support to the tree,
> especially, since the new scrips are not tested.
OK. We will update this patch with the scan-build-py changes and remove the
ctu-build.py and ctu-analyze.py scripts.
> I am curious which op
zaks.anna added a comment.
> -(Anna) Scan-build-py integration of the functionality is nearly finished
> (see https://github.com/rizsotto/scan-build/issues/83) (--ctu switch performs
> both analysis phases at once). This I think could go in a different patch,
> but until we could keep the ctu-b
a.sidorin accepted this revision.
a.sidorin added a comment.
Hello Daniel & Gabor,
Thank you very much for your work. This patch looks good to me but I think such
a change should also be approved by maintainers.
https://reviews.llvm.org/D30691
___
dkrupp added a comment.
Thanks for the reviews so far.
I think we have addressed all major concerns regarding this patch:
-(Anna) Scan-build-py integration of the functionality is nearly finished (see
https://github.com/rizsotto/scan-build/issues/83) (--ctu switch performs both
analysis phases
xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#731617, @zaks.anna wrote:
> I agree that scan-build or scan-build-py integration is an important issue to
> resolve here. What I envision is that users will just call scan-build and
> pass
xazax.hun updated this revision to Diff 101695.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Migrate to use USR instead of mangled names. Name mangling related changes
are reverted.
- Better error handling in some cases.
- File paths containing spaces are now handle
a.sidorin added inline comments.
Comment at: lib/AST/ASTContext.cpp:1481
+ assert(!FD->hasBody() && "FD has a definition in current translation unit!");
+ if (!FD->getType()->getAs())
+return nullptr; // Cannot even mangle that.
xazax.hun wrote:
> a.sidorin
xazax.hun updated this revision to Diff 96727.
xazax.hun marked 5 inline comments as done.
xazax.hun added a comment.
- Updates according to review comments
- Improvements to the python scripts
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Mangle.h
whisperity added inline comments.
Comment at: lib/AST/ASTContext.cpp:1497
+ auto FnUnitCacheEntry = FunctionAstUnitMap.find(MangledFnName);
+ if (FnUnitCacheEntry == FunctionAstUnitMap.end()) {
+if (FunctionFileMap.empty()) {
danielmarjamaki wrote:
> How ab
danielmarjamaki added inline comments.
Comment at: lib/AST/ASTContext.cpp:1495
+ ASTUnit *Unit = nullptr;
+ StringRef ASTFileName;
+ auto FnUnitCacheEntry = FunctionAstUnitMap.find(MangledFnName);
As far as I see you can move this ASTFileName declaration down.
xazax.hun updated this revision to Diff 96377.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.
- Removed more unused code.
- Remove the usages of posix API.
- Changes according to some review comments.
- Add a test to the clang-func-mapping tool.
https://reviews.llvm.org/D
xazax.hun marked 6 inline comments as done and an inline comment as not done.
xazax.hun added a comment.
In https://reviews.llvm.org/D30691#731617, @zaks.anna wrote:
> I agree that scan-build or scan-build-py integration is an important issue to
> resolve here. What I envision is that users will
dcoughlin added inline comments.
Comment at: include/clang/AST/Decl.h:53
class VarTemplateDecl;
+class CompilerInstance;
Is this needed? It seems like a layering violation.
Comment at: include/clang/AST/Mangle.h:59
+ // the static analyzer.
zaks.anna added a comment.
I agree that scan-build or scan-build-py integration is an important issue to
resolve here. What I envision is that users will just call scan-build and pass
-whole-project as an option to it. Everything else will happen automagically:)
Another concern is dumping the A
xazax.hun updated this revision to Diff 94814.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Removed a clang tool, replaced with python tool functionality.
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Decl.h
include/
xazax.hun marked 6 inline comments as done.
xazax.hun added inline comments.
Comment at: test/Analysis/Inputs/externalFnMap.txt:1
+_Z7h_chaini@x86_64 xtu-chain.cpp.ast
+_ZN4chns4chf2Ei@x86_64 xtu-chain.cpp.ast
NoQ wrote:
> These tests use a pre-made external func
xazax.hun updated this revision to Diff 94709.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Fixed some memory leaks.
- Removed some unrelated style changes.
- Simplifications to the python scripts.
- Removed debug prints.
https://reviews.llvm.org/D30691
Files:
i
NoQ added a comment.
Yeah, of course, ideally sticking this into scan-build, one way or another,
would be great. I understand that it'd require quite a bit of communication
with Laszlo. Otherwise we're just duplicating a whole lot of things.
Thanks for digging into this, guys. Really.
==
NoQ added a comment.
One more obvious observation regarding scan-build: because you are already
reading a compilation database, the whole tool is essentially usable in
combination with the current scan-build-py (which can create compilation
databases). So it's already quite usable, but you're f
danielmarjamaki added inline comments.
Comment at: tools/xtu-analysis/xtu-analyze.py:29
+
+threading_factor = int(multiprocessing.cpu_count() * 1.5)
+analyser_output_formats = ['plist-multi-file', 'plist', 'plist-html',
gerazo wrote:
> danielmarjamaki wrote:
> >
gerazo added inline comments.
Comment at: tools/xtu-analysis/xtu-analyze.py:29
+
+threading_factor = int(multiprocessing.cpu_count() * 1.5)
+analyser_output_formats = ['plist-multi-file', 'plist', 'plist-html',
danielmarjamaki wrote:
> does this mean that if ther
xazax.hun updated this revision to Diff 93658.
xazax.hun marked 11 inline comments as done.
xazax.hun added a comment.
- Fixed some of the review comments and some other cleanups.
https://reviews.llvm.org/D30691
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Decl.h
include/clang/
danielmarjamaki added inline comments.
Comment at: include/clang/AST/ASTContext.h:42
#include "clang/Basic/Specifiers.h"
+#include "clang/Basic/VersionTuple.h"
#include "llvm/ADT/APSInt.h"
I don't see why this is included here.
Comment at: in
xazax.hun added a comment.
Guide to run the two pass analysis:
Process
---
These are the steps of XTU analysis:
1. `xtu-build.py` script uses your compilation database and extracts all
necessary information from files compiled. It puts all its generated data into
a folder (.xtu by default
xazax.hun created this revision.
Herald added a subscriber: mgorny.
This patch adds support for naive cross translational unit analysis.
The aim of this patch is to be minimal to enable the development of the feature
on the top of tree. This patch should be an NFC in case XTUDir is not provided
82 matches
Mail list logo
