#76 Fix broken configure files in-place
Opened 2 months ago by law. Modified 14 days ago
Unknown source fix-configure  into  master

file modified
+25

@@ -94,8 +94,33 @@

  # Eventually we'll want to turn this on by default, but this gives packagers a

  # way to turn it back off.

  # %_configure_disable_silent_rules 1

+ 

+ # This fixes various easy resolved configure tests that are compromised by LTO.

+ #

+ # We use this within the standard %configure macro, but also make it available for

+ # packages which don't use %configure

+ #

+ # The first two are common ways to test for the existence of a function, so we ensure

+ # the reference to the function is preserved

+ #

+ # The third are constants used to then try to generate NaNs and other key

+ # floating point numbers.  We then use those special FP numbers to try and

+ # raise a SIGFPE.  By declaring x & y volatile we prevent the optimizers

+ # from removing the computation

+ #

+ %_fix_broken_configure_for_lto \

+   for file in $(find . -type f -name configure -print); do \

+     %{__sed} -r --in-place=.backup 's/^char \\(\\*f\\) \\(\\) = /__attribute__ ((used)) char (*f) () = /g' $file; \

+     diff -u $file.backup $file && mv $file.back up $file \

+     %{__sed} -r --in-place=.backup 's/^char \\(\\*f\\) \\(\\);/__attribute__ ((used)) char (*f) ();/g' $file; \

+     diff -u $file.backup $file && mv $file.back up $file \

+     %{__sed} --in-place=.backup ':a;N;\$\!ba;s/int x = 1;.\*int nan;/volatile int x = 1; volatile int y = 0; volatile int z, nan;/g' $file; \

+     diff -u $file.backup $file && mv $file.back up $file \

+   done ;

+ 

  %configure \

    %{set_build_flags}; \

+     [ "%_with_lto" = 1 ] && %{_fix_broken_configure_for_lto} \

this won't do anything since nothing is setting up %_with_lto

    [ "%_configure_gnuconfig_hack" = 1 ] && for i in $(find $(dirname %{_configure}) -name config.guess -o -name config.sub) ; do \

        [ -f /usr/lib/rpm/redhat/$(basename $i) ] && %{__rm} -f $i && %{__cp} -fv /usr/lib/rpm/redhat/$(basename $i) $i ; \

    done ; \

file modified
+4 -1

@@ -6,7 +6,7 @@

  

  Summary: Red Hat specific rpm configuration files

  Name: redhat-rpm-config

- Version: 146

+ Version: 147

  Release: 1%{?dist}

  # No version specified.

  License: GPL+

@@ -207,6 +207,9 @@

  %{_rpmconfigdir}/macros.d/macros.kmp

  

  %changelog

+ * Wed Jan 08 2020 Jeff Law <law@redhat.com> - 147-1

+ - Fix broken configure files in-place

+ 

  * Thu Dec 05 2019 Denys Vlasenko <dvlasenk@redhat.com> - 146-1

  - kmod.prov: fix and speed it up

  

Described at length in the relevant BZ
https://bugzilla.redhat.com/show_bug.cgi?id=1789149

This could go in now. The only fallout would be some packages (if rebuilt) would need to rebuild their documentation (which often depends on the generated configure/config.h files) and some of those packages fail to have a BuildRequires for texinfo. I know which packages are affected by this issue and could fix them up very quickly.

The rationale about this change belongs to the commit message rather than bugzilla, and to make matters worse the commit message doesn't even link to the bug where it's described. Also the commit summary is way too broad as it doesn't even mention that this is somehow related to LTO. A much better commit summary can be derived from this comment in the patch itself: "This fixes various easy resolved configure tests that are compromised by LTO."

I also tend to agree with the bugzilla comments that creating a backup when modifying would go a long way to help resolve magic-angst.

are these fixes already in automake/autoconf upstream/fedora?

On the actual implementation, I've no idea about the actual sed'ed stuff, but:
1) do use a shell test for the disabler instead of a macro conditional
2) use a similar naming as the other two hacks, eg %_configure_lto_hack to enable/disable

That makes it more consistent with what's already there and also more user-friendly, macro conditionals only test for whether a macro is defined or not and undefining to disable is unreliable/cumbersome (as macros can have multiple stacked definitions)

I think weak,externally_visible is the safer compiler barrier. noclone,noinline does not actually inhibit interprocedural analysis.

Florian: it's cloning/inlining that's the source of the problem, not interprocedural analysis.

The routine in question:

find_stack_direction ()
{
static char *addr = 0;
auto char dummy;
if (addr == 0)
{
addr = &dummy;
return find_stack_direction ();
}
else
return (&dummy > addr) ? 1 : -1;
}

When we inline the self-recursive call we can no longer rely on comparing &dummy in the inner call from to &dummy from the outer call frame to determine which way the stack grew. I think noinline,noclone is the right set of attributes to address this problem.

This is a change in behavior due to the retuning of the inliner and the ability to do partial cloning of self-recursive calls more than the introduction of LTO. noclone,noinline are the right attributes IMHO.

Having said that, I think the only time this shows up in autoconf generated configure tests that matter are for packages with an embedded copy of libiberty which has a C-alloca implementation. The other (non-autoconf) instances of this issue are already fixed in the appropriate upstreams.

I could take this out and do a test build over the next few days while we wait for the next GCC snapshot. That would give us a clear list of affected packages.

jeff

ignatenkobrain: My understanding from the SuSE guys is that yes, the most recent version of autoconf handles the most common cases correctly (testing for the existence of a function in libc/libm). However, I have not verified this myself. I'd be very surprised if things like find_stack_direction are fixed in upstream autoconf.

So, for some packages just re-running the autotools should be sufficient to fix the problems. There's some risk with doing that, but it's manageable, particularly since my tester can flag any changes in the generated config.h files. The most common fallout is some packages have their documentation depend on the autoconf generated files. So running the autotools will trigger a subsequent rebuild of the package docs -- and many packages do not have proper BuildRequires to rebuild their documentation. I've already stumbled over this stuff in several cases, but they're easy to fix.

Some packages are using autoconf driving scripts that are so old they can't be grokked by modern autoconf -- IIRC I found one that was autoconf-2.13, circa 1999 which is definitely not compatible with modern versions such as 2.69. So a simple autoreconf -ivf won't work, you first have to modernize all the autoconf crap in the package at which point I determined sed was a far better solution.

4 new commits added

  • Use %{__sed} and a consistent with other configure hackery
  • Drop comments for sed hackery that is no longer needed
  • Create backup file when editing configure scripts
  • Drop two sed hacks that are no longer needed
2 months ago

1 new commit added

  • Use shell test rather than simple macro-is-defined test.
2 months ago

3 new commits added

  • Drop sed hack which handled the autoconf bits that turned nm output into C
  • Put the original configure file back if we made no changes. This
  • Drop sed fix for detecting when signal handlers need to be reinstalled. It only affects a few packages -- few enough we can fix them individually.
a month ago

this won't do anything since nothing is setting up %_with_lto