Re: [PATCH] Remove debug/stack_chk_fail_local.c [BZ #21740]

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sun, Jul 16, 2017 at 12:22 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jul 10, 2017 at 10:34 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Jul 10, 2017 at 6:29 AM, Nick Alcock <nix@esperi.org.uk> wrote:
>>> On 10 Jul 2017, H. J. Lu said:
>>>> On Mon, Jul 10, 2017 at 11:50:02AM +0100, Nick Alcock wrote:
>>>>> If it passes a test build with --enable-stack-protector=all without
>>>>> pulling junk into ld.so and exploding at ld.so link time, sure. (That's
>>>>> what happened every time I tried to remove this stuff before, but I may
>>>>> have failed to notice that this may not be necessary any more.)
>>>>
>>>> Here is the updated patch.  tst-_dl_addr_inside_object should be
>>>> linked with $(dummy-stack-chk-fail).  Otherwise, __stack_chk_fail is
>>>> undefined.  OK for master branch?
>>>
>>> I presume this is because it's in $(all-nonlib)? Are other all-nonlib
>>> things similarly affected? (If they are, is the code in Makerules
>>> perhaps a better place to adjust this?)
>>>
>>> I guess the only affected nonlib things would be things that directly
>>> link against objects that will otherwise end up in the shared libc or
>>> ld.so, which means this is the only affected test (since only those
>>> things reference the usually-hidden __stack_chk_fail). If so, I have no
>>> objection to this patch, but maybe a comment explaining this would be a
>>> good idea so that if more such tests turn up in future this unusual
>>> piece of linking can be generalized a bit.
>>>
>>> Modulo that, I have no objections, but of course I also have no actual
>>> right to ack anything whatsoever :)
>>>
>>>>> > -/* On some architectures, this helps needless PIC pointer setup
>>>>> > -   that would be needed just for the __stack_chk_fail call.  */
>>>>>
>>>>> Does anyone know what architectures these might be? Presumably x86-32...
>>>>>
>>>>
>>>> config/i386/i386.c:   __stack_chk_fail_local hidden function instead of calling
>>>
>>> I presume you tested a build on x86-32 :) I guess that will suffice...
>>
>> We must keep debug/stack_chk_fail_local.c for libc_nonshared.a.
>> Here is the updated patch to add __stack_chk_fail_local alias only
>> to libc.so.
>>
>> Tested on i686 and x86-64 with and without --enable-stack-protector=all.
>> OK for master?
>>
>
> Any objections?
>
> https://sourceware.org/ml/libc-alpha/2017-07/msg00406.html
>

Here is the updated patch.   I will check it tomorrow if there are no
objections.


-- 
H.J.

From 47d95763a35fbc83ac871b10fc59f9eca1ef0a40 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 9 Jul 2017 08:39:17 -0700
Subject: [PATCH] Don't add stack_chk_fail_local.o to libc.a [BZ #21740]

commit 524a8ef2ad76af8ac049293d993a1856b0d888fb
Author: Nick Alcock <nick.alcock@oracle.com>
Date:   Mon Dec 26 10:08:57 2016 +0100

    PLT avoidance for __stack_chk_fail [BZ #7065]

    Add a hidden __stack_chk_fail_local alias to libc.so,
    and make sure that on targets which use __stack_chk_fail,
    this does not introduce a local PLT reference into libc.so.

which unconditionally added

strong_alias (__stack_chk_fail, __stack_chk_fail_local)

defines __stack_chk_fail_local as an alias of __stack_chk_fail in libc.a.
There is no need to add stack_chk_fail_local.o to libc.a.  We only need
to add stack_chk_fail_local.oS to libc_nonshared.a.

Tested on x86-64:

[hjl@gnu-skl-1 build-x86_64-linux]$ nm libc.a | grep __stack_chk_fail
0000000000000000 T __stack_chk_fail
0000000000000000 T __stack_chk_fail_local
[hjl@gnu-skl-1 build-x86_64-linux]$ nm libc_nonshared.a | grep __stack_chk_fail_local
0000000000000000 T __stack_chk_fail_local
[hjl@gnu-skl-1 build-x86_64-linux]$

	[BZ #21740]
	* debug/Makefile (elide-routines.o): New.
---
 debug/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/debug/Makefile b/debug/Makefile
index ce5fa8801f..504bf875fe 100644
--- a/debug/Makefile
+++ b/debug/Makefile
@@ -53,6 +53,10 @@ routines  = backtrace backtracesyms backtracesymsfd noophooks \
 	    $(static-only-routines)
 static-only-routines := warning-nop stack_chk_fail_local
 
+# Don't add stack_chk_fail_local.o to libc.a since __stack_chk_fail_local
+# is an alias of __stack_chk_fail in stack_chk_fail.o.
+elide-routines.o := stack_chk_fail_local
+
 # Building the stack-protector failure routines with stack protection
 # makes no sense.
 
-- 
2.13.3