Re: [PATCH v8.1 20/27] build_id: Provide ld-embedded build-ids

From: "Jan Beulich" <jbeulich@suse.com>
To: ross.lagerwall@citrix.com, konrad@kernel.org, konrad.wilk@oracle.com
Cc: sstabellini@kernel.org, keir@xen.org, andrew.cooper3@citrix.com,
	mpohlack@amazon.com, mpohlack@amazon.de, julien.grall@arm.com,
	sasha.levin@oracle.com, xen-devel@lists.xenproject.org
Subject: Re: [PATCH v8.1 20/27] build_id: Provide ld-embedded build-ids
Date: Wed, 20 Apr 2016 01:14:17 -0600	[thread overview]
Message-ID: <57173A5902000078000E6E54@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1460584928-32440-21-git-send-email-konrad.wilk@oracle.com>

>>> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> 04/14/16 12:03 AM >>>
>--- a/Config.mk
>+++ b/Config.mk
>@@ -126,6 +126,17 @@ endef
 >check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1")
 >$(eval $(check-y))
 >
>+ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \
>+					grep -q unrecognized && echo n || echo y)

What about localized binutils?

>@@ -87,7 +93,8 @@ endif
 >
 >$(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
 	>./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x100000 \
>-	`$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
>+	`$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'` \
>+	$(notes_phdrs)

 The placement makes it somewhat non-obvious that this is an option passed
to mkelf32 - it'd probably better go on the first line.

>+note.o: $(TARGET)-syms
>+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id  $(BASEDIR)/xen-syms $@.bin
>+	$(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 \
>+		--rename-section=.data=.note.gnu.build-id -S $@.bin $@
>+	rm -f $@.bin

This, btw, should be accompanied by an adjustment to the clean target, for the
case where a build gets interrupted before the rule above manages to delete the
intermediate file.

>@@ -99,6 +112,21 @@ SECTIONS
        >_erodata = .;
   >} :text
 >
>+#if defined(BUILD_ID) && !defined(EFI)
>+/*
>+ * What a strange section name. The reason is that on ELF builds this section
>+ * is extracted to notes.o (which then is ingested in the EFI file). But the
>+ * compiler may want to inject other things in the .note which we don't care
>+ * about - hence this unique name.
>+ */
>+  . = ALIGN(4);
>+  .note.gnu.build-id : {
>+       __note_gnu_build_id_start = .;
>+       *(.note.gnu.build-id)
>+       __note_gnu_build_id_end = .;
>+  } :note :text
>+#endif

Am I misremembering that we had agreed for this to live ahead of _erodata?

>@@ -61,6 +66,51 @@ const char *xen_deny(void)
     >return "<denied>";
 >}
 >
>+static const void *build_id_p;
>+static unsigned int build_id_len;

__read_mostly? Also these variables are kind of pointless to have in a build-id-
less build. But I guess that's going to be rare anyway.

Anyway,
Acked-by: Jan Beulich <jbeulich@suse.com>
with or without any of the above suggestions taken care of.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel