[report-converter] Sparse parser #3160

jay24rajput · 2021-01-21T18:40:45Z

Sparse report converter tool for parsing Sparse output of kernel sources

bulwahn · 2021-01-24T15:14:12Z

sparse creates multi-line error messages, but this report-converter here does not parse those correctly. Please rework that.

jay24rajput · 2021-01-24T17:15:30Z

@bulwahn @csordasmarton So should we include that multi-line messages with the message of the original file? Because those lines to have a path,line and column number?

bulwahn · 2021-01-24T17:25:12Z

Well, sparse has a very special syntax for its output... it would be best to just repair the sparse output to be more like clang-analyzer or clang-tidy, but for the time being, you need understand which lines belong to previous warning line and when a new warning line starts. You can have a look at Argert's implementation (he got that right).

jay24rajput · 2021-01-31T12:16:12Z

@csordasmarton @bulwahn I think I have made the required changes to recognize the multi-line comments Kindly have a look and let me know if any modifications are required.

csordasmarton

I have some tiny little comments. The biggest one is to update the test case with a multiline warning. Otherwise LGTM!

tools/report-converter/README.md

tools/report-converter/codechecker_report_converter/sparse/output_parser.py

tools/report-converter/tests/unit/sparse_output_test_files/sample.out

jay24rajput · 2021-02-03T19:11:07Z

@csordasmarton Apologies for changing the approved code of the Parser. I noticed a bug while re-writing the test cases and hence decided to fix it. Kindly have a look at the modified code

tools/report-converter/tests/unit/sparse_output_test_files/sample.out

tools/report-converter/tests/unit/sparse_output_test_files/files/sample.h

bulwahn · 2021-02-08T08:12:00Z

It is "its converter", not "it's converter" in the commit message header of the third commit.

bulwahn · 2021-02-08T08:15:34Z

Review is done; I will start testing...

jay24rajput · 2021-02-15T19:39:03Z

@csordasmarton Kindly have a look 😄

tools/report-converter/tests/unit/sparse_output_test_files/files/sample.c

tools/report-converter/tests/unit/sparse_output_test_files/sample.out

tools/report-converter/codechecker_report_converter/sparse/output_parser.py

tools/report-converter/tests/unit/sparse_output_test_files/sample.out

jay24rajput · 2021-02-19T21:13:33Z

@csordasmarton removing this and the above-mentioned regex was actually creating a separate sample.h.plist file and not appending it to sample.c. Nevertheless, I have modified the code to not include the files/sample.c: note: in included file: line in the output.

Kindly have a look 😄

tools/report-converter/tests/unit/sparse_output_test_files/Makefile

tools/report-converter/codechecker_report_converter/sparse/output_parser.py

tools/report-converter/tests/unit/sparse_output_test_files/sample.out

csordasmarton · 2021-02-23T13:59:51Z

tools/report-converter/codechecker_report_converter/sparse/output_parser.py

+            r'(?P<message>[\S \t]+)\s*'
+        )
+
+    def parse_message(self, it, line):


I tried to convert what the sample.out with the report-converter tool and then store the results to a running server. Now there is only one report with a lot of bug steps:

I don't think this is what you want.
In my previous comment (#3160 (comment)) my recommendation was the following:

Every line which looks like this will be a new report (Message): ./files/sample.h:3:40: warning: incorrect type in argument 2 (different address spaces) (the warning keyword identifies that we have a new report).

Every line which looks like this will be an event / bug step item of the previous Message object: ./files/sample.h:3:40: expected struct task_struct *p1. (So it doesn't contain the warning keyword like in the previous case).

Every line can be skipped which looks like this files/sample.c: note: in included file:.

So in case of the following output:

files/sample.c:4:1: warning: symbol 'machine_id' was not declared. Should it be static? files/sample.h:3:40: warning: incorrect type in argument 2 (different address spaces) files/sample.c: note: in included file: files/sample.h:3:40: warning: incorrect type in argument 1 (different address spaces) files/sample.h:3:40: expected struct task_struct *p1 files/sample.h:3:40: got struct spinlock [noderef] __rcu *

I want to see 3 reports:

files/sample.c:4:1: warning: symbol 'machine_id' was not declared. Should it be static?

files/sample.h:3:40: warning: incorrect type in argument 2 (different address spaces)

files/sample.h:3:40: warning: incorrect type in argument 1 (different address spaces)
files/sample.h:3:40: expected struct task_struct *p1
files/sample.h:3:40: got struct spinlock [noderef] __rcu *

In the third case the report will have multiple bug steps.

I tried to convert what the sample.out with the report-converter tool and then store the results to a running server. Now there is only one report with a lot of bug steps:

I don't think this is what you want.

Well Actually I thought that is what we wanted, a single report for all the files and hence changed the code to include it in the single report. 😢

In my previous comment (#3160 (comment)) my recommendation was the following:

Every line which looks like this will be a new report (Message): ./files/sample.h:3:40: warning: incorrect type in argument 2 (different address spaces) (the warning keyword identifies that we have a new report).

This logic won't always stand true as some lines also start with error. Do you recommend modifying the regex as well now?

Every line which looks like this will be an event / bug step item of the previous Message object: ./files/sample.h:3:40: expected struct task_struct *p1. (So it doesn't contain the warning keyword like in the previous case).

Every line can be skipped which looks like this files/sample.c: note: in included file:.

So in case of the following output:

files/sample.c:4:1: warning: symbol 'machine_id' was not declared. Should it be static? files/sample.h:3:40: warning: incorrect type in argument 2 (different address spaces) files/sample.c: note: in included file: files/sample.h:3:40: warning: incorrect type in argument 1 (different address spaces) files/sample.h:3:40: expected struct task_struct *p1 files/sample.h:3:40: got struct spinlock [noderef] __rcu *

I want to see 3 reports:

files/sample.c:4:1: warning: symbol 'machine_id' was not declared. Should it be static?

files/sample.h:3:40: warning: incorrect type in argument 2 (different address spaces)

files/sample.h:3:40: warning: incorrect type in argument 1 (different address spaces)
files/sample.h:3:40: expected struct task_struct *p1
files/sample.h:3:40: got struct spinlock [noderef] __rcu *

In the third case the report will have multiple bug steps.

Alright so in the tests, we will also have two sample.c.expected.plist and sample.h.expected.plist, this is after the assumption that second line in the sample.out file is removed. Correct me if I am wrong

jay24rajput · 2021-02-24T12:02:46Z

@csordasmarton I think the current code solves the issues discussed above by us. Kindly have a look and let me know what you think

csordasmarton

I have just 2 tiny comments, otherwise its LGTM!

tools/report-converter/codechecker_report_converter/sparse/output_parser.py

csordasmarton · 2021-02-26T11:01:02Z

tools/report-converter/tests/unit/sparse_output_test_files/sample.out

+./files/sample.h:3:40: warning: incorrect type in argument 1 (different address spaces)
+./files/sample.h:3:40:    expected struct task_struct *p1
+./files/sample.h:3:40:    got struct spinlock [noderef] __rcu *


In my previous review comments I asked what is the reason that one line starts with ./ and the others are not (#3160 (comment)).
Now with the regexes we will identify every line which stars with ./ as notes and the others as reports. Are you sure that every note line will start with ./?

Yes as far as I have checked, all the note lines start with ./

- Sparse output is parsed and stored into the format: file:line:column:message - Parser is called from report converter CLI with type 'sparse'

- Sparse Parser is tested with sample.c, sample.h and sample.out files

- Sparse documentation in report converter explains the usage of sparse tool on kernel sources - Sparse is accessible from the main readme file and supported code analyzer file - Sparse updated in usage

jay24rajput · 2021-02-26T12:29:37Z

@csordasmarton I have made the changes suggested by you! Kindly have a look😄

csordasmarton

LGTM! Thank you for your hard work 😊

jay24rajput requested review from bruntib, csordasmarton and dkrupp as code owners January 21, 2021 18:40

jay24rajput force-pushed the sparse_parser branch from b4ad7cd to 6372ea6 Compare January 21, 2021 18:55

jay24rajput force-pushed the sparse_parser branch from 6372ea6 to 9118724 Compare January 31, 2021 11:54

csordasmarton suggested changes Feb 2, 2021

View reviewed changes

tools/report-converter/README.md Outdated Show resolved Hide resolved

tools/report-converter/codechecker_report_converter/sparse/output_parser.py Outdated Show resolved Hide resolved

tools/report-converter/tests/unit/sparse_output_test_files/sample.out Outdated Show resolved Hide resolved

csordasmarton added this to the release 6.16.0 milestone Feb 2, 2021

csordasmarton added documentation 📖 Changes to documentation. test ☑️ Adding or refactoring tests tools 🛠️ Meta-tag for all the additional tools supplied with CodeChecker: plist2html, tu_collector, etc. labels Feb 2, 2021

jay24rajput force-pushed the sparse_parser branch from 9118724 to 45abc16 Compare February 3, 2021 19:01

jay24rajput requested a review from csordasmarton February 3, 2021 19:11

bulwahn reviewed Feb 8, 2021

View reviewed changes

tools/report-converter/tests/unit/sparse_output_test_files/sample.out Show resolved Hide resolved

bulwahn reviewed Feb 8, 2021

View reviewed changes

tools/report-converter/tests/unit/sparse_output_test_files/files/sample.h Outdated Show resolved Hide resolved

bulwahn reviewed Feb 8, 2021

View reviewed changes

tools/report-converter/tests/unit/sparse_output_test_files/files/sample.h Outdated Show resolved Hide resolved

jay24rajput force-pushed the sparse_parser branch from 45abc16 to 2b150c3 Compare February 15, 2021 19:31

csordasmarton suggested changes Feb 18, 2021

View reviewed changes

jay24rajput commented Feb 18, 2021

View reviewed changes

tools/report-converter/codechecker_report_converter/sparse/output_parser.py Outdated Show resolved Hide resolved

jay24rajput commented Feb 18, 2021

View reviewed changes

tools/report-converter/codechecker_report_converter/sparse/output_parser.py Outdated Show resolved Hide resolved

csordasmarton suggested changes Feb 19, 2021

View reviewed changes

jay24rajput force-pushed the sparse_parser branch from 2b150c3 to 7c79b0d Compare February 19, 2021 21:06

jay24rajput requested a review from csordasmarton February 19, 2021 21:14

csordasmarton suggested changes Feb 23, 2021

View reviewed changes

jay24rajput force-pushed the sparse_parser branch from 7c79b0d to 3a5865e Compare February 24, 2021 11:50

jay24rajput requested a review from csordasmarton February 24, 2021 12:02

csordasmarton suggested changes Feb 26, 2021

View reviewed changes

jay24rajput force-pushed the sparse_parser branch from 3a5865e to cfe107d Compare February 26, 2021 12:14

jay24rajput added 3 commits February 26, 2021 17:47

[report-converter][cmd] Parse Sparse Output

636b35b

- Sparse output is parsed and stored into the format: file:line:column:message - Parser is called from report converter CLI with type 'sparse'

[test] Test sparse output with sample output file

ad43dc8

- Sparse Parser is tested with sample.c, sample.h and sample.out files

[docs] Explain how to use Sparse and its converter on the kernel

eb42f47

- Sparse documentation in report converter explains the usage of sparse tool on kernel sources - Sparse is accessible from the main readme file and supported code analyzer file - Sparse updated in usage

jay24rajput force-pushed the sparse_parser branch from cfe107d to eb42f47 Compare February 26, 2021 12:18

jay24rajput requested a review from csordasmarton February 26, 2021 12:28

csordasmarton approved these changes Mar 2, 2021

View reviewed changes

csordasmarton merged commit ce64dfb into Ericsson:master Mar 2, 2021

csordasmarton mentioned this pull request Mar 2, 2021

[report-converter] Support for Sparse tool #2950

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[report-converter] Sparse parser #3160

[report-converter] Sparse parser #3160

jay24rajput commented Jan 21, 2021

bulwahn commented Jan 24, 2021

jay24rajput commented Jan 24, 2021

bulwahn commented Jan 24, 2021

jay24rajput commented Jan 31, 2021 •

edited

Loading

csordasmarton left a comment

jay24rajput commented Feb 3, 2021

bulwahn commented Feb 8, 2021

bulwahn commented Feb 8, 2021

jay24rajput commented Feb 15, 2021

jay24rajput commented Feb 19, 2021 •

edited

Loading

csordasmarton Feb 23, 2021

jay24rajput Feb 24, 2021

jay24rajput commented Feb 24, 2021

csordasmarton left a comment

csordasmarton Feb 26, 2021

jay24rajput Feb 26, 2021

jay24rajput commented Feb 26, 2021

csordasmarton left a comment

[report-converter] Sparse parser #3160

[report-converter] Sparse parser #3160

Conversation

jay24rajput commented Jan 21, 2021

bulwahn commented Jan 24, 2021

jay24rajput commented Jan 24, 2021

bulwahn commented Jan 24, 2021

jay24rajput commented Jan 31, 2021 • edited Loading

csordasmarton left a comment

Choose a reason for hiding this comment

jay24rajput commented Feb 3, 2021

bulwahn commented Feb 8, 2021

bulwahn commented Feb 8, 2021

jay24rajput commented Feb 15, 2021

jay24rajput commented Feb 19, 2021 • edited Loading

csordasmarton Feb 23, 2021

Choose a reason for hiding this comment

jay24rajput Feb 24, 2021

Choose a reason for hiding this comment

jay24rajput commented Feb 24, 2021

csordasmarton left a comment

Choose a reason for hiding this comment

csordasmarton Feb 26, 2021

Choose a reason for hiding this comment

jay24rajput Feb 26, 2021

Choose a reason for hiding this comment

jay24rajput commented Feb 26, 2021

csordasmarton left a comment

Choose a reason for hiding this comment

jay24rajput commented Jan 31, 2021 •

edited

Loading

jay24rajput commented Feb 19, 2021 •

edited

Loading