[PATCH] Fix for linker build ID in Fedora 8

[PATCH] Fix for linker build ID in Fedora 8

[Pavel Roskin]

Hello!

[Sorry for not replying earlier.  All messages from my gnu.org account
were getting lost.  Anyway, I'll need to retire that address due to
massive spam.]

Here's an alternate patch for the issue that appeared on Fedora 8.  The
linker adds a "build ID" (.not.gnu.build-id) by default.  That section
makes objcopy create output files about 128M long when asked to use
binary format.  Needless to say, they are not suitable as grub images.
Also, the build ID section breaks the test for objcopy in the configure
script.

This patch is less intrusive than the one I posted earlier, so I hope it
will be accepted.

2007-10-29  Pavel Roskin  <proski-grub@xemaps.com>

	* configure.ac: Test if '--build-id=none' is supported by the
	linker.  Add it to TARGET_LDFLAGS if possible.  Build ID
	interferes with objcopy.
	* aclocal.m4 (grub_PROG_OBJCOPY_ABSOLUTE): Use LDFLAGS when
	linking, so that build ID doesn't break the test.

--- aclocal.m4
+++ aclocal.m4
@@ -57,7 +57,7 @@
 fi
 grub_cv_prog_objcopy_absolute=yes
 for link_addr in 2000 8000 7C00; do
-  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} -nostdlib -Wl,-N -Wl,-Ttext -Wl,$link_addr conftest.o -o conftest.exec]); then :
+  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} ${LDFLAGS} -nostdlib -Wl,-N -Wl,-Ttext -Wl,$link_addr conftest.o -o conftest.exec]); then :
   else
     AC_MSG_ERROR([${CC-cc} cannot link at address $link_addr])
   fi
--- configure.ac
+++ configure.ac
@@ -229,6 +229,16 @@
   TARGET_LDFLAGS="$TARGET_LDFLAGS -m32"
 fi
 
+AC_MSG_CHECKING([whether linker accepts `--build-id=none'])
+save_LDFLAGS="$LDFLAGS"
+LDFLAGS="$LDFLAGS -Wl,--build-id=none"
+AC_TRY_LINK(, , build_id_flag=yes, build_id_flag=no)
+AC_MSG_RESULT([$build_id_flag])
+LDFLAGS="$save_LDFLAGS"
+if test "x$build_id_flag" = xyes; then
+  TARGET_LDFLAGS="$TARGET_LDFLAGS -Wl,--build-id=none"
+fi
+
 #
 # Compiler features.
 #


-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix for linker build ID in Fedora 8

[Robert Millan]

On Wed, Nov 14, 2007 at 02:39:02AM -0500, Pavel Roskin wrote:
> Hello!
> 
> [Sorry for not replying earlier.  All messages from my gnu.org account
> were getting lost.  Anyway, I'll need to retire that address due to
> massive spam.]
> 
> Here's an alternate patch for the issue that appeared on Fedora 8.  The
> linker adds a "build ID" (.not.gnu.build-id) by default.  That section
> makes objcopy create output files about 128M long when asked to use
> binary format.  Needless to say, they are not suitable as grub images.
> Also, the build ID section breaks the test for objcopy in the configure
> script.
> 
> This patch is less intrusive than the one I posted earlier, so I hope it
> will be accepted.
> 
> 2007-10-29  Pavel Roskin  <proski-grub@xemaps.com>
> 
> 	* configure.ac: Test if '--build-id=none' is supported by the
> 	linker.  Add it to TARGET_LDFLAGS if possible.  Build ID
> 	interferes with objcopy.
> 	* aclocal.m4 (grub_PROG_OBJCOPY_ABSOLUTE): Use LDFLAGS when
> 	linking, so that build ID doesn't break the test.

Looks fine to me.  Seeing that there were no objections after a few days, I
suggest you check it in.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[PATCH] Fix for linker build ID in Fedora 8

[Pavel Roskin]

Hello!

Sorry for re-sending this patch.  My e-mail was incorrectly set up.  It
should be fixed now.  Please ignore the previous post from my xemaps.com
address.

Here's an alternate patch for the issue that appeared on Fedora 8.  The
linker adds a "build ID" (.not.gnu.build-id) by default.  That section
makes objcopy create output files about 128M long when asked to use
binary format.  Needless to say, they are not suitable as grub images.
Also, the build ID section breaks the test for objcopy in the configure
script.

This patch is less intrusive than the one I posted earlier, so I hope it
will be accepted.

2007-11-23  Pavel Roskin  <proski@gnu.org>
	* configure.ac: Test if '--build-id=none' is supported by the
	linker.  Add it to TARGET_LDFLAGS if possible.  Build ID
	interferes with objcopy.
	* aclocal.m4 (grub_PROG_OBJCOPY_ABSOLUTE): Use LDFLAGS when
	linking, so that build ID doesn't break the test.

--- aclocal.m4
+++ aclocal.m4
@@ -57,7 +57,7 @@
 fi
 grub_cv_prog_objcopy_absolute=yes
 for link_addr in 2000 8000 7C00; do
-  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} -nostdlib -Wl,-N -Wl,-Ttext -Wl,$link_addr conftest.o -o conftest.exec]); then :
+  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} ${LDFLAGS} -nostdlib -Wl,-N -Wl,-Ttext -Wl,$link_addr conftest.o -o conftest.exec]); then :
   else
     AC_MSG_ERROR([${CC-cc} cannot link at address $link_addr])
   fi
--- configure.ac
+++ configure.ac
@@ -229,6 +229,16 @@
   TARGET_LDFLAGS="$TARGET_LDFLAGS -m32"
 fi
 
+AC_MSG_CHECKING([whether linker accepts `--build-id=none'])
+save_LDFLAGS="$LDFLAGS"
+LDFLAGS="$LDFLAGS -Wl,--build-id=none"
+AC_TRY_LINK(, , build_id_flag=yes, build_id_flag=no)
+AC_MSG_RESULT([$build_id_flag])
+LDFLAGS="$save_LDFLAGS"
+if test "x$build_id_flag" = xyes; then
+  TARGET_LDFLAGS="$TARGET_LDFLAGS -Wl,--build-id=none"
+fi
+
 #
 # Compiler features.
 #


-- 
Regards,
Pavel Roskin
-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix for linker build ID in Fedora 8

[Lubomir Kundrak]

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

Hi,

Thanks for the patch!

On Fri, 2007-11-23 at 22:07 -0500, Pavel Roskin wrote:
> Hello!
> Here's an alternate patch for the issue that appeared on Fedora 8.  The
> linker adds a "build ID" (.not.gnu.build-id) by default.  That section
> makes objcopy create output files about 128M long when asked to use
> binary format.  Needless to say, they are not suitable as grub images.

I'd say this is a binutils bug, it wrecks the usability of binary output
format of objcopy. It doesn't make much sense to me, that it needs to be
mapped to the process' address space, but maybe gdb needs it that way, I
really have no clue -- I'm tempted to ask the binutils developers.

> --- configure.ac
> +++ configure.ac
> @@ -229,6 +229,16 @@
>    TARGET_LDFLAGS="$TARGET_LDFLAGS -m32"
>  fi
>  
> +AC_MSG_CHECKING([whether linker accepts `--build-id=none'])
> +save_LDFLAGS="$LDFLAGS"
> +LDFLAGS="$LDFLAGS -Wl,--build-id=none"
> +AC_TRY_LINK(, , build_id_flag=yes, build_id_flag=no)
> +AC_MSG_RESULT([$build_id_flag])
> +LDFLAGS="$save_LDFLAGS"
> +if test "x$build_id_flag" = xyes; then
> +  TARGET_LDFLAGS="$TARGET_LDFLAGS -Wl,--build-id=none"
> +fi
> +
>  #
>  # Compiler features.
>  #

I disagree with a solution like this -- you completely disable
generation of build-ids. They are highly usable and I want them to be
included in our debuginfo package. They're basically okay in *.exec
files, so why not just leave them out of *.img-s, which don't contain
any debugging information anyways.

I attach the patch I use. The first hunk is basically the same as yours,
the second one is self-explanatory.

Regards,
-- 
Lubomir Kundrak (Red Hat Security Response Team)

[-- Attachment #2: grub-1.95-buildid.patch --]
[-- Type: text/x-patch, Size: 1684 bytes --]

Index: aclocal.m4
===================================================================
RCS file: /sources/grub/grub2/aclocal.m4,v
retrieving revision 1.6
diff -u -p -r1.6 aclocal.m4
--- aclocal.m4	3 Feb 2007 11:36:13 -0000	1.6
+++ aclocal.m4	18 Nov 2007 16:48:12 -0000
@@ -57,7 +57,7 @@ else
 fi
 grub_cv_prog_objcopy_absolute=yes
 for link_addr in 2000 8000 7C00; do
-  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} -nostdlib -Wl,-N -Wl,-Ttext -Wl,$link_addr conftest.o -o conftest.exec]); then :
+  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} -nostdlib -Wl,-N -Wl,-Ttext -Wl,$link_addr -Wl,--build-id=none conftest.o -o conftest.exec]); then :
   else
     AC_MSG_ERROR([${CC-cc} cannot link at address $link_addr])
   fi
Index: genmk.rb
===================================================================
RCS file: /sources/grub/grub2/genmk.rb,v
retrieving revision 1.29
diff -u -p -r1.29 genmk.rb
--- genmk.rb	20 Oct 2007 18:32:17 -0000	1.29
+++ genmk.rb	25 Nov 2007 21:57:04 -0000
@@ -57,7 +57,7 @@ class Image
 MOSTLYCLEANFILES += #{deps_str}
 
 #{@name}: #{exe}
-	$(OBJCOPY) -O binary -R .note -R .comment $< $@
+	$(OBJCOPY) -O binary -R .note -R .comment -R .note.gnu.build-id $< $@
 
 #{exe}: #{objs_str}
 	$(TARGET_CC) -o $@ $^ $(TARGET_LDFLAGS) $(#{prefix}_LDFLAGS)
@@ -115,7 +115,7 @@ UNDSYMFILES += #{undsym}
 #{@name}: #{pre_obj} #{mod_obj}
 	-rm -f $@
 	$(TARGET_CC) $(#{prefix}_LDFLAGS) $(TARGET_LDFLAGS) -Wl,-r,-d -o $@ $^
-	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@
+	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment -R .note.gnu.build-id $@
 
 #{pre_obj}: $(#{prefix}_DEPENDENCIES) #{objs_str}
 	-rm -f $@

Re: [PATCH] Fix for linker build ID in Fedora 8

[Lubomir Kundrak]

On Sun, 2007-11-25 at 23:26 +0100, Lubomir Kundrak wrote:
> Hi,
> 
> Thanks for the patch!
> 
> On Fri, 2007-11-23 at 22:07 -0500, Pavel Roskin wrote:
> > Hello!
> > Here's an alternate patch for the issue that appeared on Fedora 8.  The
> > linker adds a "build ID" (.not.gnu.build-id) by default.  That section
> > makes objcopy create output files about 128M long when asked to use
> > binary format.  Needless to say, they are not suitable as grub images.

> I attach the patch I use. The first hunk is basically the same as yours,
> the second one is self-explanatory.

Actually, the first hunk should better look like this:
http://lists.gnu.org/archive/html/grub-devel/2007-11/msg00187.html

-- 
Lubomir Kundrak (Red Hat Security Response Team)

Re: [PATCH] Fix for linker build ID in Fedora 8

[Pavel Roskin]

Hello!

On Mon, 2007-11-26 at 11:28 +0100, Lubomir Kundrak wrote:

> > I attach the patch I use. The first hunk is basically the same as yours,
> > the second one is self-explanatory.
> 
> Actually, the first hunk should better look like this:
> http://lists.gnu.org/archive/html/grub-devel/2007-11/msg00187.html

The objcopy test exists to detect certain breakage in objcopy.  I'm not
sure that testing it in the condition that it only copies .text is
sufficient to find the original problem.  Surely, that will need some
software archeology.  The test comes from GRUB 1.  Anyway, I'll rather
test objcopy under conditions close to those used in the build process.

And what's the build ID for?  Why do we want to keep it?

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix for linker build ID in Fedora 8

[Lubomir Kundrak]

On Mon, 2007-11-26 at 07:35 -0500, Pavel Roskin wrote:
> Hello!
> 
> On Mon, 2007-11-26 at 11:28 +0100, Lubomir Kundrak wrote:
> 
> > > I attach the patch I use. The first hunk is basically the same as yours,
> > > the second one is self-explanatory.
> > 
> > Actually, the first hunk should better look like this:
> > http://lists.gnu.org/archive/html/grub-devel/2007-11/msg00187.html
> 
> The objcopy test exists to detect certain breakage in objcopy.  I'm not
> sure that testing it in the condition that it only copies .text is
> sufficient to find the original problem.  Surely, that will need some
> software archeology.  The test comes from GRUB 1.  Anyway, I'll rather
> test objcopy under conditions close to those used in the build process.

To be honest, I'm not completely sure either.

> And what's the build ID for?  Why do we want to keep it?

It basically makes it possible to match an executable or library with
its core dump and sources it was compiled for. The latter is achieved by
placing the sources to build-specific directory.

In Fedora and Red Hat Enterprise Linux, debugging information (debugging
symbols and source code) are packged together in -debuginfo packages.
Since Fedora 8 it is for example possible to comfortably find the
debugging information and respective binary package when everything you
have is the core dump. You can have multiple versions debugging
informations for different builds of the same package installed at the
same time and debugger will find the matching code. Moreover, package
that doesn't link with build id won't build in our build system, unless
you work around it (which would you do for example if you were building
encumbered binary-only package, which is definitely not the case).

-- 
Lubomir Kundrak (Red Hat Security Response Team)

Re: [PATCH] Fix for linker build ID in Fedora 8

[Pavel Roskin]

On Mon, 2007-11-26 at 13:54 +0100, Lubomir Kundrak wrote:
> On Mon, 2007-11-26 at 07:35 -0500, Pavel Roskin wrote:

> > The objcopy test exists to detect certain breakage in objcopy.  I'm not
> > sure that testing it in the condition that it only copies .text is
> > sufficient to find the original problem.  Surely, that will need some
> > software archeology.  The test comes from GRUB 1.  Anyway, I'll rather
> > test objcopy under conditions close to those used in the build process.
> 
> To be honest, I'm not completely sure either.

OK, I understand that you also want the test to pass on native x86_64.
I'll try to find more information about the original objcopy problem and
make a fix that addresses both the build ID issue and the x86_64 issue.

> > And what's the build ID for?  Why do we want to keep it?
> 
> It basically makes it possible to match an executable or library with
> its core dump and sources it was compiled for. The latter is achieved by
> placing the sources to build-specific directory.

Fair enough.  I've been helped by Fedora's debuginfo more than once.  

I understand it's OK to strip build ID from anything that is used in the
real boot (whether it's affected by the new objcopy bug or not), as we
want space saving and we cannot debug it in gdb.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix for linker build ID in Fedora 8

[Robert Millan]

Is this fixed now?

On Mon, Nov 26, 2007 at 08:06:27AM -0500, Pavel Roskin wrote:
> On Mon, 2007-11-26 at 13:54 +0100, Lubomir Kundrak wrote:
> > On Mon, 2007-11-26 at 07:35 -0500, Pavel Roskin wrote:
> 
> > > The objcopy test exists to detect certain breakage in objcopy.  I'm not
> > > sure that testing it in the condition that it only copies .text is
> > > sufficient to find the original problem.  Surely, that will need some
> > > software archeology.  The test comes from GRUB 1.  Anyway, I'll rather
> > > test objcopy under conditions close to those used in the build process.
> > 
> > To be honest, I'm not completely sure either.
> 
> OK, I understand that you also want the test to pass on native x86_64.
> I'll try to find more information about the original objcopy problem and
> make a fix that addresses both the build ID issue and the x86_64 issue.
> 
> > > And what's the build ID for?  Why do we want to keep it?
> > 
> > It basically makes it possible to match an executable or library with
> > its core dump and sources it was compiled for. The latter is achieved by
> > placing the sources to build-specific directory.
> 
> Fair enough.  I've been helped by Fedora's debuginfo more than once.  
> 
> I understand it's OK to strip build ID from anything that is used in the
> real boot (whether it's affected by the new objcopy bug or not), as we
> want space saving and we cannot debug it in gdb.
> 
> -- 
> Regards,
> Pavel Roskin

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] Fix for linker build ID in Fedora 8

[Pavel Roskin]

Quoting Robert Millan <rmh@aybabtu.com>:

> Is this fixed now?

Yes, it's fixed now.  Binaries for the target have build ID stripped.   
Native binaries have build ID, as it was asked by Lubomir.  Everything  
compiles out-of-box with no extra options.

Sorry if it was unclear.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix for linker build ID in Fedora 8

[Pavel Roskin]

Quoting Robert Millan <rmh@aybabtu.com>:

> Is this fixed now?

P.S.  Actually, I realize now that the fix I applied wasn't the best  
approach.  The issue is with objcopy, not with ld, and should be  
addressed with objcopy flags.  It can be gone with objcopy flags (-R  
.note.gnu.build-id) without any tests, whereas ld needs to be tested  
for --build-id=none support.  And even if the build ID is only useful  
for the executables in Fedora, there is no need to strip it from the  
temporary files that are processed by objcopy.

I considered both patches and chose the "least intrusive" one based on  
the patch size, which wasn't a good idea.

I also assumed that "-R .note.gnu.build-id" should be added to strip  
invocations, not only to objcopy.  While it could be a good idea,  
there is no harm other than wasting a few bytes in having build ID on  
modules.  The issue could be addressed separately.  Maybe there are  
more sections to strip in the module files.

It's only the *.img files that are mangled beyond recognition if the  
build ID is not stripped, and it's entirely an objcopy bug.  Strip  
does the right thing.

I'm not trying to deal with the native x86_64 patch here.  It's a  
separate issue with the objcopy test.  It may be another objcopy bug.   
It may warrant introducing OBJCOPY_FLAGS to hold the increasing number  
of objcopy flags.  But it's out of scope for now.

Please consider this patch.  In case it's mangled, I put a copy to  
http://red-bean.com/proski/grub/grub2-objcopy-f8.diff

Index: ChangeLog
===================================================================
RCS file: /cvsroot/grub/grub2/ChangeLog,v
retrieving revision 1.452
diff -u -p -r1.452 ChangeLog
--- ChangeLog	25 Nov 2007 02:01:29 -0000	1.452
+++ ChangeLog	14 Dec 2007 05:31:11 -0000
@@ -1,3 +1,11 @@
+2007-12-14  Pavel Roskin  <proski@gnu.org>
+
+	* configure.ac: Revert previous patch.
+	* aclocal.m4: Likewise.  Instead, strip build ID when calling
+	objcopy.  It's a bug in objcopy, and should be worked around in
+	objcopy.
+	* genmk.rb: Strip build ID when calling objcopy.
+
  2007-11-24  Pavel Roskin  <proski@gnu.org>

  	* configure.ac: Test if '--build-id=none' is supported by the
Index: aclocal.m4
===================================================================
RCS file: /cvsroot/grub/grub2/aclocal.m4,v
retrieving revision 1.7
diff -u -p -r1.7 aclocal.m4
--- aclocal.m4	25 Nov 2007 02:01:30 -0000	1.7
+++ aclocal.m4	14 Dec 2007 05:31:11 -0000
@@ -61,7 +61,7 @@ for link_addr in 2000 8000 7C00; do
    else
      AC_MSG_ERROR([${CC-cc} cannot link at address $link_addr])
    fi
-  if AC_TRY_COMMAND([${OBJCOPY-objcopy} -O binary conftest.exec  
conftest]); then :
+  if AC_TRY_COMMAND([${OBJCOPY-objcopy} -O binary -R  
.note.gnu.build-id conftest.exec conftest]); then :
    else
      AC_MSG_ERROR([${OBJCOPY-objcopy} cannot create binary files])
    fi
Index: configure.ac
===================================================================
RCS file: /cvsroot/grub/grub2/configure.ac,v
retrieving revision 1.40
diff -u -p -r1.40 configure.ac
--- configure.ac	25 Nov 2007 02:01:30 -0000	1.40
+++ configure.ac	14 Dec 2007 05:31:12 -0000
@@ -229,16 +229,6 @@ if test "x$target_m32" = x1; then
    TARGET_LDFLAGS="$TARGET_LDFLAGS -m32"
  fi

-AC_MSG_CHECKING([whether the linker accepts `--build-id=none'])
-save_LDFLAGS="$LDFLAGS"
-LDFLAGS="$LDFLAGS -Wl,--build-id=none"
-AC_TRY_LINK(, , build_id_flag=yes, build_id_flag=no)
-AC_MSG_RESULT([$build_id_flag])
-LDFLAGS="$save_LDFLAGS"
-if test "x$build_id_flag" = xyes; then
-  TARGET_LDFLAGS="$TARGET_LDFLAGS -Wl,--build-id=none"
-fi
-
  #
  # Compiler features.
  #
Index: genmk.rb
===================================================================
RCS file: /cvsroot/grub/grub2/genmk.rb,v
retrieving revision 1.29
diff -u -p -r1.29 genmk.rb
--- genmk.rb	20 Oct 2007 18:32:17 -0000	1.29
+++ genmk.rb	14 Dec 2007 05:31:12 -0000
@@ -57,7 +57,7 @@ class Image
  MOSTLYCLEANFILES += #{deps_str}

  #{@name}: #{exe}
-	$(OBJCOPY) -O binary -R .note -R .comment $< $@
+	$(OBJCOPY) -O binary -R .note -R .comment -R .note.gnu.build-id $< $@

  #{exe}: #{objs_str}
  	$(TARGET_CC) -o $@ $^ $(TARGET_LDFLAGS) $(#{prefix}_LDFLAGS)

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix for linker build ID in Fedora 8

[Bean]

Hi,

In fact, i would prefer to use linker script to solve this problem.
Linker script direct the link process, you can combine the sections in
any way you want. For example, the following linker script:

SECTIONS
     {
       .=0x8000
       .text :
       {
         *(.text)
       }
     }

should only copy .text section to the output expectable.

To use the linker script:

ld -T my_script ...

I use this method to solve a problem in the cygwin ld. Its default
linker script create __CTOR_LIST__ and __DTOR_LIST__ variable at the
end of the text section, so it can't get the length right. After using
the new linker script, I'm able to create the exact content that I
want.

-- 
Bean

Re: [PATCH] Fix for linker build ID in Fedora 8

[Pavel Roskin]

Quoting Bean <bean123ch@gmail.com>:

> Hi,
>
> In fact, i would prefer to use linker script to solve this problem.

OK, one approach would be to have an equivalent to "--build-id=none"  
that would work with all versions of ld.  That would be a nice  
simplification, but it would still be working around objcopy bugs  
before objcopy.

Another, more radical approach, would be to make ld work as objcopy  
and output the binary, thus avoiding the stage with *.exec files.

We would depend on the fact that if objcopy can output in the binary  
format, ld can do it as well.  I think it's a safe assumption.

We could have separate ld scripts for different targets and different  
outputs, such as *.mod and *.img files.

> Linker script direct the link process, you can combine the sections in
> any way you want. For example, the following linker script:
>
> SECTIONS
>      {
>        .=0x8000
>        .text :
>        {
>          *(.text)
>        }
>      }
>
> should only copy .text section to the output expectable.

We would need a script to remove a section (.note.gnu.build-id) or  
maybe all sections with names starting with ".note" and a command to  
set output format to  binary (if possible, it can be done on the  
command line too).

-- 
Regards,
Pavel Roskin