You can subscribe to this list here.
| 2002 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
(1) |
Oct
(122) |
Nov
(152) |
Dec
(69) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2003 |
Jan
(6) |
Feb
(25) |
Mar
(73) |
Apr
(82) |
May
(24) |
Jun
(25) |
Jul
(10) |
Aug
(11) |
Sep
(10) |
Oct
(54) |
Nov
(203) |
Dec
(182) |
| 2004 |
Jan
(307) |
Feb
(305) |
Mar
(430) |
Apr
(312) |
May
(187) |
Jun
(342) |
Jul
(487) |
Aug
(637) |
Sep
(336) |
Oct
(373) |
Nov
(441) |
Dec
(210) |
| 2005 |
Jan
(385) |
Feb
(480) |
Mar
(636) |
Apr
(544) |
May
(679) |
Jun
(625) |
Jul
(810) |
Aug
(838) |
Sep
(634) |
Oct
(521) |
Nov
(965) |
Dec
(543) |
| 2006 |
Jan
(494) |
Feb
(431) |
Mar
(546) |
Apr
(411) |
May
(406) |
Jun
(322) |
Jul
(256) |
Aug
(401) |
Sep
(345) |
Oct
(542) |
Nov
(308) |
Dec
(481) |
| 2007 |
Jan
(427) |
Feb
(326) |
Mar
(367) |
Apr
(255) |
May
(244) |
Jun
(204) |
Jul
(223) |
Aug
(231) |
Sep
(354) |
Oct
(374) |
Nov
(497) |
Dec
(362) |
| 2008 |
Jan
(322) |
Feb
(482) |
Mar
(658) |
Apr
(422) |
May
(476) |
Jun
(396) |
Jul
(455) |
Aug
(267) |
Sep
(280) |
Oct
(253) |
Nov
(232) |
Dec
(304) |
| 2009 |
Jan
(486) |
Feb
(470) |
Mar
(458) |
Apr
(423) |
May
(696) |
Jun
(461) |
Jul
(551) |
Aug
(575) |
Sep
(134) |
Oct
(110) |
Nov
(157) |
Dec
(102) |
| 2010 |
Jan
(226) |
Feb
(86) |
Mar
(147) |
Apr
(117) |
May
(107) |
Jun
(203) |
Jul
(193) |
Aug
(238) |
Sep
(300) |
Oct
(246) |
Nov
(23) |
Dec
(75) |
| 2011 |
Jan
(133) |
Feb
(195) |
Mar
(315) |
Apr
(200) |
May
(267) |
Jun
(293) |
Jul
(353) |
Aug
(237) |
Sep
(278) |
Oct
(611) |
Nov
(274) |
Dec
(260) |
| 2012 |
Jan
(303) |
Feb
(391) |
Mar
(417) |
Apr
(441) |
May
(488) |
Jun
(655) |
Jul
(590) |
Aug
(610) |
Sep
(526) |
Oct
(478) |
Nov
(359) |
Dec
(372) |
| 2013 |
Jan
(467) |
Feb
(226) |
Mar
(391) |
Apr
(281) |
May
(299) |
Jun
(252) |
Jul
(311) |
Aug
(352) |
Sep
(481) |
Oct
(571) |
Nov
(222) |
Dec
(231) |
| 2014 |
Jan
(185) |
Feb
(329) |
Mar
(245) |
Apr
(238) |
May
(281) |
Jun
(399) |
Jul
(382) |
Aug
(500) |
Sep
(579) |
Oct
(435) |
Nov
(487) |
Dec
(256) |
| 2015 |
Jan
(338) |
Feb
(357) |
Mar
(330) |
Apr
(294) |
May
(191) |
Jun
(108) |
Jul
(142) |
Aug
(261) |
Sep
(190) |
Oct
(54) |
Nov
(83) |
Dec
(22) |
| 2016 |
Jan
(49) |
Feb
(89) |
Mar
(33) |
Apr
(50) |
May
(27) |
Jun
(34) |
Jul
(53) |
Aug
(53) |
Sep
(98) |
Oct
(206) |
Nov
(93) |
Dec
(53) |
| 2017 |
Jan
(65) |
Feb
(82) |
Mar
(102) |
Apr
(86) |
May
(187) |
Jun
(67) |
Jul
(23) |
Aug
(93) |
Sep
(65) |
Oct
(45) |
Nov
(35) |
Dec
(17) |
| 2018 |
Jan
(26) |
Feb
(35) |
Mar
(38) |
Apr
(32) |
May
(8) |
Jun
(43) |
Jul
(27) |
Aug
(30) |
Sep
(43) |
Oct
(42) |
Nov
(38) |
Dec
(67) |
| 2019 |
Jan
(32) |
Feb
(37) |
Mar
(53) |
Apr
(64) |
May
(49) |
Jun
(18) |
Jul
(14) |
Aug
(53) |
Sep
(25) |
Oct
(30) |
Nov
(49) |
Dec
(31) |
| 2020 |
Jan
(87) |
Feb
(45) |
Mar
(37) |
Apr
(51) |
May
(99) |
Jun
(36) |
Jul
(11) |
Aug
(14) |
Sep
(20) |
Oct
(24) |
Nov
(40) |
Dec
(23) |
| 2021 |
Jan
(14) |
Feb
(53) |
Mar
(85) |
Apr
(15) |
May
(19) |
Jun
(3) |
Jul
(14) |
Aug
(1) |
Sep
(57) |
Oct
(73) |
Nov
(56) |
Dec
(22) |
| 2022 |
Jan
(3) |
Feb
(22) |
Mar
(6) |
Apr
(55) |
May
(46) |
Jun
(39) |
Jul
(15) |
Aug
(9) |
Sep
(11) |
Oct
(34) |
Nov
(20) |
Dec
(36) |
| 2023 |
Jan
(79) |
Feb
(41) |
Mar
(99) |
Apr
(169) |
May
(48) |
Jun
(16) |
Jul
(16) |
Aug
(57) |
Sep
(19) |
Oct
|
Nov
|
Dec
|
| S | M | T | W | T | F | S |
|---|---|---|---|---|---|---|
|
|
|
|
|
|
|
1
(24) |
|
2
(50) |
3
(26) |
4
(17) |
5
(18) |
6
(20) |
7
(20) |
8
(16) |
|
9
(19) |
10
(11) |
11
(17) |
12
(17) |
13
(20) |
14
(18) |
15
(18) |
|
16
(15) |
17
(27) |
18
(22) |
19
(31) |
20
(19) |
21
(21) |
22
(14) |
|
23
(14) |
24
(18) |
25
(15) |
26
|
27
(1) |
28
|
29
(2) |
|
30
(16) |
|
|
|
|
|
|
|
From: <sv...@va...> - 2012-09-24 21:50:24
|
philippe 2012-09-24 22:50:16 +0100 (Mon, 24 Sep 2012)
New Revision: 13015
Log:
fix n-i-bz report error for vgdb snapshot requested before execution
Massif does not accept to take snapshots of heap before execution has started.
So, if such a snapshot is requested (using vgdb and option --vgdb-error=0),
then such a snapshot must be refused rather than causing an assert.
(problem reported by dar...@ya...)
Modified files:
trunk/NEWS
trunk/massif/ms_main.c
Modified: trunk/NEWS (+1 -1)
===================================================================
--- trunk/NEWS 2012-09-24 22:37:02 +01:00 (rev 13014)
+++ trunk/NEWS 2012-09-24 22:50:16 +01:00 (rev 13015)
@@ -32,8 +32,8 @@
305948 [390] ppc64: code generation for ShlD64 / ShrD64 asserts
306054 [390] s390x: Condition code computation for convert-to-int/logical
307155 [390] filter_gdb should filter out syscall-template.S T_PSEUDO
+n-i-bz [390] report error for vgdb snapshot requested before execution
-
Release 3.8.1 (19 September 2012)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3.8.1 is a bug fix release. It fixes some assertion failures in 3.8.0
Modified: trunk/massif/ms_main.c (+9 -1)
===================================================================
--- trunk/massif/ms_main.c 2012-09-24 22:37:02 +01:00 (rev 13014)
+++ trunk/massif/ms_main.c 2012-09-24 22:50:16 +01:00 (rev 13015)
@@ -2392,9 +2392,17 @@
{
Snapshot snapshot;
+ if (!clo_pages_as_heap && !have_started_executing_code) {
+ // See comments of variable have_started_executing_code.
+ VG_(gdb_printf)
+ ("error: cannot take snapshot before execution has started\n");
+ return;
+ }
+
clear_snapshot(&snapshot, /* do_sanity_check */ False);
take_snapshot(&snapshot, Normal, get_time(), detailed);
- write_snapshots_to_file ((filename == NULL) ? (Char*) "massif.vgdb.out" : filename,
+ write_snapshots_to_file ((filename == NULL) ?
+ (Char*) "massif.vgdb.out" : filename,
&snapshot,
1);
delete_snapshot(&snapshot);
|
|
From: <sv...@va...> - 2012-09-24 21:37:09
|
philippe 2012-09-24 22:37:02 +0100 (Mon, 24 Sep 2012)
New Revision: 13014
Log:
Unbreak build on Mac_OS where __NR_mprotect is not defined.
The test is not executed on Darwin but should compile.
(based on fix suggestion by Rich Coe)
Modified files:
trunk/memcheck/tests/leak-segv-jmp.c
trunk/memcheck/tests/leak-segv-jmp.stderr.exp
Modified: trunk/memcheck/tests/leak-segv-jmp.stderr.exp (+4 -4)
===================================================================
--- trunk/memcheck/tests/leak-segv-jmp.stderr.exp 2012-09-24 22:12:41 +01:00 (rev 13013)
+++ trunk/memcheck/tests/leak-segv-jmp.stderr.exp 2012-09-24 22:37:02 +01:00 (rev 13014)
@@ -14,8 +14,8 @@
expecting a leak
1,000 bytes in 1 blocks are definitely lost in loss record ... of ...
at 0x........: malloc (vg_replace_malloc.c:...)
- by 0x........: f (leak-segv-jmp.c:167)
- by 0x........: main (leak-segv-jmp.c:214)
+ by 0x........: f (leak-segv-jmp.c:171)
+ by 0x........: main (leak-segv-jmp.c:218)
LEAK SUMMARY:
definitely lost: 1,000 bytes in 1 blocks
@@ -30,8 +30,8 @@
expecting a leak again
1,000 bytes in 1 blocks are definitely lost in loss record ... of ...
at 0x........: malloc (vg_replace_malloc.c:...)
- by 0x........: f (leak-segv-jmp.c:167)
- by 0x........: main (leak-segv-jmp.c:214)
+ by 0x........: f (leak-segv-jmp.c:171)
+ by 0x........: main (leak-segv-jmp.c:218)
LEAK SUMMARY:
definitely lost: 1,000 bytes in 1 blocks
Modified: trunk/memcheck/tests/leak-segv-jmp.c (+4 -0)
===================================================================
--- trunk/memcheck/tests/leak-segv-jmp.c 2012-09-24 22:12:41 +01:00 (rev 13013)
+++ trunk/memcheck/tests/leak-segv-jmp.c 2012-09-24 22:37:02 +01:00 (rev 13014)
@@ -136,6 +136,10 @@
}
#else
+// Ensure the file compiles even if the syscall nr is not defined.
+#ifndef __NR_mprotect
+#define __NR_mprotect 0
+#endif
UWord do_syscall_WRK (UWord syscall_no,
UWord a1, UWord a2, UWord a3,
UWord a4, UWord a5, UWord a6
|
|
From: <sv...@va...> - 2012-09-24 21:12:52
|
philippe 2012-09-24 22:12:41 +0100 (Mon, 24 Sep 2012)
New Revision: 13013
Log:
fix 307155 filter_gdb should filter out syscall-template.S T_PSEUDO
With some glibc version (e.g. on fedora 16), gdb output contains
a line with T_PSEUDO which should be filtered out.
Patch from Mark Wielaard.
Modified files:
trunk/NEWS
trunk/gdbserver_tests/filter_gdb
Modified: trunk/NEWS (+1 -1)
===================================================================
--- trunk/NEWS 2012-09-23 01:42:49 +01:00 (rev 13012)
+++ trunk/NEWS 2012-09-24 22:12:41 +01:00 (rev 13013)
@@ -31,9 +31,9 @@
275800 [390] s390x: Add support for the ecag instruction (part 1)
305948 [390] ppc64: code generation for ShlD64 / ShrD64 asserts
306054 [390] s390x: Condition code computation for convert-to-int/logical
+307155 [390] filter_gdb should filter out syscall-template.S T_PSEUDO
-
Release 3.8.1 (19 September 2012)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3.8.1 is a bug fix release. It fixes some assertion failures in 3.8.0
Modified: trunk/gdbserver_tests/filter_gdb (+1 -0)
===================================================================
--- trunk/gdbserver_tests/filter_gdb 2012-09-23 01:42:49 +01:00 (rev 13012)
+++ trunk/gdbserver_tests/filter_gdb 2012-09-24 22:12:41 +01:00 (rev 13013)
@@ -85,6 +85,7 @@
-e '/^[ ]*in \.\.\/sysdeps\/unix\/syscall-template\.S/d' \
-e '/^[1-9][0-9]*[ ]*\.\.\/sysdeps\/unix\/syscall-template\.S/d' \
-e '/^[1-9][0-9]*[ ]in *\.\.\/sysdeps\/unix\/syscall-template\.S/d' \
+ -e '/^[1-9][0-9]*[ ]T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)/d' \
-e 's/\(Could not write register \)".*"/\1 "xxx"/' \
-e 's/\(ERROR changing register \).*$/\1 xxx regno y/' \
-e 's/0x........ in \(main (argc=1, argv=0x........) at watchpoints.c:[24][3689]\)/\1/' \
|
|
From: Josef W. <Jos...@gm...> - 2012-09-24 19:15:05
|
Am 24.09.2012 20:34, schrieb Julian Seward: > On Monday, September 24, 2012, Carl E. Love wrote: > >> I was just looking into the POWER architectures a bit more to make sure >> they would be reasonably easy to support. Just to be clear, are there >> any Valgrind restrictions on the cache sizes, specifically must be a >> power of 2? >> >> I see the Power 5 has an L2 unified cache of 1.875MB and and L3 unified, >> shared cache of size 36MB. I was doing some cache studies last year and >> I remember there being issues where the cache size must be a power of 2. >> I don't remember what tool it was now that had that restriction. >> >> Similarly, must the Valgrind cache associativity be a power of 2? The >> POWER 5 processor's L2 cache is 10-way set associative. > > The same thing happens with server-level CPUs from Intel and AMD. > > I think Florian is presenting a general mechanism that allows recording of > the cache details, regardless of whether they are something the various > tools can handle, or not. And I think that's the right approach. > > As you say, though, some of the cache simulators have problems with > non-power-of-2 sizes or associativities (can't remember which), so that > the number of cache sets isn't a power of 2. So far that has been kludged > up by postprocessing the cache info so as (in the right circumstances) > increase the stated associativity by 50% (eg, a factor of 3/2) and > decreasing the number of lines by the same factor, so as to make the > number of lines be a power of 2 whilst not changing the overall capacity > of the cache that is simulated. > > This kind of gets around the problem for cache sizes of (eg) 12MB > (viz, 3/2 * 8MB) but does not fix it for cache sizes of (eg) 10MB > since there is no code to do rescaling for the ratio 5/4. > > This stuff (+ big comment) is in get_caches_from_CPUID in > cachegrind/cg-x86-amd64.c. If you or anybody else wants to do the 5/4 > rescaling case, pls feel free :) This is not possible with trying to keep the cache size the same and the number of sets a power of two. It is important for the performance of the simulator to have a fast way to get from address to set number. Bit mangling is fine, but modulo is prohibitively slow. Better are small lookup tables. If the number of sets is a power of two, the "best" way is somehow straightforward: every cache should cover the address space in an as uniform way as possible. So let's take to lowest bits of the address (not counting the bits needed for the offset inside of a cache line). For every other number of sets, the best solution is not obvious. Josef |
|
From: Carl E. L. <ce...@li...> - 2012-09-24 19:11:45
|
On Mon, 2012-09-24 at 20:34 +0200, Julian Seward wrote: > On Monday, September 24, 2012, Carl E. Love wrote: > > > I was just looking into the POWER architectures a bit more to make sure > > they would be reasonably easy to support. Just to be clear, are there > > any Valgrind restrictions on the cache sizes, specifically must be a > > power of 2? > > > > I see the Power 5 has an L2 unified cache of 1.875MB and and L3 unified, > > shared cache of size 36MB. I was doing some cache studies last year and > > I remember there being issues where the cache size must be a power of 2. > > I don't remember what tool it was now that had that restriction. > > > > Similarly, must the Valgrind cache associativity be a power of 2? The > > POWER 5 processor's L2 cache is 10-way set associative. > > The same thing happens with server-level CPUs from Intel and AMD. > > I think Florian is presenting a general mechanism that allows recording of > the cache details, regardless of whether they are something the various > tools can handle, or not. And I think that's the right approach. > > As you say, though, some of the cache simulators have problems with > non-power-of-2 sizes or associativities (can't remember which), so that > the number of cache sets isn't a power of 2. So far that has been kludged > up by postprocessing the cache info so as (in the right circumstances) > increase the stated associativity by 50% (eg, a factor of 3/2) and > decreasing the number of lines by the same factor, so as to make the > number of lines be a power of 2 whilst not changing the overall capacity > of the cache that is simulated. > > This kind of gets around the problem for cache sizes of (eg) 12MB > (viz, 3/2 * 8MB) but does not fix it for cache sizes of (eg) 10MB > since there is no code to do rescaling for the ratio 5/4. > > This stuff (+ big comment) is in get_caches_from_CPUID in > cachegrind/cg-x86-amd64.c. If you or anybody else wants to do the 5/4 > rescaling case, pls feel free :) > > I suppose this stuff should get lifted out, as part of Florian's reorg, > and made general. > > J Yup, from the general recording of cache size, line size, type, associativity the data structures seem to cover all of the info needed for POWER. The hope would be to see if some of the lower code requirements on powers of two could be removed with the code restructuring. I haven't really dived into the cachegrind and other tool implementations to see why the restrictions are there or what it would take to change the restrictions. Good to know that the power of 2 issue is not just a POWER problem. > |
|
From: Josef W. <Jos...@gm...> - 2012-09-24 18:43:56
|
Am 24.09.2012 20:18, schrieb Carl E. Love: > I was just looking into the POWER architectures a bit more to make sure > they would be reasonably easy to support. Just to be clear, are there > any Valgrind restrictions on the cache sizes, specifically must be a > power of 2? The simulator in Cachegrind/Callgrind currently has various constrains about the sizes: * the cache line size in bytes must be a power of two (at least 16B) * the number of sets (= cache size / line size / associativity) must be a power of two. This is used to allow fast set calculation from address of a memory access. It is already now the case that sometimes values are adjusted (with an corresponding warning printed) to make the simulator happy. If that seems impossible, the simulator will error out, and ask the user to specify parameters via command line. However, this all is the responsibility of the tool. The interface asking for hardware parameters seems fine to me, and should be able to return any numbers. > I see the Power 5 has an L2 unified cache of 1.875MB and and L3 unified, > shared cache of size 36MB. I was doing some cache studies last year and > I remember there being issues where the cache size must be a power of 2. > I don't remember what tool it was now that had that restriction. > > Similarly, must the Valgrind cache associativity be a power of 2? No. > The > POWER 5 processor's L2 cache is 10-way set associative. Josef |
|
From: Julian S. <js...@ac...> - 2012-09-24 18:35:50
|
On Monday, September 24, 2012, Carl E. Love wrote: > I was just looking into the POWER architectures a bit more to make sure > they would be reasonably easy to support. Just to be clear, are there > any Valgrind restrictions on the cache sizes, specifically must be a > power of 2? > > I see the Power 5 has an L2 unified cache of 1.875MB and and L3 unified, > shared cache of size 36MB. I was doing some cache studies last year and > I remember there being issues where the cache size must be a power of 2. > I don't remember what tool it was now that had that restriction. > > Similarly, must the Valgrind cache associativity be a power of 2? The > POWER 5 processor's L2 cache is 10-way set associative. The same thing happens with server-level CPUs from Intel and AMD. I think Florian is presenting a general mechanism that allows recording of the cache details, regardless of whether they are something the various tools can handle, or not. And I think that's the right approach. As you say, though, some of the cache simulators have problems with non-power-of-2 sizes or associativities (can't remember which), so that the number of cache sets isn't a power of 2. So far that has been kludged up by postprocessing the cache info so as (in the right circumstances) increase the stated associativity by 50% (eg, a factor of 3/2) and decreasing the number of lines by the same factor, so as to make the number of lines be a power of 2 whilst not changing the overall capacity of the cache that is simulated. This kind of gets around the problem for cache sizes of (eg) 12MB (viz, 3/2 * 8MB) but does not fix it for cache sizes of (eg) 10MB since there is no code to do rescaling for the ratio 5/4. This stuff (+ big comment) is in get_caches_from_CPUID in cachegrind/cg-x86-amd64.c. If you or anybody else wants to do the 5/4 rescaling case, pls feel free :) I suppose this stuff should get lifted out, as part of Florian's reorg, and made general. J |
|
From: Carl E. L. <ce...@li...> - 2012-09-24 18:20:12
|
On Fri, 2012-09-21 at 13:01 -0400, Florian Krohm wrote:
> We had a discussion about this a few weeks back. Here are my thoughts.
>
>
> Objective:
> ----------
> Have coregrind query the properties of the host's cache system. Make
> this information available in a simple interface that hides all
> architecture-specific details e.g. the existence of a cpuid instruction.
>
>
> Benefits:
> ---------
> This is conceptually cleaner than the status quo. Detection of cache
> properties does not belong in the realm of the tools. Additionally,
> if several tools required information about caches there would be code
> duplication.
>
>
> Representation of cache information:
> ------------------------------------
>
> /* The various kinds of caches */
> typedef enum {
> DATA_CACHE,
> INSN_CACHE,
> DATA_INSN_CACHE // combined data and insn cache
> } cache_kind;
>
> /* Information about a particular cache */
> typedef struct {
> cache_kind kind;
> UInt level; /* level this cache is at, e.g. 1 for L1 cache */
> UInt sizeB; /* size of this cache in bytes */
> UInt line_sizeB; /* cache line size in bytes */
> UInt associativity;
> } cache_t;
>
> /* Information about the cache system as a whole */
> typedef struct {
> UInt num_levels;
> UInt num_caches;
> /* Unordered array of caches for this host. NULL if there are
> no caches. Users can assume that the array contains at most one
> cache of a given kind per cache level. */
> cache_t *caches;
> } cacheinfo_t;
>
I was just looking into the POWER architectures a bit more to make sure
they would be reasonably easy to support. Just to be clear, are there
any Valgrind restrictions on the cache sizes, specifically must be a
power of 2?
I see the Power 5 has an L2 unified cache of 1.875MB and and L3 unified,
shared cache of size 36MB. I was doing some cache studies last year and
I remember there being issues where the cache size must be a power of 2.
I don't remember what tool it was now that had that restriction.
Similarly, must the Valgrind cache associativity be a power of 2? The
POWER 5 processor's L2 cache is 10-way set associative.
<snip>
|
|
From: Tom H. <to...@co...> - 2012-09-24 17:40:53
|
On 24/09/12 18:37, Petar Jovanovic wrote: > @Julian,Tom > So, for the si_code, the answer is 'no', the value was not correct. The issue is > that the si_code variable can have two values (see the part of the original code > that we are deleting with this patch). The value in si_code depends on the > 'code' field in the TEQ instruction. So, with a single synth_sigfpe we can only > cover one case, and ignore the second case (or maybe even assert for this case > in priv/guest_mips_toIR.c, which we should avoid, if possible - see the change > in the attached patch - I would like to remove that but I am leaving it for the > sake of conversation). Should we have two Ijk_SigFPEs, like Ijk_SigFPE1 and > Ijk_SigFPE2, and multiply the rest of the code needed for this? Or something > else? Yes, generating the right code is a sticky issue. I'm pretty sure some of the existing synthesised signals get it wrong because of this lack of information. What we've generally done I think is punted and just generated one of the codes, or maybe zero and hoped nothing minds too much about the details being exactly right. Tom -- Tom Hughes (to...@co...) http://compton.nu/ |
|
From: Petar J. <mip...@gm...> - 2012-09-24 17:38:01
|
@Julian
Which line has exceeded 80 chars? The extra char ('+/-') in the patch might have
caused it to look that way.
@Julian,Tom
So, for the si_code, the answer is 'no', the value was not correct. The issue is
that the si_code variable can have two values (see the part of the original code
that we are deleting with this patch). The value in si_code depends on the
'code' field in the TEQ instruction. So, with a single synth_sigfpe we can only
cover one case, and ignore the second case (or maybe even assert for this case
in priv/guest_mips_toIR.c, which we should avoid, if possible - see the change
in the attached patch - I would like to remove that but I am leaving it for the
sake of conversation). Should we have two Ijk_SigFPEs, like Ijk_SigFPE1 and
Ijk_SigFPE2, and multiply the rest of the code needed for this? Or something
else?
P.
Index: coregrind/m_signals.c
===================================================================
--- coregrind/m_signals.c (revision 13012)
+++ coregrind/m_signals.c (working copy)
@@ -1919,27 +1919,6 @@
info.si_signo = VKI_SIGTRAP;
info.si_code = VKI_TRAP_BRKPT; /* tjh: only ever called for a brkpt ins */
-# if defined(VGP_mips32_linux) || defined(VGP_mips64_linux)
- /* This is for teq on mips. Teq on mips for ins: 0xXXX1f4
- * cases VKI_SIGFPE not VKI_SIGTRAP
- */
- // JRS 2012-Jun-06: commented out until we know we need it
- // This isn't a clean solution; need something that avoids looking
- // at the guest code.
- //UInt *ins = (void*)(vgPlain_threads[tid].arch.vex.guest_PC-4);
- //UInt tcode = (((*ins) >> 6) & ((1 << 10) - 1));
- //if (tcode == VKI_BRK_OVERFLOW || tcode == VKI_BRK_DIVZERO) {
- // if (tcode == VKI_BRK_DIVZERO)
- // info.si_code = VKI_FPE_INTDIV;
- // else
- // info.si_code = VKI_FPE_INTOVF;
- // info.si_signo = VKI_SIGFPE;
- // info.si_errno = 0;
- // info.VKI_SIGINFO_si_addr
- // = (void*)(vgPlain_threads[tid].arch.vex.guest_PC-4);
- //}
-# endif
-
# if defined(VGP_x86_linux) || defined(VGP_amd64_linux)
uc.uc_mcontext.trapno = 3; /* tjh: this is the x86 trap number
for a breakpoint trap... */
@@ -1962,6 +1941,32 @@
resume_scheduler(tid);
}
+// Synthesise a SIGFPE.
+void VG_(synth_sigfpe)(ThreadId tid)
+{
+// Only tested on mips32
+#if !defined(VGA_mips32)
+ vg_assert(0);
+#else
+ vki_siginfo_t info;
+ struct vki_ucontext uc;
+
+ vg_assert(VG_(threads)[tid].status == VgTs_Runnable);
+
+ VG_(memset)(&info, 0, sizeof(info));
+ VG_(memset)(&uc, 0, sizeof(uc));
+ info.si_signo = VKI_SIGFPE;
+ info.si_code = VKI_FPE_INTDIV;
+
+ if (VG_(gdbserver_report_signal) (VKI_SIGFPE, tid)) {
+ resume_scheduler(tid);
+ deliver_signal(tid, &info, &uc);
+ }
+ else
+ resume_scheduler(tid);
+#endif
+}
+
/* Make a signal pending for a thread, for later delivery.
VG_(poll_signals) will arrange for it to be delivered at the right
time.
Index: coregrind/m_scheduler/scheduler.c
===================================================================
--- coregrind/m_scheduler/scheduler.c (revision 13012)
+++ coregrind/m_scheduler/scheduler.c (working copy)
@@ -200,6 +200,7 @@
case VEX_TRC_JMP_SIGTRAP: return "SIGTRAP";
case VEX_TRC_JMP_SIGSEGV: return "SIGSEGV";
case VEX_TRC_JMP_SIGBUS: return "SIGBUS";
+ case VEX_TRC_JMP_SIGFPE: return "SIGFPE";
case VEX_TRC_JMP_EMWARN: return "EMWARN";
case VEX_TRC_JMP_EMFAIL: return "EMFAIL";
case VEX_TRC_JMP_CLIENTREQ: return "CLIENTREQ";
@@ -1424,6 +1425,10 @@
VG_(synth_sigbus)(tid);
break;
+ case VEX_TRC_JMP_SIGFPE:
+ VG_(synth_sigfpe)(tid);
+ break;
+
case VEX_TRC_JMP_NODECODE: {
Addr addr = VG_(get_IP)(tid);
Index: coregrind/pub_core_signals.h
===================================================================
--- coregrind/pub_core_signals.h (revision 13012)
+++ coregrind/pub_core_signals.h (working copy)
@@ -77,6 +77,7 @@
extern void VG_(synth_sigill) (ThreadId tid, Addr addr);
extern void VG_(synth_sigtrap) (ThreadId tid);
extern void VG_(synth_sigbus) (ThreadId tid);
+extern void VG_(synth_sigfpe) (ThreadId tid);
/* Extend the stack to cover addr, if possible */
extern Bool VG_(extend_stack)(Addr addr, UInt maxsize);
Index: VEX/priv/host_mips_isel.c
===================================================================
--- VEX/priv/host_mips_isel.c (revision 2544)
+++ VEX/priv/host_mips_isel.c (working copy)
@@ -3049,6 +3049,7 @@
case Ijk_NoRedir:
case Ijk_SigBUS:
case Ijk_SigTRAP:
+ case Ijk_SigFPE:
case Ijk_Sys_syscall:
case Ijk_TInval:
{
Index: VEX/priv/ir_defs.c
===================================================================
--- VEX/priv/ir_defs.c (revision 2544)
+++ VEX/priv/ir_defs.c (working copy)
@@ -1231,6 +1231,7 @@
case Ijk_SigTRAP: vex_printf("SigTRAP"); break;
case Ijk_SigSEGV: vex_printf("SigSEGV"); break;
case Ijk_SigBUS: vex_printf("SigBUS"); break;
+ case Ijk_SigFPE: vex_printf("SigFPE"); break;
case Ijk_Sys_syscall: vex_printf("Sys_syscall"); break;
case Ijk_Sys_int32: vex_printf("Sys_int32"); break;
case Ijk_Sys_int128: vex_printf("Sys_int128"); break;
Index: VEX/priv/guest_mips_toIR.c
===================================================================
--- VEX/priv/guest_mips_toIR.c (revision 2544)
+++ VEX/priv/guest_mips_toIR.c (working copy)
@@ -3126,43 +3126,84 @@
case 0x30: { /* TGE */
/*tge */ DIP("tge r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt), getIReg (rs)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt),
getIReg (rs)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else if (trap_code == 6)
+ goto decode_failure;
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt),
getIReg (rs)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x31: { /* TGEU */
/*tgeu */ DIP("tgeu r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt), getIReg (rs)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt),
getIReg (rs)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else if (trap_code == 6)
+ goto decode_failure;
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt),
getIReg (rs)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x32: { /* TLT */
/*tlt */ DIP("tlt r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs),
getIReg (rt)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else if (trap_code == 6)
+ goto decode_failure;
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs),
getIReg (rt)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x33: { /* TLTU */
/*tltu */ DIP("tltu r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs),
getIReg (rt)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else if (trap_code == 6)
+ goto decode_failure;
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs),
getIReg (rt)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x34: { /* TEQ */
/*teq */ DIP("teq r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP, IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 7)
+ stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg (rt)),
+ Ijk_SigFPE, IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ else if (trap_code == 6)
+ goto decode_failure;
+ else
+ stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg (rt)),
+ Ijk_SigTRAP, IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x36: { /* TNE */
/*tne */ DIP("tne r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg (rt)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else if (trap_code == 6)
+ goto decode_failure;
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg (rt)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x0F: {
Index: VEX/priv/host_mips_defs.c
===================================================================
--- VEX/priv/host_mips_defs.c (revision 2544)
+++ VEX/priv/host_mips_defs.c (working copy)
@@ -3296,7 +3296,8 @@
case Ijk_NoRedir: trcval = VEX_TRC_JMP_NOREDIR; break;
case Ijk_SigTRAP: trcval = VEX_TRC_JMP_SIGTRAP; break;
//case Ijk_SigSEGV: trcval = VEX_TRC_JMP_SIGSEGV; break;
- case Ijk_SigBUS: trcval = VEX_TRC_JMP_SIGBUS; break;
+ case Ijk_SigBUS: trcval = VEX_TRC_JMP_SIGBUS; break;
+ case Ijk_SigFPE: trcval = VEX_TRC_JMP_SIGFPE; break;
case Ijk_Boring: trcval = VEX_TRC_JMP_BORING; break;
/* We don't expect to see the following being assisted. */
//case Ijk_Ret:
Index: VEX/pub/libvex_ir.h
===================================================================
--- VEX/pub/libvex_ir.h (revision 2544)
+++ VEX/pub/libvex_ir.h (working copy)
@@ -1872,6 +1872,7 @@
Ijk_SigTRAP, /* current instruction synths SIGTRAP */
Ijk_SigSEGV, /* current instruction synths SIGSEGV */
Ijk_SigBUS, /* current instruction synths SIGBUS */
+ Ijk_SigFPE, /* current instruction synths SIGFPE */
/* Unfortunately, various guest-dependent syscall kinds. They
all mean: do a syscall before continuing. */
Ijk_Sys_syscall, /* amd64 'syscall', ppc 'sc', arm 'svc #0' */
Index: VEX/pub/libvex_trc_values.h
===================================================================
--- VEX/pub/libvex_trc_values.h (revision 2544)
+++ VEX/pub/libvex_trc_values.h (working copy)
@@ -62,6 +62,8 @@
continuing */
#define VEX_TRC_JMP_SIGBUS 93 /* deliver SIGBUS before continuing */
+#define VEX_TRC_JMP_SIGFPE 97 /* deliver SIGFPE before continuing */
+
#define VEX_TRC_JMP_EMWARN 63 /* deliver emulation warning before
continuing */
#define VEX_TRC_JMP_EMFAIL 83 /* emulation fatal error; abort system */
|
|
From: Julian S. <js...@ac...> - 2012-09-24 15:10:18
|
On Monday, September 24, 2012, Tom Hughes wrote:
> On 24/09/12 15:35, Petar Jovanovic wrote:
> > +// Synthesise a SIGFPE.
> > +void VG_(synth_sigfpe)(ThreadId tid)
> > +{
> > + vki_siginfo_t info;
> > + struct vki_ucontext uc;
> > +
> > + vg_assert(VG_(threads)[tid].status == VgTs_Runnable);
> > +
> > + VG_(memset)(&info, 0, sizeof(info));
> > + VG_(memset)(&uc, 0, sizeof(uc));
> > + info.si_signo = VKI_SIGFPE;
> > + info.si_code = VKI_TRAP_BRKPT;
>
> Is that really the right code value?
Hmm, good point. Maybe not. Petar, if this is a mips-specific
value, then I think it would be safer to put it inside ifdef(VGA_mips32),
and put vg_assert(0) for all other platforms, since right now
it is only on mips that this will be used.
J
|
|
From: Tom H. <to...@co...> - 2012-09-24 14:58:17
|
On 24/09/12 15:35, Petar Jovanovic wrote:
> +// Synthesise a SIGFPE.
> +void VG_(synth_sigfpe)(ThreadId tid)
> +{
> + vki_siginfo_t info;
> + struct vki_ucontext uc;
> +
> + vg_assert(VG_(threads)[tid].status == VgTs_Runnable);
> +
> + VG_(memset)(&info, 0, sizeof(info));
> + VG_(memset)(&uc, 0, sizeof(uc));
> + info.si_signo = VKI_SIGFPE;
> + info.si_code = VKI_TRAP_BRKPT;
Is that really the right code value?
Tom
--
Tom Hughes (to...@co...)
http://compton.nu/
|
|
From: Julian S. <js...@ac...> - 2012-09-24 14:47:58
|
Yes, looks fine (OK to land). Very minor thing -- the guest_mips_toIR.c
changes, can you limit the width to 80 chars pls? I try to keep the
width to 80 chars for efficiency of screen space use.
J
On Monday, September 24, 2012, Petar Jovanovic wrote:
> Hi Julian,
>
> thanks for the reply. Sure it makes sense.
>
> Check the patch below.
>
> Regards,
> Petar
>
> Index: coregrind/m_signals.c
> ===================================================================
> --- coregrind/m_signals.c (revision 13012)
> +++ coregrind/m_signals.c (working copy)
> @@ -1919,27 +1919,6 @@
> info.si_signo = VKI_SIGTRAP;
> info.si_code = VKI_TRAP_BRKPT; /* tjh: only ever called for a brkpt ins
> */
>
> -# if defined(VGP_mips32_linux) || defined(VGP_mips64_linux)
> - /* This is for teq on mips. Teq on mips for ins: 0xXXX1f4
> - * cases VKI_SIGFPE not VKI_SIGTRAP
> - */
> - // JRS 2012-Jun-06: commented out until we know we need it
> - // This isn't a clean solution; need something that avoids looking
> - // at the guest code.
> - //UInt *ins = (void*)(vgPlain_threads[tid].arch.vex.guest_PC-4);
> - //UInt tcode = (((*ins) >> 6) & ((1 << 10) - 1));
> - //if (tcode == VKI_BRK_OVERFLOW || tcode == VKI_BRK_DIVZERO) {
> - // if (tcode == VKI_BRK_DIVZERO)
> - // info.si_code = VKI_FPE_INTDIV;
> - // else
> - // info.si_code = VKI_FPE_INTOVF;
> - // info.si_signo = VKI_SIGFPE;
> - // info.si_errno = 0;
> - // info.VKI_SIGINFO_si_addr
> - // = (void*)(vgPlain_threads[tid].arch.vex.guest_PC-4);
> - //}
> -# endif
> -
> # if defined(VGP_x86_linux) || defined(VGP_amd64_linux)
> uc.uc_mcontext.trapno = 3; /* tjh: this is the x86 trap number
> for a breakpoint trap... */
> @@ -1962,6 +1941,27 @@
> resume_scheduler(tid);
> }
>
> +// Synthesise a SIGFPE.
> +void VG_(synth_sigfpe)(ThreadId tid)
> +{
> + vki_siginfo_t info;
> + struct vki_ucontext uc;
> +
> + vg_assert(VG_(threads)[tid].status == VgTs_Runnable);
> +
> + VG_(memset)(&info, 0, sizeof(info));
> + VG_(memset)(&uc, 0, sizeof(uc));
> + info.si_signo = VKI_SIGFPE;
> + info.si_code = VKI_TRAP_BRKPT;
> +
> + if (VG_(gdbserver_report_signal) (VKI_SIGFPE, tid)) {
> + resume_scheduler(tid);
> + deliver_signal(tid, &info, &uc);
> + }
> + else
> + resume_scheduler(tid);
> +}
> +
> /* Make a signal pending for a thread, for later delivery.
> VG_(poll_signals) will arrange for it to be delivered at the right
> time.
> Index: coregrind/pub_core_signals.h
> ===================================================================
> --- coregrind/pub_core_signals.h (revision 13012)
> +++ coregrind/pub_core_signals.h (working copy)
> @@ -77,6 +77,7 @@
> extern void VG_(synth_sigill) (ThreadId tid, Addr addr);
> extern void VG_(synth_sigtrap) (ThreadId tid);
> extern void VG_(synth_sigbus) (ThreadId tid);
> +extern void VG_(synth_sigfpe) (ThreadId tid);
>
> /* Extend the stack to cover addr, if possible */
> extern Bool VG_(extend_stack)(Addr addr, UInt maxsize);
> Index: coregrind/m_scheduler/scheduler.c
> ===================================================================
> --- coregrind/m_scheduler/scheduler.c (revision 13012)
> +++ coregrind/m_scheduler/scheduler.c (working copy)
> @@ -200,6 +200,7 @@
> case VEX_TRC_JMP_SIGTRAP: return "SIGTRAP";
> case VEX_TRC_JMP_SIGSEGV: return "SIGSEGV";
> case VEX_TRC_JMP_SIGBUS: return "SIGBUS";
> + case VEX_TRC_JMP_SIGFPE: return "SIGFPE";
> case VEX_TRC_JMP_EMWARN: return "EMWARN";
> case VEX_TRC_JMP_EMFAIL: return "EMFAIL";
> case VEX_TRC_JMP_CLIENTREQ: return "CLIENTREQ";
> @@ -1424,6 +1425,10 @@
> VG_(synth_sigbus)(tid);
> break;
>
> + case VEX_TRC_JMP_SIGFPE:
> + VG_(synth_sigfpe)(tid);
> + break;
> +
> case VEX_TRC_JMP_NODECODE: {
> Addr addr = VG_(get_IP)(tid);
>
> Index: VEX/priv/host_mips_defs.c
> ===================================================================
> --- VEX/priv/host_mips_defs.c (revision 2544)
> +++ VEX/priv/host_mips_defs.c (working copy)
> @@ -3296,7 +3296,8 @@
> case Ijk_NoRedir: trcval = VEX_TRC_JMP_NOREDIR; break;
> case Ijk_SigTRAP: trcval = VEX_TRC_JMP_SIGTRAP; break;
> //case Ijk_SigSEGV: trcval = VEX_TRC_JMP_SIGSEGV;
> break; - case Ijk_SigBUS: trcval = VEX_TRC_JMP_SIGBUS;
> break; + case Ijk_SigBUS: trcval = VEX_TRC_JMP_SIGBUS;
> break; + case Ijk_SigFPE: trcval = VEX_TRC_JMP_SIGFPE;
> break; case Ijk_Boring: trcval = VEX_TRC_JMP_BORING; break;
> /* We don't expect to see the following being assisted. */ //case Ijk_Ret:
> Index: VEX/priv/ir_defs.c
> ===================================================================
> --- VEX/priv/ir_defs.c (revision 2544)
> +++ VEX/priv/ir_defs.c (working copy)
> @@ -1231,6 +1231,7 @@
> case Ijk_SigTRAP: vex_printf("SigTRAP"); break;
> case Ijk_SigSEGV: vex_printf("SigSEGV"); break;
> case Ijk_SigBUS: vex_printf("SigBUS"); break;
> + case Ijk_SigFPE: vex_printf("SigFPE"); break;
> case Ijk_Sys_syscall: vex_printf("Sys_syscall"); break;
> case Ijk_Sys_int32: vex_printf("Sys_int32"); break;
> case Ijk_Sys_int128: vex_printf("Sys_int128"); break;
> Index: VEX/priv/guest_mips_toIR.c
> ===================================================================
> --- VEX/priv/guest_mips_toIR.c (revision 2544)
> +++ VEX/priv/guest_mips_toIR.c (working copy)
> @@ -3126,43 +3126,72 @@
>
> case 0x30: { /* TGE */
> /*tge */ DIP("tge r%d, r%d %d", rs, rt, trap_code);
> - stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt), getIReg
> (rs)), - Ijk_SigTRAP,
> - IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC)); + if (trap_code == 6 || trap_code == 7)
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt),
> getIReg (rs)),
> + Ijk_SigFPE,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> + else
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt),
> getIReg (rs)),
> + Ijk_SigTRAP,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> break;
> }
> case 0x31: { /* TGEU */
> /*tgeu */ DIP("tgeu r%d, r%d %d", rs, rt, trap_code);
> - stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt), getIReg
> (rs)), - Ijk_SigTRAP,
> - IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC)); + if (trap_code == 6 || trap_code == 7)
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt),
> getIReg (rs)),
> + Ijk_SigFPE,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> + else
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt),
> getIReg (rs)),
> + Ijk_SigTRAP,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> break;
> }
> case 0x32: { /* TLT */
> /*tlt */ DIP("tlt r%d, r%d %d", rs, rt, trap_code);
> - stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs), getIReg
> (rt)), - Ijk_SigTRAP,
> - IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC)); + if (trap_code == 6 || trap_code == 7)
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs),
> getIReg (rt)),
> + Ijk_SigFPE,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> + else
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs),
> getIReg (rt)),
> + Ijk_SigTRAP,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> break;
> }
> case 0x33: { /* TLTU */
> /*tltu */ DIP("tltu r%d, r%d %d", rs, rt, trap_code);
> - stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs), getIReg
> (rt)), - Ijk_SigTRAP,
> - IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC)); + if (trap_code == 6 || trap_code == 7)
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs),
> getIReg (rt)),
> + Ijk_SigFPE,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> + else
> + stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs),
> getIReg (rt)),
> + Ijk_SigTRAP,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> break;
> }
> case 0x34: { /* TEQ */
> /*teq */ DIP("teq r%d, r%d %d", rs, rt, trap_code);
> - stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg
> (rt)), - Ijk_SigTRAP, IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC)); + if (trap_code == 6 || trap_code == 7)
> + stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg
> (rt)), + Ijk_SigFPE, IRConst_U32 (guest_PC_curr_instr +
> 4), OFFB_PC)); + else
> + stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg
> (rt)), + Ijk_SigTRAP, IRConst_U32 (guest_PC_curr_instr +
> 4), OFFB_PC));
> break;
> }
> case 0x36: { /* TNE */
> /*tne */ DIP("tne r%d, r%d %d", rs, rt, trap_code);
> - stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg
> (rt)), - Ijk_SigTRAP,
> - IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC)); + if (trap_code == 6 || trap_code == 7)
> + stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg
> (rt)), + Ijk_SigFPE,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> + else
> + stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg
> (rt)), + Ijk_SigTRAP,
> + IRConst_U32 (guest_PC_curr_instr + 4),
> OFFB_PC));
> break;
> }
> case 0x0F: {
> Index: VEX/priv/host_mips_isel.c
> ===================================================================
> --- VEX/priv/host_mips_isel.c (revision 2544)
> +++ VEX/priv/host_mips_isel.c (working copy)
> @@ -3049,6 +3049,7 @@
> case Ijk_NoRedir:
> case Ijk_SigBUS:
> case Ijk_SigTRAP:
> + case Ijk_SigFPE:
> case Ijk_Sys_syscall:
> case Ijk_TInval:
> {
> Index: VEX/pub/libvex_ir.h
> ===================================================================
> --- VEX/pub/libvex_ir.h (revision 2544)
> +++ VEX/pub/libvex_ir.h (working copy)
> @@ -1872,6 +1872,7 @@
> Ijk_SigTRAP, /* current instruction synths SIGTRAP */
> Ijk_SigSEGV, /* current instruction synths SIGSEGV */
> Ijk_SigBUS, /* current instruction synths SIGBUS */
> + Ijk_SigFPE, /* current instruction synths SIGFPE */
> /* Unfortunately, various guest-dependent syscall kinds. They
> all mean: do a syscall before continuing. */
> Ijk_Sys_syscall, /* amd64 'syscall', ppc 'sc', arm 'svc #0' */
> Index: VEX/pub/libvex_trc_values.h
> ===================================================================
> --- VEX/pub/libvex_trc_values.h (revision 2544)
> +++ VEX/pub/libvex_trc_values.h (working copy)
> @@ -62,6 +62,8 @@
> continuing */
> #define VEX_TRC_JMP_SIGBUS 93 /* deliver SIGBUS before continuing */
>
> +#define VEX_TRC_JMP_SIGFPE 97 /* deliver SIGFPE before continuing */
> +
> #define VEX_TRC_JMP_EMWARN 63 /* deliver emulation warning before
> continuing */
> #define VEX_TRC_JMP_EMFAIL 83 /* emulation fatal error; abort system
> */
>
> ---------------------------------------------------------------------------
> --- Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
|
|
From: Petar J. <mip...@gm...> - 2012-09-24 14:36:07
|
Hi Julian,
thanks for the reply. Sure it makes sense.
Check the patch below.
Regards,
Petar
Index: coregrind/m_signals.c
===================================================================
--- coregrind/m_signals.c (revision 13012)
+++ coregrind/m_signals.c (working copy)
@@ -1919,27 +1919,6 @@
info.si_signo = VKI_SIGTRAP;
info.si_code = VKI_TRAP_BRKPT; /* tjh: only ever called for a brkpt ins */
-# if defined(VGP_mips32_linux) || defined(VGP_mips64_linux)
- /* This is for teq on mips. Teq on mips for ins: 0xXXX1f4
- * cases VKI_SIGFPE not VKI_SIGTRAP
- */
- // JRS 2012-Jun-06: commented out until we know we need it
- // This isn't a clean solution; need something that avoids looking
- // at the guest code.
- //UInt *ins = (void*)(vgPlain_threads[tid].arch.vex.guest_PC-4);
- //UInt tcode = (((*ins) >> 6) & ((1 << 10) - 1));
- //if (tcode == VKI_BRK_OVERFLOW || tcode == VKI_BRK_DIVZERO) {
- // if (tcode == VKI_BRK_DIVZERO)
- // info.si_code = VKI_FPE_INTDIV;
- // else
- // info.si_code = VKI_FPE_INTOVF;
- // info.si_signo = VKI_SIGFPE;
- // info.si_errno = 0;
- // info.VKI_SIGINFO_si_addr
- // = (void*)(vgPlain_threads[tid].arch.vex.guest_PC-4);
- //}
-# endif
-
# if defined(VGP_x86_linux) || defined(VGP_amd64_linux)
uc.uc_mcontext.trapno = 3; /* tjh: this is the x86 trap number
for a breakpoint trap... */
@@ -1962,6 +1941,27 @@
resume_scheduler(tid);
}
+// Synthesise a SIGFPE.
+void VG_(synth_sigfpe)(ThreadId tid)
+{
+ vki_siginfo_t info;
+ struct vki_ucontext uc;
+
+ vg_assert(VG_(threads)[tid].status == VgTs_Runnable);
+
+ VG_(memset)(&info, 0, sizeof(info));
+ VG_(memset)(&uc, 0, sizeof(uc));
+ info.si_signo = VKI_SIGFPE;
+ info.si_code = VKI_TRAP_BRKPT;
+
+ if (VG_(gdbserver_report_signal) (VKI_SIGFPE, tid)) {
+ resume_scheduler(tid);
+ deliver_signal(tid, &info, &uc);
+ }
+ else
+ resume_scheduler(tid);
+}
+
/* Make a signal pending for a thread, for later delivery.
VG_(poll_signals) will arrange for it to be delivered at the right
time.
Index: coregrind/pub_core_signals.h
===================================================================
--- coregrind/pub_core_signals.h (revision 13012)
+++ coregrind/pub_core_signals.h (working copy)
@@ -77,6 +77,7 @@
extern void VG_(synth_sigill) (ThreadId tid, Addr addr);
extern void VG_(synth_sigtrap) (ThreadId tid);
extern void VG_(synth_sigbus) (ThreadId tid);
+extern void VG_(synth_sigfpe) (ThreadId tid);
/* Extend the stack to cover addr, if possible */
extern Bool VG_(extend_stack)(Addr addr, UInt maxsize);
Index: coregrind/m_scheduler/scheduler.c
===================================================================
--- coregrind/m_scheduler/scheduler.c (revision 13012)
+++ coregrind/m_scheduler/scheduler.c (working copy)
@@ -200,6 +200,7 @@
case VEX_TRC_JMP_SIGTRAP: return "SIGTRAP";
case VEX_TRC_JMP_SIGSEGV: return "SIGSEGV";
case VEX_TRC_JMP_SIGBUS: return "SIGBUS";
+ case VEX_TRC_JMP_SIGFPE: return "SIGFPE";
case VEX_TRC_JMP_EMWARN: return "EMWARN";
case VEX_TRC_JMP_EMFAIL: return "EMFAIL";
case VEX_TRC_JMP_CLIENTREQ: return "CLIENTREQ";
@@ -1424,6 +1425,10 @@
VG_(synth_sigbus)(tid);
break;
+ case VEX_TRC_JMP_SIGFPE:
+ VG_(synth_sigfpe)(tid);
+ break;
+
case VEX_TRC_JMP_NODECODE: {
Addr addr = VG_(get_IP)(tid);
Index: VEX/priv/host_mips_defs.c
===================================================================
--- VEX/priv/host_mips_defs.c (revision 2544)
+++ VEX/priv/host_mips_defs.c (working copy)
@@ -3296,7 +3296,8 @@
case Ijk_NoRedir: trcval = VEX_TRC_JMP_NOREDIR; break;
case Ijk_SigTRAP: trcval = VEX_TRC_JMP_SIGTRAP; break;
//case Ijk_SigSEGV: trcval = VEX_TRC_JMP_SIGSEGV; break;
- case Ijk_SigBUS: trcval = VEX_TRC_JMP_SIGBUS; break;
+ case Ijk_SigBUS: trcval = VEX_TRC_JMP_SIGBUS; break;
+ case Ijk_SigFPE: trcval = VEX_TRC_JMP_SIGFPE; break;
case Ijk_Boring: trcval = VEX_TRC_JMP_BORING; break;
/* We don't expect to see the following being assisted. */
//case Ijk_Ret:
Index: VEX/priv/ir_defs.c
===================================================================
--- VEX/priv/ir_defs.c (revision 2544)
+++ VEX/priv/ir_defs.c (working copy)
@@ -1231,6 +1231,7 @@
case Ijk_SigTRAP: vex_printf("SigTRAP"); break;
case Ijk_SigSEGV: vex_printf("SigSEGV"); break;
case Ijk_SigBUS: vex_printf("SigBUS"); break;
+ case Ijk_SigFPE: vex_printf("SigFPE"); break;
case Ijk_Sys_syscall: vex_printf("Sys_syscall"); break;
case Ijk_Sys_int32: vex_printf("Sys_int32"); break;
case Ijk_Sys_int128: vex_printf("Sys_int128"); break;
Index: VEX/priv/guest_mips_toIR.c
===================================================================
--- VEX/priv/guest_mips_toIR.c (revision 2544)
+++ VEX/priv/guest_mips_toIR.c (working copy)
@@ -3126,43 +3126,72 @@
case 0x30: { /* TGE */
/*tge */ DIP("tge r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt), getIReg (rs)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 6 || trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt),
getIReg (rs)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rt),
getIReg (rs)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x31: { /* TGEU */
/*tgeu */ DIP("tgeu r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt), getIReg (rs)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 6 || trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt),
getIReg (rs)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rt),
getIReg (rs)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x32: { /* TLT */
/*tlt */ DIP("tlt r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 6 || trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs),
getIReg (rt)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32S, getIReg (rs),
getIReg (rt)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x33: { /* TLTU */
/*tltu */ DIP("tltu r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 6 || trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs),
getIReg (rt)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpLT32U, getIReg (rs),
getIReg (rt)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x34: { /* TEQ */
/*teq */ DIP("teq r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP, IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 6 || trap_code == 7)
+ stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg (rt)),
+ Ijk_SigFPE, IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ else
+ stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg (rt)),
+ Ijk_SigTRAP, IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x36: { /* TNE */
/*tne */ DIP("tne r%d, r%d %d", rs, rt, trap_code);
- stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg (rt)),
- Ijk_SigTRAP,
- IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
+ if (trap_code == 6 || trap_code == 7)
+ stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg (rt)),
+ Ijk_SigFPE,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
+ else
+ stmt (IRStmt_Exit (binop (Iop_CmpNE32, getIReg (rs), getIReg (rt)),
+ Ijk_SigTRAP,
+ IRConst_U32 (guest_PC_curr_instr + 4),
OFFB_PC));
break;
}
case 0x0F: {
Index: VEX/priv/host_mips_isel.c
===================================================================
--- VEX/priv/host_mips_isel.c (revision 2544)
+++ VEX/priv/host_mips_isel.c (working copy)
@@ -3049,6 +3049,7 @@
case Ijk_NoRedir:
case Ijk_SigBUS:
case Ijk_SigTRAP:
+ case Ijk_SigFPE:
case Ijk_Sys_syscall:
case Ijk_TInval:
{
Index: VEX/pub/libvex_ir.h
===================================================================
--- VEX/pub/libvex_ir.h (revision 2544)
+++ VEX/pub/libvex_ir.h (working copy)
@@ -1872,6 +1872,7 @@
Ijk_SigTRAP, /* current instruction synths SIGTRAP */
Ijk_SigSEGV, /* current instruction synths SIGSEGV */
Ijk_SigBUS, /* current instruction synths SIGBUS */
+ Ijk_SigFPE, /* current instruction synths SIGFPE */
/* Unfortunately, various guest-dependent syscall kinds. They
all mean: do a syscall before continuing. */
Ijk_Sys_syscall, /* amd64 'syscall', ppc 'sc', arm 'svc #0' */
Index: VEX/pub/libvex_trc_values.h
===================================================================
--- VEX/pub/libvex_trc_values.h (revision 2544)
+++ VEX/pub/libvex_trc_values.h (working copy)
@@ -62,6 +62,8 @@
continuing */
#define VEX_TRC_JMP_SIGBUS 93 /* deliver SIGBUS before continuing */
+#define VEX_TRC_JMP_SIGFPE 97 /* deliver SIGFPE before continuing */
+
#define VEX_TRC_JMP_EMWARN 63 /* deliver emulation warning before
continuing */
#define VEX_TRC_JMP_EMFAIL 83 /* emulation fatal error; abort system */
|
|
From: Florian K. <br...@ac...> - 2012-09-24 12:22:06
|
On 09/24/2012 05:55 AM, Julian Seward wrote:
>
>> /* The various kinds of caches */
>> typedef enum {
>> DATA_CACHE,
>> INSN_CACHE,
>> DATA_INSN_CACHE // combined data and insn cache
>> } cache_kind;
>
> Like JosefW I would prefer COMBINED_CACHE to DATA_INSN_CACHE.
Josef did suggest UNIFIED_CACHE as it is the standard term. I think I've
seen "unified cache" more often that "combined cache" so I'm going to
run with that unless you object.
>> /* Information about the cache system as a whole */
>> typedef struct {
>> UInt num_levels;
>> UInt num_caches;
>> /* Unordered array of caches for this host. NULL if there are
>> no caches. Users can assume that the array contains at most one
>> cache of a given kind per cache level. */
>> cache_t *caches;
>> } cacheinfo_t;
>
> IIUC, num_levels is the number of levels the machine really has,
Yes
> and
> num_caches <= num_levels is the number of caches for which we have
> descriptions. Which may be less than the number of levels. Yes?
Yes, num_caches is the number of caches for which we have descriptions.
But it can be >= num_levels. Think of a machine that has L1 data and L1
insn cache and L2 data and L2 insn cache. So num_caches == 4 and
num_levels == 2.
> Am I right to understand (from your comments later) that some s390s
> have icaches which are coherent, and some do not? So you need to know
> the model number in order to decide whether or not VG_(invalid_icache)
> is a no-op?
No that is not the case for s390. But it might be for some architecture.
I was trying to propose something to cover the general case.
>> (4) VEX: functions returning VexInvalRange
>> The returned address range indicates whether some cache invalidation
>> needs to occur later. What is returned here may, in general, depend
>> on the particular machine model of a given architecture. So we need
>> to query the cache info before returning anything.
>> A different possibility, which may even be cleaner, is to make these
>> functions *always* return the address range for the insns that were
>> patched. In that case we would not need cache information here.
>
> Yes. I would prefer this solution. The patchers just return the address
> range they patched, and it is the call site's problem to figure out what to
> do about cache coherence.
Excellent.
>> Tools can call the existing VG_(machine_get_VexArchInfo) to get the
>> cache information. The function would have to be exposed through
>> pub_tool_machine.h so it becomes available.
>> Alternatively, VexArchInfo could be passed to the "instrument" function
>> of the tools.
>
> I would prefer both, in fact -- pass it to the instrument function, and
> allow tools to make ad-hoc calls to get it, if they want.
OK. I can see how it might be convenient to have a functional interface.
Might eliminate the need for a global variable in the tools.
> A couple of other comments:
>
> * pls can we use type names beginning w/ capitals, as in the rest
> of the code base
Sure, will do.
>
> * another piece of complexity to be aware of (a small one, though) is
> that ARM sets its hwcaps not only by the normal game of trying insns
> and seeing which get SIGILL (in m_machine.c), but also by looking at
> the AUXV entries on the stack at startup.
I think ppc does this, too.
Florian
|
|
From: Julian S. <js...@ac...> - 2012-09-24 11:05:04
|
> Some of the remaining tests in the test suite need this. Do you think we
> should fix/model this differently,
Yes. There is already a framework in place to generate SIGSEGV, SIGBUS,
SIGTRAP, without having to look at the guest insn. So we just need to
extend it to handle SIGFPE.
* in guest_mips_toIR.c, change this to Ijk_SigFPE
case 0x34: { /* TEQ */
/*teq */ DIP("teq r%d, r%d %d", rs, rt, trap_code);
stmt (IRStmt_Exit(binop (Iop_CmpEQ32, getIReg (rs), getIReg (rt)),
Ijk_SigTRAP, IRConst_U32 (guest_PC_curr_instr + 4), OFFB_PC));
break;
}
* add Ijk_SigFPE to the IR definition, fix up the MIPS back end
to handle it,
* add VEX_TRC_JMP_SIGFPE, plus a case for it in scheduler.c
* add VG_(synth_sigfpe)
Makes sense? I am happy to review patches if you want.
J
|
|
From: Christian B. <bor...@de...> - 2012-09-24 10:09:37
|
On 21/09/12 19:01, Florian Krohm wrote: > Cache information will be determined after hwcaps and machine models > have been determined. The rationale is that not all cache information > can be figured out automatically (e.g. on s390 we cannot figure out > whether icaches are coherent). Some such information may depend on > the machine model (part of hwcaps) for which we just know what it is and > can fill it in. Florian, AFAIK icaches on s390 are always coherent Christian |
|
From: Julian S. <js...@ac...> - 2012-09-24 09:56:15
|
I agree with almost all of this, with only minor comments.
> /* The various kinds of caches */
> typedef enum {
> DATA_CACHE,
> INSN_CACHE,
> DATA_INSN_CACHE // combined data and insn cache
> } cache_kind;
Like JosefW I would prefer COMBINED_CACHE to DATA_INSN_CACHE.
> /* Information about a particular cache */
> typedef struct {
> cache_kind kind;
> UInt level; /* level this cache is at, e.g. 1 for L1 cache */
> UInt sizeB; /* size of this cache in bytes */
> UInt line_sizeB; /* cache line size in bytes */
> UInt associativity;
> } cache_t;
>
> /* Information about the cache system as a whole */
> typedef struct {
> UInt num_levels;
> UInt num_caches;
> /* Unordered array of caches for this host. NULL if there are
> no caches. Users can assume that the array contains at most one
> cache of a given kind per cache level. */
> cache_t *caches;
> } cacheinfo_t;
IIUC, num_levels is the number of levels the machine really has, and
num_caches <= num_levels is the number of caches for which we have
descriptions. Which may be less than the number of levels. Yes?
> (1) cachegrind / callgrind
> These are perfectly served by cacheinfo_t as shown above.
>
> (2) VG_(invalidate_icache)
> We need to extend the above representation, by, say, adding a
> "Bool icaches_maintain_coherence;" to cacheinfo_t
Am I right to understand (from your comments later) that some s390s
have icaches which are coherent, and some do not? So you need to know
the model number in order to decide whether or not VG_(invalid_icache)
is a no-op?
> (3) VEX: VexArchinfo contains ppc_cache_line_szB
> The cache line size is needed to implement the icbi insn.
> Can be obtained from cacheinfo_t
>
> (4) VEX: functions returning VexInvalRange
> The returned address range indicates whether some cache invalidation
> needs to occur later. What is returned here may, in general, depend
> on the particular machine model of a given architecture. So we need
> to query the cache info before returning anything.
> A different possibility, which may even be cleaner, is to make these
> functions *always* return the address range for the insns that were
> patched. In that case we would not need cache information here.
Yes. I would prefer this solution. The patchers just return the address
range they patched, and it is the call site's problem to figure out what to
do about cache coherence.
> How to make cache information available:
> ----------------------------------------
> Ideally we want the cache information to be provided in one spot only;
> be that a function call returning it or some persistent data structure
> containing it. A related question is where the definition of cacheinfo_t
> resides. If it does not reside in VEX then VEX would be dependent on
> coregrind and that's a no-go. So, adding a cacheinfo_t typed member
> to VexArchInfo looks natural.
Yes.
> It could be filled in the same way hwcaps
> are currently filled in in m_machine.c.
> The type definitions (cache_t, cacheinfo_t etc) would be included in
> libvex.h
>
> Tools can call the existing VG_(machine_get_VexArchInfo) to get the
> cache information. The function would have to be exposed through
> pub_tool_machine.h so it becomes available.
> Alternatively, VexArchInfo could be passed to the "instrument" function
> of the tools.
I would prefer both, in fact -- pass it to the instrument function, and
allow tools to make ad-hoc calls to get it, if they want.
> I can work on an implementation for this but first there should be
> agreement
>
> - about the data structures to represent cache info
What you propose seems good to me.
> - how tools access it (function call or passed to the instrument
> function)
Both!
---------
A couple of other comments:
* pls can we use type names beginning w/ capitals, as in the rest
of the code base
* another piece of complexity to be aware of (a small one, though) is
that ARM sets its hwcaps not only by the normal game of trying insns
and seeing which get SIGILL (in m_machine.c), but also by looking at
the AUXV entries on the stack at startup.
Overall, sounds good. It's quite a tricky area to tidy up since I think
it has grown without much planning, over the years.
J
|