diff mbox series

Makefile: Use relative paths for debugging symbols.

Message ID 20220818173133.12552-1-vagrant@debian.org
State Changes Requested
Delegated to: Tom Rini
Headers show
Series Makefile: Use relative paths for debugging symbols. | expand

Commit Message

Vagrant Cascadian Aug. 18, 2022, 5:31 p.m. UTC
From: Vagrant Cascadian <vagrant@reproducible-builds.org>

The KBUILD_CFLAGS and KBUILD_AFLAGS variables are adjusted to use
-ffile-prefix-map and --debug-prefix-map, respectively, to use
relative paths for occurrences of __FILE__ and debug paths.

This enables reproducible builds regardless of the absolute path to
the build directory:

  https://reproducible-builds.org/docs/build-path/

Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>

---

 Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Rasmus Villemoes Aug. 19, 2022, 9:54 a.m. UTC | #1
On 18/08/2022 19.31, Vagrant Cascadian wrote:
> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
> 
> The KBUILD_CFLAGS and KBUILD_AFLAGS variables are adjusted to use
> -ffile-prefix-map and --debug-prefix-map, respectively, to use
> relative paths for occurrences of __FILE__ and debug paths.
> 
> This enables reproducible builds regardless of the absolute path to
> the build directory:
> 
>   https://reproducible-builds.org/docs/build-path/
> 
> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
> 
> ---
> 
>  Makefile | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1a66f69a4b..b40b9b2444 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -751,14 +751,18 @@ KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
>  # Enabled with W=2, disabled by default as noisy
>  KBUILD_CFLAGS += $(call cc-disable-warning, maybe-uninitialized)
>  
> -# change __FILE__ to the relative path from the srctree
> -KBUILD_CFLAGS	+= $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
> +# change __FILE__ and debugging symbols to the relative path from the
> +# srctree
> +KBUILD_CFLAGS	+= $(call cc-option,-ffile-prefix-map=$(srctree)/=)

You do update the comment, but it might be worth spelling out in the
commit log that -ffile-prefix-map is a superset of -fmacro-prefix-map,
and that both options are supported in the same set of compilers (so we
don't lose the effect of -fmacro-prefix-map for some set of compilers),
namely gcc 8. For reference, from the gcc repo

commit 7365279fca30371b07e49bfa83a23ddc44cc3860
Author: Boris Kolpackov <boris@gcc.gnu.org>
Date:   Thu Jan 18 13:17:37 2018 +0000

  Add ability to remap file names in __FILE__, etc (PR other/70268)

  This commit adds the -fmacro-prefix-map option that allows remapping
of file
  names in __FILE__, __BASE_FILE__, and __builtin_FILE(), similar to how
  -fdebug-prefix-map allows to do the same for debug information.

  Additionally, it adds -ffile-prefix-map which can be used to specify both
  mappings with a single option (and, should we need to add more
-f*-prefix-map
  options in the future, those as well).

>  KBUILD_CFLAGS	+= -g
>  # $(KBUILD_AFLAGS) sets -g, which causes gcc to pass a suitable -g<format>
>  # option to the assembler.
>  KBUILD_AFLAGS	+= -g
>  
> +# Use relative paths in debugging symbols
> +KBUILD_AFLAGS   += --debug-prefix-map=$(srctree)/=
> +

Hm, ok, it seems this has been supported in binutils since ~2007, so I
don't suppose this needs some equivalent to the cc-option guard.

Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Tom Rini Aug. 26, 2022, 8:59 p.m. UTC | #2
On Thu, Aug 18, 2022 at 10:31:34AM -0700, Vagrant Cascadian wrote:

> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
> 
> The KBUILD_CFLAGS and KBUILD_AFLAGS variables are adjusted to use
> -ffile-prefix-map and --debug-prefix-map, respectively, to use
> relative paths for occurrences of __FILE__ and debug paths.
> 
> This enables reproducible builds regardless of the absolute path to
> the build directory:
> 
>   https://reproducible-builds.org/docs/build-path/
> 
> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

This needs some sort of clang check and then perhaps different flag
used? How does the linux kernel handle this? See:
https://source.denx.de/u-boot/u-boot/-/jobs/487013#L48
Rasmus Villemoes Aug. 28, 2022, 12:24 p.m. UTC | #3
On 26/08/2022 22.59, Tom Rini wrote:
> On Thu, Aug 18, 2022 at 10:31:34AM -0700, Vagrant Cascadian wrote:
> 
>> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>
>> The KBUILD_CFLAGS and KBUILD_AFLAGS variables are adjusted to use
>> -ffile-prefix-map and --debug-prefix-map, respectively, to use
>> relative paths for occurrences of __FILE__ and debug paths.
>>
>> This enables reproducible builds regardless of the absolute path to
>> the build directory:
>>
>>   https://reproducible-builds.org/docs/build-path/
>>
>> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
>> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> 
> This needs some sort of clang check and then perhaps different flag
> used? How does the linux kernel handle this?

Well, interestingly it seems that the kernel doesn't do anything like
this for debug info, they only apply the -fmacro-prefix-map. Which one
should probably raise with them at some point.

It seems we're not actually calling gas directly, but always invokes
$(CC) whatever that may be to compile assembler files. So I think the
right fix is to simply pass the same -ffile-prefix-map in both
KBUILD_CFLAGS as in KBUILD_AFLAGS - and if there's some variable that
ends up being included in both automatically, then just adding it there
should do the trick.

Rasmus
Vagrant Cascadian Aug. 28, 2022, 6:01 p.m. UTC | #4
On 2022-08-28, Rasmus Villemoes wrote:
> On 26/08/2022 22.59, Tom Rini wrote:
>> On Thu, Aug 18, 2022 at 10:31:34AM -0700, Vagrant Cascadian wrote:
>> 
>>> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>>
>>> The KBUILD_CFLAGS and KBUILD_AFLAGS variables are adjusted to use
>>> -ffile-prefix-map and --debug-prefix-map, respectively, to use
>>> relative paths for occurrences of __FILE__ and debug paths.
>>>
>>> This enables reproducible builds regardless of the absolute path to
>>> the build directory:
>>>
>>>   https://reproducible-builds.org/docs/build-path/
>>>
>>> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>> 
>> This needs some sort of clang check and then perhaps different flag
>> used? How does the linux kernel handle this?
>
> Well, interestingly it seems that the kernel doesn't do anything like
> this for debug info, they only apply the -fmacro-prefix-map. Which one
> should probably raise with them at some point.
>
> It seems we're not actually calling gas directly, but always invokes
> $(CC) whatever that may be to compile assembler files. So I think the
> right fix is to simply pass the same -ffile-prefix-map in both
> KBUILD_CFLAGS as in KBUILD_AFLAGS - and if there's some variable that
> ends up being included in both automatically, then just adding it there
> should do the trick.

I tried just adding -ffile-prefix-map and that helped, but was not
sufficient to solve the reproducibility issues. It also needs the
--debug-prefix-map to make it the assembly code build reproducibly.

Though I guess I didn't try adding -ffile-prefix-map to KBUILD_AFLAGS,
now that I think about it... will test that too, thanks!


live well,
  vagrant
Vagrant Cascadian Aug. 28, 2022, 9:46 p.m. UTC | #5
On 2022-08-28, Vagrant Cascadian wrote:
> On 2022-08-28, Rasmus Villemoes wrote:
>> On 26/08/2022 22.59, Tom Rini wrote:
>>> On Thu, Aug 18, 2022 at 10:31:34AM -0700, Vagrant Cascadian wrote:
>>>> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>>>
>>>> The KBUILD_CFLAGS and KBUILD_AFLAGS variables are adjusted to use
>>>> -ffile-prefix-map and --debug-prefix-map, respectively, to use
>>>> relative paths for occurrences of __FILE__ and debug paths.
>>>>
>>>> This enables reproducible builds regardless of the absolute path to
>>>> the build directory:
>>>>
>>>>   https://reproducible-builds.org/docs/build-path/
>>>>
>>>> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>>> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>>> 
>>> This needs some sort of clang check and then perhaps different flag
>>> used? How does the linux kernel handle this?
>>
>> Well, interestingly it seems that the kernel doesn't do anything like
>> this for debug info, they only apply the -fmacro-prefix-map. Which one
>> should probably raise with them at some point.
>>
>> It seems we're not actually calling gas directly, but always invokes
>> $(CC) whatever that may be to compile assembler files. So I think the
>> right fix is to simply pass the same -ffile-prefix-map in both
>> KBUILD_CFLAGS as in KBUILD_AFLAGS - and if there's some variable that
>> ends up being included in both automatically, then just adding it there
>> should do the trick.
>
> I tried just adding -ffile-prefix-map and that helped, but was not
> sufficient to solve the reproducibility issues. It also needs the
> --debug-prefix-map to make it the assembly code build reproducibly.
>
> Though I guess I didn't try adding -ffile-prefix-map to KBUILD_AFLAGS,
> now that I think about it... will test that too, thanks!

Nope, that built, but not reproducibly...

I was able to guard it with "as-option":

  +KBUILD_AFLAGS   += $(call as-option,--debug-prefix-map=$(srctree)/=)

And it builds reproducibly for me.

That should be sufficient to fix build failures with clang... or is
there a more appropriate guard to use? I am not familiar with clang and
it's relevent tools to know if there is a relevent comprable option to
--debug-prefix-map.


live well,
  vagrant
Rasmus Villemoes Aug. 29, 2022, 7:15 a.m. UTC | #6
On 28/08/2022 23.46, Vagrant Cascadian wrote:
> On 2022-08-28, Vagrant Cascadian wrote:
>> On 2022-08-28, Rasmus Villemoes wrote:
>>> On 26/08/2022 22.59, Tom Rini wrote:
>>>> On Thu, Aug 18, 2022 at 10:31:34AM -0700, Vagrant Cascadian wrote:
>>>>> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>>>>
>>>>> The KBUILD_CFLAGS and KBUILD_AFLAGS variables are adjusted to use
>>>>> -ffile-prefix-map and --debug-prefix-map, respectively, to use
>>>>> relative paths for occurrences of __FILE__ and debug paths.
>>>>>
>>>>> This enables reproducible builds regardless of the absolute path to
>>>>> the build directory:
>>>>>
>>>>>   https://reproducible-builds.org/docs/build-path/
>>>>>
>>>>> Signed-off-by: Vagrant Cascadian <vagrant@reproducible-builds.org>
>>>>> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>>>>
>>>> This needs some sort of clang check and then perhaps different flag
>>>> used? How does the linux kernel handle this?
>>>
>>> Well, interestingly it seems that the kernel doesn't do anything like
>>> this for debug info, they only apply the -fmacro-prefix-map. Which one
>>> should probably raise with them at some point.
>>>
>>> It seems we're not actually calling gas directly, but always invokes
>>> $(CC) whatever that may be to compile assembler files. So I think the
>>> right fix is to simply pass the same -ffile-prefix-map in both
>>> KBUILD_CFLAGS as in KBUILD_AFLAGS - and if there's some variable that
>>> ends up being included in both automatically, then just adding it there
>>> should do the trick.
>>
>> I tried just adding -ffile-prefix-map and that helped, but was not
>> sufficient to solve the reproducibility issues. It also needs the
>> --debug-prefix-map to make it the assembly code build reproducibly.
>>
>> Though I guess I didn't try adding -ffile-prefix-map to KBUILD_AFLAGS,
>> now that I think about it... will test that too, thanks!
> 
> Nope, that built, but not reproducibly...

That's odd. <me goes digging in gcc source> <me scratches head>

Ahh, so gcc does have logic to forward/translate a -fdebug-prefix-map to
--debug-prefix-map when invoking the assembler:

#define ASM_MAP " %{fdebug-prefix-map=*:--debug-prefix-map %*}"

And for reasons that are still beyond me, gcc seems to accept --bar-baz
as an alias for -fbar-baz. E.g. if I invoke

  gcc -v --debug-prefix-map=/tmp=xyz-tmp-ABC --data-sections -g -o
/tmp/start.o -c /tmp/start.S

gcc tells me that

COLLECT_GCC_OPTIONS='-v' '-fdebug-prefix-map=/tmp=xyz-tmp-ABC'
'-fdata-sections' '-g' '-o' '/tmp/start.o' '-c' '-mtune=generic'
'-march=x86-64'

and of course the data-sections was just a silly thing to highlight the
alias machinery, but the point is that gcc saw this as being called with
-fdebug-prefix-map=. And sure enough, the very next line which shows the
assembler invocation says

  as --gdwarf2 --debug-prefix-map /tmp=xyz-tmp-ABC -v --64 -o
/tmp/start.o /tmp/ccu8aTZH.s

If I invoke gcc with the -fdebug-prefix-map form, exactly the same
happens, i.e. gcc says it was invoked with the -fdebug form, and calls
as with the --debug form.

But here's the kicker: The union option -ffile-prefix-map is not
recognized by gcc's spec file machinery (the ASM_MAP macro above), so
while it does affect the file paths that gcc emits to .s files when
compiling .c files before handing them to gas, it has no effect when
we're using gcc directly to compile .s or .S files.

I'm inclined to say that is a bug in gcc. I'll open a bug and see what
they say.

> I was able to guard it with "as-option":
> 
>   +KBUILD_AFLAGS   += $(call as-option,--debug-prefix-map=$(srctree)/=)
> 
> And it builds reproducibly for me.
> 
> That should be sufficient to fix build failures with clang... or is
> there a more appropriate guard to use? I am not familiar with clang and
> it's relevent tools to know if there is a relevent comprable option to
> --debug-prefix-map.

I think for clang one should just use -ffile-prefix-map unconditionally;
https://reproducible-builds.org/docs/build-path/ says that should work.
And when I try with 'clang -v -no-integrated-as
-ffile-prefix-map=/tmp=xyz-tmp-ABC ...', it does end up invoking
'/usr/bin/as --64 --debug-prefix-map /tmp=xyz-tmp-ABC'.

Rasmus
Rasmus Villemoes Aug. 29, 2022, 7:55 a.m. UTC | #7
On 29/08/2022 09.15, Rasmus Villemoes wrote:

> I'm inclined to say that is a bug in gcc. I'll open a bug and see what
> they say.

And it turns out this is already reported, with the same suggested
resolution as I was thinking of, namely extending the ASM_MAP definition:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93371

I'll submit a proper patch and see if this can be moved forward.

Rasmus
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 1a66f69a4b..b40b9b2444 100644
--- a/Makefile
+++ b/Makefile
@@ -751,14 +751,18 @@  KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
 # Enabled with W=2, disabled by default as noisy
 KBUILD_CFLAGS += $(call cc-disable-warning, maybe-uninitialized)
 
-# change __FILE__ to the relative path from the srctree
-KBUILD_CFLAGS	+= $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
+# change __FILE__ and debugging symbols to the relative path from the
+# srctree
+KBUILD_CFLAGS	+= $(call cc-option,-ffile-prefix-map=$(srctree)/=)
 
 KBUILD_CFLAGS	+= -g
 # $(KBUILD_AFLAGS) sets -g, which causes gcc to pass a suitable -g<format>
 # option to the assembler.
 KBUILD_AFLAGS	+= -g
 
+# Use relative paths in debugging symbols
+KBUILD_AFLAGS   += --debug-prefix-map=$(srctree)/=
+
 # Report stack usage if supported
 # ARC tools based on GCC 7.1 has an issue with stack usage
 # with naked functions, see commit message for more details