Bug 326623 - A false positive conflict report in a field assignment in a constructor
Summary: A false positive conflict report in a field assignment in a constructor
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: drd (show other bugs)
Version: 3.9.0.SVN
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Bart Van Assche
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-25 10:43 UTC by ewirch
Modified: 2013-10-29 21:02 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
drd-r13694-conflict-trace.txt (39.87 KB, text/plain)
2013-10-25 10:43 UTC, ewirch
Details
drd-13696-conflict-trace.txt (50.35 KB, text/plain)
2013-10-25 13:01 UTC, ewirch
Details
drd-13698-conflict-trace.txt (42.93 KB, text/plain)
2013-10-25 15:10 UTC, ewirch
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ewirch 2013-10-25 10:43:16 UTC
DRD  (r13694) reports "Conflicting store by thread 2 at 0x061d6fd8 size 8" on this line:

pool_[index] = new FfJniPoolElement(*lib);

"pool_" is a class local field, and the line is inside the class constructor. The reference to the constructed instance was not leaked previously:

FfJniPool(list<Library*> libs) : pool_(libs.size()) {
	size_t index = 0;
	for (list<Library*>::iterator lib = libs.begin(); lib != libs.end(); lib++) {
		pool_[index] = new FfJniPoolElement(*lib);
		index++;
	}
}

So it is impossible that a data race is happening here.

This time it is very hard to create a simplefied test case. I attached a trace for the address 0x061d6fd8. Could you please check if intercepts are missing? If not, I'll try harder to create a test case.



Reproducible: Always
Comment 1 ewirch 2013-10-25 10:43:46 UTC
Created attachment 83105 [details]
drd-r13694-conflict-trace.txt
Comment 2 Bart Van Assche 2013-10-25 12:16:56 UTC
In the trace I saw __GI_memcpy() occur just before the race. I have added an intercept for that function in r13696. Does that help ?
Comment 3 ewirch 2013-10-25 13:01:13 UTC
Created attachment 83109 [details]
drd-13696-conflict-trace.txt

The __GI_memcpy (drd_strmem_intercepts.c:322) is now in place. But the error is still reproducable. See attached trace: drd-13696-conflict-trace.txt.
Comment 4 Bart Van Assche 2013-10-25 14:26:39 UTC
The memory accesses in trace drd-13696-conflict-trace.txt look fine to me. Since the conflicting access is reported on an eight byte store and since the trace is only for the first byte of that store operation, it would help if you could do the following:
* Check with memcheck that no code is reading outside the memory ranges it is allowed to in case you had not yet done that.
* In case memcheck does not report any complaints, update Valgrind from SVN, rebuild and reinstall and rerun your program under drd with option --ptrace-addr=0x61d6fc8+8. I have modified drd such that an address range can now be specified for the --ptrace-addr option.
Comment 5 ewirch 2013-10-25 15:10:07 UTC
Created attachment 83114 [details]
drd-13698-conflict-trace.txt

Nice. memcheck does not report errors. Recompiled and created a trace.
Comment 6 Bart Van Assche 2013-10-25 15:30:52 UTC
Are you perhaps invoking Java code from C++ ? If so, does adding the Valgrind option --smc-check=all help (see also http://valgrind.org/docs/manual/faq.html#faq.java) ?
Comment 7 ewirch 2013-10-25 15:55:41 UTC
No. The test program does emulate the jni api, but it's not linked against the java libraries. So: no scm in use.
Comment 8 Bart Van Assche 2013-10-25 16:17:38 UTC
Thread 2 allocated ten bytes of memory starting at addres 0x61d6fc0. The following code is reading past the boundaries of that memory allocation. I think this is what is causing the false positive on the eight byte read from address 0x061d6fc8:

==3008== load  0x61d6fc0 size 16 (thread 2 / vc [ 1: 1380, 2: 58, 3: 4, 4: 4, 5: 4, 6: 4, 7: 4, 8: 4, 9: 4 ])
==3008==    at 0x58B0DAF: __strstr_sse42 (emmintrin.h:685)
==3008==    by 0x79F3BF5: DefinePhoneticPart (phonfact.c:704)
==3008==    by 0x79C09E5: dllDefinePhoneticPart (wordevaluate.c:9720)
==3008==    by 0x79EBC16: Java_de_factfinder_jni_JNIFactFinder_xsDefinePhoneticPart (factfinder_JNIFactFinder.c:3244)
==3008==    by 0x4175AE: FfJniApiEmulation::xsDefinePhoneticPart(int, std::string, std::string, int, int) (FfJniApiEmulation.cpp:841)
==3008==    by 0x40DCAA: Table::definePhoneticPart(std::string, std::string, int, int) (Table.cpp:681)
==3008==    by 0x46A555: DefinePhoneticPartTest::testReplaceContainsSearch(std::string, std::string) (DefinePhoneticPartTest.cpp:41)
==3008==    by 0x46A31B: DefinePhoneticPartTest::testReplaceContainsSearch_adder::invokeTest(Test*) (in /home/factfinder/edu/ff-lib-test/ff-lib-test)
==3008==    by 0x421115: TestExecuteTask::run() (TestRunner.h:35)
==3008==    by 0x420B1E: PoolWorker::threadWork() (ThreadPool.h:62)
==3008==    by 0x484AA2: Thread::invokeRun(void*) (Thread.cpp:66)
==3008==    by 0x4C2F594: vgDrd_thread_wrapper (drd_pthread_intercepts.c:355)

I will add an intercept for that function.
Comment 9 Bart Van Assche 2013-10-25 16:52:09 UTC
Added an intercept for strstr() in r13699 and also for a whole bunch of sse-optimized variants of already intercepted functions. Further feedback is welcome.
Comment 10 ewirch 2013-10-28 08:28:21 UTC
Works like a charm. Why didn't memcheck pick up the write past the boundaries?
Comment 11 Bart Van Assche 2013-10-28 15:49:21 UTC
It's not yet clear to me why memcheck did not detect this. Anyway, I will have a look into unifying the memcheck, DRD and Helgrind strmem intercept code. It's not good to have almost the same intercept code in three different Valgrind tools.