Implements Dr Memory analysis #837

arnaudsylvestre · 2016-04-16T22:02:56Z

Dr. Memory is a memory monitoring tool capable of identifying memory-related programming errors such as accesses of uninitialized memory, accesses to unaddressable memory.

Here is a Pull Request to support this tool in sonar-cxx.

Hope it helps !

arnaudsylvestre · 2016-04-16T22:18:13Z

This is for the wiki:

Dr. Memory

SonarQube can be fed with results of a Dr. Memory analysis.

Just launch your program with Dr. Memory. The actual call should look something like:

drmemory.exe -logdir c:/logs -- c:/path/to/my/app

Now, you just have to fill the sonar.cxx.drmemory.reportPath:

sonar.cxx.drmemory.reportPath=c:/logs/**/results.txt

The results.txt file should look something like:

Dr. Memory version 1.8.0 build 8 built on Sep  9 2014 16:27:02
Dr. Memory results for pid 1952: "Mask.Acceptance.Tests.exe"
Application cmdline: "Mask.Acceptance.Tests.exe"
Recorded 108 suppression(s) from default C:\Program Files (x86)\Dr. Memory\bin\suppress-default.txt

Error #1: UNINITIALIZED READ: reading register eax
# 0 ImageElementParser::Parse                                                 [r:\developpement\mask\peage\api_masque\sources\maskcpp\core\parsing\imageelementparser.cpp:21]
# 1 MRTComponentsParser::Parse                                                [r:\developpement\mask\peage\api_masque\sources\maskcpp\core\parsing\mrtcomponentsparser.cpp:47]
# 2 MRTPageParser::Parse                                                      [r:\developpement\mask\peage\api_masque\sources\maskcpp\core\parsing\mrtpageparser.cpp:13]
# 3 MRTFileParser::Parse                                                      [r:\developpement\mask\peage\api_masque\sources\maskcpp\core\parsing\mrtfileparser.cpp:22]
# 4 MRTMasqueParserAdapter::Parser                                            [r:\developpement\mask\peage\api_masque\sources\maskcpp\core\parsing\mrtmasqueparseradapter.cpp:12]
# 5 Mask::Load                                                                [r:\developpement\mask\peage\api_masque\sources\maskcpp\core\mask.cpp:15]
# 6 Mask::Load                                                                [r:\developpement\mask\peage\api_masque\sources\maskcpp\core\mask.cpp:8]
# 7 BitmapGenerationConfiguration::AndGetResultAsBmpContent                   [r:\developpement\mask\peage\api_masque\sources\maskcpp\factory\maskfactory.h:72]
# 8 MaskAcceptanceTest::LaunchTest                                            [r:\developpement\mask\peage\api_masque\tests\acceptance\acceptancetestexecutor.cpp:68]
# 9 conditions::Conditions_OnImage_Test::TestBody                             [r:\developpement\mask\peage\api_masque\tests\acceptance\conditionstest.cpp:44]
#10 testing::internal::HandleSehExceptionsInMethodIfSupported<>               [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:2075]
#11 testing::internal::HandleExceptionsInMethodIfSupported<>                  [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:2126]
#12 testing::Test::Run                                                        [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:2162]
#13 testing::TestInfo::Run                                                    [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:2338]
#14 testing::TestCase::Run                                                    [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:2445]
#15 testing::internal::UnitTestImpl::RunAllTests                              [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:4237]
#16 testing::internal::HandleSehExceptionsInMethodIfSupported<>               [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:2075]
#17 testing::internal::HandleExceptionsInMethodIfSupported<>                  [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:2126]
#18 testing::UnitTest::Run                                                    [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest.cc:3874]
#19 main                                                                      [r:\developpement\mask\build\packages\external.google-test.1.6.2-dev\sources\src\gtest_main.cc:38]
Note: @0:00:11.965 in thread 1804
Note: instruction: test   %eax %eax

jmecosta · 2016-04-17T08:17:04Z

this seems good, however just out of curiosity why did you not use the external plugin. for example intel inspector we use it in same way. see https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Extending-the-code-analysis#resources

arnaudsylvestre · 2016-04-17T09:53:13Z

The resultst.txt file generated by Dr. Memory is not easy to transform into a xml file. I must developed a tool to parse this file (based on RegEx). So, I would like to share it with the community.

In addition, it's easier to use for other people if it is natively integrated into SonarQube (through sonar-cxx)

guwirth · 2016-04-17T09:54:30Z

sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/drmemory/CxxDrMemorySensor.java

+				}
+			}
+		} catch (IOException e1) {
+			throw new EmptyReportException();


Error handling is different. All IO failures are handled as EmptyReportException. Other sensor are doing this only if really empty.

What kind of Exception do you want to be raised ? Can I create my own Exception type ?

@arnaudsylvestre please have a look to https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java. EmptyReportException is a special case.

public void analyse(Project project, SensorContext context) { try { List<File> reports = getReports(settings, fs.baseDir(), reportPathKey()); violationsCount = 0; for (File report : reports) { int prevViolationsCount = violationsCount; CxxUtils.LOG.info("Processing report '{}'", report); try { processReport(project, context, report); CxxUtils.LOG.debug("{} processed = {}", metric == null ? "Issues" : metric.getName(), violationsCount - prevViolationsCount); } catch (EmptyReportException e) { CxxUtils.LOG.warn("The report '{}' seems to be empty, ignoring.", report); } } CxxUtils.LOG.info("{} processed = {}", metric == null ? "Issues" : metric.getName(), violationsCount); if (metric != null) { Measure measure = new Measure(metric); measure.setIntValue(violationsCount); context.saveMeasure(measure); } } catch (Exception e) { String msg = new StringBuilder() .append("Cannot feed the data into sonar, details: '") .append(e) .append("'") .toString(); throw new IllegalStateException(msg, e); } }

guwirth · 2016-04-17T10:02:19Z

@arnaudsylvestre thanks for providing this. Looks good beside some smaller points.

error handling (see comment above)
wiki item:
- would be good to have a link to Dr. Memory homepage
- would be good to add a small sample of the input file format

otherwise 👍

jmecosta · 2016-04-17T10:02:35Z

By the way how do you handle the stacktrace, single issue or multiple issues?

arnaudsylvestre · 2016-04-17T10:55:26Z

I parse the stack trace until I found a file that is handled by the analysis. After that, I produce only one issue.

jmecosta · 2016-04-17T10:57:34Z

So you can have multiple issues that are related to the same leak?

On Sun, 17 Apr 2016 13:55 Arnaud SYLVESTRE, [email protected]
wrote:

I parse the stack trace until I found a file that is handled by the
analysis. After that, I produce only one issue.

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#837 (comment)

arnaudsylvestre · 2016-04-17T10:59:16Z

No, because when I produce an issue, I go to the next leak without analyzing the rest of the stacktrace

jmecosta · 2016-04-17T11:02:13Z

So that means if the leak happens in a file included in the analysis but
after the first is found then you loose that information?

On Sun, 17 Apr 2016 13:59 Arnaud SYLVESTRE, [email protected]
wrote:

No, because when I produce an issue, I go to the next leak without
analyzing the rest of the stacktrace

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#837 (comment)

jmecosta · 2016-04-17T11:04:28Z

I suppose that the lowest you can raise the issue the best is to report the
issue? And suppose if you report the all stack in the issue message that is
also good. At least that's the approach I follow for the Intel inspector

On Sun, 17 Apr 2016 14:02 Jorge Costa, [email protected] wrote:

So that means if the leak happens in a file included in the analysis but
after the first is found then you loose that information?

On Sun, 17 Apr 2016 13:59 Arnaud SYLVESTRE, [email protected]
wrote:

No, because when I produce an issue, I go to the next leak without
analyzing the rest of the stacktrace

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#837 (comment)

arnaudsylvestre · 2016-04-17T11:58:11Z

Dr. Memory generates an entry by leak. The stacktrace is just here to inform in which context the leak happened. So, if there is another leak in the stacktrace, it will be another entry ! So, there is no problem.

In a first version, I displayed the full stack trace in the message but it was not easily readable...

jmecosta · 2016-04-17T12:19:30Z

Ok, i understand what you mean, in previous versions of sq it was easy to
display those. What we found usefull was also to to include the binary that
cause the leak. So that devs would know what test to run to check the leak

On Sun, 17 Apr 2016 14:58 Arnaud SYLVESTRE, [email protected]
wrote:

Dr. Memory generates an entry by leak. The stacktrace is just here to
inform in which context the leak happened. So, if there is another leak in
the stacktrace, it will be another entry ! So, there is no problem.

In a first version, I displayed the full stack trace in the message but it
was not easily readable...

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#837 (comment)

jmecosta · 2016-04-17T12:46:36Z

@arnaudsylvestre but other than the remediationFunctionGapMultiplier i am ok with the pr

arnaudsylvestre · 2016-04-17T16:43:02Z

Thank you for your comments and thank you for this great plugin ! If there is anything else, please tell me !

arnaudsylvestre · 2016-07-02T15:52:16Z

Do you want that I add the documentation in the Wiki ? If yes, where ? Maybe in the Code checkers part ?

jmecosta · 2016-07-02T18:50:23Z

yep seems a good place to put it

On Sat, 2 Jul 2016 at 18:52 Arnaud SYLVESTRE [email protected]
wrote:

Do you want that I add the documentation in the Wiki ? If yes, where ?
Maybe in the Code checkers part ?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#837 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA_jyJtER-OwBlZYacFw4ZbztzcJAF6gks5qRokxgaJpZM4IJB4X
.

arnaudsylvestre added 2 commits April 16, 2016 23:56

Implements Dr Memory analysis

6da9c76

Correct a test due to Dr Memory integration

fef6ea0

guwirth reviewed Apr 17, 2016
View reviewed changes

guwirth added the enhancement label Apr 17, 2016

guwirth added this to the 0.9.6 milestone Apr 17, 2016

arnaudsylvestre added 2 commits April 17, 2016 12:52

Add new line in package-info.java

acff735

Correct error handling

9b2549e

Change remediationFunctionBaseEffort to remediationFunctionGapMultiplier

dd521d6

guwirth added the docu request label Apr 18, 2016

guwirth merged commit b4676f3 into SonarOpenCommunity:master Apr 18, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements Dr Memory analysis #837

Implements Dr Memory analysis #837

arnaudsylvestre commented Apr 16, 2016

arnaudsylvestre commented Apr 16, 2016 •

edited

Loading

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

guwirth Apr 17, 2016

arnaudsylvestre Apr 17, 2016

guwirth Apr 17, 2016

guwirth commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

jmecosta commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

jmecosta commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

arnaudsylvestre commented Jul 2, 2016

jmecosta commented Jul 2, 2016

Implements Dr Memory analysis #837

Implements Dr Memory analysis #837

Conversation

arnaudsylvestre commented Apr 16, 2016

arnaudsylvestre commented Apr 16, 2016 • edited Loading

Dr. Memory

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

guwirth Apr 17, 2016

Choose a reason for hiding this comment

arnaudsylvestre Apr 17, 2016

Choose a reason for hiding this comment

guwirth Apr 17, 2016

Choose a reason for hiding this comment

guwirth commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

jmecosta commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

jmecosta commented Apr 17, 2016

jmecosta commented Apr 17, 2016

arnaudsylvestre commented Apr 17, 2016

arnaudsylvestre commented Jul 2, 2016

jmecosta commented Jul 2, 2016

arnaudsylvestre commented Apr 16, 2016 •

edited

Loading