1:38 p.m.
Hi all,
It seems GCC 9 does not put read-only zero-initialized variables into the BSS
section anymore; instead it is put into RODATA.
(See the thread at [0], especially [1])
In filter/nozero a 64M large, static, zeroed, read-only array is declared.
The new behavior of GCC puts this array as-is into the binary inflating the
size by a factor of around 10000.
(Clang 8 and older GCCs work "correctly")
It would be possible to remove the "const" qualifier or use some compiler
attribute trickery to get the previous behavior back.
Thanks,
Thomas
[0] https://gcc.gnu.org/ml/gcc/2019-04/msg00034.html
[1] https://gcc.gnu.org/ml/gcc/2019-04/msg00049.html
5:47 a.m.
On Thu, Jul 04, 2019 at 08:38:07PM +0200, Thomas Weißschuh wrote:
Hi all,
It seems GCC 9 does not put read-only zero-initialized variables into the BSS
section anymore; instead it is put into RODATA.
(See the thread at [0], especially [1])
In filter/nozero a 64M large, static, zeroed, read-only array is declared.
The new behavior of GCC puts this array as-is into the binary inflating the
size by a factor of around 10000.
(Clang 8 and older GCCs work "correctly")
It would be possible to remove the "const" qualifier or use some compiler
attribute trickery to get the previous behavior back.
Thanks,
Thomas
[0] https://gcc.gnu.org/ml/gcc/2019-04/msg00034.html
[1] https://gcc.gnu.org/ml/gcc/2019-04/msg00049.html
Thanks, I can confirm this is indeed a problem (with GCC 9.1.1):
-rwxrwxr-x. 1 rjones rjones 67141400 Jun 10 09:36 nbdkit-nozero-filter.so
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
8:04 a.m.
On 7/4/19 1:38 PM, Thomas Weißschuh wrote:
Hi all,
It seems GCC 9 does not put read-only zero-initialized variables into the BSS
section anymore; instead it is put into RODATA.
(See the thread at [0], especially [1])
Thanks for the pointers.
In filter/nozero a 64M large, static, zeroed, read-only array is
declared.
The new behavior of GCC puts this array as-is into the binary inflating the
size by a factor of around 10000.
Odd that you are only seeing this for the nozero filter, since we also
have the same sort of buffer in server/plugins.c. Oh, I see a
difference between the two - one is function-local, the other is
file-local. Does this patch make a difference? (I need to fire up a
rawhide VM to test it myself...) If it doesn't, then removing the
'const' seems like the easiest trick to work around this compiler
pessimization (yes, I can see why the compiler writers argued that
sticking things in .rodata adds a bit more security against errant code
accidentally trying to corrupt the buffer; but as the buffer shouldn't
be escaping, it's already undefined behavior for such code to happen).
diff --git i/filters/nozero/nozero.c w/filters/nozero/nozero.c
index 16ec710b..21863707 100644
--- i/filters/nozero/nozero.c
+++ w/filters/nozero/nozero.c
@@ -45,7 +45,6 @@
#define MAX_WRITE (64 * 1024 * 1024)
-static const char buffer[MAX_WRITE];
static enum ZeroMode {
NONE,
EMULATE,
@@ -111,6 +110,8 @@ nozero_zero (struct nbdkit_next_ops *next_ops, void
*nxdata,
while (count) {
uint32_t size = MIN (count, MAX_WRITE);
+ static const char buffer[MAX_WRITE]; /* Always contains zeroes */
+
if (next_ops->pwrite (nxdata, buffer, size, offs, flags, err) == -1)
return -1;
offs += size;
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
8:21 a.m.
On Fri, 2019-07-05T08:04-0500, Eric Blake wrote:
On 7/4/19 1:38 PM, Thomas Weißschuh wrote:
> In filter/nozero a 64M large, static, zeroed, read-only array is declared.
> The new behavior of GCC puts this array as-is into the binary inflating the
> size by a factor of around 10000.
Odd that you are only seeing this for the nozero filter, since we also
have the same sort of buffer in server/plugins.c. Oh, I see a
difference between the two - one is function-local, the other is
file-local. Does this patch make a difference? (I need to fire up a
rawhide VM to test it myself...) If it doesn't, then removing the
'const' seems like the easiest trick to work around this compiler
pessimization (yes, I can see why the compiler writers argued that
sticking things in .rodata adds a bit more security against errant code
accidentally trying to corrupt the buffer; but as the buffer shouldn't
be escaping, it's already undefined behavior for such code to happen).
It actually also happens for server/plugins.c.
I assumed that filter/nozero got somehow statically linked into the server
(which does not really make sense), so it would have been the same instance of
the problem. Then I forgot to confirm this afterwards.
diff --git i/filters/nozero/nozero.c w/filters/nozero/nozero.c
index 16ec710b..21863707 100644
--- i/filters/nozero/nozero.c
+++ w/filters/nozero/nozero.c
...
Nope, same thing.
Thomas
8:48 a.m.
On 7/5/19 8:21 AM, Thomas Weißschuh wrote:
On Fri, 2019-07-05T08:04-0500, Eric Blake wrote:
> On 7/4/19 1:38 PM, Thomas Weißschuh wrote:
>> In filter/nozero a 64M large, static, zeroed, read-only array is declared.
>> The new behavior of GCC puts this array as-is into the binary inflating the
>> size by a factor of around 10000.
>
> Odd that you are only seeing this for the nozero filter, since we also
> have the same sort of buffer in server/plugins.c. Oh, I see a
> difference between the two - one is function-local, the other is
> file-local. Does this patch make a difference? (I need to fire up a
> rawhide VM to test it myself...)
Thanks for your fast reply - you did beat me in getting my rawhide VM
running.
It actually also happens for server/plugins.c.
Good to know. So the fix is indeed removing 'const', rather than
reducing scope (although I might as well reduce scope while at it, for
consistency).
The fix is in git now.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
9:52 a.m.
On Fri, 2019-07-05T08:48-0500, Eric Blake wrote:
On 7/5/19 8:21 AM, Thomas Weißschuh wrote:
> On Fri, 2019-07-05T08:04-0500, Eric Blake wrote:
>> On 7/4/19 1:38 PM, Thomas Weißschuh wrote:
>>> In filter/nozero a 64M large, static, zeroed, read-only array is declared.
>>> The new behavior of GCC puts this array as-is into the binary inflating the
>>> size by a factor of around 10000.
>>
>> Odd that you are only seeing this for the nozero filter, since we also
>> have the same sort of buffer in server/plugins.c. Oh, I see a
>> difference between the two - one is function-local, the other is
>> file-local. Does this patch make a difference? (I need to fire up a
>> rawhide VM to test it myself...)
Thanks for your fast reply - you did beat me in getting my rawhide VM
running.
> It actually also happens for server/plugins.c.
Good to know. So the fix is indeed removing 'const', rather than
reducing scope (although I might as well reduce scope while at it, for
consistency).
Does this not mean, the 64M are zeroed on every loop iteration in every call to
these functions?
(Probably also depends on compiler optimizations)
Maybe this will have some performance impact, or does it not matter with these
functions?
Maybe the
The fix is in git now.
Thanks!
Thomas
10:02 a.m.
On 7/5/19 9:52 AM, Thomas Weißschuh wrote:
> Good to know. So the fix is indeed removing 'const',
rather than
> reducing scope (although I might as well reduce scope while at it, for
> consistency).
Does this not mean, the 64M are zeroed on every loop iteration in every call to
these functions?
No. static variables are guaranteed by the C standard to start life
pre-initialized (where that initialization is guaranteed to be all
zeroes if there is no explicit initializer), and that pre-initialization
is NOT performed during each call to the function (the compiler may
delay the initialization to the first function call, if that is cheaper
than always initializing the storage even if the function doesn't get
called, but the effect is the same).
(Probably also depends on compiler optimizations)
Any compiler that initializes a static variable more than once is buggy.
Maybe this will have some performance impact, or does it not matter with these
functions?
No, there should be no performance change. If there were, that's an
even more serious bug in gcc.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
11:01 a.m.
On Fri, 2019-07-05T10:02-0500, Eric Blake wrote:
On 7/5/19 9:52 AM, Thomas Weißschuh wrote:
>> Good to know. So the fix is indeed removing 'const', rather than
>> reducing scope (although I might as well reduce scope while at it, for
>> consistency).
>
> Does this not mean, the 64M are zeroed on every loop iteration in every call to
> these functions?
No. static variables are guaranteed by the C standard to start life
pre-initialized (where that initialization is guaranteed to be all
zeroes if there is no explicit initializer), and that pre-initialization
is NOT performed during each call to the function (the compiler may
delay the initialization to the first function call, if that is cheaper
than always initializing the storage even if the function doesn't get
called, but the effect is the same).
Ah, yes I forgot the 'static'. Thanks for the clarification.
> (Probably also depends on compiler optimizations)
Any compiler that initializes a static variable more than once is buggy.
>
> Maybe this will have some performance impact, or does it not matter with these
> functions?
No, there should be no performance change. If there were, that's an
even more serious bug in gcc.
Yep, my bad.
Thanks again,
Thomas
1968
days inactive
1969
days old
7 comments
3 participants
participants (3)
-
Eric Blake -
Richard W.M. Jones -
Thomas Weißschuh