|
From: Florian K. <br...@ac...> - 2012-06-28 10:19:54
|
A week or so ago somebody wanted to run with --num-callers=100 which did
not fly because there is a hard-wired maximum of 50. This is kind of
lame in particular in this instance where it can easily be avoided by
using a variable length array:
Index: coregrind/m_execontext.c
===================================================================
--- coregrind/m_execontext.c (revision 12623)
+++ coregrind/m_execontext.c (working copy)
@@ -297,7 +297,7 @@
static ExeContext* record_ExeContext_wrk ( ThreadId tid, Word
first_ip_delta,
Bool first_ip_only )
{
- Addr ips[VG_DEEPEST_BACKTRACE];
+ Addr ips[VG_(clo_backtrace_size)];
UInt n_ips;
init_ExeContext_storage();
VLAs are part of C for more than a decade now so I presume their use
should be OK (we're requiring C99 features in other places already, e.g.
mixing declarations and statements). Both gcc and clang support them.
I have a proper patch to fix the above issue but wanted to double check
first that use of VLAs is OK in valgrind.
Florian
|
|
From: Julian S. <js...@ac...> - 2012-06-28 11:29:41
|
(general question) what does the standard says happens if the array size is
zero or negative, at run time?
Ok for the change provided that
(1) the num-callers is limited to some sane values at startup so we can't
trash the stack. >= 1 and <= 500 say. The V thread stacks are small
(64k) (or were, at least, maybe not any more)
(2) this doesn't change the amount of memory required to store the ExeContexts
in the heap (but I'm sure it doesn't -- you're only talking about stack
allocation, right?)
J
On Thursday, June 28, 2012, Florian Krohm wrote:
> A week or so ago somebody wanted to run with --num-callers=100 which did
> not fly because there is a hard-wired maximum of 50. This is kind of
> lame in particular in this instance where it can easily be avoided by
> using a variable length array:
>
> Index: coregrind/m_execontext.c
> ===================================================================
> --- coregrind/m_execontext.c (revision 12623)
> +++ coregrind/m_execontext.c (working copy)
> @@ -297,7 +297,7 @@
> static ExeContext* record_ExeContext_wrk ( ThreadId tid, Word
> first_ip_delta,
> Bool first_ip_only )
> {
> - Addr ips[VG_DEEPEST_BACKTRACE];
> + Addr ips[VG_(clo_backtrace_size)];
> UInt n_ips;
>
> init_ExeContext_storage();
>
> VLAs are part of C for more than a decade now so I presume their use
> should be OK (we're requiring C99 features in other places already, e.g.
> mixing declarations and statements). Both gcc and clang support them.
> I have a proper patch to fix the above issue but wanted to double check
> first that use of VLAs is OK in valgrind.
>
> Florian
|
|
From: Florian K. <br...@ac...> - 2012-06-29 12:10:25
|
On 06/28/2012 01:27 PM, Julian Seward wrote: > > (general question) what does the standard says happens if the array size is > zero or negative, at run time? > You get "undefined behaviour". When evaluated, the array size shall be greater than zero. > Ok for the change provided that > > (1) the num-callers is limited to some sane values at startup so we can't > trash the stack. >= 1 and <= 500 say. The V thread stacks are small > (64k) (or were, at least, maybe not any more) OK. > > (2) this doesn't change the amount of memory required to store the ExeContexts > in the heap (but I'm sure it doesn't -- you're only talking about stack > allocation, right?) Right, this is only about the stack-allocated array. No effect on the memory spent for ExeContexts. Florian |