Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static analyzer HTML output has incorrect links to "note" diagnostics #64054

Closed
nicolas17 opened this issue Jul 24, 2023 · 4 comments
Closed
Assignees
Labels
clang:static analyzer good first issue https://github.com/llvm/llvm-project/contribute

Comments

@nicolas17
Copy link
Contributor

When the clang static analyzer outputs HTML reports, there is a "Bug Summary" section at the top, which has in-page anchor links to the diagnostics in the source file below. When the checker generates "notes" with BugReport.addNote, this section has links to every note in the report.

However, these links are off by one. The first link doesn't work, the second link points at the first note, the third link points at the second note, etc.

To reproduce:

struct A {
  int *iptr;
  int a;
  int b;
  int *c;
  int *d;

  A (int *iptr) : iptr(iptr) {}
};

void f() {
  A a(0);
}
clang --analyze --analyzer-output html -Xanalyzer -analyzer-checker=optin.cplusplus.UninitializedObject test-notes.cpp

The output HTML has links to #EndPath, #Note0, #Note1, #Note2, #Note3 (generated by HTMLDiagnostics::FinalizeHTML), but the notes themselves have id="Note1", id="Note2", id="Note3", id="Note4" (generated by RewriteFile and HandlePiece).

It looks like FinalizeHTML generates the links in incremental order from 0, but RewriteFile is iterating over the notes in reverse order and starts at n (not n-1).

@nicolas17
Copy link
Contributor Author

Now I see that path-sensitive diagnostic elements start at id="Path1", so probably note IDs should also start at 1. Maybe:

diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 0fe0c93dc016..f91a51cc5f8f 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -592,11 +592,11 @@ void HTMLDiagnostics::FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
             P->getLocation().asLocation().getExpansionLineNumber();
         int ColumnNumber =
             P->getLocation().asLocation().getExpansionColumnNumber();
+        ++NumExtraPieces;
         os << "<tr><td class=\"rowname\">Note:</td><td>"
            << "<a href=\"#Note" << NumExtraPieces << "\">line "
            << LineNumber << ", column " << ColumnNumber << "</a><br />"
            << P->getString() << "</td></tr>";
-        ++NumExtraPieces;
       }
     }
 

@EugeneZelenko EugeneZelenko added good first issue https://github.com/llvm/llvm-project/contribute clang:static analyzer and removed new issue labels Jul 24, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 24, 2023

@llvm/issue-subscribers-clang-static-analyzer

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 24, 2023

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Assign the issue to you.
  2. Fix the issue locally.
  3. Run the test suite locally.
    3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can
    e.g. use make check-clang-ast to only run Clang's AST tests.
  4. Create a git commit
  5. Run git clang-format HEAD~1 to format your changes.
  6. Submit the patch to Phabricator.
    6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue

@gruuprasad
Copy link
Contributor

@EugeneZelenko @nicolas17 I would like to fix this. Please assign this to me.

doru1004 pushed a commit to doru1004/llvm-project that referenced this issue Aug 3, 2023
IDs of the note list start from 1. Link generated for each note started
with index 0 i.e #Note0, #Note1 and so on.
As a result, first link ("#Note0") was invalid, subsequent links pointed
at wrong note.
Now, generated links to the notes start with index 1 i.e (#Note1, #Note2
and so on.

Patch by Guruprasad Hegde (gruuprasad)!

Fixes llvm#64054

Differential Revision: https://reviews.llvm.org/D156724
razmser pushed a commit to razmser/llvm-project that referenced this issue Sep 8, 2023
IDs of the note list start from 1. Link generated for each note started
with index 0 i.e #Note0, #Note1 and so on.
As a result, first link ("#Note0") was invalid, subsequent links pointed
at wrong note.
Now, generated links to the notes start with index 1 i.e (#Note1, #Note2
and so on.

Patch by Guruprasad Hegde (gruuprasad)!

Fixes llvm#64054

Differential Revision: https://reviews.llvm.org/D156724
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Development

No branches or pull requests

4 participants