bug#37384: Daemon does not honor “useSubstitutes” on armhf

bug#37384: Daemon does not honor “useSubstitutes” on armhf

[Timothy Sample]

Currently, the “guix” package cannot build on armhf due to test
failures: <https://ci.guix.gnu.org/build/1705321/details>.  (I can
confirm this on my local machine, too.)

It looks like there is some confusion about the layout of the “Settings”
struct.  At least, if I add some print statements in the code to print
the address of the “useSubstitutes” flag:

    printMsg(lvlError, format("XXX: %1%") % &settings.useSubstitutes);

it gives me different addresses depending on whether I’m printing from
the main daemon loop or from the “querySubstitutablePathInfos” method.
The addresses suspiciously differ by eight.

I can confirm that the addresses are the same on x86_64.

My guess is that this bug was revealed by the changes made in commit
f6919ebdc6b0ce0286814cc6ab0564b1a4c67f5f (making the daemon assume a
single substituter).  Before this commit, it looks like the daemon would
give up because there were no substituters.  Now it relies on checking
“useSubstitutes”, which is not in fact reliable.  I’ve tested everything
from before this commit, and the addresses are still different.

Weirdly, in GDB, I can be right before the following line:

    if (!settings.useSubstitutes) return;

run “p settings.useSubstitutes” and see “false”, and then continue
running through the method without hitting that “return”.  It seems GDB
is not confused about the flag, just the actual code.

Looking at the disassembly was no use to me, since I don’t know much
about ARM, and it was not simple enough for me to guess what it was
doing.


-- Tim

bug#37384: Daemon does not honor “useSubstitutes” on armhf

[Timothy Sample]

Timothy Sample <samplet@ngyro.com> writes:

> it gives me different addresses depending on whether I’m printing from
> the main daemon loop or from the “querySubstitutablePathInfos” method.
> The addresses suspiciously differ by eight.

The problem goes away if I move “useSubstitutes” above “reservedSize”,
which has type “off_t”.  I’m guessing this is because of something fishy
with “_FILE_OFFSET_BITS”, but I’m not sure what just yet.


-- Tim

bug#37384: Daemon does not honor “useSubstitutes” on armhf

[Timothy Sample]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

Timothy Sample <samplet@ngyro.com> writes:

> The problem goes away if I move “useSubstitutes” above “reservedSize”,
> which has type “off_t”.  I’m guessing this is because of something fishy
> with “_FILE_OFFSET_BITS”, but I’m not sure what just yet.

Here’s my best guess as to what is going on.  In “nix-daemon.cc” the
order of includes causes “off_t” to be defined before
“_FILE_OFFSET_BITS”, which results in it being something like a 32-bit
signed integer.  In “local-store.cc”, “_FILE_OFFSET_BITS” gets set
first, yielding a 64-bit “off_t”.  This causes all of the addresses in
the “Settings” struct to be mismatched after “reservedSize”.  I plugged
in “sizeof(off_t)” into the “printMsg” calls from before, and (lo and
behold) I saw 4 from “nix-daemon.cc” and 8 from “local-store.cc”.

The following patch fixes the problem:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-daemon-Include-config.h-in-nix-daemon.cc.patch --]
[-- Type: text/x-patch, Size: 637 bytes --]

From 3c3eafac82e0a6e8a37363d6eb46f128e585705a Mon Sep 17 00:00:00 2001
From: Timothy Sample <samplet@ngyro.com>
Date: Thu, 12 Sep 2019 00:50:54 -0400
Subject: [PATCH] daemon: Include 'config.h' in 'nix-daemon.cc'.

* nix/nix-daemon/nix-daemon.cc: Include 'config.h'.
---
 nix/nix-daemon/nix-daemon.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index ffac6cde34..1163a249d1 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -1,3 +1,4 @@
+#include "config.h"
 #include "shared.hh"
 #include "local-store.hh"
 #include "util.hh"
-- 
2.23.0


[-- Attachment #3: Type: text/plain, Size: 142 bytes --]


Is this okay to push?  It seems kind of conspicuous that it’s missing in
the first place.  Is it missing for a good reason?


-- Tim

bug#37384: Daemon does not honor “useSubstitutes” on armhf

[Ludovic Courtès]

Hello Timothy,

Timothy Sample <samplet@ngyro.com> skribis:

> The following patch fixes the problem:
>
> From 3c3eafac82e0a6e8a37363d6eb46f128e585705a Mon Sep 17 00:00:00 2001
> From: Timothy Sample <samplet@ngyro.com>
> Date: Thu, 12 Sep 2019 00:50:54 -0400
> Subject: [PATCH] daemon: Include 'config.h' in 'nix-daemon.cc'.
>
> * nix/nix-daemon/nix-daemon.cc: Include 'config.h'.
> ---
>  nix/nix-daemon/nix-daemon.cc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
> index ffac6cde34..1163a249d1 100644
> --- a/nix/nix-daemon/nix-daemon.cc
> +++ b/nix/nix-daemon/nix-daemon.cc
> @@ -1,3 +1,4 @@
> +#include "config.h"
>  #include "shared.hh"
>  #include "local-store.hh"
>  #include "util.hh"

LGTM.

Thanks for the debugging effort that led to this fix!

Ludo’.

bug#37384: Daemon does not honor “useSubstitutes” on armhf

[Timothy Sample]

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> LGTM.

Great!  I just pushed it.  Thanks for the review.


-- Tim