Control: reopen 995393 Control: tags 995393 patch Christoph Biedl wrote...
> As a workaround I propose a tactical __attribute__ to locally disable
> optimization as this issue has been blocking eatmydata for way too long,
> and performance loss should be neglectable.
So as always with a band-aid, it's wiser to address the underlying
problem than to work around it - this time fakeroot broke the glibc
autopkgtest ...
As jtrc27 pointed out in IRC, ppc64el is fairly picky when it comes to
va_arg handling, and next_openat needs a correct "..." parameter in the
prototype to have proper code generated.
As a surprise, it seems the original submitter had that in mind by
introducing a sixth column to wrapfunc.inp, although it's not used by
wrapwak (and not documented either, fix for that included).
My solution isn't nice as it hardcodes the va_arg handling into the
expansion, making it a special for wrapping the openat call. There's
always room for improvement B=)
Additionally, some headers are not necesarily re-generated - so add
an extra rm call in the clean target to enforce this.
Status: Passes on the porterbox, no compiler warnings/errors from
a prototype mismatch.
Christoph
From d30127a5b2a2f26ae2f6ddc8df077a548ec145e0 Mon Sep 17 00:00:00 2001
From: Christoph Biedl <debian.a...@manchmal.in-ulm.de>
Date: Thu, 30 Dec 2021 19:05:37 +0100
Subject: [PATCH] Fix segfault on ppc64el, take 2
---
debian/patches/fix-prototype-generation.patch | 53 +++++++++++++++++++
debian/patches/ppc64el-workaround.patch | 31 -----------
debian/patches/series | 2 +-
debian/rules | 1 +
4 files changed, 55 insertions(+), 32 deletions(-)
create mode 100644 debian/patches/fix-prototype-generation.patch
delete mode 100644 debian/patches/ppc64el-workaround.patch
diff --git a/debian/patches/fix-prototype-generation.patch
b/debian/patches/fix-prototype-generation.patch
new file mode 100644
index 0000000..d364c5c
--- /dev/null
+++ b/debian/patches/fix-prototype-generation.patch
@@ -0,0 +1,53 @@
+Subject: Fix prototype generation for openat
+Author: Christoph Biedl <debian.a...@manchmal.in-ulm.de>
+Date: 2021-12-30
+Bug-Debian: https://bugs.debian.org/995393
+Forwarded: Yes (implicitely)
+
+ As jtrc27 pointed out in IRC, ppc64el is fairly picky when it comes to
+ va_arg handling, and next_openat needs a correct "..." parameter in the
+ prototype to have proper code generated.
+
+--- a/wrapawk
++++ b/wrapawk
+@@ -37,7 +37,25 @@
+ argtype=$3;
+ argname=$4;
+ MACRO=$5;
+- if(MACRO){
++ openat_extra=$6;
++ if(openat_extra){
++ print " {(void(*))&next_" name ", \"" name "\"}," > structfile;
++ print "extern " ret " (*next_" name ")" openat_extra ";" > headerfile;
++ print ret " (*next_" name ")" openat_extra "=tmp_" name ";"> deffile;
++
++ print ret " tmp_" name, openat_extra "{" > tmpffile;
++ print " mode_t mode = 0;" > tmpffile;
++ print " if (flags & O_CREAT) {" > tmpffile;
++ print " va_list args;" > tmpffile;
++ print " va_start(args, flags);" > tmpffile;
++ print " mode = va_arg(args, int);" > tmpffile;
++ print " va_end(args);" > tmpffile;
++ print " }" > tmpffile;
++ print " load_library_symbols();" > tmpffile;
++ print " return next_" name, argname ";" > tmpffile;
++ print "}" > tmpffile;
++ print "" > tmpffile;
++ } else if(MACRO){
+ print " {(void(*))&NEXT_" MACRO "_NOARG, " name "_QUOTE}," > structfile;
+ print "extern " ret " (*NEXT_" MACRO "_NOARG)" argtype ";" > headerfile;
+ print ret " (*NEXT_" MACRO "_NOARG)" argtype "=TMP_" MACRO ";"> deffile;
+--- a/wrapfunc.inp
++++ b/wrapfunc.inp
+@@ -9,8 +9,10 @@
+ /**/ */
+ /* each line of this file lists 4 fields, seperated by a ";". */
+ /* The first field is the name of the wrapped function, then it's return */
+-/* value. After that come the function arguments with types, and the last */
++/* value. After that come the function arguments with types, and the fifth */
+ /* field contains the function arguments without types. */
++/* A sixth field is a special needed when wrapping the openat syscall. */
++/* Otherwise it's like the third (function arguments with types). */
+ /**/
+
+ /* __*xstat are used on glibc systems instead of just *xstat. */
diff --git a/debian/patches/ppc64el-workaround.patch
b/debian/patches/ppc64el-workaround.patch
deleted file mode 100644
index 95b0ff0..0000000
--- a/debian/patches/ppc64el-workaround.patch
+++ /dev/null
@@ -1,31 +0,0 @@
-Subject: Work around segfault on ppc64el
-Author: Christoph Biedl <debian.a...@manchmal.in-ulm.de>
-Date: 2021-12-20
-Bug-Debian: https://bugs.debian.org/995393
-Forwarded: Yes
-
- For whatever reason the generated code segfaults on ppc64el when
- built with the usual optimizations. The root cause is not clear,
- might be a compiler bug or the result of improperly generated
- function prototypes.
-
- As a workaround, disable optimizations for the affected function.
-
- This should be re-visted upon any major compiler release whether
- it's still needed.
-
---- a/libfakeroot.c
-+++ b/libfakeroot.c
-@@ -2596,7 +2596,11 @@
- #endif
-
- #ifdef HAVE_OPENAT
--int openat(int dir_fd, const char *pathname, int flags, ...)
-+int
-+#if defined(__powerpc__) && defined(__powerpc64__) && __BYTE_ORDER__ ==
__ORDER_LITTLE_ENDIAN__
-+__attribute__((optimize("O0")))
-+#endif
-+openat(int dir_fd, const char *pathname, int flags, ...)
- {
- mode_t mode;
-
diff --git a/debian/patches/series b/debian/patches/series
index d55c36b..38761a2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,3 +1,3 @@
fix-shell-in-fakeroot
also-wrap-stat-library-call.patch
-ppc64el-workaround.patch
+fix-prototype-generation.patch
diff --git a/debian/rules b/debian/rules
index 7f1bf74..d93587c 100755
--- a/debian/rules
+++ b/debian/rules
@@ -63,6 +63,7 @@ override_dh_auto_install:
override_dh_auto_clean:
rm -rf obj-sysv obj-tcp
+ -rm wrapdef.h wrapstruct.h
dh_autoreconf_clean
rm -f dhar-stamp
$(RM) debian/fakeroot.postinst
--
2.34.1
signature.asc
Description: PGP signature
