[master] PR sanitizer/101749 - sanitizer_common/sanitizer_posix_libcdep.cpp: Prevent generation of dependency on _cxa_guard for static initialization.

Message ID 20220311090534.1133919-1-sundeep.kokkonda@gmail.com
State New
Headers show
Series [master] PR sanitizer/101749 - sanitizer_common/sanitizer_posix_libcdep.cpp: Prevent generation of dependency on _cxa_guard for static initialization. | expand

Commit Message

Sundeep KOKKONDA March 11, 2022, 9:05 a.m. UTC
Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@gmail.com>
---
 meta/recipes-devtools/gcc/gcc-11.2.inc         |  1 +
 .../gcc/0042-Fix-thread-stack-size-init.patch  | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch

Comments

Khem Raj March 11, 2022, 5:21 p.m. UTC | #1
lgtm. you might need to add more info into patch header.

On Fri, Mar 11, 2022 at 1:05 AM Sundeep KOKKONDA
<sundeep.kokkonda@gmail.com> wrote:
>
> Signed-off-by: Sundeep KOKKONDA <sundeep.kokkonda@gmail.com>
> ---
>  meta/recipes-devtools/gcc/gcc-11.2.inc         |  1 +
>  .../gcc/0042-Fix-thread-stack-size-init.patch  | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
>  create mode 100644 meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
>
> diff --git a/meta/recipes-devtools/gcc/gcc-11.2.inc b/meta/recipes-devtools/gcc/gcc-11.2.inc
> index 2394c86e29..0c6c54888c 100644
> --- a/meta/recipes-devtools/gcc/gcc-11.2.inc
> +++ b/meta/recipes-devtools/gcc/gcc-11.2.inc
> @@ -68,6 +68,7 @@ SRC_URI = "\
>             file://0003-CVE-2021-42574.patch \
>             file://0004-CVE-2021-42574.patch \
>             file://0001-CVE-2021-46195.patch \
> +          file://0042-Fix-thread-stack-size-init.patch \
>  "
>  SRC_URI[sha256sum] = "d08edc536b54c372a1010ff6619dd274c0f1603aa49212ba20f7aa2cda36fa8b"
>
> diff --git a/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch b/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
> new file mode 100644
> index 0000000000..ab463f252a
> --- /dev/null
> +++ b/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
> @@ -0,0 +1,18 @@
> +Upstream-Status: Backport [https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=90e46074e6b3561ae7d8ebd205127f286cc0c6b6]
> +---
> +--- a/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp        2022-03-08 03:04:25.871012986 -0800
> ++++ b/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp        2022-03-10 23:28:20.131064433 -0800
> +@@ -166,9 +166,10 @@
> + #if !SANITIZER_GO
> + // TODO(glider): different tools may require different altstack size.
> + static uptr GetAltStackSize() {
> +-  // SIGSTKSZ is not enough.
> +-  static const uptr kAltStackSize = SIGSTKSZ * 4;
> +-  return kAltStackSize;
> ++  // Note: since GLIBC_2.31, SIGSTKSZ may be a function call, so this may be
> ++  // more costly that you think. However GetAltStackSize is only call 2-3 times
> ++  // per thread so don't cache the evaluation.
> ++  return SIGSTKSZ * 4;
> + }
> +
> + void SetAlternateSignalStack() {
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#163047): https://lists.openembedded.org/g/openembedded-core/message/163047
> Mute This Topic: https://lists.openembedded.org/mt/89706923/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Randy MacLeod March 11, 2022, 5:24 p.m. UTC | #2
Hi Sundeep,

I'd like to see a v2 with some log changes.

First, Anuj maintains 3.3/3.4 so no need to CC him.


I'll make some specific comment below but please also read:
https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded
    and links in the "See Also" section.

Your subject (142 chars!)
-------
Re: [OE-core] [master][PATCH]

--->
PR sanitizer/101749 - sanitizer_common/sanitizer_posix_libcdep.cpp: 
Prevent generation of dependency on _cxa_guard for static initialization.
<---
-------
is too long. Look at 'git log --oneline and you'll see that we try to 
keep the commit length
to under 60 chars and most of the exceptions come from when the project 
used SVN rather than git.

$ cd ../oe-core.git
$ git log --oneline  | cut -c 12- | awk '{print length($0)}' | sort -n | 
uniq -c



Also, we have a convention that the subject prefix is the package name
so the shortlog should be something like:

     "gcc: sanitizer: Fix asan against glibc 2.34"
taken from the shortlog for the master commit:

$ git log -1 d9f462fb372fb02da032cefd6b091d7582c425ae



The long log should explain what problem you are trying to fix
in your own words, not just as a link to a YP Bugzilla item.

As explained in:
https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
---

Short log / Statement of what needed to be changed.

(Optional pointers to external resources, such as defect tracking)
   
The intent of your change.
   
(Optional, if it's not clear from above) how your change resolves the issues in the first part.
   
Tag line(s) at the end.

---

Also, when it makes sense, it's good to include some parts of the error 
message that you
are fixing so that someone can search for that string.

Finally, since this is a cherry-pick from the release/gcc-11 branch
you should mention that in the long log as well as what you have done
in the patch itself so that when someone updates from gcc-11.2 to 11.3
they'll have two tips showing that this patch can be dropped.

On 2022-03-11 04:05, Sundeep KOKKONDA wrote:
> Signed-off-by: Sundeep KOKKONDA<sundeep.kokkonda@gmail.com>
> ---
>   meta/recipes-devtools/gcc/gcc-11.2.inc         |  1 +
>   .../gcc/0042-Fix-thread-stack-size-init.patch  | 18 ++++++++++++++++++
>   2 files changed, 19 insertions(+)
>   create mode 100644 meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
>
> diff --git a/meta/recipes-devtools/gcc/gcc-11.2.inc b/meta/recipes-devtools/gcc/gcc-11.2.inc
> index 2394c86e29..0c6c54888c 100644
> --- a/meta/recipes-devtools/gcc/gcc-11.2.inc
> +++ b/meta/recipes-devtools/gcc/gcc-11.2.inc
> @@ -68,6 +68,7 @@ SRC_URI = "\
>              file://0003-CVE-2021-42574.patch  \
>              file://0004-CVE-2021-42574.patch  \
>              file://0001-CVE-2021-46195.patch  \
> +	file://0042-Fix-thread-stack-size-init.patch  \
>   "
>   SRC_URI[sha256sum] = "d08edc536b54c372a1010ff6619dd274c0f1603aa49212ba20f7aa2cda36fa8b"
>   
> diff --git a/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch b/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
> new file mode 100644
> index 0000000000..ab463f252a
> --- /dev/null
> +++ b/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
> @@ -0,0 +1,18 @@
> +Upstream-Status: Backport [https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=90e46074e6b3561ae7d8ebd205127f286cc0c6b6]


This commit changes lots of files:

$ cd .../gcc.git && git checkout release/gcc-11 && git pull
$ git show 90e46074e6b3561ae7d8ebd205127f286cc0c6b6 | diffstat | wc -l
134

looking at:
$ git blame libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp | less

it seems you should reference this commit:

$ git show 91f8a7a34cf2
commit 91f8a7a34cf29ae7c465603a801326767f1cc7e9
Author: Martin Liska <mliska@suse.cz>
Date:   Thu Aug 5 04:43:17 2021

     sanitizer: cherry pick 414482751452e54710f16bae58458c66298aaf69

     The patch is needed in order to support recent glibc (2.34).
    ...


> +---
> +--- a/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp	2022-03-08 03:04:25.871012986 -0800
> ++++ b/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp	2022-03-10 23:28:20.131064433 -0800
> +@@ -166,9 +166,10 @@
> + #if !SANITIZER_GO
> + // TODO(glider): different tools may require different altstack size.
> + static uptr GetAltStackSize() {
> +-  // SIGSTKSZ is not enough.
> +-  static const uptr kAltStackSize = SIGSTKSZ * 4;
> +-  return kAltStackSize;
> ++  // Note: since GLIBC_2.31, SIGSTKSZ may be a function call, so this may be
> ++  // more costly that you think. However GetAltStackSize is only call 2-3 times
> ++  // per thread so don't cache the evaluation.
> ++  return SIGSTKSZ * 4;
> + }
> +
> + void SetAlternateSignalStack() {


And given that this started with glibc-2.31,
you should backport to honister, hardknott and dunfell.

https://wiki.yoctoproject.org/wiki/Releases

once this is in master.


Lots of clean-up for a modest patch but once you get into the groove
of how we do things, there should be fewer, if any, comments.

Thanks!


../Randy


>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#163047):https://lists.openembedded.org/g/openembedded-core/message/163047
> Mute This Topic:https://lists.openembedded.org/mt/89706923/3616765
> Group Owner:openembedded-core+owner@lists.openembedded.org
> Unsubscribe:https://lists.openembedded.org/g/openembedded-core/unsub  [randy.macleod@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Sundeep KOKKONDA March 14, 2022, 2:08 p.m. UTC | #3
Hi Randy,

Commit log updated. Please let me know is it ok?

I'll backport the patch to honister, hardknott and dunfell after patch taken to master.

Thanks,
Sundeep K.

Patch

diff --git a/meta/recipes-devtools/gcc/gcc-11.2.inc b/meta/recipes-devtools/gcc/gcc-11.2.inc
index 2394c86e29..0c6c54888c 100644
--- a/meta/recipes-devtools/gcc/gcc-11.2.inc
+++ b/meta/recipes-devtools/gcc/gcc-11.2.inc
@@ -68,6 +68,7 @@  SRC_URI = "\
            file://0003-CVE-2021-42574.patch \
            file://0004-CVE-2021-42574.patch \
            file://0001-CVE-2021-46195.patch \
+	   file://0042-Fix-thread-stack-size-init.patch \
 "
 SRC_URI[sha256sum] = "d08edc536b54c372a1010ff6619dd274c0f1603aa49212ba20f7aa2cda36fa8b"
 
diff --git a/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch b/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
new file mode 100644
index 0000000000..ab463f252a
--- /dev/null
+++ b/meta/recipes-devtools/gcc/gcc/0042-Fix-thread-stack-size-init.patch
@@ -0,0 +1,18 @@ 
+Upstream-Status: Backport [https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=90e46074e6b3561ae7d8ebd205127f286cc0c6b6]
+---
+--- a/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp	2022-03-08 03:04:25.871012986 -0800
++++ b/libsanitizer/sanitizer_common/sanitizer_posix_libcdep.cpp	2022-03-10 23:28:20.131064433 -0800
+@@ -166,9 +166,10 @@
+ #if !SANITIZER_GO
+ // TODO(glider): different tools may require different altstack size.
+ static uptr GetAltStackSize() {
+-  // SIGSTKSZ is not enough.
+-  static const uptr kAltStackSize = SIGSTKSZ * 4;
+-  return kAltStackSize;
++  // Note: since GLIBC_2.31, SIGSTKSZ may be a function call, so this may be
++  // more costly that you think. However GetAltStackSize is only call 2-3 times
++  // per thread so don't cache the evaluation.
++  return SIGSTKSZ * 4;
+ }
+ 
+ void SetAlternateSignalStack() {