User account creation filtered due to spam.

Bug 58306 - Broken profiling for unrar sources: error: corrupted value profile: value profile counter (X out of Y) inconsistent with basic-block count
Summary: Broken profiling for unrar sources: error: corrupted value profile: value pro...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: gcov-profile (show other bugs)
Version: 6.1.0
: P3 major
Target Milestone: ---
Assignee: Martin Liška
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-03 11:45 UTC by Artem S. Tashkinov
Modified: 2016-08-15 11:46 UTC (History)
2 users (show)

See Also:
Host: x86_64 i686
Target:
Build:
Known to work:
Known to fail: 4.7.3, 4.8.5, 4.9.4, 5.2.1, 5.3.1, 6.1.0
Last reconfirmed: 2016-08-04 00:00:00


Attachments
Sources + profile info (174.60 KB, application/x-xz)
2013-09-03 11:45 UTC, Artem S. Tashkinov
Details
Sources and Makefile (run make to reproduce) (471.31 KB, application/x-xz)
2015-02-16 17:04 UTC, Artem S. Tashkinov
Details
Sources and Makefile (run make to reproduce) (476.56 KB, application/x-xz)
2015-04-18 19:39 UTC, Artem S. Tashkinov
Details
gcc -march=native /tmp/ff.c -c -v (1.02 KB, text/plain)
2016-08-05 13:53 UTC, Artem S. Tashkinov
Details
unrarsrc-5.4.4 + profile data (191.64 KB, application/x-xz)
2016-08-05 14:05 UTC, Artem S. Tashkinov
Details
Patch candidate for the fallout (1.13 KB, patch)
2016-08-12 11:55 UTC, Martin Liška
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Artem S. Tashkinov 2013-09-03 11:45:51 UTC
Created attachment 30744 [details]
Sources + profile info

blake2s.cpp: In function ‘void blake2s_update(blake2s_state*, const byte*, size_t)’:
blake2s.cpp:133:40: error: corrupted value profile: value profile counter (11354724 out of 11329053) inconsistent with basic-block count (9600416)
blake2s.cpp:157:49: error: corrupted value profile: value profile counter (11204032 out of 11277067) inconsistent with basic-block count (10940610)
make: *** [blake2s.o] Error 1
make: *** Waiting for unfinished jobs....

CPU: Intel Core i5 2500
GCC: 4.7.3
ARC: i686

Makefile is included (just run make to see the error).
Comment 1 Paolo Carlini 2013-09-03 12:17:59 UTC
Doesn't look like a C++ front-end issue.
Comment 2 Artem S. Tashkinov 2013-09-03 12:37:47 UTC
(In reply to Paolo Carlini from comment #1)
> Doesn't look like a C++ front-end issue.

Surely, but I had to choose something not knowing what to choose.
Comment 3 Artem S. Tashkinov 2015-02-16 16:22:44 UTC
This bug affects GCC 4.5.4 as well. I guess the bug is no longer relevant since both these GCC releases are deprecated and unsupported.
Comment 4 Artem S. Tashkinov 2015-02-16 17:04:56 UTC
Created attachment 34783 [details]
Sources and Makefile (run make to reproduce)

This bug affects GCC 4.9.2 too! (I'm on i686):

blake2s.cpp: In function ‘void blake2s_update(blake2s_state*, const byte*, size_t)’:
blake2s.cpp:138:40: error: corrupted value profile: value profile counter (11286702 out of 11276532) inconsistent with basic-block count (9452310)
       memcpy( S->buf + left, in, fill ); // Fill buffer
                                        ^
blake2s.cpp:162:49: error: corrupted value profile: value profile counter (11504160 out of 11557346) inconsistent with basic-block count (11155414)
       memcpy( S->buf + left, in, (size_t)inlen );
                                                 ^
make: *** [blake2s.o] Error 1
Comment 5 Artem S. Tashkinov 2015-04-18 19:39:15 UTC
Created attachment 35355 [details]
Sources and Makefile (run make to reproduce)

GCC 5.0.1 RC2 is also affected:

g++  -O3 -march=native -Wno-attributes -fno-tree-vectorize -fprofile-use -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DRAR_SMP -DUNRAR -c hash.cpp
blake2s.cpp: In function ‘void blake2s_update(blake2s_state*, const byte*, size_t)’:
blake2s.cpp:138:40: error: corrupted value profile: value profile counter (10976192 out of 10966245) inconsistent with basic-block count (10996457)
       memcpy( S->buf + left, in, fill ); // Fill buffer
                                        ^
blake2s.cpp:162:49: error: corrupted value profile: value profile counter (10915050 out of 10973342) inconsistent with basic-block count (10932972)
       memcpy( S->buf + left, in, (size_t)inlen );
                                                 ^
make: *** [blake2s.o] Error 1
make: *** Waiting for unfinished jobs....
Comment 6 PeteVine 2015-12-23 11:50:47 UTC
I'm getting something similar on armv7:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69004
Comment 7 Artem S. Tashkinov 2016-03-10 18:27:19 UTC
Affects GCC 5.3.1 as well.
Comment 8 Artem S. Tashkinov 2016-03-10 18:31:29 UTC
Should I reproduce this bug in GCC 6.x as well or at least someone will take a look at it?
Comment 9 Artem S. Tashkinov 2016-03-10 18:38:46 UTC
Tow more errors for the same sources:

threadpool.cpp: In member function ‘bool ThreadPool::GetQueuedTask(ThreadPool::QueueEntry*)’:
threadpool.cpp:213:1: error: corrupted profile info: profile data is not flow-consistent
 }
 ^
threadpool.cpp:213:1: error: corrupted profile info: number of executions for edge 7-11 thought to be -23
threadpool.cpp:213:1: error: corrupted profile info: number of executions for edge 7-8 thought to be 93128
makefile:110: recipe for target 'threadpool.o' failed






unpack.cpp: In member function ‘bool Unpack::ReadTables(BitInput&, UnpackBlockHeader&, UnpackBlockTables&)’:
unpack.cpp:350:1: error: corrupted profile info: profile data is not flow-consistent
 }
 ^
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 7-66 thought to be 12
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 7-5 thought to be -12
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 14-15 thought to be -19
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 14-16 thought to be 10447
unpack.cpp:350:1: error: corrupted profile info: number of iterations for basic block 15 thought to be -19
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 15-24 thought to be -19
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 41-42 thought to be 184976
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 41-26 thought to be -38846
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 44-46 thought to be 1905937
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 44-45 thought to be -2633
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 55-57 thought to be 2567166
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 55-56 thought to be -2937
unpack.cpp: In member function ‘void Unpack::UnpackDecode(UnpackThreadData&)’:
unpack.cpp:350:1: error: corrupted profile info: profile data is not flow-consistent
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 15-59 thought to be 22934224
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 15-16 thought to be -22914989
unpack.cpp:350:1: error: corrupted profile info: number of iterations for basic block 16 thought to be -22888703
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 16-17 thought to be 26397
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 16-18 thought to be -22915100
unpack.cpp:350:1: error: corrupted profile info: number of iterations for basic block 18 thought to be -22890122
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 18-20 thought to be -22890122
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 36-37 thought to be 28405926
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 36-45 thought to be -867648
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 55-56 thought to be 6025631
unpack.cpp:350:1: error: corrupted profile info: number of executions for edge 55-11 thought to be -6475
makefile:110: recipe for target 'unpack.o' failed
Comment 10 PeteVine 2016-03-10 20:57:13 UTC
At least on ARM (gcc 4.9.3) it does work after a clean generate/use cycle.
Comment 11 Martin Liška 2016-08-04 13:58:21 UTC
I've just verified that GCC 5.3.1 and GCC 6.1.1 and latest trunk work fine (x86_64-linux-gnu). I built the binary and unrar a rar archive.

Can you please re-try that?
Comment 12 Artem S. Tashkinov 2016-08-05 13:05:26 UTC
(In reply to Martin Liška from comment #11)
> I've just verified that GCC 5.3.1 and GCC 6.1.1 and latest trunk work fine
> (x86_64-linux-gnu). I built the binary and unrar a rar archive.
> 
> Can you please re-try that?

Comment 7.
Comment 13 Martin Liška 2016-08-05 13:12:38 UTC
(In reply to Artem S. Tashkinov from comment #12)
> (In reply to Martin Liška from comment #11)
> > I've just verified that GCC 5.3.1 and GCC 6.1.1 and latest trunk work fine
> > (x86_64-linux-gnu). I built the binary and unrar a rar archive.
> > 
> > Can you please re-try that?
> 
> Comment 7.

Ok, (In reply to Artem S. Tashkinov from comment #12)
> (In reply to Martin Liška from comment #11)
> > I've just verified that GCC 5.3.1 and GCC 6.1.1 and latest trunk work fine
> > (x86_64-linux-gnu). I built the binary and unrar a rar archive.
> > 
> > Can you please re-try that?
> 
> Comment 7.

Ok, I'll try to reproduce, but I would need:

1) echo ""> /tmp/ff.c && gcc -march=native /tmp/ff.c -c -v
2) please upload somewhere a sample rar archive which you use for training run
3) command line passed to ./unrar binary (are you using multiple threads?)

Then it should be hopefully reproducible.
Comment 14 Artem S. Tashkinov 2016-08-05 13:53:50 UTC
Created attachment 39058 [details]
gcc -march=native /tmp/ff.c -c -v
Comment 15 Artem S. Tashkinov 2016-08-05 14:01:19 UTC
(In reply to Martin Liška from comment #13)
> Ok, I'll try to reproduce, but I would need:
> 
> 1) echo ""> /tmp/ff.c && gcc -march=native /tmp/ff.c -c -v

Done.

> 2) please upload somewhere a sample rar archive which you use for training
> run

https://bit.ly/2az6QHV

> 3) command line passed to ./unrar binary (are you using multiple threads?)
> 
> Then it should be hopefully reproducible.

./unrar t -inul /tmp/Rar.testarchive5.rar
./unrar t -inul /tmp/Rar.testarchive.rar

Just these two commands. Nothing else.

Here are GCC 6.1.0 results:

blake2s.cpp: In function ‘void blake2s_update(blake2s_state*, const byte*, size_t)’:
blake2s.cpp:138:40: error: corrupted value profile: value profile counter (11420699 out of 11426898) inconsistent with basic-block count (11501625)
       memcpy( S->buf + left, in, fill ); // Fill buffer
                                        ^
blake2s.cpp:162:49: error: corrupted value profile: value profile counter (11501951 out of 11566282) inconsistent with basic-block count (11518351)
       memcpy( S->buf + left, in, (size_t)inlen );
                                                 ^
make: *** [blake2s.o] Error 1
make: *** Waiting for unfinished jobs....
Comment 16 Artem S. Tashkinov 2016-08-05 14:05:39 UTC
Created attachment 39059 [details]
unrarsrc-5.4.4 + profile data
Comment 17 Martin Liška 2016-08-05 14:31:18 UTC
Thanks for the input data, I can confirm the. I'll work on that on Monday.
Comment 18 Martin Liška 2016-08-08 14:00:08 UTC
Ok, problem is that various value profilers are not updated atomically, fixed in:
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00600.html
Comment 19 Artem S. Tashkinov 2016-08-08 14:42:44 UTC
(In reply to Martin Liška from comment #18)
> Ok, problem is that various value profilers are not updated atomically,
> fixed in:
> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00600.html

Thank you!

Do I understand the patch correctly that it requires "-fprofile-update=atomic" option in order to eliminate this bug?
Comment 20 Martin Liška 2016-08-08 14:47:30 UTC
(In reply to Artem S. Tashkinov from comment #19)
> (In reply to Martin Liška from comment #18)
> > Ok, problem is that various value profilers are not updated atomically,
> > fixed in:
> > https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00600.html
> 
> Thank you!
> 
> Do I understand the patch correctly that it requires
> "-fprofile-update=atomic" option in order to eliminate this bug?

Exactly, I hope I'll be able to merge the whole patchset (5) to mainline soon.
Comment 21 Artem S. Tashkinov 2016-08-08 16:45:51 UTC
(In reply to Martin Liška from comment #20)
> > Do I understand the patch correctly that it requires
> > "-fprofile-update=atomic" option in order to eliminate this bug?
> 
> Exactly, I hope I'll be able to merge the whole patchset (5) to mainline
> soon.

What's the rationale behind this decision? This looks a bit counter intuitive and counter productive. People just enable profiling and don't think twice about extra requirements it might ensue.
Comment 22 Martin Liška 2016-08-08 17:12:34 UTC
(In reply to Artem S. Tashkinov from comment #21)
> (In reply to Martin Liška from comment #20)
> > > Do I understand the patch correctly that it requires
> > > "-fprofile-update=atomic" option in order to eliminate this bug?
> > 
> > Exactly, I hope I'll be able to merge the whole patchset (5) to mainline
> > soon.
> 
> What's the rationale behind this decision? This looks a bit counter
> intuitive and counter productive. People just enable profiling and don't
> think twice about extra requirements it might ensue.

Sure, as Nathan suggested, we'll select the proper default value according to -pthread argument.
Comment 23 Artem S. Tashkinov 2016-08-08 17:19:57 UTC
(In reply to Martin Liška from comment #22)
> Sure, as Nathan suggested, we'll select the proper default value according
> to -pthread argument.

Wonderful! What are the chances of this patch being merged with GCC 4.9.x?
Comment 24 Martin Liška 2016-08-08 21:32:17 UTC
> Wonderful! What are the chances of this patch being merged with GCC 4.9.x?

Any, because 4.9 was closed last week and there's not going to be any release.
If you are interested, I can back-port patches to both 5 and 6 branches?
Comment 25 PeteVine 2016-08-08 23:37:18 UTC
Great news, thanks! What about backporting to 4.9? There's not going to be another official release but manual patching could still be useful.
Comment 26 Artem S. Tashkinov 2016-08-09 07:52:53 UTC
(In reply to Martin Liška from comment #24)
> > Wonderful! What are the chances of this patch being merged with GCC 4.9.x?
> 
> Any, because 4.9 was closed last week and there's not going to be any
> release.
> If you are interested, I can back-port patches to both 5 and 6 branches?

Only if it's relatively straightforward and fast for you. Since you're such a prolific bug solver, the project will benefit a lot more if you keep on solving new unresolved bugs, instead of backporting them. So, suit yourself. And thank you very much!
Comment 27 Martin Liška 2016-08-10 13:15:27 UTC
Author: marxin
Date: Wed Aug 10 13:14:56 2016
New Revision: 239324

URL: https://gcc.gnu.org/viewcvs?rev=239324&root=gcc&view=rev
Log:
Add new *_atomic counter update function

	PR gcov-profile/58306
	* Makefile.in: New functions (modules) are added.
	* libgcov-profiler.c (__gcov_interval_profiler_atomic): New
	function.
	(__gcov_pow2_profiler_atomic): New function.
	(__gcov_one_value_profiler_body): New argument is instroduced.
	(__gcov_one_value_profiler): Call with the new argument.
	(__gcov_one_value_profiler_atomic): Likewise.
	(__gcov_indirect_call_profiler_v2): Likewise.
	(__gcov_time_profiler_atomic): New function.
	(__gcov_average_profiler_atomic): Likewise.
	(__gcov_ior_profiler_atomic): Likewise.
	* libgcov.h: Declare the aforementioned functions.
	PR gcov-profile/58306
	* gcc.dg/tree-prof/val-profiler-threads-1.c: New test.
	PR gcov-profile/58306
	* tree-profile.c (gimple_init_edge_profiler): Create conditionally
	atomic variants of profile update functions.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-prof/val-profiler-threads-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-profile.c
    trunk/libgcc/ChangeLog
    trunk/libgcc/Makefile.in
    trunk/libgcc/libgcov-profiler.c
    trunk/libgcc/libgcov.h
Comment 28 Martin Liška 2016-08-10 13:26:58 UTC
Fixed on trunk.
Comment 29 Artem S. Tashkinov 2016-08-10 13:50:22 UTC
(In reply to Martin Liška from comment #28)
> Fixed on trunk.

Thanks!

Will GCC 6.1.1 include these patches?
Comment 30 Andreas Schwab 2016-08-11 09:16:42 UTC
/daten/aranym/gcc/gcc-20160811/Build/gcc/testsuite/g++/../../libgcov.a(_gcov_time_profiler.o): In function `__gcov_time_profiler_atomic':
/daten/aranym/gcc/gcc-20160811/Build/m68k-linux/libgcc/../../../libgcc/libgcov-profiler.c:352: undefined reference to `__atomic_fetch_add_8'
collect2: error: ld returned 1 exit status
compiler exited with status 1
FAIL: g++.dg/other/pr55650.C  -std=gnu++98 (test for excess errors)
Comment 31 Artem S. Tashkinov 2016-08-11 09:31:29 UTC
(In reply to Andreas Schwab from comment #30)
> /daten/aranym/gcc/gcc-20160811/Build/gcc/testsuite/g++/../../libgcov.
> a(_gcov_time_profiler.o): In function `__gcov_time_profiler_atomic':
> /daten/aranym/gcc/gcc-20160811/Build/m68k-linux/libgcc/../../../libgcc/
> libgcov-profiler.c:352: undefined reference to `__atomic_fetch_add_8'
> collect2: error: ld returned 1 exit status
> compiler exited with status 1
> FAIL: g++.dg/other/pr55650.C  -std=gnu++98 (test for excess errors)

-march=i686 (or higher) should help but it certainly warrants some attention.
Comment 32 Martin Liška 2016-08-11 09:49:57 UTC
(In reply to Andreas Schwab from comment #30)
> /daten/aranym/gcc/gcc-20160811/Build/gcc/testsuite/g++/../../libgcov.
> a(_gcov_time_profiler.o): In function `__gcov_time_profiler_atomic':
> /daten/aranym/gcc/gcc-20160811/Build/m68k-linux/libgcc/../../../libgcc/
> libgcov-profiler.c:352: undefined reference to `__atomic_fetch_add_8'
> collect2: error: ld returned 1 exit status
> compiler exited with status 1
> FAIL: g++.dg/other/pr55650.C  -std=gnu++98 (test for excess errors)

Sorry for the breakage, I misread that gcov_type is always 64-bit long. Thus, having a system w/o memory load/store instructions of size 64-bit, -fprofile-update=atomic must be disabled and libgcov should not have *_atomic functions.
Comment 33 Martin Liška 2016-08-12 11:55:07 UTC
Created attachment 39275 [details]
Patch candidate for the fallout

Hi Andread.

Can you please test me the following patch. I understand why the symbol is undefined, but what I don't understand why is atomic update mode selected for the test-case. Following patch should detect whether a target is capable of atomic update operations, however libgcov.a would still contain __atomic_fetch_add_8, which should be fine as soon as nobody is calling functions that contain the symbol?
Comment 34 Martin Liška 2016-08-12 11:56:04 UTC
(In reply to Martin Liška from comment #33)
> Created attachment 39275 [details]
> Patch candidate for the fallout
> 
> Hi Andread.

*Andreas

> 
> Can you please test me the following patch. I understand why the symbol is
> undefined, but what I don't understand why is atomic update mode selected
> for the test-case. Following patch should detect whether a target is capable
> of atomic update operations, however libgcov.a would still contain
> __atomic_fetch_add_8, which should be fine as soon as nobody is calling
> functions that contain the symbol?
Comment 35 Andreas Schwab 2016-08-15 11:46:03 UTC
That patch doesn't change anything.