Subject: Re: bug#30820: Chunked store references in compiled code break grafting (again)

From: [email-masked] (Ludovic Courtès)
Date: Wed, 21 Mar 2018 00:07:30 +0100

Hello,

[email-masked] (Ludovic Courtès) skribis:

> So the real issue is this:
>
>> The second issue is that the patch only ever worked with literal
>> strings.  It does not “see” strings in constant arrays like the ‘str’
>> array in the example above.

Good news!  Commit e288572710250bcd2aa0f69ce88154d98ac69b29 adjusts
‘gcc-strmov-store-file-names.patch’ in ‘core-updates’ to correctly deal
with this case:

--8<---------------cut here---------------start------------->8---
$ cat strmov.c 
#define _GNU_SOURCE
#include <string.h>
static const char str[] =
  "This is a /gnu/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee string in a global variable.";

extern char *p, *q;

#ifndef MEMCPY
# define MEMCPY memcpy
#endif

void foo (char *x, char *y)
{
  MEMCPY (x, str, sizeof str);
  MEMCPY (y, "this is a literal /gnu/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee string", 35);
}
$ ./pre-inst-env guix build -e '(@@ (gnu packages commencement) gcc-final)'
/gnu/store/wzdyqkdslk1s6f0vi9qw1xha8cniijzs-gcc-5.5.0-lib
/gnu/store/46ww5s9zvsw04id438c4drpnwd9m6vl8-gcc-5.5.0
$ /gnu/store/46ww5s9zvsw04id438c4drpnwd9m6vl8-gcc-5.5.0/bin/gcc -O2 -c strmov.c
$ objdump -S strmov.o |grep movabs
$ NIX_STORE=/foo /gnu/store/46ww5s9zvsw04id438c4drpnwd9m6vl8-gcc-5.5.0/bin/gcc -O2 -c strmov.c
$ objdump -S strmov.o |grep movabs
   0:	48 b8 54 68 69 73 20 	movabs $0x2073692073696854,%rax
   a:	48 ba 74 6f 72 65 2f 	movabs $0x6565652f65726f74,%rdx
  1e:	48 b8 61 20 2f 67 6e 	movabs $0x732f756e672f2061,%rax
  30:	48 b8 65 65 65 65 65 	movabs $0x6565656565656565,%rax
  4a:	48 b8 65 65 65 65 65 	movabs $0x2065656565656565,%rax
  58:	48 b8 73 74 72 69 6e 	movabs $0x6920676e69727473,%rax
  66:	48 b8 6e 20 61 20 67 	movabs $0x626f6c672061206e,%rax
  74:	48 b8 61 6c 20 76 61 	movabs $0x6169726176206c61,%rax
  82:	48 b8 74 68 69 73 20 	movabs $0x2073692073696874,%rax
  93:	48 b8 61 20 6c 69 74 	movabs $0x61726574696c2061,%rax
  a5:	48 b8 6c 20 2f 67 6e 	movabs $0x732f756e672f206c,%rax
--8<---------------cut here---------------end--------------->8---

I built everything about to ‘gcc-final’ in ‘core-updates’.  I checked
manually that none of the /gnu/store references in libc-2.26.so were
chunked.

For the record, what the patch initially did was to skip code that would
otherwise emit a “block move” when expanding __builtin_memcpy & co.
This patch additionally skips similar code that would replace
__builtin_memcpy calls with memory moves early on, in
‘gimple_fold_builtin_memory_op’, before ‘expand_builtin’ is called.

In the example above, this transformation would lead to the code below
(as seen with ‘-fdump-tree-all’ in the ‘gimple’ phase output):

--8<---------------cut here---------------start------------->8---
foo (char * x, char * y)
{
  MEM[(char * {ref-all})x] = MEM[(char * {ref-all})&str];
  memcpy (y, "this is a literal /gnu/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee string", 35);
}
--8<---------------cut here---------------end--------------->8---

With the patch we get:

--8<---------------cut here---------------start------------->8---
foo (char * x, char * y)
{
  memcpy (x, &str, 85);
  memcpy (y, "this is a literal /gnu/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee string", 35);
}
--8<---------------cut here---------------end--------------->8---

Ludo’.