This revision was automatically updated to reflect the committed changes.
Closed by commit rL364605: [analyzer] Fix clang-tidy crash on GCCAsmStmt
(authored by Nathan-Huckleberry, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
htt
Nathan-Huckleberry updated this revision to Diff 206954.
Nathan-Huckleberry added a comment.
- Minor style change on assert
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63533/new/
https://reviews.llvm.org/D63533
Files:
clang/lib/StaticAnalyzer/
nickdesaulniers added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401
+ case Stmt::GCCAsmStmtClass:
+assert(cast(Term)->isAsmGoto() == true && "Encountered
GCCAsmStmt without labels");
+// TODO: Handle jumping to labels
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks great now, thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63533/new/
https://reviews.llvm.org/D63533
_
Nathan-Huckleberry updated this revision to Diff 206919.
Nathan-Huckleberry added a comment.
- Add assertion message and simplify test case
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63533/new/
https://reviews.llvm.org/D63533
Files:
clang/lib
xbolva00 added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401
+ case Stmt::GCCAsmStmtClass:
+return;
}
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > NoQ wrote:
> > > Please add a TODO to actually implement th
tmroeder added a comment.
I can confirm that this fixes the clang-tidy crash I observed in trying to
analyze the kernel.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63533/new/
https://reviews.llvm.org/D63533
__
Nathan-Huckleberry marked an inline comment as done.
Nathan-Huckleberry added inline comments.
Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot
%s
+// RUN: cat %t.dot | FileCheck %s
---
nickdesaulniers added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401
+ case Stmt::GCCAsmStmtClass:
+return;
}
nickdesaulniers wrote:
> NoQ wrote:
> > Please add a TODO to actually implement this functionality.
> An
nickdesaulniers added inline comments.
Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot
%s
+// RUN: cat %t.dot | FileCheck %s
NoQ wrote:
> NoQ wrote:
> > NoQ wrote:
> >
NoQ added inline comments.
Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot
%s
+// RUN: cat %t.dot | FileCheck %s
NoQ wrote:
> NoQ wrote:
> > Ugh, you picked an exotic
NoQ added inline comments.
Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot
%s
+// RUN: cat %t.dot | FileCheck %s
NoQ wrote:
> Ugh, you picked an exotic test as an exam
NoQ added inline comments.
Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot
%s
+// RUN: cat %t.dot | FileCheck %s
Ugh, you picked an exotic test as an example.
Let's t
Nathan-Huckleberry updated this revision to Diff 205642.
Nathan-Huckleberry added a comment.
- [analyzer] Revise test case and add TODO
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63533/new/
https://reviews.llvm.org/D63533
Files:
clang/lib/Sta
nickdesaulniers added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401
+ case Stmt::GCCAsmStmtClass:
+return;
}
NoQ wrote:
> Please add a TODO to actually implement this functionality.
And an `assert` that this `Stmt
NoQ added a comment.
Hey, thanks! That's a nice catch.
The code looks fine, but i don't think your test actually tests it - see inline
comments. Also i think we should put the test into the clang repo (i.e.,
`test/Analysis`), because that's where the change is. I'd like to know it if i
break y
Nathan-Huckleberry created this revision.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
Added entry in switch statement to recognize GCCAsmStmt
as a possible block t
17 matches
Mail list logo