* [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S
@ 2018-09-07 7:06 Terry Guo
2018-09-11 20:47 ` H.J. Lu
2018-09-12 7:43 ` Florian Weimer
0 siblings, 2 replies; 7+ messages in thread
From: Terry Guo @ 2018-09-07 7:06 UTC (permalink / raw)
To: libc-alpha; +Cc: hongjiu.lu
Hello,
The patch is to fix this issue by wrapping the _start function with
ENTRY and END just like what we did for 64bit target. Tested with
glibc 32bit build, there is no regression. OK for trunk?
BR,
Terry
2018-09-07 H.J. Lu <hongjiu.lu@intel.com>
Xuepeng Guo <xuepeng.guo@intel.com>
[BZ #23606]
* sysdeps/i386/start.S: Wrap _start function with ENTRY and END.
diff --git a/sysdeps/i386/start.S b/sysdeps/i386/start.S
index 91035fa83f..e35e9bd31b 100644
--- a/sysdeps/i386/start.S
+++ b/sysdeps/i386/start.S
@@ -52,10 +52,11 @@
NULL
*/
- .text
- .globl _start
- .type _start,@function
-_start:
+#include <sysdep.h>
+
+ENTRY (_start)
+ /* Clearing frame pointer is insufficient, use CFI. */
+ cfi_undefined (eip)
/* Clear the frame pointer. The ABI suggests this be done, to mark
the outermost frame obviously. */
xorl %ebp, %ebp
@@ -131,6 +132,7 @@ _start:
1: movl (%esp), %ebx
ret
#endif
+END (_start)
/* To fulfill the System V/i386 ABI we need this symbol. Yuck, it's so
meaningless since we don't support machines < 80386. */
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S
2018-09-07 7:06 [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S Terry Guo
@ 2018-09-11 20:47 ` H.J. Lu
2018-09-12 7:43 ` Florian Weimer
1 sibling, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2018-09-11 20:47 UTC (permalink / raw)
To: Terry Guo; +Cc: GNU C Library
On Fri, Sep 7, 2018 at 12:06 AM, Terry Guo <terry.xpguo@gmail.com> wrote:
> Hello,
>
> The patch is to fix this issue by wrapping the _start function with
> ENTRY and END just like what we did for 64bit target. Tested with
> glibc 32bit build, there is no regression. OK for trunk?
>
> BR,
> Terry
>
>
> 2018-09-07 H.J. Lu <hongjiu.lu@intel.com>
> Xuepeng Guo <xuepeng.guo@intel.com>
>
> [BZ #23606]
> * sysdeps/i386/start.S: Wrap _start function with ENTRY and END.
>
>
> diff --git a/sysdeps/i386/start.S b/sysdeps/i386/start.S
> index 91035fa83f..e35e9bd31b 100644
> --- a/sysdeps/i386/start.S
> +++ b/sysdeps/i386/start.S
> @@ -52,10 +52,11 @@
> NULL
> */
>
> - .text
> - .globl _start
> - .type _start,@function
> -_start:
> +#include <sysdep.h>
> +
> +ENTRY (_start)
> + /* Clearing frame pointer is insufficient, use CFI. */
> + cfi_undefined (eip)
> /* Clear the frame pointer. The ABI suggests this be done, to mark
> the outermost frame obviously. */
> xorl %ebp, %ebp
> @@ -131,6 +132,7 @@ _start:
> 1: movl (%esp), %ebx
> ret
> #endif
> +END (_start)
>
> /* To fulfill the System V/i386 ABI we need this symbol. Yuck, it's so
> meaningless since we don't support machines < 80386. */
LGTM. Please check it in.
If you don't have an account, please send me an attachment of
"git format-patch". I will check it in for you.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S
2018-09-07 7:06 [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S Terry Guo
2018-09-11 20:47 ` H.J. Lu
@ 2018-09-12 7:43 ` Florian Weimer
2018-09-12 11:57 ` H.J. Lu
1 sibling, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2018-09-12 7:43 UTC (permalink / raw)
To: Terry Guo, libc-alpha; +Cc: hongjiu.lu
On 09/07/2018 09:06 AM, Terry Guo wrote:
> + /* Clearing frame pointer is insufficient, use CFI. */
> + cfi_undefined (eip)
Isn't this a separate fix?
The ChangeLog or the commit message should mention ENDBR32 (âUse ENTRY
to insert ENDBR32 when building for CETâ or something like that).
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S
2018-09-12 7:43 ` Florian Weimer
@ 2018-09-12 11:57 ` H.J. Lu
2018-09-12 11:59 ` Florian Weimer
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2018-09-12 11:57 UTC (permalink / raw)
To: Florian Weimer; +Cc: Terry Guo, GNU C Library, Lu, Hongjiu
On Wed, Sep 12, 2018 at 12:43 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 09/07/2018 09:06 AM, Terry Guo wrote:
>>
>> + /* Clearing frame pointer is insufficient, use CFI. */
>> + cfi_undefined (eip)
>
>
> Isn't this a separate fix?
Since _start now includes CFI, without "cfi_undefined (eip)", unwinder may not
terminate at _start and one unwind test will fail.
> The ChangeLog or the commit message should mention ENDBR32 (“Use ENTRY to
> insert ENDBR32 when building for CET” or something like that).
>
Please also write ChangeLog as
* sysdeps/i386/start.S (_start): ...
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S
2018-09-12 11:57 ` H.J. Lu
@ 2018-09-12 11:59 ` Florian Weimer
2018-09-12 15:20 ` H.J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2018-09-12 11:59 UTC (permalink / raw)
To: H.J. Lu; +Cc: Terry Guo, GNU C Library, Lu, Hongjiu
On 09/12/2018 01:57 PM, H.J. Lu wrote:
> On Wed, Sep 12, 2018 at 12:43 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 09/07/2018 09:06 AM, Terry Guo wrote:
>>>
>>> + /* Clearing frame pointer is insufficient, use CFI. */
>>> + cfi_undefined (eip)
>>
>>
>> Isn't this a separate fix?
>
> Since _start now includes CFI, without "cfi_undefined (eip)", unwinder may not
> terminate at _start and one unwind test will fail.
Ah! Please include this information in the commit message or ChangeLog
entry, too.
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S
2018-09-12 11:59 ` Florian Weimer
@ 2018-09-12 15:20 ` H.J. Lu
2018-09-12 15:28 ` Florian Weimer
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2018-09-12 15:20 UTC (permalink / raw)
To: Florian Weimer; +Cc: Terry Guo, GNU C Library
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On Wed, Sep 12, 2018 at 4:59 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 09/12/2018 01:57 PM, H.J. Lu wrote:
>>
>> On Wed, Sep 12, 2018 at 12:43 AM, Florian Weimer <fweimer@redhat.com>
>> wrote:
>>>
>>> On 09/07/2018 09:06 AM, Terry Guo wrote:
>>>>
>>>>
>>>> + /* Clearing frame pointer is insufficient, use CFI. */
>>>> + cfi_undefined (eip)
>>>
>>>
>>>
>>> Isn't this a separate fix?
>>
>>
>> Since _start now includes CFI, without "cfi_undefined (eip)", unwinder
>> may not
>> terminate at _start and one unwind test will fail.
>
>
> Ah! Please include this information in the commit message or ChangeLog
> entry, too.
This is the patch I am going to check in.
--
H.J.
[-- Attachment #2: 0001-i386-Use-ENTRY-and-END-in-start.S-BZ-23606.patch --]
[-- Type: text/x-patch, Size: 3184 bytes --]
From 402ea0545c9efa49be1596522e517b196d1397a8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 5 Sep 2018 05:58:45 -0700
Subject: [PATCH] i386: Use ENTRY and END in start.S [BZ #23606]
Wrapping the _start function with ENTRY and END to insert ENDBR32 at
function entry when CET is enabled. Since _start now includes CFI,
without "cfi_undefined (eip)", unwinder may not terminate at _start
and we will get
Program received signal SIGSEGV, Segmentation fault.
0xf7dc661e in ?? () from /lib/libgcc_s.so.1
Missing separate debuginfos, use: dnf debuginfo-install libgcc-8.2.1-3.0.fc28.i686
(gdb) bt
#0 0xf7dc661e in ?? () from /lib/libgcc_s.so.1
#1 0xf7dc7c18 in _Unwind_Backtrace () from /lib/libgcc_s.so.1
#2 0xf7f0d809 in __GI___backtrace (array=array@entry=0xffffc7d0,
size=size@entry=20) at ../sysdeps/i386/backtrace.c:127
#3 0x08049254 in compare (p1=p1@entry=0xffffcad0, p2=p2@entry=0xffffcad4)
at backtrace-tst.c:12
#4 0xf7e2a28c in msort_with_tmp (p=p@entry=0xffffca5c, b=b@entry=0xffffcad0,
n=n@entry=2) at msort.c:65
#5 0xf7e29f64 in msort_with_tmp (n=2, b=0xffffcad0, p=0xffffca5c)
at msort.c:53
#6 msort_with_tmp (p=p@entry=0xffffca5c, b=b@entry=0xffffcad0, n=n@entry=5)
at msort.c:53
#7 0xf7e29f64 in msort_with_tmp (n=5, b=0xffffcad0, p=0xffffca5c)
at msort.c:53
#8 msort_with_tmp (p=p@entry=0xffffca5c, b=b@entry=0xffffcad0, n=n@entry=10)
at msort.c:53
#9 0xf7e29f64 in msort_with_tmp (n=10, b=0xffffcad0, p=0xffffca5c)
at msort.c:53
#10 msort_with_tmp (p=p@entry=0xffffca5c, b=b@entry=0xffffcad0, n=n@entry=20)
at msort.c:53
#11 0xf7e2a5b6 in msort_with_tmp (n=20, b=0xffffcad0, p=0xffffca5c)
at msort.c:297
#12 __GI___qsort_r (b=b@entry=0xffffcad0, n=n@entry=20, s=s@entry=4,
cmp=cmp@entry=0x8049230 <compare>, arg=arg@entry=0x0) at msort.c:297
#13 0xf7e2a84d in __GI_qsort (b=b@entry=0xffffcad0, n=n@entry=20, s=s@entry=4,
cmp=cmp@entry=0x8049230 <compare>) at msort.c:308
#14 0x080490f6 in main (argc=2, argv=0xffffcbd4) at backtrace-tst.c:39
FAIL: debug/backtrace-tst
2018-09-12 H.J. Lu <hongjiu.lu@intel.com>
Xuepeng Guo <xuepeng.guo@intel.com>
[BZ #23606]
* sysdeps/i386/start.S: Include <sysdep.h>
(_start): Use ENTRY/END to insert ENDBR32 at entry when CET is
enabled. Add cfi_undefined (eip).
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
sysdeps/i386/start.S | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sysdeps/i386/start.S b/sysdeps/i386/start.S
index 91035fa83f..e35e9bd31b 100644
--- a/sysdeps/i386/start.S
+++ b/sysdeps/i386/start.S
@@ -52,10 +52,11 @@
NULL
*/
- .text
- .globl _start
- .type _start,@function
-_start:
+#include <sysdep.h>
+
+ENTRY (_start)
+ /* Clearing frame pointer is insufficient, use CFI. */
+ cfi_undefined (eip)
/* Clear the frame pointer. The ABI suggests this be done, to mark
the outermost frame obviously. */
xorl %ebp, %ebp
@@ -131,6 +132,7 @@ _start:
1: movl (%esp), %ebx
ret
#endif
+END (_start)
/* To fulfill the System V/i386 ABI we need this symbol. Yuck, it's so
meaningless since we don't support machines < 80386. */
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S
2018-09-12 15:20 ` H.J. Lu
@ 2018-09-12 15:28 ` Florian Weimer
0 siblings, 0 replies; 7+ messages in thread
From: Florian Weimer @ 2018-09-12 15:28 UTC (permalink / raw)
To: H.J. Lu; +Cc: Terry Guo, GNU C Library
On 09/12/2018 05:20 PM, H.J. Lu wrote:
> This is the patch I am going to check in.
Nice commit message and ChangeLog entry, assuming that the lines with #
survive the git comment filter. 8-)
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-09-12 15:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07 7:06 [patch] Fix BZ 23606 -- Missing ENDBR32 in sysdeps/i386/start.S Terry Guo
2018-09-11 20:47 ` H.J. Lu
2018-09-12 7:43 ` Florian Weimer
2018-09-12 11:57 ` H.J. Lu
2018-09-12 11:59 ` Florian Weimer
2018-09-12 15:20 ` H.J. Lu
2018-09-12 15:28 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).