|
From: <sv...@va...> - 2005-11-18 15:10:38
|
Author: sewardj
Date: 2005-11-18 15:10:24 +0000 (Fri, 18 Nov 2005)
New Revision: 5190
Log:
Back out r5180 (sorry Josef). On consideration it just increases the dif=
ficulty
of testing the system properly.
Modified:
trunk/coregrind/m_redir.c
Modified: trunk/coregrind/m_redir.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- trunk/coregrind/m_redir.c 2005-11-18 14:59:00 UTC (rev 5189)
+++ trunk/coregrind/m_redir.c 2005-11-18 15:10:24 UTC (rev 5190)
@@ -377,19 +377,14 @@
=20
#elif defined(VGP_ppc32_linux)
=20
- /* these two drive memcheck nuts if not replaced, and unfortunately
- they need to be replaced right at the start, before the dynamic
- linker starts. */
- if (VG_(needs).malloc_replacement) {
- add_redirect_sym_to_addr(
- "soname:ld.so.1", "strlen",
- (Addr)&VG_(ppc32_linux_REDIR_FOR_strlen)
- ); =20
- add_redirect_sym_to_addr(
- "soname:ld.so.1", "strcmp",
- (Addr)&VG_(ppc32_linux_REDIR_FOR_strcmp)
- );
- }
+ add_redirect_sym_to_addr(
+ "soname:ld.so.1", "strlen",
+ (Addr)&VG_(ppc32_linux_REDIR_FOR_strlen)
+ ); =20
+ add_redirect_sym_to_addr(
+ "soname:ld.so.1", "strcmp",
+ (Addr)&VG_(ppc32_linux_REDIR_FOR_strcmp)
+ );
=20
#else
# error Unknown platform
|
|
From: Josef W. <Jos...@gm...> - 2005-11-18 16:29:46
|
On Friday 18 November 2005 16:10, sv...@va... wrote: > Author: sewardj > Date: 2005-11-18 15:10:24 +0000 (Fri, 18 Nov 2005) > New Revision: 5190 > > Log: > Back out r5180 (sorry Josef). On consideration it just increases the difficulty > of testing the system properly. What is/was the problem here? For meaningful results in the scope of redirections, I had to revert the instrument function back to *not* use the noredir_addr. I hope this is fine because I switch off chaising. The correct implementation (instead of mentioned reverting) needs some changes. Josef |
|
From: Julian S. <js...@ac...> - 2005-11-18 17:22:20
|
> > Back out r5180 (sorry Josef). On consideration it just increases the > > difficulty of testing the system properly. > > What is/was the problem here? No particular problem, but 1. The redirect/no-redirect decision was made on the basis of whether the tool overrides malloc or not. This is not a good criterion -- for example it means the redirection is done for massif too. 2. It means there are more ways the system can break -- now we have some tools using these replacement fns and some tools not using them. To tell the truth I'm not really clear what problem callgrind has with them. They just replace strlen and strcmp when called from ld.so, but they don't replace the strlen and strcmp in glibc, so I would not expect them to come very high on the profiles. So what problem are they causing? > For meaningful results in the scope of redirections, I had to revert the > instrument function back to *not* use the noredir_addr. I hope this is > fine because I switch off chaising. Well, it might work, but it's not good. I wonder if there's a better way to solve this. J |
|
From: Josef W. <Jos...@gm...> - 2005-11-18 18:59:20
|
On Friday 18 November 2005 18:23, Julian Seward wrote: > > 1. The redirect/no-redirect decision was made on the basis of > whether the tool overrides malloc or not. This is not a > good criterion -- for example it means the redirection is done > for massif too. Yes. Makes sense. > > 2. It means there are more ways the system can break -- now we > have some tools using these replacement fns and some tools not > using them. And on the other architectures? Don't you have these 2 cases there, too? Or are there no redirections? > To tell the truth I'm not really clear what problem callgrind has > with them. On the one hand, it is a technical one: I have to clearly separate the the uses of non-redirected and redirected addresses of a BB, as instruction addresses appear in the profiling output with callgrind (in contrast to cachegrind). Solvable. Another thing: because of redirection, I can not do jump/call handling at the end of a BB, because the goto target will not always be correct. > They just replace strlen and strcmp when called from > ld.so, but they don't replace the strlen and strcmp in glibc, so > I would not expect them to come very high on the profiles. Still it feels wrong. A profiling tool should run the same code as in reality. Why artificially lower the quality of the result? ld.so's strcmp is used quite often in every symbol resolution. This function can be relevant when looking at startup time of a large program. I just checked: the startup of an empty (about:blank) konqueror window calls into strcmp of ld.so 656,895 times, triggered by 38,095 symbol lookups. > > For meaningful results in the scope of redirections, I had to revert the > > instrument function back to *not* use the noredir_addr. I hope this is > > fine because I switch off chaising. > > Well, it might work, but it's not good. I wonder if there's a > better way to solve this. What is the exact problem here? If we have a redirected BB, is the problem that there can be multiple BBs for this redirected address? Or is this a problem with BB discarding? When a range with a nonredir address (!= redir) of a BB is discarded, I suppose you discard the redirected BB, too? And discard_basic_block_info only gives me the nonredir address? OK, then I would have a problem if a store the BB data by redirected address. The latter case currently is no problem, because of my design error ;-) Currently, I do not really react on BB discards. Josef |
|
From: Nicholas N. <nj...@cs...> - 2005-11-18 19:49:43
|
On Fri, 18 Nov 2005, Josef Weidendorfer wrote: > Still it feels wrong. A profiling tool should run the same code as in > reality. Why artificially lower the quality of the result? This is a good point. For example, I think it's good that tools like Callgrind don't have to override malloc() with the Valgrind versions, as was the case at one point. To do the equivalent here we can expose some of the redirection functions to tools so they can install any redirections they need in their pre_clo_init() function. This seems pretty simple. But it won't go into 3.1.0, it's too close to the freeze. Nick |
|
From: Julian S. <js...@ac...> - 2005-11-18 19:57:19
|
> > Still it feels wrong. A profiling tool should run the same code as in
> > reality. Why artificially lower the quality of the result?
>
> This is a good point.
I agree. I was just experimenting with this:
if (0==VG_(strcmp)("Memcheck", VG_(details).name)) {
add_redirect_sym_to_addr(
"soname:ld.so.1", "strlen",
(Addr)&VG_(ppc32_linux_REDIR_FOR_strlen)
);
add_redirect_sym_to_addr(
"soname:ld.so.1", "strcmp",
(Addr)&VG_(ppc32_linux_REDIR_FOR_strcmp)
);
}
Would that be an acceptable temporary fix for 3.1.0 that you'd be
happy with?
J
|
|
From: Josef W. <Jos...@gm...> - 2005-11-18 20:30:45
|
On Friday 18 November 2005 20:58, Julian Seward wrote:
> I agree. I was just experimenting with this:
>
> if (0==VG_(strcmp)("Memcheck", VG_(details).name)) {
>
> add_redirect_sym_to_addr(
> "soname:ld.so.1", "strlen",
> (Addr)&VG_(ppc32_linux_REDIR_FOR_strlen)
> );
> add_redirect_sym_to_addr(
> "soname:ld.so.1", "strcmp",
> (Addr)&VG_(ppc32_linux_REDIR_FOR_strcmp)
> );
>
> }
>
> Would that be an acceptable temporary fix for 3.1.0 that you'd be
> happy with?
Sure. Thanks.
Thinking about it, redirection needs are really specific to tools.
In the next VG version, they should be installed by the tool instead of the
core, as Nick said.
BTW: I just found a bug regarding the call graph on PPC. I have a problem
with conditional returns. They always are converted to conditional jumps
(which can get calls depending on the situation).
Of course this is because x86/amd64 does not have conditional returns.
If you do not receive a new RC from me by the time you want to do the
announcement, please still use the RC I sent to you yesterday.
I just thought about the BB discarding problem: Currently, I keep counters
for discarded BBs in the hope that the code will be mapped again. This is bad.
A solution would be to simply output the counters of these BBs at discarding
time. This probably is also a nice idea for cachegrind...
Josef
|
|
From: Nicholas N. <nj...@cs...> - 2005-11-18 20:59:56
|
On Fri, 18 Nov 2005, Josef Weidendorfer wrote: > Thinking about it, redirection needs are really specific to tools. > In the next VG version, they should be installed by the tool instead of the > core, as Nick said. Some of the redirections are needed by the core (eg. sysinfo page redirs) but some are needed by tools. > I just thought about the BB discarding problem: Currently, I keep counters > for discarded BBs in the hope that the code will be mapped again. This is bad. > A solution would be to simply output the counters of these BBs at discarding > time. This probably is also a nice idea for cachegrind... This is not an issue for Cachegrind because the counters are attributed to source code locations, which are independent of where BBs get mapped into memory. Nick |
|
From: Julian S. <js...@ac...> - 2005-11-18 21:25:54
|
> Sure. Thanks. Committed. > If you do not receive a new RC from me by the time you want to do the > announcement, please still use the RC I sent to you yesterday. Really I would like to have whatever final tarball you are happy with, to test together with all the other stuff, before making the announcement. Any chance you can make any final changes you want and send me a new tarball asap? J |